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

Issue 599203 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug

Blocking:
issue 459705



Sign in to add a comment

GN: data_deps does not cause the compilation of dependency to happen

Project Member Reported by sdefresne@chromium.org, Mar 30 2016

Issue description

build/config/ios/rules.gni adds a data_deps of all iOS unit tests on "//testing/iossim(${host_toolchain})".

According to "gn help data_deps", this means that the dependency will be build and available at runtime:

  Specifies dependencies of a target that are not actually linked into
  the current target. Such dependencies will be built and will be
  available at runtime.

However, if I "gn clean" and then build "base_unittests" only, the data_deps is not built.

$ gn clean out/Default
$ ninja -j 1000 -C out/Default base_unittests
ninja: Entering directory `out/Default'
[1/1] Regenerating ninja files
[1018/1018] STAMP obj/base/base_unittests.stamp
$ ls out/Default/clang_x64/iossim
ls: out/Default/clang_x64/iossim: No such file or directory
$ ninja -j 1000 -C out/Default clang_x64/iossim
ninja: Entering directory `out/Default'
[90/90] LINK clang_x64/iossim
$ ls out/Default/clang_x64/iossim
out/Default/clang_x64/iossim

I would have expected the clang_x64/iossim to have been built as part of building base_unittests, since it is listed as a dependency (through the data_deps):

$ gn desc out/Default base:base_unittests deps
//base:base
//base:base_unittests_bundle_data
//base:base_unittests_bundle_data_executable
//base:base_unittests_bundle_data_info_plist
//base:base_unittests_resources_bundle_data
//base:i18n
//base:message_loop_tests
//base/test:run_all_unittests
//base/test:test_support
//base/third_party/dynamic_annotations:dynamic_annotations
//build/config/sanitizers:deps
//testing/gmock:gmock
//testing/gtest:gtest
//testing/iossim:iossim(//build/toolchain/mac:clang_x64)
//third_party/icu:icu

Is this a bug in GN or a design decision?
 
Labels: -Pri-3 Pri-1
If that's true, it's a bug. Let me test it to see if it repros for me.
I thought I was having this problem last week, but it turned out I had just created a second target called "base_unittests", which ninja was building instead.

Would be to double-check if: 
    ninja -j 1000 -C out/Default base:base_unittests

Fixes things.
I've tried to build //:gn_all that depends on //base:base_unittests, and the iossim tool is not build either.

