Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10831087:
A batch file to ease copying a build from a network share to a local machine for testing. (Closed)
|
Created:
8 years, 4 months ago by grt (UTC plus 2) Modified:
8 years, 4 months ago CC:
chromium-reviews, pam+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionA batch file to ease copying a build from a network share to a local machine for testing.
NOTRY=true
BUG=none
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149483
Patch Set 1 #
Total comments: 6
Patch Set 2 : incorporated comments from robertshield #
Total comments: 12
Patch Set 3 : responses to Gab's comments #Messages
Total messages: 11 (0 generated)
Sending to you guys for review since you're likely to find it useful. Thanks.
https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:11: REM \\build.share\src\chrome\tools\win\copy-installer.bat is this under src\chrome\tools\win or src\tools\win? rietveld makes it look like the latter. https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:20: REM and/or FLAVOR variables. Mention that this uses robocopy if available, recommend installing robocopy.
Thanks. Comments below. Will update the CL when my machine is free enough to switch back to that branch. https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:11: REM \\build.share\src\chrome\tools\win\copy-installer.bat On 2012/07/31 14:05:59, robertshield wrote: > is this under src\chrome\tools\win or src\tools\win? rietveld makes it look like > the latter. Yeah, the latter. That's what I get for trying to shorten the path to one of my checkouts. I'll change it to "\\build.share\...\src\tools\win\copy-installer.bat". Does that seem clear enough to you? https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:20: REM and/or FLAVOR variables. On 2012/07/31 14:05:59, robertshield wrote: > Mention that this uses robocopy if available, recommend installing robocopy. Robocopy is included with Vista+. Does this affect your request? xcopy is good enough that I think it's probably a waste of time to suggest to XP users that they should figure out how to get/install robocopy. wdyt?
lgtm (please upload new patch) https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:11: REM \\build.share\src\chrome\tools\win\copy-installer.bat On 2012/07/31 14:19:43, grt wrote: > On 2012/07/31 14:05:59, robertshield wrote: > > is this under src\chrome\tools\win or src\tools\win? rietveld makes it look > like > > the latter. > > Yeah, the latter. That's what I get for trying to shorten the path to one of my > checkouts. I'll change it to > "\\build.share\...\src\tools\win\copy-installer.bat". Does that seem clear > enough to you? Yep, although I would suggest "..." -> "<path_to_checkout> or some such, lest "..." be read as ".." https://chromiumcodereview.appspot.com/10831087/diff/1/tools/win/copy-install... tools/win/copy-installer.bat:20: REM and/or FLAVOR variables. On 2012/07/31 14:19:43, grt wrote: > On 2012/07/31 14:05:59, robertshield wrote: > > Mention that this uses robocopy if available, recommend installing robocopy. > > Robocopy is included with Vista+. Does this affect your request? xcopy is good > enough that I think it's probably a waste of time to suggest to XP users that > they should figure out how to get/install robocopy. wdyt? SGTM
Will wait for new patch to take a look. Please ping when you do upload it. Thanks! Gab
Thanks, Robert. I went ahead and added a note about robocopy while I was there. All ready for your review, Gab.
This is the most epic batch script I have ever seen (I would have switched to python after 2 lines!). Comments below. Cheers! Gab https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:8: REM into the directory \[out|build]\[Debug|Release] on the current drive. nit: Replace [...|...] by (...|...) (...|...) is the correct notation for a regex and given this very much looks like one, might as well make it one :)! https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:16: REM "out|build" and/or "Debug|Release" (case matters) to on the command line in nit: "to on the command line" -> "on the command line"? https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:20: REM and/or FLAVOR variables. optional: I feel BUILDTYPE (which is something I intuitively know what it is) would be better than FLAVOR (which is vague out of context). https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:53: SET TOCOPY=*.pdb setup.exe setup.exe.manifest chrome.packed.7z *.dll I would really like an option to NOT copy pdb's (I very rarely need them and they are huge... I've only been copying them manually when I need them for now (i.e. once every 25 copies or something...)). https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:60: REM Branch to handle copying via robocopy (fast) or xcopy (slow). As *.dll is so big what I do is only copy the files that got updated after an incremental build. The easy technical solution is probably to only copy the files from the FROM side which are more recent than the most recent file on the TO side. Note: We probably also want an option for a clean copy (but I'm totally fine if that's just manually deleting the TO directory).
Thanks. I chose to stick with a batch script because I want to be able to run it on a fresh machine (i.e., spin up vm, run script, debug). It's a bit more cumbersome to write, but it's lightweight. https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:8: REM into the directory \[out|build]\[Debug|Release] on the current drive. On 2012/07/31 22:23:24, gab wrote: > nit: Replace [...|...] by (...|...) > > (...|...) is the correct notation for a regex and given this very much looks > like one, might as well make it one :)! [foo|bar] is closer the convention for optional arguments in a command line tool. https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:16: REM "out|build" and/or "Debug|Release" (case matters) to on the command line in On 2012/07/31 22:23:24, gab wrote: > nit: "to on the command line" -> "on the command line"? Done. https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:20: REM and/or FLAVOR variables. On 2012/07/31 22:23:24, gab wrote: > optional: I feel BUILDTYPE (which is something I intuitively know what it is) > would be better than FLAVOR (which is vague out of context). Done. https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:53: SET TOCOPY=*.pdb setup.exe setup.exe.manifest chrome.packed.7z *.dll On 2012/07/31 22:23:24, gab wrote: > I would really like an option to NOT copy pdb's (I very rarely need them and > they are huge... I've only been copying them manually when I need them for now > (i.e. once every 25 copies or something...)). patches welcome. :-) https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:60: REM Branch to handle copying via robocopy (fast) or xcopy (slow). On 2012/07/31 22:23:24, gab wrote: > As *.dll is so big what I do is only copy the files that got updated after an > incremental build. > > The easy technical solution is probably to only copy the files from the FROM > side which are more recent than the most recent file on the TO side. > > Note: We probably also want an option for a clean copy (but I'm totally fine if > that's just manually deleting the TO directory). robocopy skips files that haven't changed.
lgtm (thanks for writing this, I will definitely use this over my manual/half-scripty hacks I had before!) https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... File tools/win/copy-installer.bat (right): https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:53: SET TOCOPY=*.pdb setup.exe setup.exe.manifest chrome.packed.7z *.dll On 2012/08/01 08:10:30, grt wrote: > On 2012/07/31 22:23:24, gab wrote: > > I would really like an option to NOT copy pdb's (I very rarely need them and > > they are huge... I've only been copying them manually when I need them for now > > (i.e. once every 25 copies or something...)). > > patches welcome. :-) ok ok..! I guess it's not that bad given robocopy only copies the diffs anyways so that you don't constantly pay the overhead (although chrome.pdb is HUGE, especially in static builds... which are guess are not as relevant now as they used to be...) https://chromiumcodereview.appspot.com/10831087/diff/7001/tools/win/copy-inst... tools/win/copy-installer.bat:60: REM Branch to handle copying via robocopy (fast) or xcopy (slow). On 2012/08/01 08:10:30, grt wrote: > On 2012/07/31 22:23:24, gab wrote: > > As *.dll is so big what I do is only copy the files that got updated after an > > incremental build. > > > > The easy technical solution is probably to only copy the files from the FROM > > side which are more recent than the most recent file on the TO side. > > > > Note: We probably also want an option for a clean copy (but I'm totally fine > if > > that's just manually deleting the TO directory). > > robocopy skips files that haven't changed. Oh :)! Awesome!
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/grt@chromium.org/10831087/9001
Change committed as 149483 |
