Intent to introduce `link_deps`

[Takuto Ikuta]

Hi all,

Recently, I sometimes see serialized build when building chrome.

And some serialized build looks come from unnecessary to be there dependencies.

(e.g. wait  v8 snapshot/code generation)

For the issue, I wrote proposal to introduce new dependency type `link_deps`.

https://docs.google.com/document/d/1-XLRS21DsD3_JvhQRe79DucfAvgOTgq7uijJfd82k4Q/edit?usp=sharing

Implementation is here: https://chromium-review.googlesource.com/c/chromium/src/+/1020946

Can I ask you to review/comment for my proposal?

Thanks, Takuto.

[Dirk Pranke]

I think you've rediscovered crbug.com/578477 :).

Generally speaking, I think neither Brett nor I are wild about adding an additional dependency type (as Daniel mentioned on another thread).

However, it doesn't look like there's a great way to automatically infer that these dependencies can be optimized out, at least not without doing

something expensive like running `gn check`, which also isn't perfect.

So, it looks like we probably would have to add something to solve this, and for something that can result in a 20% speedup in the build, it's hard to argue that we shouldn't do that.

The question then becomes: how risky is such a feature, i.e., what happens if someone gets it wrong, and what can we do to minimize the risk?

In a sense, using 'link_deps' is probably comparable in risk to simply having a missing dependency, and you'd use many of the same approaches to debug it. So, maybe that's not too bad.

One variant that I thought of, though:

The problem stems from the generated files in libA, since we serialize libB's compilation targets on generateA.stamp (really libA's inputdeps). Your proposal tells us to ignore generateA.stamp. However, if we changed A down the road so that a.h became generated, then things would break.

It's probably fairly common that B will depend on A's headers but not A's sources. We can't just split things into different targets in this case, because we need the real lib/shared_lib dependencies.

However, if we moved a.h into `public`, then we could check that nothing in public was generated and the link_deps was safe.

So, a variant of your CL would be to introduce link_deps but require that a target in the link_deps expose headers only through public, and then serialize libB on [libA.public].inputdeps (if need be). This is slightly more complicated, but slightly safer. I'm not sure if this tradeoff is a good idea or a bad one.

Thoughts?

-- Dirk

[Roland McGrath]

The notion of tying this behavior to `public` is the first thing that occurred to me.  I wonder if we could just make it implicit that a normal `deps` dependency on a code-compiling target that defines `public` is only synchronously dependent on the `public` files and the target's own top-level outputs (i.e. the library file itself).  This is a bit subtle, but I'm not at all sure it's really worse in that regard than adding a new flavor of dependency and reasoning about it interacts.

[Dirk Pranke]

You might be right, maybe that's the way to go.

-- Dirk

[Takuto Ikuta]

Adding compile dependency only for target having `public` makes sense to me.

It will require to add `public` field in some code generating action of existing BUILD.gn before rolling GN, otherwise it will drop necessary dependency.

If there is no any other concern, I'll implement such feature in GN.

--
You received this message because you are subscribed to the Google Groups "gn-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gn-dev+unsubscribe@chromium.org.

[Roland McGrath]

[Sorry for the dup, Takuto.  Re-sending to the list.]

It might not be obvious, but I think `public  = []` should invoke the new behavior and only `!defined(public)` should invoke the old compatibility behavior.  That is, unless we make this a compatibility-breaking change (e.g. with a switch in .gn or something).  If we don't want to break compatibility with existing GN files that rely on the implicit dependency on header generation steps, then compilation targets with no such dependencies to worry about (i.e. most) will have to add `public = []` just to enable maximal parallelism--even if they authentically have no public headers at all to mention!  That is sure to get overlooked by 99% of people writing new GN targets.  The failure mode of the build not being as fast/parallel as it could be is very hard to notice.

So that doesn't sound great.  Perhaps we do want a compatibility-breaking change, but I don't know how everybody would feel about that (or about a .gn switch to enable such an arcane change in behavior).  It's a tough one because the failure mode that we'd introduce for old GN files with header generation steps is intermittent races in parallel builds.  Sometimes they are obvious enough when the compiler reports the header file is missing, and incremental builds tend to be fine because of depfiles in the previous runs.  But I've seen such races before (perhaps not yet with GN, I don't recall) where the compiler was reading a partially-regenerated file racing with the generator writing it and that can just produce a giant cascade of completely confusing errors (or theoretically just silently compile some Frankenstein combination of the old and new code and lead to wholly bizarre runtime lossage).  It's not like people using the new version of GN will just get a clear error message telling them how to update their GN files to adapt to the change.  I don't personally object to the breaking change, but the question merits some careful consideration.

