respecting CXXFLAGS and other environment variables

[Paweł Hajdan, Jr.]

What would it take to make GN build use CXXFLAGS and related variables from environment?

I can write patches for this.

In the past, a similar change was made to the ninja GYP generator, and the make generator already had it.

Please see the following links for context:

https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html

https://bugs.chromium.org/p/chromium/issues/detail?id=251122

https://groups.google.com/d/msg/gyp-developer/35p9mDRbNMY/vPxQ3W2G6HUJ

https://groups.google.com/d/msg/ninja-build/ppKm1kau68s/fq1xR0J67bYJ

https://codereview.chromium.org/17359002/

https://bugs.gentoo.org/show_bug.cgi?id=587408

Paweł

[Michael Achenbach]

V8 has also one configuration that relies on flags from the environment (currently using gyp):

https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder/builds/1868/steps/gclient%20runhooks/logs/stdio

Not sure how much work it would be to bake that into gn to work with gn args only. Otherwise, the CXXFLAGS would be convenient to keep this configuration alive without much work.

[Nico Weber]

FWIW, I think this has caused more harm than good in gyp land from my point of view (people didn't realize they had these set, and then their chrome builds broke).

Ah, I now remember that this has been discussed already to some length in https://bugs.chromium.org/p/chromium/issues/detail?id=595653

[Ben Boeckel]

On Wed, Jun 29, 2016 at 08:57:00 -0400, Nico Weber wrote:
> FWIW, I think this has caused more harm than good in gyp land from my point
> of view (people didn't realize they had these set, and then their chrome
> builds broke).

Just an FYI, Linux distributions building software which uses gn will
expect that CXXFLAGS and such are applied (e.g., Fedora hardens builds
this way). I expect that reproducible build efforts also utilize these
environment variables.

I don't know gn that well, but could Chrome check if these are set and
warn about it during generation?

--Ben

[Ken Rockot]

Would it be sufficient and palatable to allow developers to globally modify cxxflags etc at their own peril via gn args?

You could then template args.gn with your own tooling and fill in values from the environment, or maybe even args.gn could support environment var expansion at generation time?

In any case, globally overriding these flags seems likely to be fragile and error-prone so it would be a bit weird to make the behavior implicit.

[Nico Weber]

On Wed, Jun 29, 2016 at 9:24 AM, Ken Rockot <roc...@chromium.org> wrote:

Would it be sufficient and palatable to allow developers to globally modify cxxflags etc at their own peril via gn args?

This was discussed and rejected in https://bugs.chromium.org/p/chromium/issues/detail?id=595653 

[Brett Wilson]

You can read the bug for why this is a bad idea.

The thing we worked out with the CrOS folks is that the script that wraps the Chrome build parses the environment variables and converts known ones to the corresponding GN variables. Unknown ones can go into a new GN build variable for "additional" flags. I'm not sure they actually implemented this yet, though.

This keeps the environment variables and GN build variables at least theoretically in sync (somebody will have to maintain the script of course). Otherwise, the build configuration and compiler flags could be arbitrarily out-of-sync.

Some things like the optimization flags will have to be carefully thought through. The Chrome build modifies these on a per-target basis which is the opposite of "set externally".

Brett

[Dirk Pranke]

On Wed, Jun 29, 2016 at 9:02 AM, Brett Wilson <bre...@chromium.org> wrote:

You can read the bug for why this is a bad idea.

The thing we worked out with the CrOS folks is that the script that wraps the Chrome build parses the environment variables and converts known ones to the corresponding GN variables. Unknown ones can go into a new GN build variable for "additional" flags. I'm not sure they actually implemented this yet, though.

Yes, this is implemented, in a limited, CrOS-specific way. See //build/toolchain/cros_toolchain.gni. 

I don't particularly like the solution, because it doesn't scale well as you increase the number of toolchains you need to worry about. 

-- Dirk

[Dirk Pranke]

On Wed, Jun 29, 2016 at 4:01 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

What would it take to make GN build use CXXFLAGS and related variables from environment?

GN itself, and the chromium //build configs, do not look at the environment. You can of course customize

your own //build files to do what you like, including calling a python script which checked the environment

and set GN variables based on the results.

But, I assume that you actually want to modify the chromium //build configs to do that, and others have already

replied on this thread as to why we're *extremely* reluctant to do this.

The gist of it is that because of the way different parts of the chromium build explicitly manipulate different

compiler configs (some parts use different ABI flags, different optimization levels, different warning levels, etc.),

just blindly setting a list of flags you expect to be applied to every target either doesn't work at all, doesn't

work the way you expect it to, or, if it does work, it's largely because you're getting lucky in a rather fragile way.

That said, I mostly understand where the requirements come from, and I would like to work w/ packagers

and other folks to figure out a workable approach for this. 

crbug.com/595653 does have a good background discussion. 

People can feel free to file a new bug or bugs to talk about generalizing this support for different/non-CrOS 

use cases, and I'll treat the requests seriously, but we wish to tread very carefully here.

-- Dirk

[Dirk Pranke]

On Wed, Jun 29, 2016 at 6:06 AM, Ben Boeckel <ben.b...@kitware.com> wrote:

On Wed, Jun 29, 2016 at 08:57:00 -0400, Nico Weber wrote:
> FWIW, I think this has caused more harm than good in gyp land from my point
> of view (people didn't realize they had these set, and then their chrome
> builds broke).

Just an FYI, Linux distributions building software which uses gn will
expect that CXXFLAGS and such are applied (e.g., Fedora hardens builds
this way). I expect that reproducible build efforts also utilize these
environment variables.

I'm not sure what you mean by "Fedora hardens builds this way" or "reproducible

build efforts also utilize these environment variables". Can you point me to more

info here? 

Generally speaking, I see env vars as the opposite of reproducible,

but of course that's not necessarily true in the context of a larger build file

generation process.

-- Dirk

[Ben Boeckel]

On Wed, Jun 29, 2016 at 09:31:16 -0700, Dirk Pranke wrote:
> I'm not sure what you mean by "Fedora hardens builds this way" or
> "reproducible
> build efforts also utilize these environment variables". Can you point me
> to more
> info here?

Fedora uses CFLAGS and CXX flags to enforce policies such as this one:

https://fedoraproject.org/wiki/Changes/Harden_All_Packages#Detailed_Harden_Flags_Description

> Generally speaking, I see env vars as the opposite of reproducible,
> but of course that's not necessarily true in the context of a larger build
> file
> generation process.

It's reproducible within a given context, which may include a set of
specific compiler flags. Within the larger context, reproducible builds,
I believe, also usually scrub the external environment of state before
starting, but *within* the buildenv, there may be other settings which
are expected to be obeyed.

For example, the SOURCE_DATE_EPOCH environment varible is set within a
reproducible build (generally to the date of the release):

https://wiki.debian.org/ReproducibleBuilds/TimestampsProposal

--Ben

[Brett Wilson]

All of this should be doable using the recommended approach I discussed above. That's the only reasonable answer I've seen that keeps things sane but allows you to do what you want.

Brett 

[Dirk Pranke]

Thanks for the links. Both of these are examples of why we want to tread carefully.

On Wed, Jun 29, 2016 at 10:18 AM, Ben Boeckel <ben.b...@kitware.com> wrote:

On Wed, Jun 29, 2016 at 09:31:16 -0700, Dirk Pranke wrote:
> I'm not sure what you mean by "Fedora hardens builds this way" or
> "reproducible
> build efforts also utilize these environment variables". Can you point me
> to more
> info here?

Fedora uses CFLAGS and CXX flags to enforce policies such as this one:

    https://fedoraproject.org/wiki/Changes/Harden_All_Packages#Detailed_Harden_Flags_Description

In this case, we generally always want Chrome hardened (as I understand it), so I'm not sure if it 

would even be a good thing if this was configurable from some higher-level build flag. I'd at least want

that to be something that was discussed before we added support for it to the build.

I think we always use -fPIC, and I'm  not actually sure why we don't use -fPIE in a static build 

instead, so I'd want to treat that as a bug and investigate. However, we also build different binaries 

differently, and so it's almost certainly true that we'd want that flag on some binaries and not others, 

and I don't know how you'd specify this using a blanket env var.

It's true (and a fair objection or complaint) that by not following many of the standard packaging guidelines

and not having better support for things like autoconf/autotools, we make it harder for people to simply

adapt Chromium/Chrome to their particular environment. And, it's possible that we should (re-)evaluate

some of the choices we've made here. But, we do have to evaluate them since it's not always obvious

that supporting more knobs or conventions either makes our lives easier or is the right thing to do.

> Generally speaking, I see env vars as the opposite of reproducible,
> but of course that's not necessarily true in the context of a larger build
> file
> generation process.

It's reproducible within a given context, which may include a set of
specific compiler flags. Within the larger context, reproducible builds,
I believe, also usually scrub the external environment of state before
starting, but *within* the buildenv, there may be other settings which
are expected to be obeyed.

For example, the SOURCE_DATE_EPOCH environment varible is set within a
reproducible build (generally to the date of the release):

    https://wiki.debian.org/ReproducibleBuilds/TimestampsProposal

I grant the idea, although I don't think this particular example is applicable to a

chromium build, which I think is already reproducible without it.

-- Dirk

[Ben Boeckel]

On Wed, Jun 29, 2016 at 17:03:58 -0700, Dirk Pranke wrote:
> I grant the idea, although I don't think this particular example is
> applicable to a
> chromium build, which I think is already reproducible without it.

So I think that there's two different ways this discussion is being
viewed which may be confusing things here:

- gn as a standalone build tool; and
- the Chrome build.

If Chrome wants to ignore environment variables and offer other ways to
toggle flags for things (e.g., stack protection, error classes,
etc.[1]), fine. But the gn tool itself should make it easy for CFLAGS,
CXXFLAGS, and LDFLAGS to have effects on the build. One I can think of
is that on OS X, -header_pad_max_size (or however it is spelled) is
often wanted when not building the final deliverable artifacts directly
and they need to be relocated before packaging.

Is gn meant to be a standalone build tool (like gyp is), or is it just
the way chrome is built?

--Ben

[1]Found a more representative set of flags that Fedora sets for all
builds:

-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector-strong --param=ssp-buffer-size=4
-grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-m64 -mtune=generic

[Dirk Pranke]

On Wed, Jun 29, 2016 at 5:13 PM, Ben Boeckel <ben.b...@kitware.com> wrote:

On Wed, Jun 29, 2016 at 17:03:58 -0700, Dirk Pranke wrote:
> I grant the idea, although I don't think this particular example is
> applicable to a
> chromium build, which I think is already reproducible without it.

So I think that there's two different ways this discussion is being
viewed which may be confusing things here:

  - gn as a standalone build tool; and
  - the Chrome build.

Right, I tried to answer that already, as part of my second reply

on this thread. the rest of my replies have been Chrome-specific,

Chrome, since I am guessing that that's actually what Paweł was suggesting

writing patches for.

As far as a standalone build tools goes ...

GN (the binary) by itself is essentially currently useless (no batteries are included). 

You *have* to write some set of build files to describe how we want compiles 

to be done, and we don't currently provide a stock set of files as part of a 

distribution; we don't even have a distribution in any useful sense. That's 

what I meant when I said you can customize your own build files to do 

what you like.

I agree that if we were provide a generic set of files, you'd want to add 

support for stuff like this, and I'd want to seriously think about how

to best integrate w/ autotools as well.

We've not spent any time on this till now, and it's not actually clear to

me that it would be worth doing this, since I don't know what the 

demand for this would be. There are good arguments that projects

(especially smaller projects that are more meant to be reusable components)

should really either support autotools or perhaps switch to cmake or

bazel.

We're not currently attempting to compete with any of those tools for

market share :).

