New issue
Advanced search Search tips

Issue 890744 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

autoninja requires space after "-C" (while ninja itself doesn't)

Project Member Reported by treib@chromium.org, Oct 1

Issue description

ninja happily accepts both "-C out/Default" and "-Cout/Default" (with or without space), but the second format breaks autoninja's Goma detection, because autoninja looks for "-C" as a separate argument [1].

Not a big deal, but it took me a while to figure out why autoninja builds were taking so long ;)

[1] https://cs.chromium.org/chromium/tools/depot_tools/autoninja.py?rcl=95d4c855637a039f4befbef17af1708ae06386b0&l=38
 
Ah - I didn't realize that the space was optional. Command line parsing is arbitrarily messy. I can create a fix or would you like to? It should be easy enough.
Owner: brucedaw...@chromium.org
Status: Assigned (was: Untriaged)
Assigning to myself to ensure it doesn't get neglected, but feel free to grab this bug and create a patch.
Any fixes should be made to post_build_ninja_summary.py as well, since it is optionally invoked from autoninja.bat as well.
Status: Started (was: Assigned)
Note that post_build_ninja_summary.py handles this by default because it uses normal command-line parsing. CL up for review for autoninja.py.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/6af3aa85495695d609a881ad1c75fd2aa5407135

commit 6af3aa85495695d609a881ad1c75fd2aa5407135
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Wed Oct 03 16:39:42 2018

Support omitting the space after -C

Nina supports -C out/Default and -Cout/Default to specify the build
directory so autoninja should also. This change adds that support.

Bug:  890744 
Change-Id: I5e824242ed4b333ac99f1ee9a649ffcfa03a812e
Reviewed-on: https://chromium-review.googlesource.com/c/1257586
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

[modify] https://crrev.com/6af3aa85495695d609a881ad1c75fd2aa5407135/autoninja.py

Status: Fixed (was: Started)
Give it a try, but I think this is working correctly now.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32b130667fdebd50a87c28e652a435bdb2decf25

commit 32b130667fdebd50a87c28e652a435bdb2decf25
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Wed Oct 03 19:16:33 2018

Roll src/third_party/depot_tools 22300e1fb562..6af3aa854956 (1 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/22300e1fb562..6af3aa854956


git log 22300e1fb562..6af3aa854956 --date=short --no-merges --format='%ad %ae %s'
2018-10-03 brucedawson@chromium.org Support omitting the space after -C


Created with:
  gclient setdep -r src/third_party/depot_tools@6af3aa854956

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:890744 
TBR=agable@chromium.org

Change-Id: Icd16a680abeca3e8c534c04e6ad6a0e743a06bbd
Reviewed-on: https://chromium-review.googlesource.com/c/1259103
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@{#596317}
[modify] https://crrev.com/32b130667fdebd50a87c28e652a435bdb2decf25/DEPS

Status: Verified (was: Fixed)
Works like a charm now, thanks!

Sign in to add a comment