[mojo] Allow bindings generation with explicit relative include paths

[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.

Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"

Committed: https://crrev.com/6f8d0b04f777a8cc17d0c214100cf6b74d24f654
Cr-Commit-Position: refs/heads/master@{#407914}

[yzshen1, 2016-07-21 23:21:51]

yzshen@chromium.org changed reviewers:
+ yzshen@chromium.org

[yzshen1, 2016-07-21 23:21:52]

https://codereview.chromium.org/2171033002/diff/1/mojo/public/tools/bindings/...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/1/mojo/public/tools/bindings/...
mojo/public/tools/bindings/mojom_bindings_generator.py:85: return
os.path.join(dir_name, file_name), None
According to our discussion, I wonder why this is necessary (instead of raising
an exception.)

If it is really needed, should we return dir_name instead of None?

[Luis Héctor Chávez, 2016-07-21 23:32:00]

The CQ bit was checked by lhchavez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 23:32:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Luis Héctor Chávez, 2016-07-21 23:33:13]

https://codereview.chromium.org/2171033002/diff/1/mojo/public/tools/bindings/...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/1/mojo/public/tools/bindings/...
mojo/public/tools/bindings/mojom_bindings_generator.py:85: return
os.path.join(dir_name, file_name), None
On 2016/07/21 23:21:51, yzshen1 wrote:
> According to our discussion, I wonder why this is necessary (instead of
raising
> an exception.)
> 
> If it is really needed, should we return dir_name instead of None?

Returning this will trigger L178 to print a nice include-chain to better
diagnose errors. Will change None to |dir_name| to remove L131-132.

[commit-bot: I haz the power, 2016-07-22 00:18:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-22 00:18:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Luis Héctor Chávez, 2016-07-22 20:06:04]

Description was changed from

==========
[mojo] Relativize mojom modules based on their include path

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path.

BUG=
==========

to

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.
==========

[Luis Héctor Chávez, 2016-07-22 20:06:42]

The CQ bit was checked by lhchavez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-22 20:07:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 23:30:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-22 23:30:02]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yzshen1, 2016-07-22 23:38:10]

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:224:
args.import_directories[idx] = RelativePath(tokens[0], args.depth)
I am confused, args.depth is described as "depth from source root". but this is
passed into RelativePath as "source_root".

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:274: "a colon")

Could you please give an example why this is necessary? Maybe in the CL
description.

[Luis Héctor Chávez, 2016-07-25 21:00:14]

Description was changed from

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.
==========

to

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.


Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"
==========

[Luis Héctor Chávez, 2016-07-25 21:00:22]

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:224:
args.import_directories[idx] = RelativePath(tokens[0], args.depth)
On 2016/07/22 23:38:10, yzshen1 wrote:
> I am confused, args.depth is described as "depth from source root". but this
is
> passed into RelativePath as "source_root".

args.depth is used as "relative path from current directory to source root".
Should I change the documentation? (It confused me as well).

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:274: "a colon")
On 2016/07/22 23:38:10, yzshen1 wrote:
> 
> Could you please give an example why this is necessary? Maybe in the CL
> description.

Done.

[yzshen1, 2016-07-26 18:35:26]

One more comment. Thanks!

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:147:
RelativePath(dirname, args.depth), import_data['filename'],
Is it intended to use args.depth instead of rel_filename.source_root here?

[Luis Héctor Chávez, 2016-07-26 19:49:59]

The CQ bit was checked by lhchavez@chromium.org to run a CQ dry run

[Luis Héctor Chávez, 2016-07-26 19:50:02]

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/mojom_bindings_generator.py (right):

https://codereview.chromium.org/2171033002/diff/40001/mojo/public/tools/bindi...
mojo/public/tools/bindings/mojom_bindings_generator.py:147:
RelativePath(dirname, args.depth), import_data['filename'],
On 2016/07/26 18:35:26, yzshen1 wrote:
> Is it intended to use args.depth instead of rel_filename.source_root here?

It's not. Using rel_filename.source_root instead.

[commit-bot: I haz the power, 2016-07-26 19:50:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yzshen1, 2016-07-26 19:51:25]

LGTM

Thanks for patience.

[commit-bot: I haz the power, 2016-07-26 21:02:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 21:02:56]

Dry run: This issue passed the CQ dry run.

[Luis Héctor Chávez, 2016-07-26 21:04:48]

The CQ bit was checked by lhchavez@chromium.org

[commit-bot: I haz the power, 2016-07-26 21:05:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 21:10:20]

Description was changed from

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.


Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"
==========

to

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.


Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"
==========

[commit-bot: I haz the power, 2016-07-26 21:10:21]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-26 21:13:27]

Description was changed from

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.


Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"
==========

to

==========
[mojo] Allow bindings generation with explicit relative include paths

If an imported mojo module AND its target directory are located outside
of the normal tree structure from the rest of the files, it will cause
the generated code to have a different-than-expected include path. This
change allows each -I directive to have an optional 'depth' modifier so
that all paths are correctly relativized. Omitting the modifier is a
no-op and preserves current behavior.

Example scenario:

project_a/src/A.mojom
project_b/src/B.mojom (import "A.mojom")

mojom_bindings_generator.py \
  -g c++ \
  -d project_a/src \
  -o project_a/gen \
  project_a/src/A.mojom
# Normal invocation

mojom_bindings_generator.py \
  -I project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# This causes A's generated import to be
# "../../../project_a/src/A.mojom.h"

mojom_bindings_generator.py \
  -I project_a/src:project_a/src \
  -g c++ \
  -d project_b/src \
  -o project_b/gen \
  project_b/src/B.mojom
# The addition of the depth modified to the include
# causes A's generated import to be "A.mojom.h"

Committed: https://crrev.com/6f8d0b04f777a8cc17d0c214100cf6b74d24f654
Cr-Commit-Position: refs/heads/master@{#407914}
==========

[commit-bot: I haz the power, 2016-07-26 21:13:28]

Patchset 4 (id:??) landed as
https://crrev.com/6f8d0b04f777a8cc17d0c214100cf6b74d24f654
Cr-Commit-Position: refs/heads/master@{#407914}