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

Max Horn [off-list ref] writes:

On 14.01.2013, at 19:14, Junio C Hamano wrote:

quoted

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.

quoted

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.

On 15.01.2013, at 17:05, Junio C Hamano wrote:

Max Horn [off-list ref] writes:

quoted

On 14.01.2013, at 19:14, Junio C Hamano wrote:

quoted

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>

quoted

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