Increase flexibility for NaCl toolchain download script.

Increase flexibility for NaCl toolchain download script.

Passing arguments in from the DEPS file makes it easier to configure the
download. The main intent of this change is to make it easier to extract
the NaCl toolchain immedately after downloading it in the Mojo repo.
Currently Chromium does not need to extract the toolchain immedately, and
this would require ~920MB of disk space, so this refactoring makes
extraction possible but optional. Eventually Chromium will extract the
toolchain immedately, but this will require a deeper rework of the GYP
build.

BUG=https://code.google.com/p/chromium/issues/detail?id=401761

Committed: https://crrev.com/474ab324d17d2cd198d3fb067cabc10a775a8df7
Cr-Commit-Position: refs/heads/master@{#309963}

[Nick Bray (chromium), 2014-12-17 22:21:48]

ncbray@chromium.org changed reviewers:
+ thakis@chromium.org

[Nick Bray (chromium), 2014-12-17 22:21:48]

thakis: OWNER

Note foo[0:0] is idiomatic for "extending" at the beginning of a list.  The
underlying tool is unfortunately sensitive to the order of arguments.

dpranke: FYI. To get the GN build working for NaCl you'll need this patch, plus
add '--extract' after 'sync' in the DEPS file.  This will dump the toolchain
into //native_client/toolchain

[Dirk Pranke, 2014-12-17 22:27:56]

On 2014/12/17 22:21:48, Nick Bray (chromium) wrote:
> thakis: OWNER
> 
> Note foo[0:0] is idiomatic for "extending" at the beginning of a list.  The
> underlying tool is unfortunately sensitive to the order of arguments.

That works, but it's a bit cryptic. I'd probably just do 

args = ['--exclude', 'pnacl_newlib'] + args

instead.

lgtm either way, though.

[Nick Bray (chromium), 2014-12-17 23:05:05]

> That works, but it's a bit cryptic. I'd probably just do 
> 
> args = ['--exclude', 'pnacl_newlib'] + args
> 
> instead.


Done.

+bradnelson, binji - I am feeling a little paranoid, can you think of any other
places this script might be used?

[Nico, 2014-12-18 01:04:28]

this change lgtm, but the plans of unpacking 1GB of stuff at runhooks time in
the future doesn't sound good to me. nacl already does way too much stuff at
runhooks time; it should aim to do (way) less in the future, not more.

[Nick Bray (chromium), 2014-12-18 01:20:45]

On 2014/12/18 01:04:28, Nico wrote:
> this change lgtm, but the plans of unpacking 1GB of stuff at runhooks time in
> the future doesn't sound good to me. nacl already does way too much stuff at
> runhooks time; it should aim to do (way) less in the future, not more.


I could rant on this, but trust that 1) the current situation is worse than you
would guess and 2) memoized unpacking at runhooks will be much saner.  Right now
it's unpacking 1GB of stuff at build time into the build output directory, which
I personally think is less desirable?  Particularly because it will do it every
time there is a clobber build?  If done at gclient runhooks time, it will be
fully memoized and only occur when NaCl updates its toolchains.  Right now every
build system that uses NaCl would need to unpack the toolchain, multiplying the
unpack time and disk space.  GYP and Scons (nacl_integration), currently.  GN in
the future.  I can only describe the current state of things as "insane."  I am
trying to get it back under control. :/

[Nico, 2014-12-18 01:27:25]

On Wed, Dec 17, 2014 at 5:20 PM, <ncbray@chromium.org> wrote:

> On 2014/12/18 01:04:28, Nico wrote:
>
>> this change lgtm, but the plans of unpacking 1GB of stuff at runhooks
>> time in
>> the future doesn't sound good to me. nacl already does way too much stuff
>> at
>> runhooks time; it should aim to do (way) less in the future, not more.
>>
>
>
> I could rant on this, but trust that 1) the current situation is worse
> than you
> would guess and 2) memoized unpacking at runhooks will be much saner.
> Right now
> it's unpacking 1GB of stuff at build time into the build output directory,
> which
> I personally think is less desirable?


I think that's preferable. It'll only happen if you build with nacl enabled
(I hope), and it seems like a build action kind of thing.


>   Particularly because it will do it every
> time there is a clobber build?  If done at gclient runhooks time, it will
> be
> fully memoized and only occur when NaCl updates its toolchains.


Which happens more often than a full clobber build, I think?


>   Right now every
> build system that uses NaCl would need to unpack the toolchain,
> multiplying the
> unpack time and disk space.


Most people build with only a single build system.


>   GYP and Scons (nacl_integration), currently.  GN in
> the future.  I can only describe the current state of things as "insane."
> I am
> trying to get it back under control. :/
>
> https://codereview.chromium.org/811593005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nick Bray (chromium), 2015-01-05 21:25:21]

