src/PRESUBMIT.py crashes at checkperms if the CL changes too many files |
||||
Issue descriptionMy team Ecosystem Infra has a trybot wpt-importer importing files from a GitHub repo to Chromium periodically. Recently, we have a huge import touching a lot of files, and Chromium's PRESUBMIT.py crashes when checking file permissions because the argument list is too long. Looks like we are adding all the filenames into args: https://cs.chromium.org/chromium/src/PRESUBMIT.py?l=927&rcl=6860ea247cfccdd8042c80735de247994f0fcd8c Traceback (most recent call last): File "/data/src/depot_tools/git_cl.py", line 6132, in <module> sys.exit(main(sys.argv[1:])) File "/data/src/depot_tools/git_cl.py", line 6114, in main return dispatcher.execute(OptionParser(), argv) File "/data/src/depot_tools/subcommand.py", line 252, in execute return command(parser, args[1:]) File "/data/src/depot_tools/git_cl.py", line 4976, in CMDupload return cl.CMDUpload(options, args, orig_args) File "/data/src/depot_tools/git_cl.py", line 1643, in CMDUpload change=change) File "/data/src/depot_tools/git_cl.py", line 1575, in RunHook gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit()) File "/data/src/depot_tools/presubmit_support.py", line 1360, in DoPresubmitChecks results += executer.ExecPresubmitScript(presubmit_script, filename) File "/data/src/depot_tools/presubmit_support.py", line 1270, in ExecPresubmitScript result = eval(function_name + '(*__args)', context) File "<string>", line 1, in <module> File "<string>", line 2706, in CheckChangeOnUpload File "<string>", line 2438, in _CommonChecks File "<string>", line 929, in _CheckFilePermissions File "/data/src/depot_tools/subprocess2.py", line 524, in check_output return check_call_out(args, stdout=PIPE, **kwargs)[0] File "/data/src/depot_tools/subprocess2.py", line 484, in check_call_out out, returncode = communicate(args, **kwargs) File "/data/src/depot_tools/subprocess2.py", line 458, in communicate proc = Popen(args, **kwargs) File "/data/src/depot_tools/subprocess2.py", line 262, in __init__ % (str(e), kwargs.get('cwd'), args[0])) OSError: Execution failed with error: [Errno 7] Argument list too long. Check that None or /usr/bin/python exist and have execution permission. Full log can be found at: https://bugs.chromium.org/p/chromium/issues/detail?id=780055#c17
,
Nov 1 2017
Sorry I'm not sure which component this should go to. Please feel free to change it.
,
Nov 2 2017
I'm not quite sure either, though I do think Infra>Client>Chrome is one of the possibilities.
,
Nov 2 2017
https://chromium-review.googlesource.com/c/chromium/src/+/751504 should fix this.
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/f2d1615bc8565d1af2b0c65ddb1595db6681cf5e commit f2d1615bc8565d1af2b0c65ddb1595db6681cf5e Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Fri Nov 10 17:16:20 2017 presubmit: Add InputApi.CreateTemporaryFile() Sometimes, InputApi users need to create temporary files, write to them and pass them to another process, like this: with input_api.tempfile.NamedTemporaryFile() as f: f.write('foo') input_api.subprocess.check_output(['/path/to/script', '--reading-from', f.name]) While this works fine on Unix, on Windows subprocess cannot open and read the file while we have it open for writing. To work around this, we now offer a CreateTemporaryFile() that wraps a call to tempfile.NamedTemporaryFile(delete=False), and we then take care of removing all files created this way at the end of a presubmit run. The idea is for users to do something like this: with input_api.CreateTemporaryFile() as f: f.write('foo') f.close() input_api.subprocess.check_output(['/path/to/script', '--reading-from', f.name]) with the temporary file being removed automatically in a transparent fashion later. Bug: 780629 Change-Id: I0d705a5d52928a43f39a51f94a2c48d277bd5ced Reviewed-on: https://chromium-review.googlesource.com/758637 Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com> Reviewed-by: Aaron Gable <agable@chromium.org> [modify] https://crrev.com/f2d1615bc8565d1af2b0c65ddb1595db6681cf5e/presubmit_support.py [modify] https://crrev.com/f2d1615bc8565d1af2b0c65ddb1595db6681cf5e/tests/presubmit_unittest.py
,
Nov 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ff391dca136c9b85c7c78b4680103110bf8bb53 commit 6ff391dca136c9b85c7c78b4680103110bf8bb53 Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Mon Nov 13 16:43:39 2017 checkperms: Add a --file-list option. And use it from //PRESUBMIT.py instead of passing --file <file> for each file that needs to be processed. The idea is to read the list of files we want to process from a text file instead of reading everything from the command-line. web-platform-test imports were hitting an "argument list too long" error because there's a particularly huge import with several directories being renamed; at one point, we ended up with a command-line with ~260k characters. Bug: 780055 , 780629 Change-Id: Ib1c8b902c42ade77ccbd2662905301d7f766f9d9 Reviewed-on: https://chromium-review.googlesource.com/751504 Reviewed-by: Aaron Gable <agable@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com> Cr-Commit-Position: refs/heads/master@{#515960} [modify] https://crrev.com/6ff391dca136c9b85c7c78b4680103110bf8bb53/PRESUBMIT.py [modify] https://crrev.com/6ff391dca136c9b85c7c78b4680103110bf8bb53/tools/checkperms/checkperms.py
,
Nov 13 2017
Fixed at last! |
||||
►
Sign in to add a comment |
||||
Comment 1 by robertma@chromium.org
, Nov 1 2017