yuhangwang / gyp

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

duplicate basenames warning is obsolete #384

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
gyp is no longer tested on vs2008, and I don't think there's other build 
systems that have the limitation that cause this error:

static library src/clank/src/chrome/chrome.gyp:unit_tests#target has several 
files with the same basename: 
update_manifest_unittest: 
browser/component_updater/test/update_manifest_unittest.cc 
common/extensions/update_manifest_unittest.cc 
Some build systems, e.g. MSVC08, cannot handle that. 

See also https://codereview.chromium.org/83663007/

Original issue reported on code.google.com by scottmg@chromium.org on 23 Nov 2013 at 3:42

GoogleCodeExporter commented 9 years ago
Any plans for this? It is blocking one of my CLs.

Original comment by mtomasz@chromium.org on 27 Mar 2014 at 3:40

GoogleCodeExporter commented 9 years ago
I think we could probably delete the warning at this point, but there are some 
non-Chromium users of gyp that are still using 2008. Presumably you could just 
rename one of your files as we've been doing up until now?

Original comment by scottmg@chromium.org on 27 Mar 2014 at 5:11

GoogleCodeExporter commented 9 years ago
I've been applying brandon...@gmail.com's patch which is attached to issue 
https://code.google.com/p/gyp/issues/detail?id=264
that downgrades the error to a warning if the generator is not msvs2008.

It has applied cleanly to all of the gyp repos revisions I've updated to, 
including very recent ones.  I recommend that it be reviewed and integrated.

/Steve Hartwell

https://gyp.googlecode.com/issues/attachment?aid=2640002000&name=0b2eb1f758bbe24
75626e8a1a8e4fcebaeb027a9.patch.txt&token=jXE76LOB3jiPqA6vUuRJVpVqH_w%3A13959836
26090

Original comment by steve.ha...@gmail.com on 28 Mar 2014 at 5:22

GoogleCodeExporter commented 9 years ago
@scottmg: Whith the amount of files in Chromium it is sometimes very difficult 
to rename. If you use namespaces well to avoid long class names, then this 
becomes a problem.

Eg. I wanted to have a AsyncFileUtil class in my own file_system_provider 
namespace to avoid FileSystemProviderAsyncFileUtil longish class name. But it 
can't be done because of this GYP error. As a result, I had to add some 
unnatural prefix like ProviderAsyncFileUtil, which is inconsistent in naming 
with all other classes in the namespace.

How about adding some switch enabling support for vs2008, which would be 
disabled by default?

Original comment by mtomasz@chromium.org on 28 Mar 2014 at 5:30

GoogleCodeExporter commented 9 years ago
FYI, in gyp r1947 and later, you can opt out this validation with 
'--no-duplicate-basename-check' GYP option.

See the following threads about the discussion behind r1947.
https://groups.google.com/d/msg/gyp-developer/DP6ZWsDkW4o/e2IFn8-5lAYJ
https://groups.google.com/d/msg/gyp-developer/qQ7RxD7yCUg/GRn7AbUtygAJ

Hopefully this validation will be retired gradually (opt-in -> opt-out -> 
removed), but I'm not sure when it will happen though.

As an interim solution for Chromium, it might be acceptable to add 
'--no-duplicate-basename-check' to src/build/gyp_chromium because I guess we no 
longer care about VC++2008 and make on Mac in Chromium.  This could be 
justified because we have already relied on '--no-circular-check' for Mac in 
gyp_chromium.
https://code.google.com/p/chromium/codesearch#chromium/src/build/gyp_chromium

Once Chromium have accepted '--no-duplicate-basename-check', we can safely 
remove the following workaround from GYP. It should be helpful for GYP.
https://code.google.com/p/gyp/source/browse/trunk/pylib/gyp/input.py?r=1947#2842

Perhaps it should also be helpful for GN folks because I guess we don't want GN 
to inherit this limitation from GYP.

Original comment by yukawa@chromium.org on 25 Jun 2014 at 2:48

GoogleCodeExporter commented 9 years ago
Fixed as r1993 by a patch from Matthew.

Original comment by yukawa@chromium.org on 24 Oct 2014 at 1:53

GoogleCodeExporter commented 9 years ago
Turns out this isn't obsolete after all, libtool prints a warning about this on 
OS X.

Original comment by thakis@chromium.org on 21 May 2015 at 7:38

GoogleCodeExporter commented 9 years ago
(See e.g. "[chromium-dev] Does chromium require .cc/.h files to have unique 
basenames?" – this should've been a gyp-time thing, not a 
land-revert-since-it-added-a-warning)

Original comment by thakis@chromium.org on 21 May 2015 at 7:39

GoogleCodeExporter commented 9 years ago
Sorry for the trouble.  When I wrote r1947, I confirmed that max/xcode can 
handle duplicated base names (by internally renaming them) in a static library, 
but apparently I should have checked and mentioned in the case of mac/ninja 
scenario.

Original comment by yukawa@chromium.org on 21 May 2015 at 8:12