-- Dirk

[Paweł Hajdan, Jr.]

Yes, I intended this thread to be in the Chrome context - packaging Chromium for open source distros.

I really appreciate all the context and open discussion about this. GN is much more thoughtful about this than GYP, and I think that's good.

I was looking at cros_toolchain.gni, and indeed having a way to pass things like cros_target_cxx and cros_target_extra_cxxflags would satisfy my use case. I believe I can convince distro packagers to use such explicit way to pass flags/settings, rather than environment being implicitly taken into account.

Would you have suggestions how to accomplish that? cros_toolchain.gni seems cros-specific.

Paweł

[Dirk Pranke]

On Thu, Jun 30, 2016 at 2:11 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Yes, I intended this thread to be in the Chrome context - packaging Chromium for open source distros.

I really appreciate all the context and open discussion about this. GN is much more thoughtful about this than GYP, and I think that's good.

I was looking at cros_toolchain.gni, and indeed having a way to pass things like cros_target_cxx and cros_target_extra_cxxflags would satisfy my use case. I believe I can convince distro packagers to use such explicit way to pass flags/settings, rather than environment being implicitly taken into account.

Would you have suggestions how to accomplish that? cros_toolchain.gni seems cros-specific.

At the moment, no. cros_toolchain.gni is indeed intentionally cros-specific, but I've been planning to clean it up and figure out how to make it less cros-specific, I just haven't really had time to think about the best way to do that, particularly given the concerns listed earlier in this thread.

