New issue
Advanced search Search tips
Starred by 2 users
Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment
contra-intuitive error messages in presence of sources_assignment_filter
Project Member Reported by primiano@chromium.org, Dec 21 2015 Back to list
Small issue today while using GN.
TL;DR: the sources_assignment_filter per-OS exclusion rule (_android, _linux etc) seem to apply at sources assignment time (As the "assignment_filter" name suggests). The resulting error message in the case of a further removal is a bit confusing.

Repro case:

diff --git a/base/BUILD.gn b/base/BUILD.gn
@@ -50,30 +50,34 @@ if (is_nacl_nonsfi) {
 
 source_set("base_paths") {
   sources = [
     "base_paths.cc",
     "base_paths.h",
     "base_paths_android.cc",
     "base_paths_android.h",
     "base_paths_mac.h",
     "base_paths_mac.mm",
     "base_paths_posix.cc",
     "base_paths_posix.h",
     "base_paths_win.cc",
     "base_paths_win.h",
   ]
 
+  sources -= [
+    "base_paths_android.cc",
+  ]
+


ERROR at //base/BUILD.gn:66:5: Item not found
    "base_paths_android.cc",
    ^----------------------
You were trying to remove "base_paths_android.cc"
from the list but it wasn't there.

If I try to remove base_paths.cc everything works fine.

I think the contra-intuitive part here is:
I add X to a list. On the next "statement" I remove X from the same list. I wouldn't expect an error.


The real use case where this happened is:
+peter was writing a CL, and he wanted to express something like this:

sources = [..., "foo_android.cc", "bar.cc"]
if (some_condition)
  sources -=  ["foo_android.cc", "bar.cc"]

it seems that the way this should be encoded is
if (some_condition) {
  sources -=  ["bar.cc"]
  if (OS=="android") {
    sources -=  ["foo_android.cc", "bar.cc"]
  }
}

Maybe WAI? Maybe the sources removed by sources_assignment_filter should be tracked in some list so that the error message could be more explicative (e.g., "You were trying to remove "foo.cc" but it was already removed by a sources_assignment_filter)
 
Cc: peter@chromium.org
Ehm, +peter for real.
Comment 2 by peter@chromium.org, Dec 21 2015
For the actual context, I added a file named "gcm_stats_recorder_android_unittest.cc" and removed it from the |sources| list manually, not being aware that it matches a mask in the sources_assignment_filter. (I don't add _android_unittest.cc files very frequently.)

The cause was obvious and it was an easy fix, but since GN looks like a scripting language I didn't expect this kind of magic to happen during assignment.
Cc: brettw@chromium.org
We actually want sources_assignment_filter to go away eventually.

In the meantime, the recommended style (which also works around this) is to build up your sources list incrementally, rather than listing them all up front and subtracting out things like we did in GYP.

There are various things that might be nice to have if we did keep track of all of the sources across various conditionals but from my understanding that would require a fair amount of reworking of how GN is implemented, so I don't know that we'll see this any time soon.
Components: Build
Sign in to add a comment