[PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Joern Hees]

6796d49 introduced a bug by making shared_path == ".git/hg' which
will most likely exist already, causing a new remote never to be
cloned and subsequently causing hg.share to fail with error msg:
"mercurial.error.RepoError: repository .git/hg not found"

Changing gitdir to dirname causes shared_path ==
.git/hg/<remote_name>/hg. The call to hg.share with local_path ==
.git/hg/<remote_name>/clone works again.

Signed-off-by: Joern Hees <dev@joernhees.de>
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 0194c67..89dd4cc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -390,7 +390,7 @@ def get_repo(url, alias):
         if not os.path.exists(dirname):
             os.makedirs(dirname)
     else:
-        shared_path = os.path.join(gitdir, 'hg')
+        shared_path = os.path.join(dirname, 'hg')
         if not os.path.exists(shared_path):
             try:
                 hg.clone(myui, {}, url, shared_path, update=False, pull=True)
-- 
1.8.3.3

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Antoine Pelisse]

On Tue, Jul 23, 2013 at 11:40 PM, Joern Hees <dev@joernhees.de> wrote:
> 6796d49 introduced a bug by making shared_path == ".git/hg' which
> will most likely exist already, causing a new remote never to be
> cloned and subsequently causing hg.share to fail with error msg:
> "mercurial.error.RepoError: repository .git/hg not found"

Indeed, no clone is performed if the .git/hg dir already exists.
I think it assumes that it's already done.
That will certainly lead to the failure you are reporting.

Also, the directory can be created to store marks for a local repository.
remote-hg won't require nor do a local clone in .git/hg for local repositories.

It should also be noted that once .git/hg is not empty, it will no
longer be possible to create a mercurial repository in there (it will
die with "destination '.git/hg'  is not empty")

I think the best way would be to create the shared repository in
.git/hg/$share, with $share being a path that can't be a remote name
(so that it doesn't conflict with remote directories),
and then apply the following patch (copied in gmail)

diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 0194c67..21c8091 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -390,7 +390,7 @@ def get_repo(url, alias):
         if not os.path.exists(dirname):
             os.makedirs(dirname)
     else:
-        shared_path = os.path.join(gitdir, 'hg')
+        shared_path = os.path.join(gitdir, 'hg', $share)
         if not os.path.exists(shared_path):
             try:
                 hg.clone(myui, {}, url, shared_path, update=False, pull=True)

That way, the share can be created even if .git/hg already exists
(because of a previous import, before the shared machinery existed, or
because you already have a local remote).

> Changing gitdir to dirname causes shared_path ==
> .git/hg/<remote_name>/hg. The call to hg.share with local_path ==
> .git/hg/<remote_name>/clone works again.

I think that will be a problem, because then the shared_path will no
longer be shared, will it ?

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Jörn Hees]

Hi,

On 24.07.2013, at 10:52, Antoine Pelisse <apelisse@gmail.com> wrote:
> I think the best way would be to create the shared repository in
> .git/hg/$share, with $share being a path that can't be a remote name
> (so that it doesn't conflict with remote directories),
> and then apply the following patch (copied in gmail)

Maybe ".git/hg/.share"?


> diff --git a/contrib/remote-helpers/git-remote-hg
> b/contrib/remote-helpers/git-remote-hg
> index 0194c67..21c8091 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -390,7 +390,7 @@ def get_repo(url, alias):
>         if not os.path.exists(dirname):
>             os.makedirs(dirname)
>     else:
> -        shared_path = os.path.join(gitdir, 'hg')
> +        shared_path = os.path.join(gitdir, 'hg', $share)
>         if not os.path.exists(shared_path):
>             try:
>                 hg.clone(myui, {}, url, shared_path, update=False, pull=True)
> 
> That way, the share can be created even if .git/hg already exists
> (because of a previous import, before the shared machinery existed, or
> because you already have a local remote).

I like the idea of having independent remotes (fetching one, doesn't update another). http://mercurial.selenic.com/wiki/ShareExtension warns about this, and i wasn't sure it wouldn't cause intricate bugs. This is why I opted for the explicit cloning, no shared history for several remotes.

I'd really like some feedback on this one as he probably knows the hg internals well enough that he can make a more educated guess on this than I can: when you import several hg remotes and fetch them / push to one, wouldn't such a shared repo cause problems?
If unsure i still opt for my version as it keeps things isolated at the cost of some optimization.


>> Changing gitdir to dirname causes shared_path ==
>> .git/hg/<remote_name>/hg. The call to hg.share with local_path ==
>> .git/hg/<remote_name>/clone works again.
> 
> I think that will be a problem, because then the shared_path will no
> longer be shared, will it ?

Yupp, the shared_paths won't be shared, so it's not as optimal as possible, but it will work at least ;)

Cheers,
Jörn

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Antoine Pelisse]

