ninja: Add target_rpath generator flag

ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

Contributed by Julien Brianceau <jbriance@cisco.com>;

BUG=none
R=sdefresne@chromium.org, thakis@chromium.org

Committed: https://chromium.googlesource.com/external/gyp/+/fbcb317c7513c1e3bd671a9e5deb8c936e39bf4c

[jbriance, 2016-03-18 10:26:16]

Description was changed from

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

BUG=none
==========

to

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

BUG=none
==========

[jbriance, 2016-03-18 10:26:16]

jbriance@cisco.com changed reviewers:
+ sbaig1@bloomberg.net, scottmg@chromium.org, sdefresne@chromium.org,
thakis@chromium.org

[Nico, 2016-03-18 14:00:21]

lgtm

If you don't want others to accidentally break this, consider adding a test.
(But if you're fine with that risk, ok as-is too.)

[jbriance, 2016-03-18 14:07:42]

On 2016/03/18 14:00:21, Nico wrote:
> lgtm
> 
> If you don't want others to accidentally break this, consider adding a test.
> (But if you're fine with that risk, ok as-is too.)

I think your approval and my second patch including a test were sent
simultaneously :)

If you're ok with the test, how should I proceed to commit ? I'm don't think I
have commit privileges and there is no commit queue.

[sdefresne, 2016-03-18 14:12:40]

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.p...
pylib/gyp/generator/ninja.py:382: self.target_rpath =
generator_flags.get('target_rpath', r'\$$ORIGIN/lib/')
Instead of passing this as a generator flag, why not allow this to be
configurable in the .gyp files instead?

[Nico, 2016-03-18 14:17:49]

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.p...
pylib/gyp/generator/ninja.py:382: self.target_rpath =
generator_flags.get('target_rpath', r'\$$ORIGIN/lib/')
On 2016/03/18 14:12:40, sdefresne wrote:
> Instead of passing this as a generator flag, why not allow this to be
> configurable in the .gyp files instead?

I feel that if you can get away with a less general thing, you should always do
that. Flexibility enables complexity. So if nobody needs this at a per-target
level, I wouldn't support it there.

[Nico, 2016-03-18 14:18:58]

Test looks fine to me too, thanks! Can you run `git cl try` and check if that
triggers try bots?

[jbriance, 2016-03-18 14:20:37]

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.p...
pylib/gyp/generator/ninja.py:382: self.target_rpath =
generator_flags.get('target_rpath', r'\$$ORIGIN/lib/')
On 2016/03/18 14:12:40, sdefresne wrote:
> Instead of passing this as a generator flag, why not allow this to be
> configurable in the .gyp files instead?

"output_dir" is a generator flag, so I thought this new "target_rpath" could be
a generator flag as well.
I also think that rpath is more a matter that should be up to the gyp caller. I
mean from same .gyp file, you might want two different rpath for two different
target systems.

[jbriance, 2016-03-18 14:21:47]

On 2016/03/18 14:18:58, Nico wrote:
> Test looks fine to me too, thanks! Can you run `git cl try` and check if that
> triggers try bots?

It seems I can't:

  $ git cl try
  ERROR: Error in response. Reason: . Message: User user:jbriance@cisco.com
cannot add builds to bucket master.client.gyp.

[Nico, 2016-03-18 14:28:35]

Ok, I started try bots. If they come back green, I'll land this for you.

[jbriance, 2016-03-18 14:31:10]

Description was changed from

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

BUG=none
==========

to

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

Contributed by Julien Brianceau <jbriance@cisco.com>

BUG=none
==========

[jbriance, 2016-03-18 14:33:09]

On 2016/03/18 14:28:35, Nico wrote:
> Ok, I started try bots. If they come back green, I'll land this for you.

Great, thanks.
I've added a "Contributed by" line in the change description, to follow
"Committing changes" guidance explained in
https://www.chromium.org/developers/contributing-code

[sdefresne, 2016-03-18 15:41:38]

lgtm

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/1813053003/diff/1/pylib/gyp/generator/ninja.p...
pylib/gyp/generator/ninja.py:382: self.target_rpath =
generator_flags.get('target_rpath', r'\$$ORIGIN/lib/')
On 2016/03/18 at 14:17:49, Nico wrote:
> On 2016/03/18 14:12:40, sdefresne wrote:
> > Instead of passing this as a generator flag, why not allow this to be
> > configurable in the .gyp files instead?
> 
> I feel that if you can get away with a less general thing, you should always
do that. Flexibility enables complexity. So if nobody needs this at a per-target
level, I wouldn't support it there.

Acknowdledged.

[Nico, 2016-03-18 17:42:58]

Description was changed from

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

Contributed by Julien Brianceau <jbriance@cisco.com>

BUG=none
==========

to

==========
ninja: Add target_rpath generator flag

Current rpath is hardcoded to $ORIGIN/lib. Allow the user to use
a custom one through a new target_rpath generator flag.

Contributed by Julien Brianceau <jbriance@cisco.com>

BUG=none
R=sdefresne@chromium.org, thakis@chromium.org

Committed:
https://chromium.googlesource.com/external/gyp/+/fbcb317c7513c1e3bd671a9e5deb...
==========

[Nico, 2016-03-18 17:42:59]

Committed patchset #2 (id:20001) manually as
fbcb317c7513c1e3bd671a9e5deb8c936e39bf4c (presubmit successful).