I'd be happy to work on this over the next few weeks with you, though.

-- Dirk

[Paweł Hajdan, Jr.]

Okay. What would be next steps to get this started?

As GN will be the only option to build M54 and later versions, this'll be really important for downstream users of chromium.

Paweł

[Dirk Pranke]

I posted a CL in https://codereview.chromium.org/2202873002/ that is a step towards making things more generic.

However, I want to be really careful here because, just as we predicted, we've seen that people have tried to set flags via these env vars and gotten busted builds that are taking up time to debug and fix as a result.

So I want to see what we can do to try and limit our exposure there and try to keep things from being used incorrectly.

-- Dirk

[Paweł Hajdan, Jr.]

Looks like above CL hit some chromeos-specific questions.

I'm wondering what can be done for the generic CFLAGS question.

To clarify, this will be a major issue for Chromium packagers, and if an upstream solution can't be found they'll have to use custom patches.

It's somewhat worrying it's already taking over 1.5 months to tackle this. I'm totally willing to write patches and have solutions for this - it's still unclear to me what solution you'd find acceptable, as the criteria don't seem to be so obvious to me.

Paweł

[Torne (Richard Coles)]

I'd guess the most likely to be accepted short term option would be some kind of flag that enabled respecting the environment variables that was off by default. That would enable distro packaging to pass their controlled set of flags/etc, but not cause any random environment variables that were already set by people who've just checked out and tried to build chromium to break their builds.

