[PATCH] pylib: gyp: input: don't raise an error but warn on duplicate basenames

65 views
Skip to first unread message

Brandon Philips

unread,
Jun 10, 2012, 3:40:07 PM6/10/12
to gyp-de...@googlegroups.com
libuv has two files which are only used on unix systems which share a
duplicate basename: unix/core.c and unix/linux/core.c

Obviously we won't be compiling these files under MSVC08 so it would be
nice to only warn about Duplicate basenames and raise an error if we are
using the affected versions of MSVS.

Tested this change with MSVS 2010 on Windows against luvit.

---

Also attached the patch to this issue:
http://code.google.com/p/gyp/issues/detail?id=264

Thanks,

Brandon

http://ifup.org

---
pylib/gyp/generator/msvs.py | 6 ++++++
pylib/gyp/input.py | 3 ++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/pylib/gyp/generator/msvs.py b/pylib/gyp/generator/msvs.py
index c01e955..27adf5b 100644
--- a/pylib/gyp/generator/msvs.py
+++ b/pylib/gyp/generator/msvs.py
@@ -1776,6 +1776,12 @@ def GenerateOutput(target_list, target_dicts, data, params):
# GeneratorCalculatedVariables.
msvs_version = params['msvs_version']

+ for val in target_dicts.itervalues():
+ if val.get('duplicate_basename', False):
+ if msvs_version.short_name == "2008":
+ raise KeyError, 'Duplicate basenames not supported under VS 2008'
+
+
generator_flags = params.get('generator_flags', {})

# Optionally shard targets marked with 'msvs_shard': SHARD_COUNT.
diff --git a/pylib/gyp/input.py b/pylib/gyp/input.py
index 2678bab..90ef32c 100644
--- a/pylib/gyp/input.py
+++ b/pylib/gyp/input.py
@@ -2165,7 +2165,8 @@ def ValidateSourcesInTarget(target, target_dict, build_file):
print ('static library %s has several files with the same basename:\n' %
target + error + 'Some build systems, e.g. MSVC08, '
'cannot handle that.')
- raise KeyError, 'Duplicate basenames in sources section, see list above'
+ print ('Duplicate basenames in sources section, see list above')
+ target_dict['duplicate_basename'] = True


def ValidateRulesInTarget(target, target_dict, extra_sources_for_rules):
--
1.7.10

Ryan Sleevi

unread,
Jun 11, 2012, 1:11:57 PM6/11/12
to Brandon Philips, gyp-de...@googlegroups.com
On Sun, Jun 10, 2012 at 12:40 PM, Brandon Philips <bra...@ifup.org> wrote:
libuv has two files which are only used on unix systems which share a
duplicate basename: unix/core.c and unix/linux/core.c

Obviously we won't be compiling these files under MSVC08 so it would be
nice to only warn about Duplicate basenames and raise an error if we are
using the affected versions of MSVS.

Tested this change with MSVS 2010 on Windows against luvit.


Hi Brandon,

Part of GYP's original design philosophy has been to design for the lowest common denominator, so that the build files can truly be usable with a variety of build systems.

Consider, for example, a very useful feature of MSVS that allows you to exclude source files based upon the current configuration. You'll see that, within GYP, it's not possible to modify the sources based on the configuration. This is because other build systems don't support this concept.

Please see the original codereview that introduced this - http://codereview.chromium.org/10010028/ - to understand that the motivation was one of consistency.

Note for future GYP contributions, you'll likely want to look at http://dev.chromium.org/developers/contributing-code , which has more or less the necessary steps (Adhere to the style, preferably add unit tests, fill out a Contributor License Agreement, update the AUTHORS file, upload a codereview using depot_tools)

Hope this helps!

Ryan

Brandon Philips

unread,
Jun 11, 2012, 3:07:12 PM6/11/12
to Ryan Sleevi, gyp-de...@googlegroups.com
On 10:11 Mon 11 Jun 2012, Ryan Sleevi wrote:
> On Sun, Jun 10, 2012 at 12:40 PM, Brandon Philips <bra...@ifup.org> wrote:
>
> > libuv has two files which are only used on unix systems which share a
> > duplicate basename: unix/core.c and unix/linux/core.c
>
> Please see the original codereview that introduced this -
> http://codereview.chromium.org/10010028/ - to understand that the
> motivation was one of consistency.

Thanks for the context Ryan. I didn't realize it was something that
libtool warns about also. I filed a bug against libuv to fix it up:

https://github.com/joyent/libuv/issues/464

For now I am using a patch to keep luvit compiling:

https://github.com/luvit/gyp/commit/66665cc0f78933a7a118603928be059fed23f673

Cheers,

Brandon

Evan Martin

unread,
Jun 11, 2012, 3:56:37 PM6/11/12
to Brandon Philips, Ryan Sleevi, gyp-de...@googlegroups.com
On Mon, Jun 11, 2012 at 12:07 PM, Brandon Philips <bra...@ifup.org> wrote:
> On 10:11 Mon 11 Jun 2012, Ryan Sleevi wrote:
>> On Sun, Jun 10, 2012 at 12:40 PM, Brandon Philips <bra...@ifup.org> wrote:
>>
>> > libuv has two files which are only used on unix systems which share a
>> > duplicate basename: unix/core.c and unix/linux/core.c
>>
>> Please see the original codereview that introduced this -
>> http://codereview.chromium.org/10010028/ - to understand that the
>> motivation was one of consistency.
>
> Thanks for the context Ryan. I didn't realize it was something that
> libtool warns about also. I filed a bug against libuv to fix it up:
>
>    https://github.com/joyent/libuv/issues/464

An alternative workaround might be to make two separate library
targets, one which includes unix/linux/* and another which includes
unix/* and depends on the former.
Reply all
Reply to author
Forward
0 new messages