ChrisPei / gyp

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

Duplicate target names in different directories cause problems #270

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
gyp forbids targets having the same name if they are defined in the same 
directory in the source, but allows it if they are defined in different 
directories. This is a source of bugs and confusion:

1) Using the make backend, if two targets with the same name also define 
actions/rules/copies with the same action/rule/copy name, the command for one 
will overwrite the other, because it's stored in a variable called 
cmd_targetname_actionname, which is not qualified with the directory name.

2) If two targets have the same name, the make alias for them is the same, 
which means that if you run "make foo" it builds both foo's, and there's no way 
to get it to just build the one in a particular directory without working out 
what the actual output file path in out/Debug/whatever is.

3) If two executable targets have the same name, the default install behaviour 
is to install them to the same location, which makes a conflicting rule in make.

4) There is at least one gyp backend under development (for the Android 
makefile system) which requires that target names are unique, and thus would 
have to mangle the path into the target name during generation.

To avoid all these issues in one go, and prevent future similar bugs from 
creeping in, I propose we forbid duplicate target names no matter what 
directory they are in.

Chromium currently has several duplicate target names, so probably the easiest 
way is if this was added as an option defaulting to off initially, and then 
changed to default to on (or made mandatory) once Chromium is fixed.

Original issue reported on code.google.com by torne@chromium.org on 18 May 2012 at 3:02

GoogleCodeExporter commented 9 years ago
Patch implementing the option: https://chromiumcodereview.appspot.com/10408025

Original comment by torne@chromium.org on 18 May 2012 at 3:05

GoogleCodeExporter commented 9 years ago
The outcome of the recent discussion on gyp-developer [1] is that gyp should 
allow duplicate target names in different directories and that we should fix 
the individual backends to either support this, or at least fail gracefully.

I've spoken to Torne about this and we're on the same page - he's abandoned 
https://chromiumcodereview.appspot.com/10408025 and I'll upload a new patch.

[1] https://groups.google.com/forum/?fromgroups#!topic/gyp-developer/6fbaqLfvAU0

Original comment by stevebl...@chromium.org on 29 May 2012 at 11:39

GoogleCodeExporter commented 9 years ago
The current behaviour of the ninja backend ...
- If multiple targets in different directories use the same name, ninja outputs 
a warning that 'multiple rules generate target. build will not be correct'. The 
build continues, but only one target is generated.
- If such duplicate targets also define actions/rules/copies with the same 
name, ninja outputs an error that there is a duplicate rule, and fails.

Original comment by stevebl...@chromium.org on 29 May 2012 at 11:56

GoogleCodeExporter commented 9 years ago
Commit: 159fe3f09a7398b3bf48200855888596c1b95110
 Email: steveblock@chromium.org@78cadc50-ecff-11dd-a971-7dbc132099af

Fix make and ninja backends to sensibly handle duplicate target names in 
different directories

Currently, if two multiple targets in different directories use the same name, 
gyp's behaviour depends on the particular backend and is confusing at best. See 
bug for details. 

This change fixes the make and ninja backends to ... 
- always build all targets when duplicate targets exist 
- use the correct action/rule/copy when such duplicate targets define 
actions/rules/copies with the same name 

It also adds a test for this. 

BUG=gyp:270
TEST=test/same-target-name-different-directory/ 
Review URL: https://chromiumcodereview.appspot.com/10447063

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

M   pylib/gyp/generator/make.py
M   pylib/gyp/generator/ninja.py
M   pylib/gyp/generator/ninja_test.py
M   test/intermediate_dir/gyptest-intermediate-dir.py
A   test/same-target-name-different-directory/gyptest-all.py
A   test/same-target-name-different-directory/src/subdir1/subdir1.gyp
A   test/same-target-name-different-directory/src/subdir2/subdir2.gyp
A   test/same-target-name-different-directory/src/subdirs.gyp
A   test/same-target-name-different-directory/src/touch.py

Original comment by bugdroid1@chromium.org on 11 Jun 2012 at 10:01

GoogleCodeExporter commented 9 years ago

Original comment by stevebl...@chromium.org on 11 Jun 2012 at 10:02

GoogleCodeExporter commented 9 years ago
This change seems to break ninja builds due to the rsp generated file name's 
length:

ninja: ERROR: 
WriteFile(cloud_policy_backend_header_compile_target_src_chrome_app_policy_cloud
_policy_codegen_gyp_cloud_policy_backend_header_compile_target_genproto.pyproto_
chrome_browser_policy_proto_device_management_backend_pb2_py.rsp): Unable to 
create file. No such file or directory

Original comment by jaysoff...@gmail.com on 22 Jun 2012 at 9:04

GoogleCodeExporter commented 9 years ago
The full command is:

cmd /s /c "python gyp-win-tool action-wrapper environment.x86 
cloud_policy_backend_header_compile_target_src_chrome_app_policy_cloud_policy_co
degen_gyp_cloud_policy_backend_header_compile_target_genproto.pyproto_chrome_bro
wser_policy_proto_device_management_backend_pb2_py.rsp"
WriteFile(cloud_policy_backend_header_compile_target_src_chrome_app_policy_cloud
_policy_codegen_gyp_cloud_policy_backend_header_compile_target_genproto.pyproto_
chrome_browser_policy_proto_device_management_backend_pb2_py.rsp): Unable to 
create file. No such file or directory

Original comment by jaysoff...@gmail.com on 22 Jun 2012 at 9:06

GoogleCodeExporter commented 9 years ago
Opened https://code.google.com/p/gyp/issues/detail?id=276 to track the issue 
describe in previous two comments.

Original comment by jaysoff...@gmail.com on 22 Jun 2012 at 9:28