Removing source exclusion filters

19 views
Skip to first unread message

Fabrice de Gans-Riberi

unread,
Apr 18, 2018, 7:49:12 PM4/18/18
to Chromium-dev
Hi all!

Right now, platform-exclusive files are automagically excludes from source sets through the use of exclusion filters defined in BUILDCONFIG.gn [1] I am working on a change to stop using the posix exclusion filter for the Fuchsia build. While working on it, I figured that with a bit more work, I could remove the posix exclusion filter altogether [2].
My question is, is it the right thing to do? A few things to consider:
-It seems a bit awkward to keep the other platform specific files through the exclusion filter and not the posix ones.
-It doesn't always make it easier to read the gn code. Some of the conditions become a bit more obtuse because _posix files now need to be explicitly included only for Posix - and sometimes Fuchsia - platforms.
-OTOH we want to stop treating Fuchsia as a Posix platform from a build perspective anyway so we'd need to temporarily remove the filter when including posix files for Fuchsia.

I welcome your thoughts on the question.

Thanks!
Fabrice

[2] WIP CL here: https://crrev.com/c/1013061

Scott Graham

unread,
Apr 19, 2018, 12:37:36 PM4/19/18
to Fabrice de Gans-Riberi, chromium-dev, gn-dev
+gn-dev for visibility

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJ0M83WrBdnmnrTqhFb8NeTf2Jkp5Yri%3DZbcc7qyMR%3DXELiOqQ%40mail.gmail.com.

Wez

unread,
Apr 19, 2018, 1:02:48 PM4/19/18
to sco...@chromium.org, fde...@chromium.org, chromi...@chromium.org, gn-...@chromium.org
In the past I've heard it expressed that the current platform filters were provided as a band-aid during the GYP->GN transition, and would ideally be removed?

Looking at https://crrev.com/c/1013061 on the one hand the change adds lines to the GN files, but on the other, a chunk of that is due to is_posix / is_fuchsia duplication, which would increase over time in any case, as we introduce native Fuchsia implementations of more things.

Nico Weber

unread,
Apr 19, 2018, 1:05:13 PM4/19/18
to Scott Graham, Fabrice de Gans-Riberi, chromium-dev, gn-dev
Like wez said, eventually all the implicit source filters should go away. So if you could remove the implicit filter for posix files, that'd be a step in that direction, and would be appreciated.

Dirk Pranke

unread,
Apr 19, 2018, 2:27:26 PM4/19/18
to Nico Weber, Scott Graham, Fabrice de Gans-Riberi, chromium-dev, gn-dev
Yes, we consider sources_assignment_filter to be something of a misfeature in GN, though it turns out to be rather useful when writing templates.

One minor downside to removing the general filters is that we don't really have a good approach to handling conditionals in an orderly way (i.e., specifying what order conditionals should be in) and so things can become a little more ad-hoc when writing a target as a result. That's something I've always wanted to come up with a proposal for, but never had the time to work on.

But, in practice, we have a mix of things already and so I doubt removing the filters would really make anything much worse.

-- Dirk

Fabrice de Gans-Riberi

unread,
Apr 20, 2018, 3:12:05 PM4/20/18
to Dirk Pranke, Nico Weber, Scott Graham, Chromium-dev, gn-...@chromium.org
Thanks everyone!

I am going to start breaking down the main CL and hopefully remove the posix filter next week. I want to keep the main CL down to a single file change so it's less of a hassle to revert if some configuration not included in the PSQ breaks.
Regarding conditionals, I have tried to keep it more or less consistent across the files I modified. I took the occasion to clean up some double negatives - stuff like if (!config) { sources -= [ source_config.cc ] } - but it's clear some files need a larger cleanup, especially //base/BUILD.gn.

Fabrice

Lei Zhang

unread,
Apr 26, 2018, 2:43:41 AM4/26/18
to Chromium-dev, Dirk Pranke, Nico Weber, Scott Graham, gn-...@chromium.org, Fabrice de Gans-Riberi
FYI, https://crrev.com/553344 safely landed yesterday to remove the
posix filter.

On Fri, Apr 20, 2018 at 12:10 PM, Fabrice de Gans-Riberi
> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJ0M83VkpTpiy3EuR9F5o_RcGWhDF6ULf0%3DGgUROD594SMNYjQ%40mail.gmail.com.

Wez

unread,
Apr 26, 2018, 6:40:21 AM4/26/18
to the...@chromium.org, chromi...@chromium.org, dpr...@chromium.org, tha...@chromium.org, sco...@chromium.org, gn-...@chromium.org, fde...@chromium.org
Reply all
Reply to author
Forward
0 new messages