$ gn clean out/Default
$ ninja -j 1000 -C out/Default :gn_all
ninja: Entering directory `out/Default'
[1/1] Regenerating ninja files
[2133/7065] ACTION //ios/web/shell:main_v..._xib(//build/toolchain/mac:ios_clang_arm
2016-03-30 16:43:45.034 Interface Builder Cocoa Touch Tool[12334:405076] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:45.043 Interface Builder Cocoa Touch Tool[12334:405076] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:45.084 Interface Builder Cocoa Touch Tool[12334:405076] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:45.090 Interface Builder Cocoa Touch Tool[12334:405076] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:46.459 Interface Builder Cocoa Touch Tool[12389:405139] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:46.460 Interface Builder Cocoa Touch Tool[12389:405139] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:46.647 Interface Builder Cocoa Touch Tool[12389:405139] Error creating notification handler for simulator graphics quality override: 1000000
2016-03-30 16:43:46.711 Interface Builder Cocoa Touch Tool[12389:405139] Please stop using -[UIToolbar _setForceTopBarAppearance:]
2016-03-30 16:43:46.712 Interface Builder Cocoa Touch Tool[12389:405139] Please stop using -[UIToolbar _setForceTopBarAppearance:]
/* com.apple.ibtool.document.warnings */
/Users/sdefresne/Developer/upstream/src/ios/web/shell/MainView.xib:global: warning: This file is set to build for a version older than the deployment target. Functionality may be limited. [9]
[7065/7065] STAMP obj/gn_all.stamp
$ find out/Default -name iossim -type f

Blocking: 459705
Labels: Proj-GN-Migration
Owner: sdefresne@chromium.org
Status: Assigned (was: Available)
I'm pretty sure the problem is that when you generate the ninja target that corresponds to the create_bundle() target that has the data_dep, you're not checking for data dependencies.

(compare the code in ninja_create_bundle_target_writer.cc w/ ninja_create_target_writer.cc).

We can work around this by making iossim a data_dep of the _generate_executable GN target in the app() template for now, but we should fix create_bundle() to look for data deps.

Sylvain, can you look into the GN change?

Here's a workaround in the meantime:

https://codereview.chromium.org/1845753002/

We can land it if we think fixing GN will be non-trivial or take some time.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd03fa72212077a789679a77992529c3aa6c08fe

commit cd03fa72212077a789679a77992529c3aa6c08fe
Author: dpranke <dpranke@chromium.org>
Date: Wed Mar 30 23:17:58 2016

Work around a bug where create_bundle() is ignoring data_deps() in iOS GN build.

It looks like create_bundle() is not checking for data_deps when it
generates its ninja rules, and as a result we don't build iossim
when we build base_unittests.app. By shifting the data_dep to the
underlying executable target, this works around the issue, but we
should probably fix the GN code generator for create_bundle() instead.

R=sdefresne@chromium.org
BUG= 599203 

Review URL: https://codereview.chromium.org/1845753002

Cr-Commit-Position: refs/heads/master@{#384141}

[modify] https://crrev.com/cd03fa72212077a789679a77992529c3aa6c08fe/build/config/ios/rules.gni

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8bb64b067eb0e61773b87c86fdda45f53e47498

commit f8bb64b067eb0e61773b87c86fdda45f53e47498
Author: sdefresne <sdefresne@chromium.org>
Date: Tue Apr 05 23:38:22 2016

[GN] Fix create_bundle to respect "data_deps" dependencies.

Add "data_deps" output dependency files as order only dependencies of
"create_bundle" target to fix the issue that "data_deps" are not build.

BUG= 599203 

Review URL: https://codereview.chromium.org/1855523003

Cr-Commit-Position: refs/heads/master@{#385325}

[modify] https://crrev.com/f8bb64b067eb0e61773b87c86fdda45f53e47498/tools/gn/ninja_create_bundle_target_writer.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d76422e697ad4de2edac5b23f12da1ee5e9dcd56

commit d76422e697ad4de2edac5b23f12da1ee5e9dcd56
Author: sdefresne <sdefresne@chromium.org>
Date: Wed Apr 13 15:14:06 2016

Revert of Work around a bug where create_bundle() is ignoring data_deps() in iOS GN build. (patchset #1 id:1 of https://codereview.chromium.org/1845753002/ )

Reason for revert:
gn binary as been fixed and create_bundle now causes its data_deps to be built, reverting this temporary change.

Original issue's description:
> Work around a bug where create_bundle() is ignoring data_deps() in iOS GN build.
>
> It looks like create_bundle() is not checking for data_deps when it
> generates its ninja rules, and as a result we don't build iossim
> when we build base_unittests.app. By shifting the data_dep to the
> underlying executable target, this works around the issue, but we
> should probably fix the GN code generator for create_bundle() instead.
>
> R=sdefresne@chromium.org
> BUG= 599203 
>
> Committed: https://crrev.com/cd03fa72212077a789679a77992529c3aa6c08fe
> Cr-Commit-Position: refs/heads/master@{#384141}

TBR=dpranke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 599203 

Review URL: https://codereview.chromium.org/1883843002

Cr-Commit-Position: refs/heads/master@{#386994}

[modify] https://crrev.com/d76422e697ad4de2edac5b23f12da1ee5e9dcd56/build/config/ios/rules.gni

Status: Fixed (was: Assigned)

Sign in to add a comment