'git cl upload' keeps failing with 'Got 400 status code' when uploading a huge CL |
||
Issue description
To update ICU in Chrome to 58.1 (the latest), I need to upload a huge CL (~ 5000 files). I've done that before in the previous update, but this time around 'git cl upload' keeps failing. I dropped some files to cut down the number of files and the size of CLs, but it didn't help.
Below is a partial output from git cl upload with the stack trace at the end.
It'd be great if this can be quickly resolved. If not, perhaps I could push the CL directly (this is just taking the upstream as they're). It should be followed up by local customizations.
Uploaded patch for source/samples/translit/util.h
Uploaded patch for source/samples/uciter8/Makefile
Uploaded patch for source/samples/uciter8/readme.txt
Uploaded patch for source/samples/uciter8/uciter8.c
Uploaded patch for source/samples/uciter8/uit_len8.c
--> Failed to upload patch for source/samples/ucnv/Makefile. Got 400 status code.
--> Failed to upload patch for source/samples/uciter8/uit_len8.h. Got 400 status code.
--> Failed to upload patch for source/samples/ucnv/convsamp.cpp. Got 400 status code.
........... snip ........
--> Failed to upload patch for source/tools/toolutil/xmlparser.cpp. Got 400 status code.
--> Failed to upload patch for source/tools/toolutil/xmlparser.h. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/icuzdump.cpp. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/icuzones. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/icuregions. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/tz2icu.cpp. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/tz2icu.h. Got 400 status code.
--> Failed to upload patch for source/tools/tzcode/readme.txt. Got 400 status code.
Got exception while uploading -- saving description to /usr/local/google/home/jungshik/.git_cl_description_backup
Traceback (most recent call last):
File "/usr/local/google/home/jungshik/cr/depot_tools/git_cl.py", line 5379, in <module>
sys.exit(main(sys.argv[1:]))
File "/usr/local/google/home/jungshik/cr/depot_tools/git_cl.py", line 5361, in main
return dispatcher.execute(OptionParser(), argv)
File "/usr/local/google/w/cr/depot_tools/subcommand.py", line 252, in execute
return command(parser, args[1:])
File "/usr/local/google/home/jungshik/cr/depot_tools/git_cl.py", line 4079, in CMDupload
return cl.CMDUpload(options, args, orig_args)
File "/usr/local/google/home/jungshik/cr/depot_tools/git_cl.py", line 1522, in CMDUpload
ret = self.CMDUploadChange(options, git_diff_args, change)
File "/usr/local/google/home/jungshik/cr/depot_tools/git_cl.py", line 2163, in CMDUploadChange
issue, patchset = upload.RealMain(upload_args)
File "/usr/local/google/w/cr/depot_tools/third_party/upload.py", line 2501, in RealMain
result = UploadSeparatePatches(issue, rpc_server, patchset, data, options)
File "/usr/local/google/w/cr/depot_tools/third_party/upload.py", line 2101, in UploadSeparatePatches
result = t.get(timeout=60)
File "/usr/lib/python2.7/multiprocessing/pool.py", line 554, in get
raise TimeoutError
multiprocessing.TimeoutError
,
Oct 21 2016
I'm totally unsurprised that this is failing. It's a huge number of files, with large diffs for each file. Fixing this in Rietveld isn't really feasible. This is one of the reasons we want to switch to Gerrit. You're trying to land a change in icu.git, not chromium/src.git. Get an over-the-shoulder review of the change (in particular, any changes you've made to README.chromium, so that others can reproduce your work), and then feel free to land directly. For landing, you have two options: * 'git cl land' should work. The files don't get uploaded until after the CL has been created on Rietveld, so there should be a CL sitting around in your Rietveld account which has a commit description but doesn't have any files. Using 'git cl land' should take advantage of that. * 'git push origin' should also work. Assuming you have appropriate ACLs to run 'git cl land' in this repo, then doing a 'git push' should use those same permissions. Again, just make sure you get a second pair of eyes on your change, especially before it gets rolled into src/DEPS. P.S. It seems like at some point it would be good to have a larger conversation about how the icu repo is maintained. Perhaps the "import source from upstream" bit could be automatic (we have lots of mirroring systems), and then the only human interaction for an actual update would be applying patches and re-running some of the other scripts?
,
Oct 21 2016
https://codereview.chromium.org/2436413002/ is one of 'partially' uploaded CLs. It's rather hard to view it because Chrome gets unresponsive.
,
Oct 21 2016
Yeah, I don't expect a partially-uploaded CL to be viewable in the browser. I'd suggest getting an in-person review, or doing something like the following: $ git cl issue 0 $ git reset HEAD~ $ git add README.chromium $ git commit -m "Update to icu58" $ git stash $ git cl upload $ git stash pop $ git add . $ git commit -m "The rest of the update" $ <get review> $ git cl land That should upload just the README.chromium for review, in case it needs to be checked by someone else, but then will land the whole diff.
,
Oct 21 2016
Thanks a lot ! 'git cl upload' made a 'partial upload to Rietveld' and an issue was created (see comment 3). Would 'git cl land' still work? As for mirroring and how to get the upstream copy before applying local changes, I fully agree that we need to discuss about that. The ICU upstream uses svn, but I guess we can make a git mirror if necessary.
,
Oct 21 2016
Yep, 'git cl land' lands your current *local* diff, but then also updates the (partially-)uploaded issue to mark it as closed. It should work fine. We mirror many external/upstream repositories from SVN to Git. At some point, file a go/fix-chrome-git ticket to have us look at setting up solutions to make keeping icu up-to-date a bit easier.
,
Oct 24 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by js...@chromium.org
, Oct 21 2016