> >> this change lgtm, but the plans of unpacking 1GB of stuff at runhooks
> >> time in
> >> the future doesn't sound good to me. nacl already does way too much stuff
> >> at
> >> runhooks time; it should aim to do (way) less in the future, not more.
> >>
> >
> >
> > I could rant on this, but trust that 1) the current situation is worse
> > than you
> > would guess and 2) memoized unpacking at runhooks will be much saner.
> > Right now
> > it's unpacking 1GB of stuff at build time into the build output directory,
> > which
> > I personally think is less desirable?
> 
> 
> I think that's preferable. It'll only happen if you build with nacl enabled
> (I hope), and it seems like a build action kind of thing.

My main goal here is to reduce complexity.  Experience on this issue says that
build systems deal badly with dynamically constructed toolchains, they expect
them to exist prior to the build starting because this is how 99.99% of software
is built.  Trying to make it a build action adds a large amount of complexity to
the build system, and build systems are generally cope badly with complexity. 
Runhooks is the correct place to update toolchains, and after runhooks the
toochains should be ready to be used.

If we care about optimizing the case where NaCl is disabled, we should probably
rig the DEPS file to not DEPS in //native_client/ and not even download the
toolchains.  For non-Google developers download >> unpacking.  For Google
developers, if they are disabling NaCl, there is a deeper issue that needs to be
addressed.

> >   Particularly because it will do it every
> > time there is a clobber build?  If done at gclient runhooks time, it will
> > be
> > fully memoized and only occur when NaCl updates its toolchains.
> 
> 
> Which happens more often than a full clobber build, I think?

You will unpack the NaCl toolchain every time the toolchain changes for a normal
build of Chrome.  Unpacking into the output directory means you must _also_
unpack it when you do a clobber build.  In general, we try not to update the
toolchains unless we need to.


> >   Right now every
> > build system that uses NaCl would need to unpack the toolchain,
> > multiplying the
> > unpack time and disk space.
> 
> 
> Most people build with only a single build system.

Currently the bots use 3: gyp, gn, and scons.  For human developers, every build
configuration (debug and release) requires a separate copy, if the toolchains
are unpacked to the output directory.

[Nick Bray (chromium), 2015-01-05 21:26:33]

The CQ bit was checked by ncbray@chromium.org

[commit-bot: I haz the power, 2015-01-05 21:27:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/811593005/20001

[Nico, 2015-01-05 21:29:11]

> My main goal here is to reduce complexity.  Experience on this issue says that
> build systems deal badly with dynamically constructed toolchains, they expect
> them to exist prior to the build starting because this is how 99.99% of
software
> is built.

We control Chromium's build- and meta-build-systems. If they don't handle this
well, they can be taught to handle this. For clang, we just add a
-DIGNORED_CLANG_REVISION=${clang_revision} define through gyp, and then ninja's
command-line tracking causes rebuilds of everything when a new clang binary
appears. Doing this costs one line of gyp code.

[Nick Bray (chromium), 2015-01-05 21:51:27]

On 2015/01/05 21:29:11, Nico wrote:
> > My main goal here is to reduce complexity.  Experience on this issue says
that
> > build systems deal badly with dynamically constructed toolchains, they
expect
> > them to exist prior to the build starting because this is how 99.99% of
> software
> > is built.
> 
> We control Chromium's build- and meta-build-systems. If they don't handle this
> well, they can be taught to handle this. For clang, we just add a
> -DIGNORED_CLANG_REVISION=${clang_revision} define through gyp, and then
ninja's
> command-line tracking causes rebuilds of everything when a new clang binary
> appears. Doing this costs one line of gyp code.

Yes, I need to rig something like this for the NaCl GN build, but this isn't the
main issue.  When you start saying this like a particular action must occur
before a particular toolchain is used, this is the badly supported case.  In GN
it is almost possible to express this, but there are bugs.  As I understand it,
this support was only grudgingly added to GN and is basically unsupported.  In
Gyp you crappify every NaCl target by making it have a dependency on the unpack
action.  Forget the dependancy?  Have fun with flake.

Even worse, NaCl currently says that the toolchain must be unpacked and used to
build a few libraries before anything can be linked.  GN doesn't support this
easily, at all.

All told, trying to make toolchains a build system issue (beyond invalidating
previous builds) results in too many moving parts.  There aren't a lot of people
working on Chrome's build, and it isn't worth spending time maintaining this
complexity on any level.  Unpacking in gclient runhooks is dead simple.

[commit-bot: I haz the power, 2015-01-05 22:04:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-05 22:05:51]

Patchset 2 (id:??) landed as
https://crrev.com/474ab324d17d2cd198d3fb067cabc10a775a8df7
Cr-Commit-Position: refs/heads/master@{#309963}