Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1152823005:
Use script in depot_tools to retrieve builders from new cq.cfg (Closed)
|
Created:
5 years, 6 months ago by Sergiy Byelozyorov Modified:
5 years, 6 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse script in depot_tools to retrieve builders from new cq.cfg
R=pgervais@chromium.org
TEST=ran git-cl-try on this CL
Committed: https://crrev.com/87e80c7a628381644013e3b96b437276d9b19bcd
Cr-Commit-Position: refs/heads/master@{#332200}
Patch Set 1 #Patch Set 2 : Added an import #Patch Set 3 : Fix #Patch Set 4 : Renamed variable #
Total comments: 4
Patch Set 5 : Removed old config since it is not used anymore #Patch Set 6 : Addressed comments #
Total comments: 6
Patch Set 7 : Added sys.executable #Patch Set 8 : Addressed comments #Patch Set 9 : Removed unneeded import #Patch Set 10 : Also pass shell=True to allow resolution of the binary without extension on Windows #
Messages
Total messages: 34 (12 generated)
lgtm with one nit. https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py#newcode1794 PRESUBMIT.py:1794: ['commit_queue.py', 'builders', cq_config_path])) The name 'commit_queue.py' is so confusing in this context. There's nothing we can do about it right now, but we should keep that in mind. https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py#newcode1796 PRESUBMIT.py:1796: # Explicitly iterate over copies of dicts since we mutate them. 'of dicts' -> 'of keys' ?
https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py#newcode1794 PRESUBMIT.py:1794: ['commit_queue.py', 'builders', cq_config_path])) On 2015/05/29 17:05:51, pgervais wrote: > The name 'commit_queue.py' is so confusing in this context. There's nothing we > can do about it right now, but we should keep that in mind. I'll add a comment. https://codereview.chromium.org/1152823005/diff/60001/PRESUBMIT.py#newcode1796 PRESUBMIT.py:1796: # Explicitly iterate over copies of dicts since we mutate them. On 2015/05/29 17:05:50, pgervais wrote: > 'of dicts' -> 'of keys' ? Done.
The CQ bit was checked by sergiyb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pgervais@chromium.org Link to the patchset: https://codereview.chromium.org/1152823005/#ps100001 (title: "Addressed comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1152823005/100001
sergiyb@chromium.org changed reviewers: + brettw@chromium.org, cpu@chromium.org, jam@chromium.org
Added a few folks to review for PRESUBMIT.py rubber-stump
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') how does this work? i.e. if i do fetch chromium, I don't have infra/config?
https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') On 2015/05/29 18:07:27, jam wrote: > how does this work? i.e. if i do fetch chromium, I don't have infra/config? Yes you do :-) CQ configuration now lives in project repositories, not in an hidden infra repository anymore.
https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') On 2015/05/29 18:14:45, pgervais wrote: > On 2015/05/29 18:07:27, jam wrote: > > how does this work? i.e. if i do fetch chromium, I don't have infra/config? > > Yes you do :-) CQ configuration now lives in project repositories, not in an > hidden infra repository anymore. ah, looks like the checkout i checked was a day old and it didn't have this file :) I just synced, applied this patch, and ran git cl try and got the following crash on Windows: F:\src\chrome2\src>git cl try Traceback (most recent call last): File "f:\src\depot_tools\git_cl.py", line 3349, in <module> sys.exit(main(sys.argv[1:])) File "f:\src\depot_tools\git_cl.py", line 3331, in main return dispatcher.execute(OptionParser(), argv) File "f:\src\depot_tools\subcommand.py", line 252, in execute return command(parser, args[1:]) File "f:\src\depot_tools\git_cl.py", line 2973, in CMDtry masters = GetMasterMap() File "f:\src\depot_tools\git_cl.py", line 2935, in GetMasterMap sys.stdout) File "f:\src\depot_tools\presubmit_support.py", line 1286, in DoGetTryMasters presubmit_script, filename, project, change)) File "f:\src\depot_tools\presubmit_support.py", line 1144, in ExecPresubmitScript return get_preferred_try_masters(project, change) File "<string>", line 1796, in GetPreferredTryMasters File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 566, in check_output process = Popen(stdout=PIPE, *popenargs, **kwargs) File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 709, in __init__ errread, errwrite) File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 957, in _execute_child startupinfo) WindowsError: [Error 193] %1 is not a valid Win32 application Sending crash report ... args: ['f:\\src\\depot_tools\\git_cl.py', 'try'] cwd: F:\src\chrome2\src exception: [Error 193] %1 is not a valid Win32 application host: JABDELMALEK3-W.ad.corp.google.com stack: File "f:\src\depot_tools\git_cl.py", line 3349, user: jabdelmalek version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 A stack trace has been sent to the maintainers.
https://chromiumcodereview.appspot.com/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://chromiumcodereview.appspot.com/1152823005/diff/100001/PRESUBMIT.py#ne... PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') On 2015/05/29 18:19:46, jam wrote: > On 2015/05/29 18:14:45, pgervais wrote: > > On 2015/05/29 18:07:27, jam wrote: > > > how does this work? i.e. if i do fetch chromium, I don't have infra/config? > > > > Yes you do :-) CQ configuration now lives in project repositories, not in an > > hidden infra repository anymore. > > ah, looks like the checkout i checked was a day old and it didn't have this file > :) > > I just synced, applied this patch, and ran git cl try and got the following > crash on Windows: > > F:\src\chrome2\src>git cl try > Traceback (most recent call last): > File "f:\src\depot_tools\git_cl.py", line 3349, in <module> > sys.exit(main(sys.argv[1:])) > File "f:\src\depot_tools\git_cl.py", line 3331, in main > return dispatcher.execute(OptionParser(), argv) > File "f:\src\depot_tools\subcommand.py", line 252, in execute > return command(parser, args[1:]) > File "f:\src\depot_tools\git_cl.py", line 2973, in CMDtry > masters = GetMasterMap() > File "f:\src\depot_tools\git_cl.py", line 2935, in GetMasterMap > sys.stdout) > File "f:\src\depot_tools\presubmit_support.py", line 1286, in DoGetTryMasters > presubmit_script, filename, project, change)) > File "f:\src\depot_tools\presubmit_support.py", line 1144, in > ExecPresubmitScript > return get_preferred_try_masters(project, change) > File "<string>", line 1796, in GetPreferredTryMasters > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 566, in > check_output > process = Popen(stdout=PIPE, *popenargs, **kwargs) > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 709, in > __init__ > errread, errwrite) > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 957, in > _execute_child > startupinfo) > WindowsError: [Error 193] %1 is not a valid Win32 application > Sending crash report ... > args: ['f:\\src\\depot_tools\\git_cl.py', 'try'] > cwd: F:\src\chrome2\src > exception: [Error 193] %1 is not a valid Win32 application > host: http://JABDELMALEK3-W.ad.corp.google.com > stack: File "f:\src\depot_tools\git_cl.py", line 3349, > user: jabdelmalek > version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 > A stack trace has been sent to the maintainers. sys.executable is missing on line 1796
https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') On 2015/05/29 18:23:32, pgervais wrote: > On 2015/05/29 18:19:46, jam wrote: > > On 2015/05/29 18:14:45, pgervais wrote: > > > On 2015/05/29 18:07:27, jam wrote: > > > > how does this work? i.e. if i do fetch chromium, I don't have > infra/config? > > > > > > Yes you do :-) CQ configuration now lives in project repositories, not in an > > > hidden infra repository anymore. > > > > ah, looks like the checkout i checked was a day old and it didn't have this > file > > :) > > > > I just synced, applied this patch, and ran git cl try and got the following > > crash on Windows: > > > > F:\src\chrome2\src>git cl try > > Traceback (most recent call last): > > File "f:\src\depot_tools\git_cl.py", line 3349, in <module> > > sys.exit(main(sys.argv[1:])) > > File "f:\src\depot_tools\git_cl.py", line 3331, in main > > return dispatcher.execute(OptionParser(), argv) > > File "f:\src\depot_tools\subcommand.py", line 252, in execute > > return command(parser, args[1:]) > > File "f:\src\depot_tools\git_cl.py", line 2973, in CMDtry > > masters = GetMasterMap() > > File "f:\src\depot_tools\git_cl.py", line 2935, in GetMasterMap > > sys.stdout) > > File "f:\src\depot_tools\presubmit_support.py", line 1286, in > DoGetTryMasters > > presubmit_script, filename, project, change)) > > File "f:\src\depot_tools\presubmit_support.py", line 1144, in > > ExecPresubmitScript > > return get_preferred_try_masters(project, change) > > File "<string>", line 1796, in GetPreferredTryMasters > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 566, in > > check_output > > process = Popen(stdout=PIPE, *popenargs, **kwargs) > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 709, in > > __init__ > > errread, errwrite) > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 957, in > > _execute_child > > startupinfo) > > WindowsError: [Error 193] %1 is not a valid Win32 application > > Sending crash report ... > > args: ['f:\\src\\depot_tools\\git_cl.py', 'try'] > > cwd: F:\src\chrome2\src > > exception: [Error 193] %1 is not a valid Win32 application > > host: http://JABDELMALEK3-W.ad.corp.google.com > > stack: File "f:\src\depot_tools\git_cl.py", line 3349, > > user: jabdelmalek > > version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 > > A stack trace has been sent to the maintainers. > > sys.executable is missing on line 1796 This would help, because python doesn't use $PATH or $PYTHONPATH to find scripts. Instead I've removed the extension and will add commit_queue.bat and commit_queue scripts to the depot_tools.
On 2015/05/29 18:57:55, Sergiy Byelozyorov wrote: > https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py > File PRESUBMIT.py (right): > > https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 > PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') > On 2015/05/29 18:23:32, pgervais wrote: > > On 2015/05/29 18:19:46, jam wrote: > > > On 2015/05/29 18:14:45, pgervais wrote: > > > > On 2015/05/29 18:07:27, jam wrote: > > > > > how does this work? i.e. if i do fetch chromium, I don't have > > infra/config? > > > > > > > > Yes you do :-) CQ configuration now lives in project repositories, not in > an > > > > hidden infra repository anymore. > > > > > > ah, looks like the checkout i checked was a day old and it didn't have this > > file > > > :) > > > > > > I just synced, applied this patch, and ran git cl try and got the following > > > crash on Windows: > > > > > > F:\src\chrome2\src>git cl try > > > Traceback (most recent call last): > > > File "f:\src\depot_tools\git_cl.py", line 3349, in <module> > > > sys.exit(main(sys.argv[1:])) > > > File "f:\src\depot_tools\git_cl.py", line 3331, in main > > > return dispatcher.execute(OptionParser(), argv) > > > File "f:\src\depot_tools\subcommand.py", line 252, in execute > > > return command(parser, args[1:]) > > > File "f:\src\depot_tools\git_cl.py", line 2973, in CMDtry > > > masters = GetMasterMap() > > > File "f:\src\depot_tools\git_cl.py", line 2935, in GetMasterMap > > > sys.stdout) > > > File "f:\src\depot_tools\presubmit_support.py", line 1286, in > > DoGetTryMasters > > > presubmit_script, filename, project, change)) > > > File "f:\src\depot_tools\presubmit_support.py", line 1144, in > > > ExecPresubmitScript > > > return get_preferred_try_masters(project, change) > > > File "<string>", line 1796, in GetPreferredTryMasters > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 566, in > > > check_output > > > process = Popen(stdout=PIPE, *popenargs, **kwargs) > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 709, in > > > __init__ > > > errread, errwrite) > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 957, in > > > _execute_child > > > startupinfo) > > > WindowsError: [Error 193] %1 is not a valid Win32 application > > > Sending crash report ... > > > args: ['f:\\src\\depot_tools\\git_cl.py', 'try'] > > > cwd: F:\src\chrome2\src > > > exception: [Error 193] %1 is not a valid Win32 application > > > host: http://JABDELMALEK3-W.ad.corp.google.com > > > stack: File "f:\src\depot_tools\git_cl.py", line 3349, > > > user: jabdelmalek > > > version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 > > > A stack trace has been sent to the maintainers. > > > > sys.executable is missing on line 1796 > > This would help, because python doesn't use $PATH or $PYTHONPATH to find > scripts. Instead I've removed the extension and will add commit_queue.bat and > commit_queue scripts to the depot_tools. Added binaries in https://codereview.chromium.org/1154623004
https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1152823005/diff/100001/PRESUBMIT.py#newcode1792 PRESUBMIT.py:1792: change.RepositoryRoot(), 'infra', 'config', 'cq.cfg') On 2015/05/29 18:57:55, Sergiy Byelozyorov wrote: > On 2015/05/29 18:23:32, pgervais wrote: > > On 2015/05/29 18:19:46, jam wrote: > > > On 2015/05/29 18:14:45, pgervais wrote: > > > > On 2015/05/29 18:07:27, jam wrote: > > > > > how does this work? i.e. if i do fetch chromium, I don't have > > infra/config? > > > > > > > > Yes you do :-) CQ configuration now lives in project repositories, not in > an > > > > hidden infra repository anymore. > > > > > > ah, looks like the checkout i checked was a day old and it didn't have this > > file > > > :) > > > > > > I just synced, applied this patch, and ran git cl try and got the following > > > crash on Windows: > > > > > > F:\src\chrome2\src>git cl try > > > Traceback (most recent call last): > > > File "f:\src\depot_tools\git_cl.py", line 3349, in <module> > > > sys.exit(main(sys.argv[1:])) > > > File "f:\src\depot_tools\git_cl.py", line 3331, in main > > > return dispatcher.execute(OptionParser(), argv) > > > File "f:\src\depot_tools\subcommand.py", line 252, in execute > > > return command(parser, args[1:]) > > > File "f:\src\depot_tools\git_cl.py", line 2973, in CMDtry > > > masters = GetMasterMap() > > > File "f:\src\depot_tools\git_cl.py", line 2935, in GetMasterMap > > > sys.stdout) > > > File "f:\src\depot_tools\presubmit_support.py", line 1286, in > > DoGetTryMasters > > > presubmit_script, filename, project, change)) > > > File "f:\src\depot_tools\presubmit_support.py", line 1144, in > > > ExecPresubmitScript > > > return get_preferred_try_masters(project, change) > > > File "<string>", line 1796, in GetPreferredTryMasters > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 566, in > > > check_output > > > process = Popen(stdout=PIPE, *popenargs, **kwargs) > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 709, in > > > __init__ > > > errread, errwrite) > > > File "f:\src\depot_tools\python276_bin\lib\subprocess.py", line 957, in > > > _execute_child > > > startupinfo) > > > WindowsError: [Error 193] %1 is not a valid Win32 application > > > Sending crash report ... > > > args: ['f:\\src\\depot_tools\\git_cl.py', 'try'] > > > cwd: F:\src\chrome2\src > > > exception: [Error 193] %1 is not a valid Win32 application > > > host: http://JABDELMALEK3-W.ad.corp.google.com > > > stack: File "f:\src\depot_tools\git_cl.py", line 3349, > > > user: jabdelmalek > > > version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 > > > A stack trace has been sent to the maintainers. > > > > sys.executable is missing on line 1796 > > This would help, because python doesn't use $PATH or $PYTHONPATH to find > scripts. Instead I've removed the extension and will add commit_queue.bat and > commit_queue scripts to the depot_tools. Added binaries in https://codereview.chromium.org/1154623004
sergiyb@chromium.org changed reviewers: + jochen@chromium.org
added Jochen for rubber-stump
I've also tested this on Windows. It works.
lgtm
The CQ bit was checked by sergiyb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pgervais@chromium.org Link to the patchset: https://codereview.chromium.org/1152823005/#ps160001 (title: "Removed unneeded import")
The CQ bit was unchecked by sergiyb@chromium.org
The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1152823005/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sergiyb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org, pgervais@chromium.org Link to the patchset: https://codereview.chromium.org/1152823005/#ps180001 (title: "Also pass shell=True to allow resolution of the binary without extension on Windows")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1152823005/180001
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/87e80c7a628381644013e3b96b437276d9b19bcd Cr-Commit-Position: refs/heads/master@{#332200}
Message was sent while issue was closed.
A revert of this CL (patchset #10 id:180001) has been created in https://codereview.chromium.org/1147003007/ by thakis@chromium.org. The reason for reverting is: Speculative, looks like this broke `git cl try` for me (crbug.com/495219). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
