Do not treat duplicate basenames as an error except for MSVC2008 generator and Make generator on OSX (issue 344573002 by yukawa@chromium.org)

37 views
Skip to first unread message

yuk...@chromium.org

unread,
Jun 18, 2014, 1:31:03 PM6/18/14
to sco...@chromium.org, tha...@chromium.org, to...@chromium.org, rsl...@chromium.org, gyp-de...@googlegroups.com, ir...@google.com, s...@chromium.org, bradn...@chromium.org, mto...@chromium.org, ev...@chromium.org
Reviewers: scottmg, Nico (away), Torne, Ryan Sleevi,

Message:
Hi Scott, Nico. Torne, Ryan.

I know that I'm not the first person who requested this, but I'd like to
hear
comments about the possibility to relax the condition when duplicate
basenames
are treated as an error.

My assumption is that relaxing this condition shouldn't be so dangerous as
long
as duplicate basenames continue to be detected as an error on platforms
where
they are actually troublesome. What do you think about this approach?

Just FYI, here is the list of related discussions I found. Sorry if I
missed
some important discussions.
- https://code.google.com/p/gyp/issues/detail?id=264
- https://code.google.com/p/gyp/issues/detail?id=384
- https://groups.google.com/d/msg/gyp-developer/QN-nYWTMUkM/utHOh9kjx9oJ
- https://codereview.chromium.org/12063003/
- https://codereview.chromium.org/12381003/


Description:
Do not treat duplicate basenames as an error except for MSVC2008 generator
and
Make generator on OSX

With this CL, duplicate basenames in the same source list are no longer
treated
as an error except for MSVC2008 generator and Make generator on OSX.

This change should not be so dangerous because those generators that don't
support duplicate basename continue to detect duplicate basenames as an
error
even with this CL. In other words, it becomes much more important for each
project owner to set up and maintain try servers/build bots to detect build
breakage as soon as possible.

BUG=gyp:264, gyp:384

Please review this at https://codereview.chromium.org/344573002/

SVN Base: http://gyp.googlecode.com/svn/trunk

Affected files (+117, -73 lines):
M pylib/gyp/generator/make.py
M pylib/gyp/generator/msvs.py
M pylib/gyp/input.py
M test/errors/gyptest-errors.py
D test/same-source-file-name/gyptest-fail-shared.py
D test/same-source-file-name/gyptest-fail-static.py
A + test/same-source-file-name/gyptest-shared.py
A + test/same-source-file-name/gyptest-static.py
M test/same-source-file-name/src/double-shared.gyp
M test/same-source-file-name/src/double-static.gyp


sco...@chromium.org

unread,
Jun 18, 2014, 3:47:38 PM6/18/14
to yuk...@chromium.org, tha...@chromium.org, to...@chromium.org, rsl...@chromium.org, gyp-de...@googlegroups.com, ir...@google.com, s...@chromium.org, bradn...@chromium.org, mto...@chromium.org, ev...@chromium.org
Per Ryan's comment on the thread linked to, I think this is contrary to gyp
design, though it's also kind of a trivial thing to have this many people
fiddle
with at this point.

I assume if make on other platforms can work, then Mac could be made to work
too? I think (?) we can deprecate 2008 now (if v8 has transitioned to
something
newer), so then we would be able to keep the lowest-common-denominator
behaviour.

https://codereview.chromium.org/344573002/

sco...@chromium.org

unread,
Jun 18, 2014, 3:49:21 PM6/18/14
to yuk...@chromium.org, tha...@chromium.org, to...@chromium.org, rsl...@chromium.org, gyp-de...@googlegroups.com, ir...@google.com, s...@chromium.org, bradn...@chromium.org, mto...@chromium.org, ev...@chromium.org
Oh, I see now it's a libtool thing. Hm, so I guess we'd have to deprecate
both
mac/make and vs2008. I don't know if anyone cares about mac/make or not.

https://codereview.chromium.org/344573002/

yuk...@chromium.org

unread,
Jun 18, 2014, 8:36:39 PM6/18/14
to sco...@chromium.org, tha...@chromium.org, to...@chromium.org, rsl...@chromium.org, gyp-de...@googlegroups.com, ir...@google.com, s...@chromium.org, bradn...@chromium.org, mto...@chromium.org, ev...@chromium.org
Thank you for the comment. Yes, I agree that deprecating these two
generators
first is the best and reasonable way. BTW, deprecating something in a
(somewhat) widely used OSS project is difficult in general. So I'd like to
know
what is the best way to make this happen step by step.

Do you think it is acceptable to introduce a new command line flag like
--no-duplicate-basenames-check [default:false]
like originally proposed in https://codereview.chromium.org/12063003/ ?

Or, do you think we can now introduce a new command line flag like
--check-compatibility-with-legacy-generators [default:false]
as an first step toward deprecating vs2008/mac-make generator?

Either is fine with me. I'd like to choose the first option if it allows
us to
reach consensus more easily. On the other hand, if people agree to have the
second option, it would be a good progress to deprecate these generators.

Any thoughts?

https://codereview.chromium.org/344573002/

Nico Weber

unread,
Jun 18, 2014, 9:23:45 PM6/18/14
to yuk...@chromium.org, Scott Graham, Nico Weber, Torne (Richard Coles), Ryan Sleevi, gyp-developer, Ian Roth, Sam Clegg, bradn...@chromium.org, mto...@chromium.org, Evan Martin
The Xcode generator is still used by chromium/ios fwiw

Scott Graham

unread,
Jun 18, 2014, 10:24:04 PM6/18/14
to Nico Weber, yuk...@chromium.org, Torne (Richard Coles), Ryan Sleevi, gyp-developer, Ian Roth, Sam Clegg, bradn...@chromium.org, mto...@chromium.org, Evan Martin
Nico, you mean that Xcode has this limitation too due to libtool? In that case, I don't think we can really pursue this, at least for Chromium, so I'd be inclined to say we could add something like a --no-duplicate-basenames-check, but we'd probably want it to default to checking (i.e. maintaining current behaviour).

Nico Weber

unread,
Jun 18, 2014, 10:33:44 PM6/18/14
to Scott Graham, Torne, (Richard Coles), Sam Clegg, Ian Roth, bradn...@chromium.org, Ryan Sleevi, mto...@chromium.org, yuk...@chromium.org, Evan Martin, gyp-developer

Oh, I thought you said that above when you said "Mac has this problem", but reading more carefully you refer to make/Mac. I don't think anyone cares about that; ninja/Mac is strictly better. So I think removing the warning makes sense.

Yohei Yukawa

unread,
Jun 19, 2014, 1:54:56 AM6/19/14
to Nico Weber, Scott Graham, Torne, (Richard Coles), Sam Clegg, Ian Roth, bradn...@chromium.org, Ryan Sleevi, mto...@chromium.org, Evan Martin, gyp-developer
Thanks. I double-checked that Xcode can handle a static_library target that contains duplicated basenames so iOS folks shouldn't be unhappy anyway.

I also dig into the build log of Xcode to see why duplicated basenames isn't a problem for Xcode, and found that Xcode intentionally generates .o files with different basenames to avoid conflict.
For example, test/same-source-file-name/src/{func.c, /subdir1/func.c, /subdir2/func.c} were compiled as func-D255E5F19577A1B1.o, func-F05DC6C08E5692C1.o, and func-D11A781EF6FD3AA2.o.  I think this approach is exactly what Scott mentioned in the following thread. 

Anyway, as Nico mentioned, ninja/Mac should be much better solution than make/Mac, even though we could do the same thing for make/Mac generator.
Reply all
Reply to author
Forward
0 new messages