New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 781858 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Dec 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Allow “config” to expose “inputs”

Project Member Reported by mark@chromium.org, Nov 6 2017

Issue description

Referring to https://chromium-review.googlesource.com/c/chromium/src/+/751403/5/build/secondary/third_party/crashpad/crashpad/tools/BUILD.gn#63 as being checked in, and the discussion we had at https://chromium-review.googlesource.com/c/chromium/src/+/751403/1/build/secondary/third_party/crashpad/crashpad/tools/BUILD.gn#11:

I have some ldflags that I’d like to specify for more than one target. I’m passing “-sectcreate __TEXT __info_plist (filename)” to the macOS linker to get it to create a section in its output containing the contents of the specified file. In order to make this work properly, the link step must depend on the file, which can be expressed in GN by adding the file to an “inputs” list. (Otherwise, the link would not be re-run if only the file changes.)

Since this needs to happen for more than one target (in this case, both generate_dump and exception_port_tool need it), it seemed like an ideal use for a GN config. But although I can specify ldflags in this way, it’s not possible to expose inputs to a target via a config.

As a further refinement, overbuilding can be prevented by a mechanism to express that the file is only an input to the link step, and not to any of the compile steps in the target.


 
Cc: brettw@chromium.org
Status: Available (was: Untriaged)

Comment 2 by mark@chromium.org, Dec 19 2017

“inputs” is a hammer, but it results in unnecessary recompilation when the input is only relevant to the link step. Bug 796187 expands on the last paragraph of the initial bug report here, and requests a more targeted solution to this problem.
What happens if instead of a config that multiple targets use, you make them list a common target in deps?

That target can be:

group("foo") {
  public_configs = [ ":foo_config" ]
  public_deps = [ ":action_that_creates_plist" ]
}
config("foo_config") {
  visibility = [ ":foo" ]
  ldflags = [ "-sectcreate __TEXT __info_plist $plist" ]
}

I think you might get a ninja dep from anything with deps=["foo"] to a stamp file for the action running, which is a proxy for the plist file changing.
But I'm not really sure.
There might be some slightly different trick along these lines that would have the desired effect.

Comment 4 by brettw@chromium.org, Dec 27 2017

The current rule for configs is that it's for things non-file and -target related and it doesn't participate in the dependency graph. Generally people ask for "deps" in a config which creates significant problems that "inputs" doesn't have.

I would try starting with Roland's solution to see if you can make something work. There is a conceptual simplicity in terms of having configs be for flags only.

Comment 5 by mark@chromium.org, Dec 27 2017

It certainly seems like an awkward hack to have to indirect my static in-tree plist through what amounts to a copy action just to make this work. I think I prefer bug 796187 comment 3.

Regardless, bug 796183 is a little frustrating: whether I achieve what needs to be done via a config or a dep exposing a public_config, ldflags can only be propagated out to a single dependent target, and not all the way to dependent targets that produce linker output.
Project Member

Comment 6 by sheriffbot@chromium.org, Dec 28

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: WontFix (was: Untriaged)
I don't think we should do this. Configs are for flags and such, while "inputs" seems more like part of the dependency tree. We don't allow sources for the same reason.
The use case is flags that need an auxiliary input file, such as a linker script or other parameter file.  For such cases configs is the sensible mechanism to use, and inputs in a config is the only way to represent the necessary ninja file dependencies.
If “inputs” is a no-go, bug 796187 frames the problem in a more general way. Inputs may be the wrong tool for the job (which I concede in comment 2), but it’s not accurate to say that “configs are for flags and such”, when some tools (such as linkers) accept configuration not via flags but via files (which they may be directed to via flags). If this is indeed a WontFix, please weigh in on bug 796187 with an acceptable framework to help this move forward.

Sign in to add a comment