[Dirk Pranke]

On Tue, Aug 16, 2016 at 5:19 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Looks like above CL hit some chromeos-specific questions.

I'm wondering what can be done for the generic CFLAGS question.

To clarify, this will be a major issue for Chromium packagers, and if an upstream solution can't be found they'll have to use custom patches.

It's somewhat worrying it's already taking over 1.5 months to tackle this. I'm totally willing to write patches and have solutions for this - it's still unclear to me what solution you'd find acceptable, as the criteria don't seem to be so obvious to me.

As I think various people have said repeatedly on this thread and the linked bugs and CLs, adding a generic escape hatch is somewhere between dangerous and flat-out bad. The Chromium build is simply too complicated to think that you can just tack on any flag you want and have everything just work. We can clearly see that in the attempts that have been made, where people like myself and llozano@ -- who theoretically have a decent idea what we're doing and know the gotchas better than most -- still keep getting things wrong and hitting serious, hard-to-debug bugs.

So, we continue to want to be very careful. The CL I linked to, above, is at least close to what we think we want to do, but got hung up on a syntactic issue (converting between strings that contain lists of switches and an actual list of switches).

The way to make progress is to show us *actual cases of actual sets of flags that need to be set* so that we can figure out if the approaches we're trying to implement will actually work. The one example given earlier in this thread, for the hardening flags in Fedora, is a good example since I think it probably would've also broken NaCl depending on how it was implemented.

However, even that example was somewhat hypothetical. So far I have not seen any actual requests with actual needs from packagers. 

I do really want to provide something that works for packagers, but any patch that just adds a single blanket escape hatch is going to be met with skepticism.

