Re: [PATCH] remote-hg: store converted URL
On 14.01.2013, at 19:14, Junio C Hamano wrote: Max Horn m...@quendi.de writes: From: Felipe Contreras felipe.contre...@gmail.com Mercurial might convert the URL to something more appropriate, like an absolute path. What it is converted *TO* is fairly clear with , like an ..., but from the first reading it was unclear to me what it is converted *FROM* and *WHEN* the conversion happens. Do you mean that the user gives git clone an URL ../hg-repo via the command line (e.g. the argument to git clone is spelled something like hg::../hg-repo), and that ../hg-repo is rewritten to something else (an absolute path, e.g. /srv/project/hg-repo)? Yes, that was meant. Lets store that instead of the original URL, which won't work from a different working directory if it's relative. What is lacking from this description is why it even needs to work from a different working directory. I am guessing that remote-hg later creates a hidden Hg repository or something in a different place and still tries to use the URL to interact with the upstream, and that is what breaks, but with only the above description without looking at your original report, people who will read the git log output and find this change will not be able to tell why this was needed, I am afraid. Of course, the above guess of mine may even be wrong, but then that is yet another reason that the log needs to explain the change better. Fully agreed. How about this commit message: -- 8 -- remote-hg: store converted URL of hg repo in git config When remote-hg is invoked, read the remote repository URL from the git config, give Mercurial a chance to expand it, and if changed, store it back into the git config. This fixes the following problem: Suppose you clone a local hg repository using a relative path, e.g. git clone hg::hgrepo gitrepo This stores hg::hgrepo in gitrepo/.git/config. However, no information about the PWD is stored, making it impossible to correctly interpret the relative path later on. Thus when latter attempting to, say, git pull from inside gitrepo, remote-hg cannot resolve the relative path correctly, and the user sees an unexpected error. With this commit, the URL hg::hgrepo gets expanded (during cloning, but also during any other remote operation) and the resulting absolute URL (e.g. hg::/abspath/hgrepo) is stored in gitrepo/.git/config. Thus the git clone of hgrepo becomes usable. -- 8 -- Suggested-by: Max Horn m...@quendi.de Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Max Horn m...@quendi.de --- For a discussion of the problem, see also http://article.gmane.org/gmane.comp.version-control.git/210250 I do not see any discussion; only your problem report. Aha, an english language issue on my side I guess: For me, a single person can discuss a problem (often, a research paper is said to be discussing a problem). Sorry for that. Anyway, the reason I gave that link was because it attempts explains the problem and one solution (which this patch ended up implementing), but also express that I feel a bit uncomfortable with this. Which I still do. Relying on the remote helper to invoke git config feels like a hack and I was wondering whether this is deemed an acceptable solution -- or whether one should instead extend the remote-helper protocol, allowing the remote helper to signal a rewritten remote URL (perhaps only directly after a clone?). As it is, the remote helper seems (?) to have no way to distinguish whether it is being called during a clone or a pull; hence it has to expand and rewrite the URL every time it is called, just in case. Anyway, as long as this particular command works somehow, I am fine: git clone hg::../relative/path/to/hg-repo git-repo Was this work done outside the list? I just want to make sure this patch is not something Felipe did not want to sign off for whatever reason but you are passing it to the list as a patch signed off by him. The work was done by Felipe's and published in his github repository: https://github.com/felipec/git/commit/605bad5b52d2fcf3d8f5fd782a87d7c97d1b040a See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 I took his sign-off from there and interpreted it as saying that Felipe was OK with this being pushed to git.git. But perhaps this is not what I should have done? In that case I am very sorry :-(. It's just that I feel this patch is quite useful and important for daily use (which is why I suggested it in the first place ;-), so I was/am quite eager to see it in. Cheers, Max PS: recently, yet another tool has (re)emerged for using hg repos from inside git: https://github.com/buchuki/gitifyhg This is partially based on Felipe's work, but has several bug fixes atop that. It is also seems to be a priority for its author, so it os more actively developed... anyway, that's now, what, solution #5 or #6? I
Re: [PATCH] remote-hg: store converted URL
Junio C Hamano gits...@pobox.com writes: Max Horn m...@quendi.de writes: ... See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 ... If I understand correctly, the $backend::$opaqueToken is a contract between the remote-helper and the remote-$backend that says When user wants to interact with the same (foreign) repository, we agreed to let her use 'origin' nickname. The remote-helper looks up this opaque token that corresponds to 'origin' and gives it to the remote-$backend, and whatever is in the opaque token should be sufficient for the remote-$backend to figure out how to proceed from there. But in this hg::../over/there case, it seems that string is not sufficient for remote-hg to do so and the contract is broken. When git clone $backend::$opaqueToken repo is run in /dir/ecto/ry, and then subsequent git fetch origin will be run in (a subdirectory of) /dir/ecto/ry/repo, but anything relative to /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo. The create a new repository here argument could even be an absolute path to a totally different place, so if the remote-$backend wants to use $opaqueToken as anything relative to the $(cwd) when git clone was invoked, that original location needs to be available somehow. Would a new helper protocol message be necessary, so that the backend can rewrite the $opaqueToken at clone time and tell the helper what to store as URL instead of the original? I do not think that is much different from remote-$backend updating the value of the remote.origin.URL using git config. An alternative approach may be for somebody (either the git clone or the remote-$backend) to store a base directory when git clone was invoked in remote.origin.dirAtCloneTime variable, so that the next time remote-$backend runs, it can read that directory and interpret the $opaqueToken as a relative path to that directory if it wants to. That way, nobody needs to rewrite $opaqueToken. How do other remote helpers solve this, I have to wonder, though. By not allowing relative paths to a directory? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
On 15.01.2013, at 17:05, Junio C Hamano wrote: Max Horn m...@quendi.de writes: On 14.01.2013, at 19:14, Junio C Hamano wrote: What is lacking from this description is why it even needs to work from a different working directory In your rewrite below, this is still lacking, I think. Hm, I thought I made it clear: It has to change because relative paths only make sense when you know the reference point they are relative with. Typically. This is the pwd. But when I git clone repo newrepo cd newrepo I just changed the PWD. The clone command was given the relative path repo. If git were to use that, it would suddenly refer to a directory inside newrepo, not next to it. Bang. Hence, git expands the relative path to an absolute one in the above example. But git cannot do that for URLs in the form HELPER::PATH, because such a string is necessarily opaque to git. snip Thus when latter attempting to, say, git pull from inside gitrepo, remote-hg cannot resolve the relative path correctly, and the user sees an unexpected error. ... cannot resolve the relative path correctly above sound like a bug in remote-hg. Something like: Cloning a local hg repository using a relative path, e.g. git clone hg::hgrepo gitrepo stores hg::hgrepo in gitrepo/.git/config as its URL. When remote-hg is invoked by git fetch, it chdirs to X (which is different from the gitrepo directory) and uses the URL (which is not correct, as it is a relative path but the cwd is different when it is used) to interact with the original hgrepo, which will fail. is needed, but you didn't explain what that X is. Perhaps it is a temporary directory. Perhaps it is a hidden Hg repository somewhere in gitrepo/.git directory. Or something else. None of the above. Nor does the remote helper chdir anywhere. It is the user who has done the chdir: Away from the location he invoked git clone at, and into the new repository directory that previously did not even exist. Cheers, Max-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
On 15.01.2013, at 17:51, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Max Horn m...@quendi.de writes: ... See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 ... If I understand correctly, the $backend::$opaqueToken is a contract between the remote-helper and the remote-$backend Just to clarify: What is the remote-helper ? So far, I thought of this being a contract between git (or some part of git), and remote-$backend (i.e. remote-hg in this case). that says When user wants to interact with the same (foreign) repository, we agreed to let her use 'origin' nickname. The remote-helper looks up this opaque token that corresponds to 'origin' and gives it to the remote-$backend, and whatever is in the opaque token should be sufficient for the remote-$backend to figure out how to proceed from there. Supposing that I interpret remote-helper correctly, that sounds about right to me. But in this hg::../over/there case, it seems that string is not sufficient for remote-hg to do so and the contract is broken. One could put it that way. When git clone $backend::$opaqueToken repo is run in /dir/ecto/ry, and then subsequent git fetch origin will be run in (a subdirectory of) /dir/ecto/ry/repo, but anything relative to /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo. The create a new repository here argument could even be an absolute path to a totally different place, so if the remote-$backend wants to use $opaqueToken as anything relative to the $(cwd) when git clone was invoked, that original location needs to be available somehow. That would be one option. Another is to do what the proposed patch does, and what git itself does: Change the relative path into an absolute one. This requires remote-$backend to be able to modify the opaque token supplied by the user. Yet another would be to be more strict in remote-$backend as to which opaque tokens to accept: When it contains a relative path, simply always refuse to work, even if the PWD happens to be set the right way. Of course this would be quite undesirable from a user's perspective. Would a new helper protocol message be necessary, so that the backend can rewrite the $opaqueToken at clone time and tell the helper what to store as URL instead of the original? I do not think that is much different from remote-$backend updating the value of the remote.origin.URL using git config. An alternative approach may be for somebody (either the git clone or the remote-$backend) to store a base directory when git clone was invoked in remote.origin.dirAtCloneTime variable, so that the next time remote-$backend runs, it can read that directory and interpret the $opaqueToken as a relative path to that directory if it wants to. That way, nobody needs to rewrite $opaqueToken. As I said above, this would be an option. However, I would prefer rewriting the $opaqueToken, as that would be closer to what git does for native tokens passed to git clone Specifically, I am talking about get_repo_path() in builtin/clone.c which is called by cmd_clone. What this does is in my eyes essentially the equivalent of what the patch discussed here is doing. Anyway, at the end of the day, I mainly care about relative paths working, somehow :-). But I think it would be important to make the issue easy to resolve for all remote-$backend authors, as many of them are affected (see below). How do other remote helpers solve this, I have to wonder, though. By not allowing relative paths to a directory? So far, all I look at do not deal with this at all. Any attempts to deal with it should be pretty easy to recognize: The remote-$backend would have to store something into the git config, or else, verify the opaque token and refuse to work with it under certain conditions (e.g. when it contains a relative path). But they don't. E.g. git-remote-testgit has the exact same problem. Doing git init repo cd repo echo a a git add a git ci -m a a git clone testgit::repo clone cd clone results in a .git/config file containing [remote origin] url = testgit::repo fetch = +refs/heads/*:refs/remotes/origin/* Trying to do a git push from within the clone then gives this: fatal: 'repo/.git' does not appear to be a git repository fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Traceback (most recent call last): File /usr/local/libexec/git-core/git-remote-testgit, line 272, in module sys.exit(main(sys.argv)) File /usr/local/libexec/git-core/git-remote-testgit, line 261, in main repo = get_repo(alias, url) File /usr/local/libexec/git-core/git-remote-testgit, line 39, in get_repo repo.get_revs() File /usr/local/lib/python2.7/site-packages/git_remote_helpers/git/repo.py, line 59, in get_revs check_call(args,
Re: [PATCH] remote-hg: store converted URL
Max Horn m...@quendi.de writes: So far, all I look at do not deal with this at all. Any attempts to deal with it should be pretty easy to recognize: The remote-$backend would have to store something into the git config, or else, verify the opaque token and refuse to work with it under certain conditions (e.g. when it contains a relative path). But they don't. E.g. git-remote-testgit has the exact same problem. Thanks for confirming what I suspected. I think the way Felipe's patch makes remote-hg take responsibility of how $opaqueToken should look like for future invocations is the simplest and makes the most sense. We could try to go fancier and end up over-engineering, but I'd rather have a simple fix in the tree first. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
Max Horn m...@quendi.de writes: From: Felipe Contreras felipe.contre...@gmail.com Mercurial might convert the URL to something more appropriate, like an absolute path. What it is converted *TO* is fairly clear with , like an ..., but from the first reading it was unclear to me what it is converted *FROM* and *WHEN* the conversion happens. Do you mean that the user gives git clone an URL ../hg-repo via the command line (e.g. the argument to git clone is spelled something like hg::../hg-repo), and that ../hg-repo is rewritten to something else (an absolute path, e.g. /srv/project/hg-repo)? Lets store that instead of the original URL, which won't work from a different working directory if it's relative. What is lacking from this description is why it even needs to work from a different working directory. I am guessing that remote-hg later creates a hidden Hg repository or something in a different place and still tries to use the URL to interact with the upstream, and that is what breaks, but with only the above description without looking at your original report, people who will read the git log output and find this change will not be able to tell why this was needed, I am afraid. Of course, the above guess of mine may even be wrong, but then that is yet another reason that the log needs to explain the change better. Suggested-by: Max Horn m...@quendi.de Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Max Horn m...@quendi.de --- For a discussion of the problem, see also http://article.gmane.org/gmane.comp.version-control.git/210250 I do not see any discussion; only your problem report. Was this work done outside the list? I just want to make sure this patch is not something Felipe did not want to sign off for whatever reason but you are passing it to the list as a patch signed off by him. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] remote-hg: store converted URL
From: Felipe Contreras felipe.contre...@gmail.com Mercurial might convert the URL to something more appropriate, like an absolute path. Lets store that instead of the original URL, which won't work from a different working directory if it's relative. Suggested-by: Max Horn m...@quendi.de Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Max Horn m...@quendi.de --- For a discussion of the problem, see also http://article.gmane.org/gmane.comp.version-control.git/210250 While I am not quite happy with using git config to solve it, there doesn't seem to be a better way right now. contrib/remote-helpers/git-remote-hg | 11 +++ 1 file changed, 11 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index c700600..7c74d8b 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -720,6 +720,14 @@ def do_export(parser): if peer: parser.repo.push(peer, force=False) +def fix_path(alias, repo, orig_url): +repo_url = util.url(repo.url()) +url = util.url(orig_url) +if str(url) == str(repo_url): +return +cmd = ['git', 'config', 'remote.%s.url' % alias, hg::%s % repo_url] +subprocess.call(cmd) + def main(args): global prefix, dirname, branches, bmarks global marks, blob_marks, parsed_refs @@ -766,6 +774,9 @@ def main(args): repo = get_repo(url, alias) prefix = 'refs/hg/%s' % alias +if not is_tmp: +fix_path(alias, peer or repo, url) + if not os.path.exists(dirname): os.makedirs(dirname) -- 1.8.0.1.525.gaaf5ad5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
