Re: [PATCH] remote-hg: store converted URL

[Junio C Hamano <gitster@pobox.com>]

Max Horn <max@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.

> 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.

Here, you say "correctly interpret relative path later on", but it
is not made clear why it is even necessary.  And that makes ...

> 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.

With that explained ...

> 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.

... the description of the fix start making sense, but not without.

>> 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?

You did nothing wrong, other than not having given the necessary
context to understand how the change flowed here and it is kosher.

Which you now have, so it is OK.

Thanks.