On a bit of tangent, it might also be a bit subtle that when generated headers are in a directory added to `include_dirs` by a config, then there has to be a `deps` link to the generation action, i.e. the config should really only be used via a group that sets `deps` and `public_configs`.  I guess nothing about that case changes here, but it comes to mind as the other pattern by which targets come to require header generation step dependencies that are easy to overlook (and these won't be caught by `gn check`).  This is not a pattern used much in Chromium, but in parts of Fuchsia we use per-component include dirs and `#include <...>` rather than `#include "..."` and relying on `gn check`.

I'm not sure there is any way the new `public` semantics would or should interact with that case, but maybe there is.  Perhaps if a file listed in some target A's `public` resides in a directory listed in some other target B's `include_dirs` (including any config that percolates into B by whatever means) then GN should warn (or error? or `--check` error?) if  B has no direct `deps` or (indirect `deps` via `public_deps`) dependency on A.  I guess the simpler answer there would be just to give `gn check` an option (in .gn, I guess) to pay attention to `#include <name>` when `name` exists in some `include_dirs` used with that source file and only ignore it as a presumed system header when GN can't figure out where the header will be found (and perhaps when the place it's found is not under // or $root_build_dir).

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

[Takuto Ikuta]

Thank you for good ideas!

2018-04-23 5:36 GMT+09:00 Roland McGrath <mcgr...@chromium.org>:

[Sorry for the dup, Takuto.  Re-sending to the list.]

It might not be obvious, but I think `public  = []` should invoke the new behavior and only `!defined(public)` should invoke the old compatibility behavior.  That is, unless we make this a compatibility-breaking change (e.g. with a switch in .gn or something).  If we don't want to break compatibility with existing GN files that rely on the implicit dependency on header generation steps, then compilation targets with no such dependencies to worry about (i.e. most) will have to add `public = []` just to enable maximal parallelism--even if they authentically have no public headers at all to mention!  That is sure to get overlooked by 99% of people writing new GN targets.  The failure mode of the build not being as fast/parallel as it could be is very hard to notice.

This is nice idea! Then only I need to do will be adding a `public = []` in an action target of v8's BUILD.gn.

 

So that doesn't sound great.  Perhaps we do want a compatibility-breaking change, but I don't know how everybody would feel about that (or about a .gn switch to enable such an arcane change in behavior).  It's a tough one because the failure mode that we'd introduce for old GN files with header generation steps is intermittent races in parallel builds.  Sometimes they are obvious enough when the compiler reports the header file is missing, and incremental builds tend to be fine because of depfiles in the previous runs.  But I've seen such races before (perhaps not yet with GN, I don't recall) where the compiler was reading a partially-regenerated file racing with the generator writing it and that can just produce a giant cascade of completely confusing errors (or theoretically just silently compile some Frankenstein combination of the old and new code and lead to wholly bizarre runtime lossage).  It's not like people using the new version of GN will just get a clear error message telling them how to update their GN files to adapt to the change.  I don't personally object to the breaking change, but the question merits some careful consideration.

On a bit of tangent, it might also be a bit subtle that when generated headers are in a directory added to `include_dirs` by a config, then there has to be a `deps` link to the generation action, i.e. the config should really only be used via a group that sets `deps` and `public_configs`.  I guess nothing about that case changes here, but it comes to mind as the other pattern by which targets come to require header generation step dependencies that are easy to overlook (and these won't be caught by `gn check`).  This is not a pattern used much in Chromium, but in parts of Fuchsia we use per-component include dirs and `#include <...>` rather than `#include "..."` and relying on `gn check`.

I'm not sure there is any way the new `public` semantics would or should interact with that case, but maybe there is.  Perhaps if a file listed in some target A's `public` resides in a directory listed in some other target B's `include_dirs` (including any config that percolates into B by whatever means) then GN should warn (or error? or `--check` error?) if  B has no direct `deps` or (indirect `deps` via `public_deps`) dependency on A.  I guess the simpler answer there would be just to give `gn check` an option (in .gn, I guess) to pay attention to `#include <name>` when `name` exists in some `include_dirs` used with that source file and only ignore it as a presumed system header when GN can't figure out where the header will be found (and perhaps when the place it's found is not under // or $root_build_dir).

I will introduce this new semantic of `public` to action target, and I'd like to that is only used to tell GN that the action does not block compile.

Also action does not have `config` variable now. So if we want to check such things, is it better to make another shared_library/source_set/static_library having generated header in its `public` and dependency to generate action in its `deps`?

 

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

--

Takuto Ikuta

Software Engineer in Tokyo

Chrome Infrastructure (goma team)

[Dirk Pranke]

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

And, I don't think we see parallelism as a bottleneck in *that* many places, though tikuta@ or brucedawson@ or bratell@ could likely correct me here.

There is still a risk that we'll break something for targets that do use public but somehow there's a dependency on a non-public file that isn't waited on.

I also don't have a problem with making a breaking / compatibility change, if we think the change itself is safe enough (and I think this qualifies, though I'm not positive yet). There's not many GN users, and we try to encourage people to version the GN binary they use alongside the code, so we don't tend to worry about "new GN, old code" that much.

-- Dirk

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

[Dirk Pranke]

On Sun, Apr 22, 2018 at 7:39 PM, Takuto Ikuta <tik...@google.com> wrote:

Thank you for good ideas!

2018-04-23 5:36 GMT+09:00 Roland McGrath <mcgr...@chromium.org>:

[Sorry for the dup, Takuto.  Re-sending to the list.]

It might not be obvious, but I think `public  = []` should invoke the new behavior and only `!defined(public)` should invoke the old compatibility behavior.  That is, unless we make this a compatibility-breaking change (e.g. with a switch in .gn or something).  If we don't want to break compatibility with existing GN files that rely on the implicit dependency on header generation steps, then compilation targets with no such dependencies to worry about (i.e. most) will have to add `public = []` just to enable maximal parallelism--even if they authentically have no public headers at all to mention!  That is sure to get overlooked by 99% of people writing new GN targets.  The failure mode of the build not being as fast/parallel as it could be is very hard to notice.

This is nice idea! Then only I need to do will be adding a `public = []` in an action target of v8's BUILD.gn.

 

So that doesn't sound great.  Perhaps we do want a compatibility-breaking change, but I don't know how everybody would feel about that (or about a .gn switch to enable such an arcane change in behavior).  It's a tough one because the failure mode that we'd introduce for old GN files with header generation steps is intermittent races in parallel builds.  Sometimes they are obvious enough when the compiler reports the header file is missing, and incremental builds tend to be fine because of depfiles in the previous runs.  But I've seen such races before (perhaps not yet with GN, I don't recall) where the compiler was reading a partially-regenerated file racing with the generator writing it and that can just produce a giant cascade of completely confusing errors (or theoretically just silently compile some Frankenstein combination of the old and new code and lead to wholly bizarre runtime lossage).  It's not like people using the new version of GN will just get a clear error message telling them how to update their GN files to adapt to the change.  I don't personally object to the breaking change, but the question merits some careful consideration.

On a bit of tangent, it might also be a bit subtle that when generated headers are in a directory added to `include_dirs` by a config, then there has to be a `deps` link to the generation action, i.e. the config should really only be used via a group that sets `deps` and `public_configs`.  I guess nothing about that case changes here, but it comes to mind as the other pattern by which targets come to require header generation step dependencies that are easy to overlook (and these won't be caught by `gn check`).  This is not a pattern used much in Chromium, but in parts of Fuchsia we use per-component include dirs and `#include <...>` rather than `#include "..."` and relying on `gn check`.

I'm not sure there is any way the new `public` semantics would or should interact with that case, but maybe there is.  Perhaps if a file listed in some target A's `public` resides in a directory listed in some other target B's `include_dirs` (including any config that percolates into B by whatever means) then GN should warn (or error? or `--check` error?) if  B has no direct `deps` or (indirect `deps` via `public_deps`) dependency on A.  I guess the simpler answer there would be just to give `gn check` an option (in .gn, I guess) to pay attention to `#include <name>` when `name` exists in some `include_dirs` used with that source file and only ignore it as a presumed system header when GN can't figure out where the header will be found (and perhaps when the place it's found is not under // or $root_build_dir).

I will introduce this new semantic of `public` to action target, and I'd like to that is only used to tell GN that the action does not block compile.

Also action does not have `config` variable now. So if we want to check such things, is it better to make another shared_library/source_set/static_library having generated header in its `public` and dependency to generate action in its `deps`?

I'm a bit lost on the second half of this note. I think I follow the point that Roland is making, but I'm not sure how often it comes up in practice.

However, I'm not seeing how 'public' on an action either makes sense or addresses the problem? I'm not sure what it would mean for a target that depends on an action to *not* block on the action.

-- Dirk

 

 

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

[Takuto Ikuta]

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

 

And, I don't think we see parallelism as a bottleneck in *that* many places, though tikuta@ or brucedawson@ or bratell@ could likely correct me here.

Yes, there are not many places that parallelism become bottleneck in chrome build, but restricted parallelism in a few places harm build speed, that I want to remove.

 

There is still a risk that we'll break something for targets that do use public but somehow there's a dependency on a non-public file that isn't waited on.

 

I also don't have a problem with making a breaking / compatibility change, if we think the change itself is safe enough (and I think this qualifies, though I'm not positive yet). There's not many GN users, and we try to encourage people to version the GN binary they use alongside the code, so we don't tend to worry about "new GN, old code" that much. 

-- Dirk

2018-04-24 0:38 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Sun, Apr 22, 2018 at 7:39 PM, Takuto Ikuta <tik...@google.com> wrote:

Thank you for good ideas!

2018-04-23 5:36 GMT+09:00 Roland McGrath <mcgr...@chromium.org>:

[Sorry for the dup, Takuto.  Re-sending to the list.]

It might not be obvious, but I think `public  = []` should invoke the new behavior and only `!defined(public)` should invoke the old compatibility behavior.  That is, unless we make this a compatibility-breaking change (e.g. with a switch in .gn or something).  If we don't want to break compatibility with existing GN files that rely on the implicit dependency on header generation steps, then compilation targets with no such dependencies to worry about (i.e. most) will have to add `public = []` just to enable maximal parallelism--even if they authentically have no public headers at all to mention!  That is sure to get overlooked by 99% of people writing new GN targets.  The failure mode of the build not being as fast/parallel as it could be is very hard to notice.

This is nice idea! Then only I need to do will be adding a `public = []` in an action target of v8's BUILD.gn.

 

So that doesn't sound great.  Perhaps we do want a compatibility-breaking change, but I don't know how everybody would feel about that (or about a .gn switch to enable such an arcane change in behavior).  It's a tough one because the failure mode that we'd introduce for old GN files with header generation steps is intermittent races in parallel builds.  Sometimes they are obvious enough when the compiler reports the header file is missing, and incremental builds tend to be fine because of depfiles in the previous runs.  But I've seen such races before (perhaps not yet with GN, I don't recall) where the compiler was reading a partially-regenerated file racing with the generator writing it and that can just produce a giant cascade of completely confusing errors (or theoretically just silently compile some Frankenstein combination of the old and new code and lead to wholly bizarre runtime lossage).  It's not like people using the new version of GN will just get a clear error message telling them how to update their GN files to adapt to the change.  I don't personally object to the breaking change, but the question merits some careful consideration.

On a bit of tangent, it might also be a bit subtle that when generated headers are in a directory added to `include_dirs` by a config, then there has to be a `deps` link to the generation action, i.e. the config should really only be used via a group that sets `deps` and `public_configs`.  I guess nothing about that case changes here, but it comes to mind as the other pattern by which targets come to require header generation step dependencies that are easy to overlook (and these won't be caught by `gn check`).  This is not a pattern used much in Chromium, but in parts of Fuchsia we use per-component include dirs and `#include <...>` rather than `#include "..."` and relying on `gn check`.

I'm not sure there is any way the new `public` semantics would or should interact with that case, but maybe there is.  Perhaps if a file listed in some target A's `public` resides in a directory listed in some other target B's `include_dirs` (including any config that percolates into B by whatever means) then GN should warn (or error? or `--check` error?) if  B has no direct `deps` or (indirect `deps` via `public_deps`) dependency on A.  I guess the simpler answer there would be just to give `gn check` an option (in .gn, I guess) to pay attention to `#include <name>` when `name` exists in some `include_dirs` used with that source file and only ignore it as a presumed system header when GN can't figure out where the header will be found (and perhaps when the place it's found is not under // or $root_build_dir).

I will introduce this new semantic of `public` to action target, and I'd like to that is only used to tell GN that the action does not block compile.

Also action does not have `config` variable now. So if we want to check such things, is it better to make another shared_library/source_set/static_library having generated header in its `public` and dependency to generate action in its `deps`?

I'm a bit lost on the second half of this note. I think I follow the point that Roland is making, but I'm not sure how often it comes up in practice.

However, I'm not seeing how 'public' on an action either makes sense or addresses the problem? I'm not sure what it would mean for a target that depends on an action to *not* block on the action.

I'm planning to use `public` in action like below.

component("take_long_time_to_finish") { # This corresponds to //tools/v8:v8_base

  sources = [...]

}

executable("cc_generator") { # This corresponds to //tools/v8:mksnapshot

  sources = [...]

  deps = [ ":take_long_time_to_finish"]

}

action("generate_cc") { # This corresponds to //tools/v8:run_mksnapshot_default

  outputs = [ "$target_gen_dir/generated.cc"]

  # This action does not generate any headers.

  public = []

  deps = [":cc_generator"]

}

source_set("generate_obj") { # This corresponds to //tools/v8:v8_snapshot

  sources = [ "$target_gen_dir/generated.cc", "generated.h"]

  deps = [ ":generate_cc" ]

}

component("public_library") { # This corresponds to //tools/v8:v8

  deps = [ ":generate_obj" ]

}

 

component("library_user") {  # This corresponds to many libraries depend on //tools/v8:v8

  # Compile of a.cc does not start before finish of "generate_cc" action, because current GN has no way to know compiling of a.cc does not depend on generated.cc.

  sources  = [ "a.cc", "a.h" ]

  deps = [ ":public_library" ]

}

If we can tell GN that an action has no public header, then the components having dependency to such action does not need to wait to start compiling.

And serialized dependency between v8 and chrome can be removed by this CL.

[Dirk Pranke]

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

-- Dirk

[Takuto Ikuta]

2018-04-25 7:34 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

Some action targets generate header files (e,g, idl compiler, mojo generator), but some action targets do not.

I want to specify empty list in public for no header generating action targets to tell GN that the action targets do not have public header and no need to wait compile in targets that have dependency to the action targets.

Following test code might be explain my thought well.

https://chromium-review.googlesource.com/c/chromium/src/+/1023771/7/tools/gn/ninja_binary_target_writer_unittest.cc

[Dirk Pranke]

On Tue, Apr 24, 2018 at 4:50 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 7:34 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

Some action targets generate header files (e,g, idl compiler, mojo generator), but some action targets do not.

I want to specify empty list in public for no header generating action targets to tell GN that the action targets do not have public header and no need to wait compile in targets that have dependency to the action targets.

Ah, I understand now, thank you. This seems a bit harder to me to do correctly. Maybe specific examples would be useful. I.e., I would think that most targets that depend on an action depend on either a header or a source file being generated. But, perhaps there are some that aren't doing either?

-- Dirk

[Takuto Ikuta]

2018-04-25 9:21 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Tue, Apr 24, 2018 at 4:50 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 7:34 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

Some action targets generate header files (e,g, idl compiler, mojo generator), but some action targets do not.

I want to specify empty list in public for no header generating action targets to tell GN that the action targets do not have public header and no need to wait compile in targets that have dependency to the action targets.

Ah, I understand now, thank you. This seems a bit harder to me to do correctly. Maybe specific examples would be useful. I.e., I would think that most targets that depend on an action depend on either a header or a source file being generated. But, perhaps there are some that aren't doing either?

From your comment, I found that `public` in action does not work as I expected.

We want compile of "libA" waits "generate" action, not want for "libB". But following BUILD.gn starts compiling of generated.cc even if "generate" is not finished.

action("generate") {

  public = []

  outputs = [ "$target_gen_dir/generated.cc"]

}

shared_library("libA") {

  sources = [ "a.cc", "a.h", "$target_gen_dir/generated.cc" ]

  deps = [ ":generate" ]

}

shared_library("libB") {

  sources = [ "b.cc", "b.h" ]

  deps = [ ":libA"]

}

 

So, still I need to introduce `link_deps` to use link only dependency.

One variant that I thought of, though:
The problem stems from the generated files in libA, since we serialize libB's compilation targets on generateA.stamp (really libA's inputdeps). Your proposal tells us to ignore generateA.stamp. However, if we changed A down the road so that a.h became generated, then things would break.
It's probably fairly common that B will depend on A's headers but not A's sources. We can't just split things into different targets in this case, because we need the real lib/shared_lib dependencies.
However, if we moved a.h into `public`, then we could check that nothing in public was generated and the link_deps was safe.
So, a variant of your CL would be to introduce link_deps but require that a target in the link_deps expose headers only through public, and then serialize libB on [libA.public].inputdeps (if need be). This is slightly more complicated, but slightly safer. I'm not sure if this tradeoff is a good idea or a bad one.

Dirk, do you think it is doable introducing link_deps if I implemented with the feature like above?

[Dirk Pranke]

On Wed, Apr 25, 2018 at 1:59 AM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 9:21 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Tue, Apr 24, 2018 at 4:50 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 7:34 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

Some action targets generate header files (e,g, idl compiler, mojo generator), but some action targets do not.

I want to specify empty list in public for no header generating action targets to tell GN that the action targets do not have public header and no need to wait compile in targets that have dependency to the action targets.

Ah, I understand now, thank you. This seems a bit harder to me to do correctly. Maybe specific examples would be useful. I.e., I would think that most targets that depend on an action depend on either a header or a source file being generated. But, perhaps there are some that aren't doing either?

From your comment, I found that `public` in action does not work as I expected.

We want compile of "libA" waits "generate" action, not want for "libB". But following BUILD.gn starts compiling of generated.cc even if "generate" is not finished.

I think I've gotten confused over the examples somewhere, either in this thread or your doc, or both.

Using your example below, I thought we cared more about unblocking the targets in :libB, and less about unblocking :libA.

If that was the case, then I'd expect to accomplish this by leaving :generate alone and by adding `public = []` to :libA.

If we wanted to unblock :libA as well, you could (I think) then move generated.cc to a :libA_generated target that :libA depends on. This seems okay to me because the "public interface" of :libA remains unchanged, and the optimization remains local to a single target.

Does that work? I would prefer to avoid adding public = [] to actions or adding link_deps if we can.

-- Dirk

[Takuto Ikuta]

2018-04-26 4:22 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Wed, Apr 25, 2018 at 1:59 AM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 9:21 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Tue, Apr 24, 2018 at 4:50 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-25 7:34 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

On Mon, Apr 23, 2018 at 6:42 PM, Takuto Ikuta <tik...@google.com> wrote:

2018-04-24 0:36 GMT+09:00 Dirk Pranke <dpr...@chromium.org>:

If you don't define public, that effectively makes everything in sources public, so I think you get the old behavior, right? (I'm not sure if that's what you were saying).

Yes.

 

At any rate, I think this is conservatively correct and so probably the right thing to do; while we try hard to make GN fast, we prioritize getting the build graph correct higher.

I will introduce `public' to few action targets, so GN won't be slow.

I'm still confused by this. Action targets are generating files, they shouldn't need public?

Some action targets generate header files (e,g, idl compiler, mojo generator), but some action targets do not.

I want to specify empty list in public for no header generating action targets to tell GN that the action targets do not have public header and no need to wait compile in targets that have dependency to the action targets.

Ah, I understand now, thank you. This seems a bit harder to me to do correctly. Maybe specific examples would be useful. I.e., I would think that most targets that depend on an action depend on either a header or a source file being generated. But, perhaps there are some that aren't doing either?

From your comment, I found that `public` in action does not work as I expected.

We want compile of "libA" waits "generate" action, not want for "libB". But following BUILD.gn starts compiling of generated.cc even if "generate" is not finished.

I think I've gotten confused over the examples somewhere, either in this thread or your doc, or both.

Using your example below, I thought we cared more about unblocking the targets in :libB, and less about unblocking :libA.

If that was the case, then I'd expect to accomplish this by leaving :generate alone and by adding `public = []` to :libA.

If we wanted to unblock :libA as well, you could (I think) then move generated.cc to a :libA_generated target that :libA depends on. This seems okay to me because the "public interface" of :libA remains unchanged, and the optimization remains local to a single target.

Does that work? I would prefer to avoid adding public = [] to actions or adding link_deps if we can.

Sorry for confusion.

And I did not fully understand Roland's thought, probably

It works, thanks.

I'd like to do such way.