gn: Multiple conditional blocks removing an item from a list

20 views
Skip to first unread message

Vincent Scheib

unread,
Mar 29, 2015, 6:25:59 PM3/29/15
to Chromium-dev
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

unread,
Mar 29, 2015, 6:39:24 PM3/29/15
to sch...@chromium.org, Chromium-dev
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

Brett Wilson

unread,
Mar 29, 2015, 10:05:39 PM3/29/15
to Mostyn Bramley-Moore, Vincent Scheib, Chromium-dev
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

unread,
Mar 30, 2015, 1:01:51 PM3/30/15
to Brett Wilson, Mostyn Bramley-Moore, Vincent Scheib, Chromium-dev
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

unread,
Mar 30, 2015, 4:53:13 PM3/30/15
to Dirk Pranke, Brett Wilson, Mostyn Bramley-Moore, Chromium-dev

Dirk Pranke

unread,
Mar 30, 2015, 4:55:40 PM3/30/15
to Brett Wilson, Mostyn Bramley-Moore, Vincent Scheib, Chromium-dev
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

unread,
Mar 30, 2015, 5:02:46 PM3/30/15
to Dirk Pranke, Mostyn Bramley-Moore, Vincent Scheib, Chromium-dev
You'll probably need the same number of conditionals, but it often ends up much easier to follow.

Brett

Vincent Scheib

unread,
Mar 30, 2015, 5:20:12 PM3/30/15
to Brett Wilson, Dirk Pranke, Mostyn Bramley-Moore, Chromium-dev
"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.
Reply all
Reply to author
Forward
0 new messages