ChrisPei / gyp

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

Use of file lists leads to output outside the build output directory, even with the ninja generator #316

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Build chromium using ninja.

What is the expected output? What do you see instead?

I expect to not have .gypcmd files littering my source tree. Instead, I do, and 
my directory listings are unreadable.

This is due to the use of the '<|(filename.txt entry1 entry2 entry3)' syntax in 
Chromium's src/native_client/build/untrusted.gypi. This feature was added in 
http://code.google.com/p/gyp/source/detail?r=789 (before ninja, I guess?).

Possibly, these files should go into the build output somewhere (shared 
intermediates?), though the syntax suggests that you'd be able to use 
'filename.txt' elsewhere.

I'm not entirely sure why we even allow the filename to be specified; it'd 
perhaps be viable to just have '<|(filename.txt entry1 entry2 entry3)' ignore 
filename.txt and write the entries to some file (probably not named 
filename.txt), and substitute an appropriate path to that file.

(Either way, we'd be breaking strict backwards compatibility, but it probably 
doesn't matter.)

Original issue reported on code.google.com by viettrun...@chromium.org on 3 Jan 2013 at 7:03

GoogleCodeExporter commented 9 years ago
I suppose the expedient fix is to change untrusted.gypi in Chromium/NaCl to 
explicitly specify <(SHARED_INTERMEDIATE_DIR)/filename.txt, instead of just 
filename.txt.

Still, it seems rather bad that the default mode of operation for <|(...) is to 
dump files in the source tree.

The difficulty with fixing this is that this is handled in input.py, which 
doesn't know anything about output directories. Hrm.

Original comment by viettrun...@chromium.org on 3 Jan 2013 at 7:29

GoogleCodeExporter commented 9 years ago
Ugh, I'm totally wrong, and things are even more busted than I thought.

The problem is that we're running commands and producing output while loading 
the .gyp file. Obviously, you can't produce output at that stage into a build 
output directory, since a) the build output directory depends on the generator, 
b) the build output directory probably doesn't exist yet, c) the build output 
directory will be created by the build tool (e.g., ninja or make) and depends 
on, e.g., the build configuration (which can be specified at build time).

The design of <|(...) (etc.) is basically broken. It leads to strange things to 
files in 'inputs' lists that don't exist and aren't explicitly created by 
anything (e.g., it's not in any 'outputs' list), but are only implicitly 
created by (e.g.) the action itself when gyp is run.

Probably, file lists should:
a) be anonymous (i.e., you don't get to specified a name for a file list),
b) inputs/dependencies should only be on the sources (i.e., contents of the 
file lists),
c) be produced at build time (with appropriate build rules/dependencies to 
produce the file list file and insert its name into an action produced by the 
generator).

Original comment by viettrun...@chromium.org on 3 Jan 2013 at 9:28

GoogleCodeExporter commented 9 years ago
This also causes unnecessary rebuilds on the Chromium bots: when "gclient 
revert" is run, it removes all .gypcmd files, which cause all dependent targets 
to be rebuilt. See 
http://build.chromium.org/p/chromium.mac/builders/Mac%20Builder%20%28dbg%29/buil
ds/52954/steps/compile/logs/stdio for an example.

Original comment by binji@chromium.org on 18 Sep 2013 at 9:16

GoogleCodeExporter commented 9 years ago
See also https://code.google.com/p/chromium/issues/detail?id=297186

Original comment by thakis@chromium.org on 8 Oct 2013 at 3:34

GoogleCodeExporter commented 9 years ago
https://codereview.chromium.org/27418008/

Original comment by thakis@chromium.org on 16 Oct 2013 at 10:36

GoogleCodeExporter commented 9 years ago

Original comment by thakis@chromium.org on 16 Oct 2013 at 10:42

GoogleCodeExporter commented 9 years ago
Fixed for ninja, which I declare as Good Enough.

Original comment by thakis@chromium.org on 17 Oct 2013 at 1:18