Fix Java file generation rule in java_aidl.gypi

Fix Java file generation rule in java_aidl.gypi

The Java file generation rule in java_aidl.gypi assumes the wrong output path
for the generated file. This leads to spurious rebuilds.

The problem is that by default, the aidl tool outputs the Java file to
<output-path>/<package-path>/<input-file-root>.java, not to
<output-path>/<input-file-root>.java as assumed by the rule. This change fixes
the problem by explicitly specifying the output path to aidl.

Also remove superfluous path components on input files.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=145982

[michaelbai, 2012-07-09 18:28:11]

http://codereview.chromium.org/10690092/diff/1/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/10690092/diff/1/content/content.gyp#newcode234
content/content.gyp:234: 'input_package_path': 'org/chromium/content/common',
It is a little weird to have input_package_path here, Could we find out the
package name in aidl file?

Did you force the source aidl files to be in the same package?

I added armand as the reviewer, as he is the original author .

[Steve Block, 2012-07-09 19:52:07]

http://codereview.chromium.org/10690092/diff/1/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/10690092/diff/1/content/content.gyp#newcode234
content/content.gyp:234: 'input_package_path': 'org/chromium/content/common',
Yes, we can get the package name from the aidl file.

I tried to do this by using a gyp command expansion to set 'outputs' for the
rule. However, I couldn't work out how to have this command use
<(RULE_INPUT_PATH) to get the name of the aidl file. I think the problem is that
<(RULE_INPUT_ROOT) isn't expanded by gyp when executing the command. I'm not
sure if this is a bug or a known limitation.

I'll continue to look into this, but it would be great to land this now with a
TODO, as it's causing lots of spurious rebuilds.

[michaelbai, 2012-07-09 20:54:57]

LGTM, providing add 'TODO', also add john for content.gyp

[jam, 2012-07-10 00:53:59]

+mark for gyp wisdom

http://codereview.chromium.org/10690092/diff/1/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/10690092/diff/1/content/content.gyp#newcode234
content/content.gyp:234: 'input_package_path': 'org/chromium/content/common',
On 2012/07/09 19:52:07, Steve Block wrote:
> Yes, we can get the package name from the aidl file.
> 
> I tried to do this by using a gyp command expansion to set 'outputs' for the
> rule. However, I couldn't work out how to have this command use
> <(RULE_INPUT_PATH) to get the name of the aidl file. I think the problem is
that
> <(RULE_INPUT_ROOT) isn't expanded by gyp when executing the command. I'm not
> sure if this is a bug or a known limitation.
> 
> I'll continue to look into this, but it would be great to land this now with a
> TODO, as it's causing lots of spurious rebuilds.

I don't know much about gyp, but I'd prefer that this is checked first.

[Steve Block, 2012-07-10 11:24:55]

I looked into this and it seems that gyp executes command expansions very early
on in the processing of gyp files. In particular, the commands are executed
before the gyp file input is passed to the generator. It's the generator that
does the expansion of built-in variables like RULE_INPUT_PATH, so the expanded
value isn't available when the command is executed.

I'm not sure whether it's a conscious design choice that command expansions
can't make use of these built-in variables, but it looks to me like allowing
this would require a significant change to gyp. Mark, please correct me if I'm
wrong or if you can see a way to work around this.

[Mark Mentovai, 2012-07-10 12:30:04]

http://codereview.chromium.org/10690092/diff/1/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/10690092/diff/1/content/content.gyp#newcode234
content/content.gyp:234: 'input_package_path': 'org/chromium/content/common',
You can’t make this work automatically without running some script at GYP time
to figure out where the aidl file’s output will be written. You could do that,
but it’d slow GYP down.

What you can do, that might be cleaner, is to have

'input_package_path': 'org/chromium/content/common',
'aidl_interface_file':
'public/android/java/src/<(input_package_path)/common.aidl'

http://codereview.chromium.org/10690092/diff/1/content/content.gyp#newcode237
content/content.gyp:237:
'public/android/java/src/org/chromium/content/common/ISandboxedProcessCallback.aidl',
and use  input_package_path here too? Or maybe even another variable that also
includes public/android/java/src?

[Steve Block, 2012-07-10 12:38:13]

Thanks for the confirmation Mark.

It turns out that in this particular case, no other rules depend on the exact
path of the output Java files, so we can avoid the problem altogether by
explicitly specifying a fixed output path to aidl.

[michaelbai, 2012-07-10 18:16:33]

LGTM, thanks for handling this.

[Steve Block, 2012-07-10 18:25:26]

Thanks!

Need OWNERS approval

[navabi, 2012-07-10 20:14:57]

lgtm

[jam, 2012-07-10 22:26:49]

lgtm