Torsten Bögershausen <tboegi@xxxxxx> writes:
> I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
> The remote-helper tests for hg-git worked OK
> with both hg version 2.9 and 3.0 under both Mac OS and Linux.
>
> Should we consider 58aee086 to be included in Git 2.0 ?
It is way too late for Git 2.0, unless we are willing to pretend
that it is before 2.0-rc1, and cut one rc release tarball or two and
delay the final release by a few weeks. And "I tested it with 2.9
and 3.0" is not the kind of test that is sufficient before the
release (e.g. my desktop seems to have 2.0.2-1ubuntu1); it is the
kind of test we want to see before the patch is sent to the list,
which I know Felipe did.
A clarification for new people might be in order, as there seem to
be some confusions in some recent threads.
Code freeze in preparation for the next release is *not* the time to
test new code. It is time to catch regressions by asking wider
audiences who do not normally follow Git development (i.e. those who
are not the ones that follow 'master' and rebuild/install it once or
twice a week for their daily use).
These people (and I am one of them for other projects whose products
I use myself and whose development I do not follow) may be curious
what will be in the upcoming release, but what they care more about
is that the upcoming release will *not* break their established
usage. If the version of Git they currently run allows them to do
something, and if the upcoming release stops them from doing that
thing they care, that will prevent them from using the new version
of Git.
That is what we want to catch by announcing release candidate
tarballs; in exchange for giving an early access to those who do not
always live on the edge by having a clone of git.git and following
its development, we want them to catch regressions and report to us,
so that we can "fix" (and the fix often is to revert when deep in
the release candidate cycles).
On the other hand, if their current Git did not allow them to do
something, and if the new version still does not, it is unfortunate
but it is the same flaw they have been living with.
My understanding is that versions of remote-hg shipped with all
versions of Git did not work with Hg 3.0, so 58aee08 is not a
regression fix at all. Is working with Hg 3.0 at such a high
priority that it makes it worth to slip the whole release schedule
by a few weeks? I do not think so.
Another thing to consider is that fewer and fewer people test later
release candidates, so issuing a 2.0-rc4 tarball that wasn't
scheduled and giving it a soak time of two weeks will not get as
wide a testing as we would get for 2.0-rc0/rc1.
So the answer to your question is an unambiguous "no".
It is a different matter if we want a change to allow us to operate
with both older and newer version of mercurial in a release of Git
in near future. The answer is a resounding "yes", even if the
solution may not be the exact 58aee0864 commit [*2*]. I think an
update should eventurally go to the maintenance tracks of even older
releases, perhaps maint-1.8.5 and upwards, after we merge it to
'master' in preparation for the feature release that comes after Git
2.0, which probably will be called 2.1 but do not quote me on that.
Regarding the commit in question, I asked Felipe a question and
never got a straight answer.
Why do we "import changegroup" unconditionally, even though it
is only used in the new codepath meant only for version 3.0 or
higher, not inside the "if" block that decides if we need that
module?
I had a few more questions in mind that I didn't have a chance to
ask, and the commit log message did not explain.
Is the reason why this fix is needed is because "repo" stopped
being a way to ask ".getbundle()" and in the new world order
"changegroup" is the new way to ask ".getbundle()"?
If the answer is "yes", then asking "are we with 3.0 or
later---if so ask changegroup for ".getbundle()?", which is this
patch does, may not be the right condition to trigger the new
codepath. "Does repo know how to do .getbundle()? If not,
import changegroup and ask that module to perform .getbundle()
instead" may be a way to base the decision on a more directly
relevant condition.
Answers to these questions, if they came, were meant to convince
myself that the patch 58aee0864 is the best solution to the problem,
but because I only got "Of course it is not a mistake" [*1*], seeing
it did not lead to a productive discussion, I gave up. So I haven't
even managed to convince myself that that commit is the best
solution to the problem.
[References]
*1* http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248348
*2*
commit 58aee0864adeeb5363f2a06728596f9c9315811f
Author: Felipe Contreras <felipe.contreras@xxxxxxxxx>
Date: Sat May 3 21:16:54 2014 -0500
remote-hg: add support for hg v3.0
Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 34cda02..8b02803 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -13,6 +13,7 @@
# "$GIT_DIR/hg/origin/clone/.hg/".
from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
+from mercurial import changegroup
import re
import sys
@@ -985,7 +986,10 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
if not checkheads(repo, remote, p_revs):
return None
- cg = repo.getbundle('push', heads=list(p_revs), common=common)
+ if check_version(3, 0):
+ cg = changegroup.getbundle(repo, 'push', heads=list(p_revs), common=common)
+ else:
+ cg = repo.getbundle('push', heads=list(p_revs), common=common)
unbundle = remote.capable('unbundle')
if unbundle:
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html