Added new command line option: --allow-duplicate-basenames

Added new command line option: --allow-duplicate-basenames 

Previously files with the same basename would trigger a gyp error because MSVC08 can't handle this. For users who don't care about MSVC08, this restriction is unnecessary.

[bradn, 2013-01-29 18:02:43]

lgtm

[Sam Clegg, 2013-01-29 18:55:15]

https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py#newcode2663
pylib/gyp/input.py:2663: if 'arm' not in variables.get('target_arch', ''):
Could we have some gyp variable / define that we can add
to the chromium gyp files.. then this TODO could be removed?

(Assuming VC2008 is not longer supported in chromium?)

[iroth, 2013-01-30 20:23:40]

On 2013/01/29 18:55:15, Sam Clegg wrote:
> https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py
> File pylib/gyp/input.py (right):
> 
> https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py#newcode2663
> pylib/gyp/input.py:2663: if 'arm' not in variables.get('target_arch', ''):
> Could we have some gyp variable / define that we can add
> to the chromium gyp files.. then this TODO could be removed?
> 
> (Assuming VC2008 is not longer supported in chromium?)

Added an additional target variable: allow_duplicate_basenames.  If true, has
the same effect as the command line parameter.

[iroth, 2013-02-04 23:47:11]

On 2013/01/30 20:23:40, iroth wrote:
> On 2013/01/29 18:55:15, Sam Clegg wrote:
> > https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py
> > File pylib/gyp/input.py (right):
> > 
> >
https://codereview.chromium.org/12063003/diff/1/pylib/gyp/input.py#newcode2663
> > pylib/gyp/input.py:2663: if 'arm' not in variables.get('target_arch', ''):
> > Could we have some gyp variable / define that we can add
> > to the chromium gyp files.. then this TODO could be removed?
> > 
> > (Assuming VC2008 is not longer supported in chromium?)
> 
> Added an additional target variable: allow_duplicate_basenames.  If true, has
> the same effect as the command line parameter.

Any comments on the new variable? Otherwise will submit tomorrow. Thanks!

[Sam Clegg, 2013-02-09 17:52:47]

https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:2660: target_variables = target_dict['variables']
You can just do:

target_variables = target_dict.get('variables', {})

https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:2665: allow_duplicates_variable =
target_variables['allow_duplicate_basenames']
You can just do:

allow_duplicates_variable = target_variables.get('allow_duplicate_basenames')

https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:2670: ValidateSourcesInTarget(target, target_dict,
build_file)
Seems like it would be nice to remove this TODO, and the
one one on line 2329.

ValidateSourcesInTarget only really needs to generate this
error for msvs_version == 2008 right?  If we make the
errors conditional on this wouldn't that aleviate the need
for this new option/variable altogether.  The result would
be that your project (as well as arm/chrome) would generate
errors if somebody tried to use VS2008, which it seems to
me if the desired behaviour.

with the change as it stands somebody using VS2008 would
have this error silently suppressed, no?

[iroth, 2013-02-28 00:00:36]

On 2013/02/09 17:52:47, Sam Clegg wrote:
> https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py
> File pylib/gyp/input.py (right):
> 
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> pylib/gyp/input.py:2660: target_variables = target_dict['variables']
> You can just do:
> 
> target_variables = target_dict.get('variables', {})
> 
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> pylib/gyp/input.py:2665: allow_duplicates_variable =
> target_variables['allow_duplicate_basenames']
> You can just do:
> 
> allow_duplicates_variable = target_variables.get('allow_duplicate_basenames')
> 
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> pylib/gyp/input.py:2670: ValidateSourcesInTarget(target, target_dict,
> build_file)
> Seems like it would be nice to remove this TODO, and the
> one one on line 2329.
> 
> ValidateSourcesInTarget only really needs to generate this
> error for msvs_version == 2008 right?  If we make the
> errors conditional on this wouldn't that aleviate the need
> for this new option/variable altogether.  The result would
> be that your project (as well as arm/chrome) would generate
> errors if somebody tried to use VS2008, which it seems to
> me if the desired behaviour.
> 
> with the change as it stands somebody using VS2008 would
> have this error silently suppressed, no?

That's the way I had originally intended to make this change, so I'm in favor of
it. The only downside is that users who want to keep their options open and be
warned about potential breakages if/when they port to new platforms will no
longer be warned. But I think that's OK since porting to a new platform almost
always raises new unexpected issues. Also MSVS 2008 is an old IDE and hopefully
will not be supported much longer.

Created a new change for this approach:
https://codereview.chromium.org/12381003/

[Sam Clegg, 2014-11-14 17:18:57]

On 2013/02/28 00:00:36, iroth wrote:
> On 2013/02/09 17:52:47, Sam Clegg wrote:
> > https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py
> > File pylib/gyp/input.py (right):
> > 
> >
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> > pylib/gyp/input.py:2660: target_variables = target_dict['variables']
> > You can just do:
> > 
> > target_variables = target_dict.get('variables', {})
> > 
> >
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> > pylib/gyp/input.py:2665: allow_duplicates_variable =
> > target_variables['allow_duplicate_basenames']
> > You can just do:
> > 
> > allow_duplicates_variable =
target_variables.get('allow_duplicate_basenames')
> > 
> >
>
https://codereview.chromium.org/12063003/diff/8001/pylib/gyp/input.py#newcode...
> > pylib/gyp/input.py:2670: ValidateSourcesInTarget(target, target_dict,
> > build_file)
> > Seems like it would be nice to remove this TODO, and the
> > one one on line 2329.
> > 
> > ValidateSourcesInTarget only really needs to generate this
> > error for msvs_version == 2008 right?  If we make the
> > errors conditional on this wouldn't that aleviate the need
> > for this new option/variable altogether.  The result would
> > be that your project (as well as arm/chrome) would generate
> > errors if somebody tried to use VS2008, which it seems to
> > me if the desired behaviour.
> > 
> > with the change as it stands somebody using VS2008 would
> > have this error silently suppressed, no?
> 
> That's the way I had originally intended to make this change, so I'm in favor
of
> it. The only downside is that users who want to keep their options open and be
> warned about potential breakages if/when they port to new platforms will no
> longer be warned. But I think that's OK since porting to a new platform almost
> always raises new unexpected issues. Also MSVS 2008 is an old IDE and
hopefully
> will not be supported much longer.
> 
> Created a new change for this approach:
> https://codereview.chromium.org/12381003/

Can this issue be closed now?