[PATCH 3 of 3] setup: improve tag retrieval for archives and fallback to lasttag otherwise

Gilles Moris gilles.moris at free.fr
Sun Aug 9 15:43:12 CDT 2009


On Sun August 9 2009 21:29:12 Mads Kiilerich wrote:
> > diff --git a/setup.py b/setup.py
> > --- a/setup.py
> > +++ b/setup.py
> > @@ -135,12 +135,35 @@
> >               version = l[-1] # latest tag or revision number
> >               if version.endswith('+'):
> >                   version += time.strftime('%Y%m%d')
> >    
> 
> If the working directory is dirty then we might want to include the 
> timestamp also in the next case
> 

It is already not working for if we are on a tag.
That's why I was thinking about an independent patch.

> > +        if len(l) == 1: # no tag found for that rev
> > +            cmd = [sys.executable, 'hg', 'parents',
> > +                   '--template', '{lasttag}:{lasttagdistance}']
> >    
> 
> Slightly confusing that you don't use '+' as separator here. But you are 
> right, ':' is more stable for parsing. But again, why parse the output? 
> Why not generate the right format and use it directly?
> 
> > +            p = subprocess.Popen(cmd, stdout=subprocess.PIPE,
> > +                                 stderr=subprocess.PIPE, env=env)
> > +            out, err = p.communicate()
> > +
> > +            err = [e for e in err.splitlines()
> > +                   if not e.startswith('Not trusting file')]
> > +            if err:
> > +                sys.stderr.write('warning: could not establish Mercurial '
> > +                                 'version:\n%s\n' % '\n'.join(err))
> > +            else:
> > +                l = out.split(':')
> >    
> 
> l = out.rsplit(':', 1)
> 
> 
> > +                # append distance to the last tag
> > +                version += ' (%s+%s)' % (l[0], l[-1])
> >    
> 
> Why -1? The list is assumed/known to have length 2, so index 1 would be 
> more intuitive.

I guess I will just generate the right format directly, as you suggest, to
simplify all that.

> 
> >   elif os.path.exists('.hg_archival.txt'):
> >       hgarchival = open('.hg_archival.txt')
> > +    lasttag = None
> >       for line in hgarchival:
> >           if line.startswith('node:'):
> >               version = line.split(':')[1].strip()[:12]
> > +        if line.startswith('tag:'):
> > +            version = line.split(':')[1].strip()
> >               break
> >    
> 
> It took me some time to figure out that this handles the situation where 
> lasttagdistance is 0. That might deserve a comment.
> 
> (It could perhaps be convenient to also include the hash in the version 
> string in all cases, but that is a different story.)
> 

That's not the case if we are on a tag, but in any other case we have a hash.
You want to stick with that, right ?

> > +        if line.startswith('lasttag:'):
> > +            lasttag = line.split(':')[1].strip()
> >    
> 
> lasttag = line.split(':', 1)[1].strip()

Or may be even simpler:
  line[9:].strip()

> 
> > +        if lasttag and line.startswith('lasttagdistance:'):
> > +            version += ' (%s+%s)' % (lasttag, line.split(':')[1].strip())
> >    
> 
> This introduces dependencies to the order of the lines in .hg_archivel; 
> lasttagdistance must come after lasttag and no node lines between. 
> Perhaps a less fragile parser would be less ... fragile ...
> 

Normally, we control that piece of code, and lasttag comes first.
But it should not be that difficult to make it more robust.

Another question about the format:
# hg version -q
Mercurial Distributed SCM (version f61394b23964 (1.3.1+81))

Do we want to replace parenthesis by brace or bracket ?
Spaces around the "+" ?

Regards.
Gilles.


More information about the Mercurial-devel mailing list