On Wed, Jul 24, 2013 at 11:59 AM, Jörn Hees <dev@joernhees.de> wrote:
> On 24.07.2013, at 10:52, Antoine Pelisse <apelisse@gmail.com> wrote:
>> I think the best way would be to create the shared repository in
>> .git/hg/$share, with $share being a path that can't be a remote name
>> (so that it doesn't conflict with remote directories),
>
> Maybe ".git/hg/.share"?

According to Documentation/git-check-ref-format.txt, I'm not sure if
we should start with a dot, or end with it.

>> That way, the share can be created even if .git/hg already exists
>> (because of a previous import, before the shared machinery existed, or
>> because you already have a local remote).
>
> I like the idea of having independent remotes (fetching one, doesn't update another). http://mercurial.selenic.com/wiki/ShareExtension warns about this, and i wasn't sure it wouldn't cause intricate bugs. > This is why I opted for the explicit cloning, no shared history for several remotes.

I think the goal of using sharing here is that Mercurial and Git may
use different schemes to handle branches. Mercurial may lead you to
have separate repositories for each branch (They seem to do it for its
own development [1]). All these branches actually share most of the
same history, and are fully related, and we usually handle this
situation in Git with one repository with multiple branches.
Using "hg share", we allow a smooth transition from Mercurial model to
Git model by merging all Mercurial repositories into one, and then map
this single repository to the Git repository.
IOW, the goal is to have only one copy of each "hg object" that are
shared amongst many "remotes" (and potentially import them only once,
though I don't think it currently works for me).

>>> Changing gitdir to dirname causes shared_path ==
>>> .git/hg/<remote_name>/hg. The call to hg.share with local_path ==
>>> .git/hg/<remote_name>/clone works again.
>>
>> I think that will be a problem, because then the shared_path will no
>> longer be shared, will it ?
>
> Yupp, the shared_paths won't be shared, so it's not as optimal as possible, but it will work at least ;)

If we decided to remove the sharing idea, I think we should revert
Felipe's commit rather than leave the shared_path variable, and call
hg.share() on repository we don't even mean to share. That would be
very confusing.

[1]: http://mercurial.selenic.com/wiki/DeveloperRepos

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Jörn Hees]

On 24.07.2013, at 15:14, Antoine Pelisse <apelisse@gmail.com> wrote:

> On Wed, Jul 24, 2013 at 11:59 AM, Jörn Hees <dev@joernhees.de> wrote:
>> On 24.07.2013, at 10:52, Antoine Pelisse <apelisse@gmail.com> wrote:
>>> I think the best way would be to create the shared repository in
>>> .git/hg/$share, with $share being a path that can't be a remote name
>>> (so that it doesn't conflict with remote directories),
>> 
>> Maybe ".git/hg/.share"?
> 
> According to Documentation/git-check-ref-format.txt, I'm not sure if
> we should start with a dot, or end with it.

I favor starting with a dot as it's nothing the user should fiddle with ;)

>>> That way, the share can be created even if .git/hg already exists
>>> (because of a previous import, before the shared machinery existed, or
>>> because you already have a local remote).
>> 
>> I like the idea of having independent remotes (fetching one, doesn't update another). http://mercurial.selenic.com/wiki/ShareExtension warns about this, and i wasn't sure it wouldn't cause intricate bugs. > This is why I opted for the explicit cloning, no shared history for several remotes.
> 
> I think the goal of using sharing here is that Mercurial and Git may
> use different schemes to handle branches. Mercurial may lead you to
> have separate repositories for each branch (They seem to do it for its
> own development [1]). All these branches actually share most of the
> same history, and are fully related, and we usually handle this
> situation in Git with one repository with multiple branches.
> Using "hg share", we allow a smooth transition from Mercurial model to
> Git model by merging all Mercurial repositories into one, and then map
> this single repository to the Git repository.
> IOW, the goal is to have only one copy of each "hg object" that are
> shared amongst many "remotes" (and potentially import them only once,
> though I don't think it currently works for me).

Alright, i just tested it out by sharing several repos and pushing to one of them, then fetching all again. Behavior seems as expected, so the remotes and their branches shown are isolated correctly.
Plus the initial fetching is quite a lot faster, less disk space used, etc…
So i think this is the way to go, thanks for the nudge.


>>>> Changing gitdir to dirname causes shared_path ==
>>>> .git/hg/<remote_name>/hg. The call to hg.share with local_path ==
>>>> .git/hg/<remote_name>/clone works again.
>>> 
>>> I think that will be a problem, because then the shared_path will no
>>> longer be shared, will it ?
>> 
>> Yupp, the shared_paths won't be shared, so it's not as optimal as possible, but it will work at least ;)
> 
> If we decided to remove the sharing idea, I think we should revert
> Felipe's commit rather than leave the shared_path variable, and call
> hg.share() on repository we don't even mean to share. That would be
> very confusing.

+1

I'll prepare a v2 of the patch.

Cheers,
Jörn

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> On Wed, Jul 24, 2013 at 11:59 AM, Jörn Hees <dev@joernhees.de> wrote:
>> On 24.07.2013, at 10:52, Antoine Pelisse <apelisse@gmail.com> wrote:
>>> I think the best way would be to create the shared repository in
>>> .git/hg/$share, with $share being a path that can't be a remote name
>>> (so that it doesn't conflict with remote directories),
>>
>> Maybe ".git/hg/.share"?
>
> According to Documentation/git-check-ref-format.txt, I'm not sure if
> we should start with a dot, or end with it.

What are in these directories under .git/hg?  Surely they cannot be
refs in Git's sense, as that hierarchy is not known to anything and
will not be protected from "git gc".

Puzzled...

	Goes and looks...

OK, the tracking branches for these are created under refs/hg/*
using the same name.

A refname shouldn't begin or end with a dot, because the range

	master...share

will become ambiguous if you allowed ".share" as a refname
shorthand.  It could mean either one of these:

	master..refs/heads/.share
        master...refs/heads/share

The same for the trailing dot "share."; the range "share...master"
becomes ambiguous.

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Jörn Hees]

On 24 Jul 2013, at 17:20, Junio C Hamano <gitster@pobox.com> wrote:

> Antoine Pelisse <apelisse@gmail.com> writes:
> 
>> On Wed, Jul 24, 2013 at 11:59 AM, Jörn Hees <dev@joernhees.de> wrote:
>>> On 24.07.2013, at 10:52, Antoine Pelisse <apelisse@gmail.com> wrote:
>>>> I think the best way would be to create the shared repository in
>>>> .git/hg/$share, with $share being a path that can't be a remote name
>>>> (so that it doesn't conflict with remote directories),
>>> 
>>> Maybe ".git/hg/.share"?
>> 
>> According to Documentation/git-check-ref-format.txt, I'm not sure if
>> we should start with a dot, or end with it.
> 
> What are in these directories under .git/hg?  Surely they cannot be
> refs in Git's sense, as that hierarchy is not known to anything and
> will not be protected from "git gc".
> 
> Puzzled...
> 
> 	Goes and looks...
> 
> OK, the tracking branches for these are created under refs/hg/*
> using the same name.
> 
> A refname shouldn't begin or end with a dot, because the range
> 
> 	master...share
> 
> will become ambiguous if you allowed ".share" as a refname
> shorthand.  It could mean either one of these:
> 
> 	master..refs/heads/.share
>        master...refs/heads/share
> 
> The same for the trailing dot "share."; the range "share...master"
> becomes ambiguous.


I think there is a slight misunderstanding here:
.git/hg/<remote_name> will be the actual directory for a hg:: remote, which will then use mercurial internal magic to refer to the shared repo .git/hg/.shared in case the remote is not somewhere on the local filesystem, otherwise that path is used.

What will appear in the refs is something like: hg/<remote_name>/{branches,bookmarks}/{master,default,…}.
So the .shared will correctly never appear in a git ref, which is what we want. It can also not clash with a remote as ".shared" is not a valid name… also what we want ;)

Cheers,
Jörn

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Junio C Hamano]

Jörn Hees <dev@joernhees.de> writes:

> On 24 Jul 2013, at 17:20, Junio C Hamano <gitster@pobox.com> wrote:
>
>> 	Goes and looks...
>> 
>> OK, the tracking branches for these are created under refs/hg/*
>> using the same name.
>> ...
>> A refname shouldn't begin or end with a dot, because the range
>> ... ellided a correct description of the reason behind rule, which is
>> ... irrelevant to the topic
>
> I think there is a slight misunderstanding here:
>
> .git/hg/<remote_name> will be the actual directory for a hg::
> remote, which will then use mercurial internal magic to refer to
> the shared repo .git/hg/.shared in case the remote is not
> somewhere on the local filesystem, otherwise that path is used.

Yes, it is not a "slight" but a "huge" misunderstanding.

I saw the caller of get_repo(url, alias) doing a

    'refs/hg/%s' % alias

immediately after the call, and somehow assumed that the proposal
would result in stuffing .shared to the "alias", which is not the
case.  The hg/$name directories may be used to store Hg repositories
for 'alias' that is passed by the caller, but hg/.shared thing you
guys were discussing is internal to get_repo() implementation and
there won't be refs/hg/.shared created because of this change.

Re: [PATCH] [SIGNED-OFF] remotes-hg: bugfix for fetching non local remotes

[Felipe Contreras]

On Wed, Jul 24, 2013 at 8:14 AM, Antoine Pelisse <apelisse@gmail.com> wrote:

> IOW, the goal is to have only one copy of each "hg object" that are
> shared amongst many "remotes" (and potentially import them only once,
> though I don't think it currently works for me).

That's right. I had code to import only once, but it didn't work
correctly; we would need a way to have shared fast-import/export
marks, and I don't think it's even possible from Mercurial's API to
figure out which objects are shared and which specific, so I gave up
on that. Sharing the repository is the only thing we can do safely and
sanely.

-- 
Felipe Contreras