[PATCH] hooks: allow Unix style environment variables on external Windows hooks

Yuya Nishihara yuya at tcha.org
Wed Jul 5 13:01:20 UTC 2017


On Tue, 04 Jul 2017 23:39:59 -0400, Matt Harbison wrote:
> On Tue, 04 Jul 2017 09:13:46 -0400, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> > On Tue, 04 Jul 2017 12:42:08 +0900, FUJIWARA Katsunori wrote:
> >> At Mon, 03 Jul 2017 21:57:01 -0400,
> >> Matt Harbison wrote:
> >> > On Mon, 03 Jul 2017 12:15:49 -0400, FUJIWARA Katsunori
> >> > <foozy at lares.dti.ne.jp> wrote:
> >> > >> >  def _exthook(ui, repo, htype, name, cmd, args, throw):
> >> > >> > +    if pycompat.osname == 'nt':
> >> > >> > +        # Replace Unix style environment variables with Windows
> >> > >> style variables,
> >> > >> > +        # for replacement by cmd.exe.  Treat '\$' as a literal  
> >> $.
> >> > >> > +        def replacer(m):
> >> > >> > +            prefix = m.group(1)
> >> > >> > +            if prefix == '\\':
> >> > >> > +                return '$' + m.group(2)
> >> > >> > +            return prefix + '%' + m.group(2) + '%'
> >> > >> > +
> >> > >> > +        cmd = re.sub(br'(.?)\$(.+?)\b', replacer, cmd)
> >> > >
> >> > > How about '(.?)\$([A-Za-z_]\w*)' for more strict matching against
> >> > > variable name?
> >> >
> >> > Unfortunately, that strictness is... optimistic:
> >> >
> >> > https://ss64.com/nt/syntax-variables.html
> >> >
> >> > Names can include spaces, '{', '}', and '$'.  Ugh.
> >> >
> >> > Or are you saying we don't need to support those characters, because  
> >> they
> >> > wouldn't be portable anyway?  (The fact that there are built-ins with  
> >> some
> >> > non portable characters like 'COMMONPROGRAMFILES(X86)' gives me slight
> >> > pause, though I've never seen any of the other characters in practice.
> >> > And mostly the convenience is when using variables exported by hg.)
> >>
> >> Yes, I think that platform specific characters and features
> >> (substitutions, and so on) can be omitted, because main concern of
> >> this patch is portability of external hook command line between
> >> platforms.
> >
> > FWIW, I found ntpath.expandvars() does support $name and ${name} on  
> > Windows.
> >
> > https://docs.python.org/2/library/os.path.html#os.path.expandvars
> 
> Thanks, I forgot about that too.  I switched to that, and got some  
> unexpected failures.  I'll have to dig in more, and that should give time  
> for stable to merge back into default to pick up the help text changes.

Oops, I wanted to tell you that we could copy the expansion rule (and maybe
a part of the implementation.) Variable expansion should be done only once,
but cmd.exe will do that again.

> BTW, if I end up using this, should it be conditionalized to just  
> Windows?  (In theory, it shouldn't matter for non-Windows, but just in  
> case...)

Just Windows. Unix shell has much more complex syntax.



More information about the Mercurial-devel mailing list