Latest autoninja.py in depot_tools is broken on Windows |
||||||
Issue descriptionExecute autoninja.bat to build Chrome/Chromium What is the expected result? ninja runs and builds targets. What happens instead? Error message printed: The filename, directory name, or volume label syntax is incorrect.
,
Nov 7
,
Nov 8
Thanks for the fix. I guess quote might also be needed for non-Windows because people may put ninja under directories with ' '.
,
Nov 8
autoninja.py was broken on Windows even without spaces in directory names. It was completely broken on Windows. depot_tools doesn't work in directories with spaces anyway, so there won't be any of those out there, I'm pretty sure. I don't fully understand why the quotes were needed, but they were.
,
Nov 8
I am trying to understand how returned value of autoninja.py is evaluated by cmd.exe, but I am not sure. Technically, no documentation prohibit to put depot_tools under directories with spaces, let me try it to work with that. Also, I would like to change the code shellcheck free.
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4186454ed32ad6d9f2a41378a87e1d599fe9c18 commit b4186454ed32ad6d9f2a41378a87e1d599fe9c18 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Thu Nov 08 03:01:50 2018 Roll src/third_party/depot_tools 49c8eafcd54c..75fa8552337b (1 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/49c8eafcd54c..75fa8552337b git log 49c8eafcd54c..75fa8552337b --date=short --no-merges --format='%ad %ae %s' 2018-11-07 kylixrd@chromium.org Quote the full path to ninja.exe. Created with: gclient setdep -r src/third_party/depot_tools@75fa8552337b The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG= chromium:902930 TBR=agable@chromium.org Change-Id: I1b849e997ef8e1b4a02affbd5dc8575bfb3b8664 Reviewed-on: https://chromium-review.googlesource.com/c/1324856 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#606307} [modify] https://crrev.com/b4186454ed32ad6d9f2a41378a87e1d599fe9c18/DEPS
,
Nov 8
autoninja is still broken on git bash on Windows: $ ninja -C out/Debug chrome ui_base_unittests "C:\src\depot_tools\ninja.exe" -C out/Debug chrome ui_base_unittests -j 1120 /c/src/depot_tools/autoninja: line 14: "C:\src\depot_tools\ninja.exe": No such file or directory I will try to figure out what's up but I have only a few minutes right now.
,
Nov 8
The culprit is kylixrd's patch on this bug. Rolling it back locally fixes git bash.
,
Nov 9
eval should be needed for shell. I have written the cl for that. https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1325249
,
Nov 9
,
Nov 14
This is a p0 that's seen no activity for 5 days. Is the patch in comment 9 the solution here? Does something else need to occur?
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/43a35d299bbc044957eea5f82eae5a2f189a2b3c commit 43a35d299bbc044957eea5f82eae5a2f189a2b3c Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Date: Thu Nov 15 03:00:51 2018 autoninja: quote the arguments when needed. On Windows, without quote, whole return value would be treated as a command, and the execution would fail. On Linux and Mac, without quote, if depot_tools is settled under directories with ' ', command execution would fail because paths are separated in a wrong way. To make such a return value work on Linux and Mac, the shell script started to use eval. Bug: 902930 Change-Id: I9bb74585294af565988c0b844b6b113a5c685530 Reviewed-on: https://chromium-review.googlesource.com/c/1325249 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org> [modify] https://crrev.com/43a35d299bbc044957eea5f82eae5a2f189a2b3c/autoninja.py [modify] https://crrev.com/43a35d299bbc044957eea5f82eae5a2f189a2b3c/autoninja
,
Nov 15
I believe it fixed by #13.
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ad0bdf7c41df7d9b2d12a011f93add529c687f45 commit ad0bdf7c41df7d9b2d12a011f93add529c687f45 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Thu Nov 15 04:03:24 2018 Roll src/third_party/depot_tools f66e5510327b..43a35d299bbc (1 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/f66e5510327b..43a35d299bbc git log f66e5510327b..43a35d299bbc --date=short --no-merges --format='%ad %ae %s' 2018-11-15 yyanagisawa@chromium.org autoninja: quote the arguments when needed. Created with: gclient setdep -r src/third_party/depot_tools@43a35d299bbc The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG= chromium:902930 TBR=agable@chromium.org Change-Id: Ie4967480952faab9f57be654ea1bb7f6042ec17d Reviewed-on: https://chromium-review.googlesource.com/c/1336926 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#608250} [modify] https://crrev.com/ad0bdf7c41df7d9b2d12a011f93add529c687f45/DEPS
,
Nov 15
Thanks very much yyanagisawa! Verified things work on my end. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Nov 7