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

[Mads Kiilerich]

Gilles Moris wrote, On 08/09/2009 07:46 PM:
>   setup.py |  23 +++++++++++++++++++++++
>   1 files changed, 23 insertions(+), 0 deletions(-)
>
>
> # HG changeset patch
> # User Gilles Moris<gilles.moris at free.fr>
> # Date 1249839101 -7200
> # Node ID f2d442385d9c8e9fb82f9acc030371de6d7b9573
> # Parent  5819241694828b709e33b4ebcbcb74dbafc48857
> setup: improve tag retrieval for archives and fallback to lasttag otherwise
>
> 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

> +        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.

>   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.)

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

lasttag = line.split(':', 1)[1].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 ...

/Mads