wutingcindy / gyp

Automatically exported from code.google.com/p/gyp
0 stars 0 forks source link

AdjustStaticLibraryDependencies may re-order hard_dependency-s incorrectly #215

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For a given project, there exist a set of targets [A,B,C,D], where the 
dependencies are
A<-B<-C<-D

A performs some step which generates a file needed to compile B (a Protobuf 
header, a version stamp, etc).

If A is a static library, it should be marked hard_dependency ( 
http://code.google.com/p/gyp/wiki/InputFormatReference#Linking_Dependencies ), 
so that A is always built before B.

Further, It is possible that B may include the generated header as part of it's 
public interface. According to 
http://code.google.com/p/gyp/wiki/InputFormatReference#Dependent_Settings , the 
correct thing for B to do is export_dependent_settings for A, so that C can 
properly locate the headers that B includes.

However, if C is a static library, the build-time ordering/dependency between 
A, B, and C will be split - C will be allowed to be built before A and B. This 
is incorrect, as C cannot build before A has generated the necessary headers 
that B includes, since C includes B.

The current solution is that B must also be declared hard_dependency. Likewise, 
if D is a static library and C includes headers from B as part of it's public 
interface, C must also be declared hard_dependency.

This requirement runs counter to the design philosophy of GYP, in that it 
requires dependencies much further up the dependency graph to know that A is 
running some action with side-effects. It is also a frequent cause for error 
(eg: http://crbug.com/87995 , http://crrev.com/98346 , http://crrev.com/97366 ).

Given that export_dependent_settings is used to indicate a public header 
dependency (or is documented as such), if a hard_dependency (A) is exported by 
a project (B), it should be imported by any dependents (C) before removing the 
original project (B). This will preserve the build sequence ordering.

Original issue reported on code.google.com by ryan.sle...@gmail.com on 5 Sep 2011 at 9:13

GoogleCodeExporter commented 9 years ago
r1028

Original comment by thakis@chromium.org on 5 Sep 2011 at 9:25

GoogleCodeExporter commented 9 years ago
Commit: 8d5f6b1c060384213b15eb44347cb9ea5b20e4d1
 Email: thakis@chromium.org@78cadc50-ecff-11dd-a971-7dbc132099af

Before adjusting/removing dependencies between two static_library targets, add 
dependencies on any exported targets that are marked hard_dependency, in order 
to preserve the build sequence ordering.

BUG=gyp:215
TEST=test/hard_dependency/gtest-*
Review URL: http://codereview.chromium.org/7748010
Patch from Ryan Sleevi <rsleevi@chromium.org>

git-svn-id: http://gyp.googlecode.com/svn/trunk@1028 
78cadc50-ecff-11dd-a971-7dbc132099af

M   pylib/gyp/input.py
A   test/hard_dependency/gyptest-exported-hard-dependency.py
A   test/hard_dependency/gyptest-no-exported-hard-dependency.py
A   test/hard_dependency/src/a.c
A   test/hard_dependency/src/a.h
A   test/hard_dependency/src/b.c
A   test/hard_dependency/src/b.h
A   test/hard_dependency/src/c.c
A   test/hard_dependency/src/c.h
A   test/hard_dependency/src/d.c
A   test/hard_dependency/src/emit.py
A   test/hard_dependency/src/hard_dependency.gyp

Original comment by bugdroid1@chromium.org on 5 Sep 2011 at 9:26

GoogleCodeExporter commented 9 years ago
Attached are the before and after JSON dumps of the Chromium dependency tree 
with static library dependencies adjusted.

With this change, a total of 28 additional/new dependencies were introduced. Of 
these dependencies, only one of them (chrome/chrome.gyp:sync) did not directly 
generate files/headers.

Original comment by rsleevi@chromium.org on 5 Sep 2011 at 11:27

Attachments:

GoogleCodeExporter commented 9 years ago
Fixed by rsleevi long ago.

Original comment by thakis@chromium.org on 15 Nov 2011 at 12:34