gn: Multiple conditional blocks removing an item from a list

[Vincent Scheib]

Seeking advice on best form to handle this situation:

I'm looking to make a GN file like this:

  #deps and sources added.

  deps = [ "a", "b", "c" ]

  #Some conditional reason1 a,b shouldn't be included

 deps -= [ "a", "b" ]

  #Some conditional reason2 b,c shouldn't be included

 deps -= [ "b", "c" ]

It's valuable to keep the conditional blocks separate, as they're fairly orthogonal logical reasons, but do have overlapping consequences of removing some of the same items.

I run into the error

  You were trying to remove "b"

  from the list but it wasn't there.

And https://code.google.com/p/chromium/wiki/GNLanguage states this is by current GN design, and that testing for inclusion in a list doesn't exist. 

What's the recommended solution then? 

a) I could only remove once, by munging together the logic blocks

  if (reason1)

    deps -= [ "a" ]

  if (reason2)

    deps -= [ "c" ]

  if (reason1 || reason2)

    deps -= [ "b" ]

b) I could have each block add to a list, to always be removed.

  if (reason1)

    deps_to_remove = [ "a", "b" ]

  if (reason2)

    deps_to_remove = [ "b", "c" ]

  deps -= deps_to_remove

c) I could modify GN to support removal, without error if the item doesn't exist. Let's say -= has value producing a warning, I could add --=

  if (reason1)

    deps --= [ "a", "b" ]

  if (reason2)

    deps --= [ "b", "c" ]

[Mostyn Bramley-Moore]

I encountered the same issue recently, and settled on a variation of (b).  But I would prefer option (c) if there's no strong reason for this to be considered an error.

Aside: another option (which I don't recommend unless it makes a long if/else if/else block more readable):

d) If you're not sure if an item you want removed is still in the list, add it first:

if (condition1) {

  deps -= [ "a", "b" ]

}

if (condition2) {

  deps += [ "b" ] # not sure if "b" was already removed or not

  deps -= [ "b", "c" ]

}

-Mostyn.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

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

--

Mostyn Bramley-Moore

TV and Connected Devices

Opera Software ASA

mos...@opera.com

[Brett Wilson]

There are some simple cases like the example you give where this behavior might be nice. But I specifically throw an error in this case because I felt like this behavior in GYP led to a very bad place. There are a lot of duplicated conditions and files excluded that don't exist any more. Instead, GN forces you to have to know what's going on when modifying sources lists as much as possible, as a design goal.

When you're converting GYP files, this is very annoying because you have no idea what's going on and then you have to tease apart all the overlapping conditionals (this is where I find bitrotted GYP code).

In general, though, if it's inconvenient to write conditions to support removal of a file in some circumstances, then it's also difficult for a reader to tell which files are compiled in any case, and your GN code is bad.

In GYP we encouraged the pattern where you listed all files and then subtracted the ones you didn't need. In GYP there is no such preference. So it's probably a lot cleaner to conditionally include instead:

  if (foo || bar) {

    source += [ "asdf.cc" ]

  } 

  if (foo) {

    sources += [ "foo.cc" ]

  }

Brett

[Dirk Pranke]

For what's it's worth, there was some additional discussion on this in crbug.com/364204 , which was WontFixed (for the same reasons).

-- Dirk

[Vincent Scheib]

FYI Brett has updated style guide: 

https://code.google.com/p/chromium/wiki/GNStyleGuide#Sources

[Dirk Pranke]

On Sun, Mar 29, 2015 at 7:04 PM, Brett Wilson <bre...@chromium.org> wrote:

There are some simple cases like the example you give where this behavior might be nice. But I specifically throw an error in this case because I felt like this behavior in GYP led to a very bad place. There are a lot of duplicated conditions and files excluded that don't exist any more. Instead, GN forces you to have to know what's going on when modifying sources lists as much as possible, as a design goal.

When you're converting GYP files, this is very annoying because you have no idea what's going on and then you have to tease apart all the overlapping conditionals (this is where I find bitrotted GYP code).

In general, though, if it's inconvenient to write conditions to support removal of a file in some circumstances, then it's also difficult for a reader to tell which files are compiled in any case, and your GN code is bad.

In GYP we encouraged the pattern where you listed all files and then subtracted the ones you didn't need. In GYP there is no such preference. So it's probably a lot cleaner to conditionally include instead:

  if (foo || bar) {

    source += [ "asdf.cc" ]

  } 

  if (foo) {

    sources += [ "foo.cc" ]

  }

Wouldn't you have the same problem w/ the additive approach as you do with the subtractive approach? I don't see how additive makes this less of a problem.

-- Dirk

(though I tend to prefer additive for other reasons).

[Brett Wilson]

You'll probably need the same number of conditionals, but it often ends up much easier to follow.

Brett

[Vincent Scheib]

"same problem", well, not the original thread problem (can't remove twice).

And, in the GYP points, there are problems have having many related and possibly overlapping conditions, but the choice is to either have 

- simpler condition logic with redundant listing of sources/deps to remove

or

- single locations adding sources/deps with conditional logic referencing logic concepts multiple times.

I, too, am favoring a single listing of a source/dep in a condition block that has all logic that relates to that being included or not.