Patchwork [1,of,3] subrepo: only retrieve the first two components of the Git version

login
register
mail settings
Submitter Siddharth Agarwal
Date March 21, 2014, 2:58 a.m.
Message ID <fae4002ec0e4060ffe84.1395370697@dev1738.prn1.facebook.com>
Download mbox | patch
Permalink /patch/4014/
State Accepted
Commit 6a2acb0d9352d5be5c495ad332497ad60a19578e
Headers show

Comments

Siddharth Agarwal - March 21, 2014, 2:58 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1395369421 25200
#      Thu Mar 20 19:37:01 2014 -0700
# Node ID fae4002ec0e4060ffe84d6fb11d8ab1f566b2aa1
# Parent  170d6d591a7dbc09bfe1b509dfd8f39991e653a9
subrepo: only retrieve the first two components of the Git version

This makes the version detection compatible with Git versions like '1.9-rc0'.
We only cared about the first two components of the version anyway.
David Soria Parra - March 21, 2014, 10 p.m.
Siddharth Agarwal <sid0@fb.com> writes:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1395369421 25200
> #      Thu Mar 20 19:37:01 2014 -0700
> # Node ID fae4002ec0e4060ffe84d6fb11d8ab1f566b2aa1
> # Parent  170d6d591a7dbc09bfe1b509dfd8f39991e653a9
> subrepo: only retrieve the first two components of the Git version
>
> This makes the version detection compatible with Git versions like '1.9-rc0'.
> We only cared about the first two components of the version 

Series looks good to me.

I would be nice to have tests for this by providing a bashscript that
replaces in the necessary test
Siddharth Agarwal - March 21, 2014, 10:29 p.m.
On 03/21/2014 03:00 PM, David Soria Parra wrote:
> Siddharth Agarwal <sid0@fb.com> writes:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1395369421 25200
>> #      Thu Mar 20 19:37:01 2014 -0700
>> # Node ID fae4002ec0e4060ffe84d6fb11d8ab1f566b2aa1
>> # Parent  170d6d591a7dbc09bfe1b509dfd8f39991e653a9
>> subrepo: only retrieve the first two components of the Git version
>>
>> This makes the version detection compatible with Git versions like '1.9-rc0'.
>> We only cared about the first two components of the version
> Series looks good to me.
>
> I would be nice to have tests for this by providing a bashscript that
> replaces in the necessary test

I actually spent some time looking at this. Doing this as an integration 
test (i.e. unified shell script) is supremely annoying because we only 
want to capture the version command and forward everything else to 
regular git. Turning version detection into a pure function and doing 
this as a doctest is likely possible but I'm not convinced the 
additional refactoring effort is worth it.


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - March 21, 2014, 10:38 p.m.
On Fri, 2014-03-21 at 15:29 -0700, Siddharth Agarwal wrote:
> On 03/21/2014 03:00 PM, David Soria Parra wrote:
> > Siddharth Agarwal <sid0@fb.com> writes:
> >
> >> # HG changeset patch
> >> # User Siddharth Agarwal <sid0@fb.com>
> >> # Date 1395369421 25200
> >> #      Thu Mar 20 19:37:01 2014 -0700
> >> # Node ID fae4002ec0e4060ffe84d6fb11d8ab1f566b2aa1
> >> # Parent  170d6d591a7dbc09bfe1b509dfd8f39991e653a9
> >> subrepo: only retrieve the first two components of the Git version
> >>
> >> This makes the version detection compatible with Git versions like '1.9-rc0'.
> >> We only cared about the first two components of the version
> > Series looks good to me.
> >
> > I would be nice to have tests for this by providing a bashscript that
> > replaces in the necessary test
> 
> I actually spent some time looking at this. Doing this as an integration 
> test (i.e. unified shell script) is supremely annoying because we only 
> want to capture the version command and forward everything else to 
> regular git. Turning version detection into a pure function and doing 
> this as a doctest is likely possible but I'm not convinced the 
> additional refactoring effort is worth it.

This would be my preferred way to do this, if anyone wants to follow-up.

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -1107,17 +1107,17 @@ 
                 raise
             self._gitexecutable = 'git.cmd'
             out, err = self._gitnodir(['--version'])
-        m = re.search(r'^git version (\d+)\.(\d+)\.(\d+)', out)
+        m = re.search(r'^git version (\d+)\.(\d+)', out)
         if not m:
             self._ui.warn(_('cannot retrieve git version'))
             return
-        version = (int(m.group(1)), m.group(2), m.group(3))
+        version = (int(m.group(1)), m.group(2))
         # git 1.4.0 can't work at all, but 1.5.X can in at least some cases,
         # despite the docstring comment.  For now, error on 1.4.0, warn on
         # 1.5.0 but attempt to continue.
-        if version < (1, 5, 0):
+        if version < (1, 5):
             raise util.Abort(_('git subrepo requires at least 1.6.0 or later'))
-        elif version < (1, 6, 0):
+        elif version < (1, 6):
             self._ui.warn(_('git subrepo requires at least 1.6.0 or later'))
 
     def _gitcommand(self, commands, env=None, stream=False):