{"thread":{"id":"34619","subject":"[PATCH] remote-hg: fix path when cloning with tilde expansion","startedAt":"2013-08-05T20:12:21Z","lastAt":"2013-08-10T15:15:07Z","messageCount":20,"participants":["Antoine Pelisse","Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224625","messageId":"1375733541-9099-1-git-send-email-apelisse@gmail.com","threadId":"34619","inReplyTo":null,"subject":"[PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-05T20:12:21Z","receivedAt":"2013-08-05T20:12:21Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"The current code fixes the path to make it absolute when cloning, but\ndoesn't consider tilde expansion, so that scenario fails throwing an\nexception because /home/myuser/~/my/repository doesn't exists:\n\n    $ git clone hg::~/my/repository && cd repository && git fetch\n\nFix that by using python os.path.expanduser method.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n contrib/remote-helpers/git-remote-hg |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\nindex 02404dc..4bbd296 100755\n--- a/contrib/remote-helpers/git-remote-hg\n+++ b/contrib/remote-helpers/git-remote-hg\n@@ -1137,7 +1137,7 @@ def fix_path(alias, repo, orig_url):\n     url = urlparse.urlparse(orig_url, 'file')\n     if url.scheme != 'file' or os.path.isabs(url.path):\n         return\n-    abs_url = urlparse.urljoin(\"%s/\" % os.getcwd(), orig_url)\n+    abs_url = os.path.abspath(os.path.expanduser(orig_url))\n     cmd = ['git', 'config', 'remote.%s.url' % alias, \"hg::%s\" % abs_url]\n     subprocess.call(cmd)\n \n-- \n1.7.9.5\n"},{"id":"224627","messageId":"CAMP44s1Jqao0YvBSh18t1C2LwAF4_u2GaTNx1RwdW+pmCFcxvQ@mail.gmail.com","threadId":"34619","inReplyTo":"1375733541-9099-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-05T20:30:04Z","receivedAt":"2013-08-05T20:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse <apelisse@gmail.com> wrote:\n> The current code fixes the path to make it absolute when cloning, but\n> doesn't consider tilde expansion, so that scenario fails throwing an\n> exception because /home/myuser/~/my/repository doesn't exists:\n>\n>     $ git clone hg::~/my/repository && cd repository && git fetch\n>\n> Fix that by using python os.path.expanduser method.\n\nShouldn't that be the job of the shell? (s/~/$HOME/)\n\n-- \nFelipe Contreras\n"},{"id":"224628","messageId":"CALWbr2zNEzcEdEGYpZYfsYSXyJyjV4J23O6=cqYD7RDJMXOxRw@mail.gmail.com","threadId":"34619","inReplyTo":"CAMP44s1Jqao0YvBSh18t1C2LwAF4_u2GaTNx1RwdW+pmCFcxvQ@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-05T20:32:05Z","receivedAt":"2013-08-05T20:32:05Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse <apelisse@gmail.com> wrote:\n>>     $ git clone hg::~/my/repository && cd repository && git fetch\n>>\n>> Fix that by using python os.path.expanduser method.\n>\n> Shouldn't that be the job of the shell? (s/~/$HOME/)\n\nI guess it is, as long as it looks like a path:\n\n    $ echo ~\n    /home/myuser\n    $ echo hg::~\n    hg::~\n"},{"id":"224920","messageId":"1376068387-28510-1-git-send-email-apelisse@gmail.com","threadId":"34619","inReplyTo":"CAMP44s1Jqao0YvBSh18t1C2LwAF4_u2GaTNx1RwdW+pmCFcxvQ@mail.gmail.com","subject":"[PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-09T17:13:07Z","receivedAt":"2013-08-09T17:13:07Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"The current code fixes the path to make it absolute when cloning, but\ndoesn't consider tilde expansion, so that scenario fails throwing an\nexception because /home/myuser/~/my/repository doesn't exists:\n\n    $ git clone hg::~/my/repository && cd repository && git fetch\n\nExpand the tilde when checking if the path is absolute, so that we don't\nfix a path that doesn't need to be.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nOn Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:\n> Shouldn't that be the job of the shell? (s/~/$HOME/)\n\nI'm not sure what you mean here. Does it mean that I should stop cloning using \"~\" ?\nI also send this patch as I think it makes more sense to keep the ~ in the path, but just make\nsure we don't build invalid absolute path.\n\nBy the way, I don't exactly understand why:\n\n    abs_url = urlparse.urljoin(\"%s/\" % os.getcwd(), orig_url)\n\nis done right after instead of:\n\n    abs_url = os.path.abspath(orig_url)\n\nCheers,\nAntoine\n\n contrib/remote-helpers/git-remote-hg |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\nindex 1897327..861c498 100755\n--- a/contrib/remote-helpers/git-remote-hg\n+++ b/contrib/remote-helpers/git-remote-hg\n@@ -1135,7 +1135,7 @@ def do_option(parser):\n\n def fix_path(alias, repo, orig_url):\n     url = urlparse.urlparse(orig_url, 'file')\n-    if url.scheme != 'file' or os.path.isabs(url.path):\n+    if url.scheme != 'file' or os.path.isabs(os.path.expanduser(url.path)):\n         return\n     abs_url = urlparse.urljoin(\"%s/\" % os.getcwd(), orig_url)\n     cmd = ['git', 'config', 'remote.%s.url' % alias, \"hg::%s\" % abs_url]\n--\n1.7.9.5\n"},{"id":"224932","messageId":"7veha266nq.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"1376068387-28510-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-09T18:49:45Z","receivedAt":"2013-08-09T18:49:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> The current code fixes the path to make it absolute when cloning, but\n> doesn't consider tilde expansion, so that scenario fails throwing an\n> exception because /home/myuser/~/my/repository doesn't exists:\n>\n>     $ git clone hg::~/my/repository && cd repository && git fetch\n>\n> Expand the tilde when checking if the path is absolute, so that we don't\n> fix a path that doesn't need to be.\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n> On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:\n>> Shouldn't that be the job of the shell? (s/~/$HOME/)\n>\n> I'm not sure what you mean here. Does it mean that I should stop cloning using \"~\" ?\n\nI think shells do not expand ~ when it appears in a string (e.g. hg::~/there);\nyou could work it around with\n\n\tgit clone hg::$(echo ~/there)\n\nand I suspect that is what Felipe is alluding to.  A tool (like\nremote-hg bridge with this patch) that expands ~ in the middle of a\nstring also may be surprising to some people, especially to those\nwho know the shell does not.\n\n> I also send this patch as I think it makes more sense to keep the\n> ~ in the path, but just make sure we don't build invalid absolute\n> path.\n>\n> By the way, I don't exactly understand why:\n>\n>     abs_url = urlparse.urljoin(\"%s/\" % os.getcwd(), orig_url)\n>\n> is done right after instead of:\n>\n>     abs_url = os.path.abspath(orig_url)\n\nThat looks like a good cleanup to me, too, but I may be missing some\nsubtle points...\n\nBy the way, you earlier sent an updated 1/2; is this supposed to be\n2/2 to conclude the two-patch series?\n\n> Cheers,\n> Antoine\n>\n>  contrib/remote-helpers/git-remote-hg |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\n> index 1897327..861c498 100755\n> --- a/contrib/remote-helpers/git-remote-hg\n> +++ b/contrib/remote-helpers/git-remote-hg\n> @@ -1135,7 +1135,7 @@ def do_option(parser):\n>\n>  def fix_path(alias, repo, orig_url):\n>      url = urlparse.urlparse(orig_url, 'file')\n> -    if url.scheme != 'file' or os.path.isabs(url.path):\n> +    if url.scheme != 'file' or os.path.isabs(os.path.expanduser(url.path)):\n>          return\n>      abs_url = urlparse.urljoin(\"%s/\" % os.getcwd(), orig_url)\n>      cmd = ['git', 'config', 'remote.%s.url' % alias, \"hg::%s\" % abs_url]\n> --\n> 1.7.9.5\n"},{"id":"224943","messageId":"CALWbr2w2JjEr_hYX9ighu_-=iTV6etG=78g4AbKko64EsecxFA@mail.gmail.com","threadId":"34619","inReplyTo":"7veha266nq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-09T20:09:17Z","receivedAt":"2013-08-09T20:09:17Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Fri, Aug 9, 2013 at 8:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Antoine Pelisse <apelisse@gmail.com> writes:\n>> On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:\n>>> Shouldn't that be the job of the shell? (s/~/$HOME/)\n>>\n>> I'm not sure what you mean here. Does it mean that I should stop cloning using \"~\" ?\n>\n> I think shells do not expand ~ when it appears in a string (e.g. hg::~/there);\n> you could work it around with\n>\n>         git clone hg::$(echo ~/there)\n>\n> and I suspect that is what Felipe is alluding to.  A tool (like\n> remote-hg bridge with this patch) that expands ~ in the middle of a\n> string also may be surprising to some people, especially to those\n> who know the shell does not.\n\nIt looks like mercurial will expand the tilde (it it starts with it):\n\n   hg init \\~\n\nwill create a $HOME/.hg. (while git init \\~ will create ./~).\n\nSo when we run:\n\ngit clone hg::~/my/repo\n\nGit will remove the \"hg::\" part, and Mercurial will expand tilde and\nclone $HOME/my/repo.\n\nSo what should we do ? I think we should stick as close as possible to\nHg behavior:\nThat is consider that a path starting with tilde is absolute, and not\ntry to fix it by building /home/user/~/repo/path.\nOf course if we could not depend on \"I think Hg works like that\", it\nwould be better if we could resolve that by asking Mercurial.\nI will dig into it.\n\n> By the way, you earlier sent an updated 1/2; is this supposed to be\n> 2/2 to conclude the two-patch series?\n\nThose two patches don't interact with each other, but you can of\ncourse join them if it makes it easier for you (and I don't think one\nis going to have to go \"faster\" than the other anyway).\n"},{"id":"224951","messageId":"7vy58a4mcy.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CALWbr2w2JjEr_hYX9ighu_-=iTV6etG=78g4AbKko64EsecxFA@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-09T20:53:33Z","receivedAt":"2013-08-09T20:53:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> So when we run:\n>\n> git clone hg::~/my/repo\n>\n> Git will remove the \"hg::\" part, and Mercurial will expand tilde and\n> clone $HOME/my/repo.\n\nNow you confused me.  If the implementation were for us to remove\nthe hg:: prefix and let Mercurial do whatever it wants to do with\nthe rest, you are right that we will not have to do any expansion\nlike your patch.  But you sent a patch to do so, so apparently it\nis not what happens.  So where does it go wrong?\n\nPuzzled...\n\n>> By the way, you earlier sent an updated 1/2; is this supposed to be\n>> 2/2 to conclude the two-patch series?\n>\n> Those two patches don't interact with each other, but you can of\n> course join them if it makes it easier for you (and I don't think one\n> is going to have to go \"faster\" than the other anyway).\n\nHmph, so there is a different 2/2 that we haven't seen recently on\nthe list (meaning you have three patches)?\n\nThanks.\n"},{"id":"224954","messageId":"CALWbr2y5H_dfHAFW_qN+j8YtF4F9+VcG8G503hr4YN2Qv69CXA@mail.gmail.com","threadId":"34619","inReplyTo":"7vy58a4mcy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-09T21:19:30Z","receivedAt":"2013-08-09T21:19:30Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Confusion everywhere :-)\n\nOn Fri, Aug 9, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Antoine Pelisse <apelisse@gmail.com> writes:\n>\n>> So when we run:\n>>\n>> git clone hg::~/my/repo\n>>\n>> Git will remove the \"hg::\" part, and Mercurial will expand tilde and\n>> clone $HOME/my/repo.\n>\n> Now you confused me.  If the implementation were for us to remove\n> the hg:: prefix and let Mercurial do whatever it wants to do with\n> the rest, you are right that we will not have to do any expansion\n> like your patch.  But you sent a patch to do so, so apparently it\n> is not what happens.  So where does it go wrong?\n>\n> Puzzled...\n\nOK, I think I see why you are puzzled.\n\nCloning works fine because we \"fix the path\" *after* the clone is done\nsuccessfully, for the following reason:\nIf you run:\n\n   git clone hg::./my_repo my_new_repo\n\nThe remote path will be hg::./my_repo, so we have to fix this path\n(otherwise you won't be able to run git fetch from inside\nmy_new_repo). It's currently done by checking if ./my_repo is an\nabsolute path or not, and try to make it absolute if required.\n\nBut my issue is when I do that:\n\n    git clone hg::~/my_repo my_new_repo\n\nThe clone works successfully by cloning $HOME/my_repo, but then, when\nwe try to fix the repo path, we think that ~/my_repo is not an\nabsolute path, so we make it absolute: /home/user/~/my_repo which is\nnow off. So I'm not able to fetch that remote.\n\nWhat the current patch does, is to expand the tilde before checking if\nthe path is absolute. So that fixes the bug, but that indeed can be\nconfusing to another user that would expect hg::~/my_repo/ to *not be*\nhg::$HOME/my_repo (because he knows the expansion should not happen in\nthat case).\n\n>>> By the way, you earlier sent an updated 1/2; is this supposed to be\n>>> 2/2 to conclude the two-patch series?\n>>\n>> Those two patches don't interact with each other, but you can of\n>> course join them if it makes it easier for you (and I don't think one\n>> is going to have to go \"faster\" than the other anyway).\n>\n> Hmph, so there is a different 2/2 that we haven't seen recently on\n> the list (meaning you have three patches)?\n\nI have 2 patch (1 from me, 1 from Felipe):\nOne with the tilde expansion, the other one with shared_path\ninitialization (which now conflicts with the resend from Felipe)\n\nI will try to provide a better versioning of the patches next time.\n\nSorry for the confusion,\nThanks,\n"},{"id":"224960","messageId":"7vfvui4jy8.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CALWbr2y5H_dfHAFW_qN+j8YtF4F9+VcG8G503hr4YN2Qv69CXA@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-09T21:45:35Z","receivedAt":"2013-08-09T21:45:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> OK, I think I see why you are puzzled.\n> ...\n> But my issue is when I do that:\n>\n>     git clone hg::~/my_repo my_new_repo\n>\n> The clone works successfully by cloning $HOME/my_repo, but then, when\n> we try to fix the repo path, we think that ~/my_repo is not an\n> absolute path, so we make it absolute: /home/user/~/my_repo which is\n> now off. So I'm not able to fetch that remote.\n\nOK, so clone works, but subsequent fetch from the cloned resoitory\ndoes not?  \"git fetch hg::~/my_repo\" will still work but the call to\n\"git config\" done near the place your patch touches does not store\n\"hg::~/my_repo\" because it thinks \"~/my_repo\" refers to\n\"./~/my_repo\" and tries to come up with an absolute path.  The patch\ntries to notice this case and return without rewriting, so that\nremote.*.url is kept as \"hg::~/my_repo\".\n\nAssuming that I am following your reasoning so far, I think I can\nagree with the patch (not that my agreement matters that much, as\nyou seem to be a lot more familiar with this codepath).\n\nThanks for explaining.\n"},{"id":"224963","messageId":"CAMP44s13y39f-eCP1sBuMEedciU230C1O11+iMb1SHi45RnSNQ@mail.gmail.com","threadId":"34619","inReplyTo":"CALWbr2y5H_dfHAFW_qN+j8YtF4F9+VcG8G503hr4YN2Qv69CXA@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-09T21:55:13Z","receivedAt":"2013-08-09T21:55:13Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Aug 9, 2013 at 4:19 PM, Antoine Pelisse <apelisse@gmail.com> wrote:\n> Confusion everywhere :-)\n>\n> On Fri, Aug 9, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Antoine Pelisse <apelisse@gmail.com> writes:\n>>\n>>> So when we run:\n>>>\n>>> git clone hg::~/my/repo\n>>>\n>>> Git will remove the \"hg::\" part, and Mercurial will expand tilde and\n>>> clone $HOME/my/repo.\n>>\n>> Now you confused me.  If the implementation were for us to remove\n>> the hg:: prefix and let Mercurial do whatever it wants to do with\n>> the rest, you are right that we will not have to do any expansion\n>> like your patch.  But you sent a patch to do so, so apparently it\n>> is not what happens.  So where does it go wrong?\n>>\n>> Puzzled...\n>\n> OK, I think I see why you are puzzled.\n>\n> Cloning works fine because we \"fix the path\" *after* the clone is done\n> successfully, for the following reason:\n\nSo if we didn't store a different path, it would work. So instead of\nexpanding '~' ourselves, it would be better to don't expand anything,\nand leave it as it is, but how to detect that in fix_path()?\n\n-- \nFelipe Contreras\n"},{"id":"224964","messageId":"CALWbr2zx7eU_SmGT1MmPvYhbmU3v8y4LxopeBAefFkUBnbRtew@mail.gmail.com","threadId":"34619","inReplyTo":"7vfvui4jy8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-09T21:55:18Z","receivedAt":"2013-08-09T21:55:18Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Fri, Aug 9, 2013 at 11:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> OK, so clone works, but subsequent fetch from the cloned resoitory\n> does not?  \"git fetch hg::~/my_repo\" will still work but the call to\n> \"git config\" done near the place your patch touches does not store\n> \"hg::~/my_repo\" because it thinks \"~/my_repo\" refers to\n> \"./~/my_repo\" and tries to come up with an absolute path.  The patch\n> tries to notice this case and return without rewriting, so that\n> remote.*.url is kept as \"hg::~/my_repo\".\n>\n> Assuming that I am following your reasoning so far, I think I can\n> agree with the patch (not that my agreement matters that much, as\n> you seem to be a lot more familiar with this codepath).\n>\n> Thanks for explaining.\n\nThanks\n"},{"id":"224965","messageId":"7v7gfu4ikb.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CAMP44s13y39f-eCP1sBuMEedciU230C1O11+iMb1SHi45RnSNQ@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-09T22:15:32Z","receivedAt":"2013-08-09T22:15:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> OK, I think I see why you are puzzled.\n>>\n>> Cloning works fine because we \"fix the path\" *after* the clone is done\n>> successfully, for the following reason:\n>\n> So if we didn't store a different path, it would work. So instead of\n> expanding '~' ourselves, it would be better to don't expand anything,\n> and leave it as it is, but how to detect that in fix_path()?\n\nI think that the patch relies on that os.path.expanduser(), if\nurl.path is such a path that begins with \"~\" (or \"~whom\"), returns\nan absolute path.  When given an absolute path, or \"~whom/path\",\nfix_path returns without running 'git config' on remote.<alias>.url\nconfiguration.\n\nPresumably this \"git config\" is to \"fix\" what is already there, and\nin the case where the path is already absolute\n(e.g. \"/home/ap/hgrepo\" as opposed to \"~ap/hgrepo\") the resulting\nrepository has a correct value for the variable set already without\nthe need to fix it (that is why the original code just returns from\nthe function), so doing the same for \"~whom\" case with this patch\nshould leave the setting, which presumably is \"hg::~ap/hgrepo\"?\n"},{"id":"224967","messageId":"CAMP44s1Ky2AkEt-XS_nAo=_RrPXSVAL=8cGiMuJabw0=BRU0Dw@mail.gmail.com","threadId":"34619","inReplyTo":"7v7gfu4ikb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-09T22:28:13Z","receivedAt":"2013-08-09T22:28:13Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>> OK, I think I see why you are puzzled.\n>>>\n>>> Cloning works fine because we \"fix the path\" *after* the clone is done\n>>> successfully, for the following reason:\n>>\n>> So if we didn't store a different path, it would work. So instead of\n>> expanding '~' ourselves, it would be better to don't expand anything,\n>> and leave it as it is, but how to detect that in fix_path()?\n>\n> I think that the patch relies on that os.path.expanduser(), if\n> url.path is such a path that begins with \"~\" (or \"~whom\"), returns\n> an absolute path.  When given an absolute path, or \"~whom/path\",\n> fix_path returns without running 'git config' on remote.<alias>.url\n> configuration.\n\nI think ~whom/path would run 'git config'.\n\n-- \nFelipe Contreras\n"},{"id":"224977","messageId":"7vmwoq304o.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CAMP44s1Ky2AkEt-XS_nAo=_RrPXSVAL=8cGiMuJabw0=BRU0Dw@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-09T23:39:03Z","receivedAt":"2013-08-09T23:39:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>>> OK, I think I see why you are puzzled.\n>>>>\n>>>> Cloning works fine because we \"fix the path\" *after* the clone is done\n>>>> successfully, for the following reason:\n>>>\n>>> So if we didn't store a different path, it would work. So instead of\n>>> expanding '~' ourselves, it would be better to don't expand anything,\n>>> and leave it as it is, but how to detect that in fix_path()?\n>>\n>> I think that the patch relies on that os.path.expanduser(), if\n>> url.path is such a path that begins with \"~\" (or \"~whom\"), returns\n>> an absolute path.  When given an absolute path, or \"~whom/path\",\n>> fix_path returns without running 'git config' on remote.<alias>.url\n>> configuration.\n>\n> I think ~whom/path would run 'git config'.\n\nHmph, do you mean the third example of this?\n\n        $ python\n        >>> import os\n        >>> os.path.expanduser(\"~/repo\")\n        '/home/junio/repo'\n        >>> os.path.expanduser(\"~junio/repo\")\n        '/home/junio/repo'\n        >>> os.path.expanduser(\"~felipe/repo\")\n        '~felipe/repo'\n\nwhich will give \"~felipe/repo\" that is _not_ an absolute repository\nbecause no such user exists on this box?\n\nIt is true that in that case fix_path() will not return early and\nwill throw a bogus path at \"git config\", but if the \"~whom\" does not\nresolve to an existing home directory of a user, I am not sure what\nwe can do better than what Antoine's patch does.\n"},{"id":"224978","messageId":"CAMP44s1Q2x9uz5Ajr=BgVjSjO88XD5UYzVSEqgMeK5_YAYSa5A@mail.gmail.com","threadId":"34619","inReplyTo":"7vmwoq304o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-09T23:43:09Z","receivedAt":"2013-08-09T23:43:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Aug 9, 2013 at 6:39 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>>> OK, I think I see why you are puzzled.\n>>>>>\n>>>>> Cloning works fine because we \"fix the path\" *after* the clone is done\n>>>>> successfully, for the following reason:\n>>>>\n>>>> So if we didn't store a different path, it would work. So instead of\n>>>> expanding '~' ourselves, it would be better to don't expand anything,\n>>>> and leave it as it is, but how to detect that in fix_path()?\n>>>\n>>> I think that the patch relies on that os.path.expanduser(), if\n>>> url.path is such a path that begins with \"~\" (or \"~whom\"), returns\n>>> an absolute path.  When given an absolute path, or \"~whom/path\",\n>>> fix_path returns without running 'git config' on remote.<alias>.url\n>>> configuration.\n>>\n>> I think ~whom/path would run 'git config'.\n>\n> Hmph, do you mean the third example of this?\n>\n>         $ python\n>         >>> import os\n>         >>> os.path.expanduser(\"~/repo\")\n>         '/home/junio/repo'\n>         >>> os.path.expanduser(\"~junio/repo\")\n>         '/home/junio/repo'\n>         >>> os.path.expanduser(\"~felipe/repo\")\n>         '~felipe/repo'\n>\n> which will give \"~felipe/repo\" that is _not_ an absolute repository\n> because no such user exists on this box?\n>\n> It is true that in that case fix_path() will not return early and\n> will throw a bogus path at \"git config\", but if the \"~whom\" does not\n> resolve to an existing home directory of a user, I am not sure what\n> we can do better than what Antoine's patch does.\n\nI was thinking something like this:\n\nif url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':\n  return\n\n-- \nFelipe Contreras\n"},{"id":"224984","messageId":"7vioze2kev.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CAMP44s1Q2x9uz5Ajr=BgVjSjO88XD5UYzVSEqgMeK5_YAYSa5A@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-10T05:18:32Z","receivedAt":"2013-08-10T05:18:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> Hmph, do you mean the third example of this?\n>>\n>>         $ python\n>>         >>> import os\n>>         >>> os.path.expanduser(\"~/repo\")\n>>         '/home/junio/repo'\n>>         >>> os.path.expanduser(\"~junio/repo\")\n>>         '/home/junio/repo'\n>>         >>> os.path.expanduser(\"~felipe/repo\")\n>>         '~felipe/repo'\n>>\n>> which will give \"~felipe/repo\" that is _not_ an absolute repository\n>> because no such user exists on this box?\n>>\n>> It is true that in that case fix_path() will not return early and\n>> will throw a bogus path at \"git config\", but if the \"~whom\" does not\n>> resolve to an existing home directory of a user, I am not sure what\n>> we can do better than what Antoine's patch does.\n>\n> I was thinking something like this:\n>\n> if url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':\n>   return\n\nThat did cross my mind.\n\nI know ~/ and ~who/ are expanded on UNIXy systems, and I read in\nPython documentation that Python on Windows treats ~/ and ~who/ the\nsame way as on UNIXy systems, so the \"begins with ~\" test would work\non both systems.  But it is probably a better design to outsource\nthat knowledge to os.path.expanduser(), with the emphasis on \"os.\"\npart of that function.  That way, we do not even have to care about\nsuch potential platform specifics, which is a big plus.  The only\npossible difference that approach makes is the above example of\nnaming a non-existent ~user, but that will not work anyway, so...\n"},{"id":"224988","messageId":"CAMP44s3ULMBg6BJr6m4zkqHyD70rHSwLcuG5ph+ABr6KME8T=w@mail.gmail.com","threadId":"34619","inReplyTo":"7vioze2kev.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-10T06:39:59Z","receivedAt":"2013-08-10T06:39:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 10, 2013 at 12:18 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>> Hmph, do you mean the third example of this?\n>>>\n>>>         $ python\n>>>         >>> import os\n>>>         >>> os.path.expanduser(\"~/repo\")\n>>>         '/home/junio/repo'\n>>>         >>> os.path.expanduser(\"~junio/repo\")\n>>>         '/home/junio/repo'\n>>>         >>> os.path.expanduser(\"~felipe/repo\")\n>>>         '~felipe/repo'\n>>>\n>>> which will give \"~felipe/repo\" that is _not_ an absolute repository\n>>> because no such user exists on this box?\n>>>\n>>> It is true that in that case fix_path() will not return early and\n>>> will throw a bogus path at \"git config\", but if the \"~whom\" does not\n>>> resolve to an existing home directory of a user, I am not sure what\n>>> we can do better than what Antoine's patch does.\n>>\n>> I was thinking something like this:\n>>\n>> if url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':\n>>   return\n>\n> That did cross my mind.\n>\n> I know ~/ and ~who/ are expanded on UNIXy systems, and I read in\n> Python documentation that Python on Windows treats ~/ and ~who/ the\n> same way as on UNIXy systems, so the \"begins with ~\" test would work\n> on both systems.  But it is probably a better design to outsource\n> that knowledge to os.path.expanduser(), with the emphasis on \"os.\"\n> part of that function.  That way, we do not even have to care about\n> such potential platform specifics, which is a big plus.  The only\n> possible difference that approach makes is the above example of\n> naming a non-existent ~user, but that will not work anyway, so...\n\nWe would be doing better than os.path.expanduser(), because if\nMercurial somehow decided to treat ~ differently, our code would still\nwork just fine. If we do os.path.expanduser(), then we are not\noutsourcing anything, we are taking ownership and fixing the path by\nourselves and dealing with all the consequences.\n\nIf I clone ~/git, and then change my username, and move my home\ndirectory, doing a 'git fetch' in ~/git wouldn't work anymore, because\nwe have expanded the path and fixed it to my old home, if instead we\nsimply return without fixing, it would still work just fine.\n\n-- \nFelipe Contreras\n"},{"id":"224992","messageId":"7v38qi2g63.fsf@alter.siamese.dyndns.org","threadId":"34619","inReplyTo":"CAMP44s3ULMBg6BJr6m4zkqHyD70rHSwLcuG5ph+ABr6KME8T=w@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-10T06:50:12Z","receivedAt":"2013-08-10T06:50:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> If I clone ~/git, and then change my username, and move my home\n> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because\n> we have expanded the path and fixed it to my old home, if instead we\n> simply return without fixing, it would still work just fine.\n\nAntoine's patch runs expanduser() only to see if the given one gets\nmodified to absolute path, and makes fix_path() return without\ncalling the extra 'git config', so it is my understanding that the\nabove describes exactly what the patch does.  Am I reading the patch\nincorrectly?\n\nIt outsources the determination of \"is this a special notation to\nname a home directory?\" logic to .isabs(.expanduser()) instead of\ndoing a .beginswith('~') ourselves.\n"},{"id":"224995","messageId":"CAMP44s1NE-ac_tWej9EhMWJLRm7aq1WKOm17fZm5y_aA4ppq5g@mail.gmail.com","threadId":"34619","inReplyTo":"7v38qi2g63.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-10T07:07:45Z","receivedAt":"2013-08-10T07:07:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 10, 2013 at 1:50 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> If I clone ~/git, and then change my username, and move my home\n>> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because\n>> we have expanded the path and fixed it to my old home, if instead we\n>> simply return without fixing, it would still work just fine.\n>\n> Antoine's patch runs expanduser() only to see if the given one gets\n> modified to absolute path, and makes fix_path() return without\n> calling the extra 'git config', so it is my understanding that the\n> above describes exactly what the patch does.  Am I reading the patch\n> incorrectly?\n\nAntoine's *second* patch, which I missed, does that, yeah. That should\nwork fine.\n\n-- \nFelipe Contreras\n"},{"id":"225021","messageId":"CALWbr2x6BH8oKSBt2xYWgEzrML2EqO=iEn4WTq7Lq=0U9C0C5A@mail.gmail.com","threadId":"34619","inReplyTo":"CAMP44s1NE-ac_tWej9EhMWJLRm7aq1WKOm17fZm5y_aA4ppq5g@mail.gmail.com","subject":"Re: [PATCH] remote-hg: fix path when cloning with tilde expansion","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-10T15:15:07Z","receivedAt":"2013-08-10T15:15:07Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Sat, Aug 10, 2013 at 9:07 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Sat, Aug 10, 2013 at 1:50 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> If I clone ~/git, and then change my username, and move my home\n>>> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because\n>>> we have expanded the path and fixed it to my old home, if instead we\n>>> simply return without fixing, it would still work just fine.\n>>\n>> Antoine's patch runs expanduser() only to see if the given one gets\n>> modified to absolute path, and makes fix_path() return without\n>> calling the extra 'git config', so it is my understanding that the\n>> above describes exactly what the patch does.  Am I reading the patch\n>> incorrectly?\n>\n> Antoine's *second* patch, which I missed, does that, yeah. That should\n> work fine.\n\nOK Cool,\nThank you both,\n"}]}