mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.51k stars 1.6k forks source link

Incorrect escaping of vcs_tag command #4285

Open ePirat opened 5 years ago

ePirat commented 5 years ago

In your meson.build we use the following command for version.h generation:

dav1d_git_dir = join_paths(dav1d_src_root, '.git')
rev_target = vcs_tag(command: [
        'git', '--git-dir', dav1d_git_dir,
        'describe', '--tags', '--long',
        '--match', '?.*.*', '--always'
    ],
    input: 'version.h.in',
    output: 'version.h'
)

Copy-pasting from our bug report:

The issue lays in the build.ninja file in the build directory. It happens on Windows/MSYS2 hosts only AFAIK. The culprit is the line

COMMAND = "D:/msys64/mingw64/bin/meson" "--internal" "vcstagger" "../include/version.h.in" "include/version.h" "0.0.1" "C:/Users/utente/Desktop/audio/dav1d/include" "@VCS_TAG@" "(.*)" "D:/msys64/usr/bin/git.EXE" "--git-dir" "C:/Users/utente/Desktop/audio/dav1d/.git" "describe" "--tags" "--long" "--match" "?.*.*" "--always"

The file '.ninja_log' gets caught in the regular expression expanded by MSYS2 bash and is fed to Git, which complains because .ninja_log is not, of course, a regular expression. The trick is simply escaping the ?, thus making the regular expression "\?.*.*" I think this needs to be fixed by Meson upstream.

I tried escaping it in the commands array but for whatever reason meson changes the \ into a /.

axxel commented 5 years ago

I had a look at the source code and believe the problem is the following line: https://github.com/mesonbuild/meson/blob/master/mesonbuild/backend/ninjabackend.py#L40. I'm not a windows + ninja + shell expert but from my understanding, you should be able to fix this by changing that line to:

quote_func = lambda s: '"{}"'.format(shlex.quote(s))
jpakkane commented 5 years ago

shlex.quote does unix shell quoting, it does not work on Windows which uses cmd.exe. It has different quoting rules.

axxel commented 5 years ago

I had a look one level deeper and agree with @jpakkane, that shlex.quote is not the right tool here. But it is not obvious where 'cmd.exe' would come into the picture. According to the reference manual of ninja, the COMMAND line gets passed to sh -c on unix but not to cmd.exe /c on windows but rather to CreateProcess directly. The process in this case is meson (or rather python?) which is directed to call the 'internal' script vcstagger.py, which in turn uses subprocess.check_output with the shell parameter set to FALSE.

It seems to me the call of D:/msys64/mingw64/bin/meson might implicitly invoke cmd.exe? Is that just the .py file or a py2exe-style binary? Anyway, the simple 'enclose the arguments in double quotes' approach in the above mentioned line is clearly not working. See also https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/