Does that make sense?

-- Dirk

[Paweł Hajdan, Jr.]

We discussed this over VC.

I wonder what the next steps are. While https://codereview.chromium.org/2202873002/ seems to be open and stalled, https://bugs.chromium.org/p/chromium/issues/detail?id=629593 is marked as fixed. Do you intend to make further changes to how cros toolchain is configured in GN?

I'm wondering what's blocking an attempt to add generic flag support (not just cros-specific).

Paweł

[Dirk Pranke]

On Wed, Aug 24, 2016 at 1:38 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

We discussed this over VC.

I wonder what the next steps are. While https://codereview.chromium.org/2202873002/ seems to be open and stalled, https://bugs.chromium.org/p/chromium/issues/detail?id=629593 is marked as fixed. Do you intend to make further changes to how cros toolchain is configured in GN?

Yes, nothing has changed since our conversation.

 

I'm wondering what's blocking an attempt to add generic flag support (not just cros-specific).

Nothing except someone having cycles to work on it. 

-- Dirk

[Paweł Hajdan, Jr.]

Looks like that CL is now closed, and https://bugs.chromium.org/p/chromium/issues/detail?id=642016 has been filed. Thanks for the details and thoughts on that bug. Let me respond to some of them here:

1. Starting with a simple scenario (host==target, no nacl) sounds good to me. What would it take to make that possible?

2. What would it take for a packager to define their own toolchain and make the build use it? We could provide a python script to generate such it (and possibly modify the build files to switch to it).

Paweł

[Brett Wilson]

There are already custom_toolchain and host_toolchain build args. If you write a file with a toolchain definition in it, you can make the build use it by setting these files. I don't know how these interact with nacl or the V8 toolchains.

Brett

[Dirk Pranke]

On Mon, Sep 5, 2016 at 8:07 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Looks like that CL is now closed, and https://bugs.chromium.org/p/chromium/issues/detail?id=642016 has been filed. Thanks for the details and thoughts on that bug. Let me respond to some of them here:

1. Starting with a simple scenario (host==target, no nacl) sounds good to me. What would it take to make that possible?

Not much. A variant of that original CL would probably work fine, i.e., define an "extra_flags" config that is included in the list of default configs, and define a couple of build args for the target and host toolchains, and apply the flags as needed in the new config based on the name of the current toolchain.

 

2. What would it take for a packager to define their own toolchain and make the build use it? We could provide a python script to generate such it (and possibly modify the build files to switch to it).

Well, it totally depends on the requirements, but you can probably use the existing //build/toolchain/cros/BUILD.gn file as a starting point.

[Paweł Hajdan, Jr.]

How does https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d6ba7bef53246b8132d81b5ef0620e32b9fbcb89 look like to you?

Any suggestions how to improve it?

Paweł

[Dirk Pranke]

At first glance that seems fine and it's not obvious to me that one would do much better; defining and using a custom toolchain is certainly what we always had in mind.

I'd probably need to stare at it longer to be sure of that evaluation, however.

-- Dirk

[Brett Wilson]

If that seems to work it looks fine to me.

Brett

[Paweł Hajdan, Jr.]

Cool. There's an updated version now which also handles cross-compiling.

Do you have recommendations where could we check-in this file to chromium sources so that it could be more easily shared by different distros and possibly further improved?

One of the possible places could be src/build/linux/unbundle . Other suggestions would also be welcome.

Paweł

[Brett Wilson]

On Wed, Jun 28, 2017 at 3:44 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Cool. There's an updated version now which also handles cross-compiling.

Do you have recommendations where could we check-in this file to chromium sources so that it could be more easily shared by different distros and possibly further improved?

One of the possible places could be src/build/linux/unbundle . Other suggestions would also be welcome.

How about //build/toolchain/linux/unbundle/BUILD.gn? That way we keep it in //build/toolchain with the others, but also segregated in an "unbundle" directory. If you do this, be sure to add really good comments so Chrome team members don't get confused and try to use it.

Brett

[Paweł Hajdan, Jr.]

Cool. What do you think about https://chromium-review.googlesource.com/c/575984/ ?

Paweł