Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(103)

Issue 1071693003: Uses mojom module names as Dart's package: import URI (Closed)

Created:
5 years, 8 months ago by zra
Modified:
5 years, 8 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, darin (slow to review), mojo-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Uses mojom module names as Dart's package: import URI At present there is a special case in Dart bindings generation which makes the Mojo SDK and things under mojo/services portable. However, we must not require things under mojo/services to be next to the Mojo SDK under mojo/public/... in client repos. Further, other collections of mojoms should also be portable. To fix this, instead of using the path in the host repo as the canonical package: import URI for the generated Dart code, this CL uses the mojom module name. BUG=https://github.com/domokit/mojo/issues/73 R=blundell@chromium.org, johnmccutchan@google.com, tonyg@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/c34c0ab8bbbcb454bd3f4183be9b1c7abdc677b3

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use mojom module name to make canonical package: URIs for Dart #

Patch Set 3 : Sky tests pass #

Patch Set 4 : Merge. Fix zip.py. #

Patch Set 5 : Merge. Fix Dart embedder package creation. #

Patch Set 6 : Merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+148 lines, -188 lines) Patch
M examples/dart/console_example/main.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M examples/dart/device_info/main.dart View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M examples/dart/netcat/main.dart View 1 1 chunk +8 lines, -9 lines 0 comments Download
M examples/dart/wget/main.dart View 1 1 chunk +2 lines, -2 lines 0 comments Download
M mojo/dart/embedder/BUILD.gn View 1 2 3 4 3 chunks +12 lines, -7 lines 0 comments Download
M mojo/dart/embedder/embedder.gni View 1 2 3 4 2 chunks +7 lines, -2 lines 0 comments Download
M mojo/dart/embedder/io/mojo_patch.dart View 1 2 3 4 1 chunk +6 lines, -6 lines 0 comments Download
M mojo/dart/embedder/packages.dart View 1 2 3 4 1 chunk +6 lines, -6 lines 0 comments Download
M mojo/dart/embedder/test/BUILD.gn View 1 2 3 4 4 chunks +4 lines, -4 lines 0 comments Download
M mojo/dart/embedder/test/dart_to_cpp_tests.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M mojo/dart/embedder/test/dart_to_cpp_tests.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M mojo/dart/embedder/test/run_dart_tests.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M mojo/dart/embedder/test/validation_unittest.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M mojo/dart/embedder/tools/dart_embedder_url_mappings.py View 1 2 3 4 3 chunks +10 lines, -14 lines 0 comments Download
M mojo/dart/test/bindings_generation_test.dart View 1 1 chunk +3 lines, -5 lines 0 comments Download
M mojo/dart/test/compile_all_interfaces_test.dart View 1 1 chunk +15 lines, -15 lines 0 comments Download
M mojo/dart/test/validation_test.dart View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/dart/application.dart View 1 1 chunk +3 lines, -5 lines 0 comments Download
M mojo/public/interfaces/application/BUILD.gn View 1 1 chunk +0 lines, -5 lines 0 comments Download
M mojo/public/tools/bindings/generators/mojom_dart_generator.py View 1 2 3 4 5 3 chunks +15 lines, -17 lines 0 comments Download
M mojo/public/tools/bindings/mojom.gni View 1 2 3 3 chunks +2 lines, -9 lines 0 comments Download
M mojo/public/tools/gn/zip.py View 1 2 3 3 chunks +12 lines, -2 lines 0 comments Download
M mojo/services/accessibility/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/clipboard/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/console/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/content_handler/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/device_info/public/interfaces/BUILD.gn View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/files/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/geometry/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/gpu/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/http_server/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/input_events/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/keyboard/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/location/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/native_viewport/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/navigation/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/service_registry/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/surfaces/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -4 lines 0 comments Download
M mojo/services/terminal/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/view_manager/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/services/window_manager/public/interfaces/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M services/dart/dart_apptests/echo_apptests.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/dart_apptests/main.dart View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M services/dart/dart_apptests/pingpong_apptests.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/test/echo/main.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/test/echo_service.mojom View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/test/pingpong/main.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/test/pingpong_service.mojom View 1 1 chunk +1 line, -1 line 0 comments Download
M services/dart/test/pingpong_target/main.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M sky/framework/embedder.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M sky/framework/net/fetch.dart View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M sky/framework/shell.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M sky/tests/editing/backspace.sky View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M sky/tests/editing/replace.sky View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M sky/tests/editing/selection.sky View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M sky/tests/editing/typing.sky View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M sky/tests/resources/event-sender.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M sky/tests/services/event-sender.sky View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M sky/tests/services/iframe-embed-vmc.sky View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M sky/tests/services/network.sky View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M sky/tools/deploy_sdk.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 30 (6 generated)
zra
This change shouldn't require any changes on the Modular side. It basically just pulls out ...
5 years, 8 months ago (2015-04-08 21:45:18 UTC) #2
tonyg
This feels like something Trung should weigh in on, but something bothers me about a ...
5 years, 8 months ago (2015-04-08 21:56:12 UTC) #3
zra
On 2015/04/08 21:56:12, tonyg wrote: > Would it make sense to explore either of these? ...
5 years, 8 months ago (2015-04-08 22:08:48 UTC) #5
viettrungluu
Also, +jamesr. On 2015/04/08 21:56:12, tonyg wrote: > This feels like something Trung should weigh ...
5 years, 8 months ago (2015-04-08 22:56:20 UTC) #7
viettrungluu
(Also +qsr, to weigh in on comparisons with Java....)
5 years, 8 months ago (2015-04-08 22:56:56 UTC) #9
blundell
On 2015/04/08 22:56:20, viettrungluu wrote: > Also, +jamesr. > > On 2015/04/08 21:56:12, tonyg wrote: ...
5 years, 8 months ago (2015-04-09 11:51:12 UTC) #10
jamesr
This seems really unfortunate. Why can't this information be derived from the path? https://codereview.chromium.org/1071693003/diff/1/mojo/public/interfaces/application/application.mojom File ...
5 years, 8 months ago (2015-04-09 22:02:34 UTC) #11
zra
https://codereview.chromium.org/1071693003/diff/1/mojo/public/interfaces/application/application.mojom File mojo/public/interfaces/application/application.mojom (right): https://codereview.chromium.org/1071693003/diff/1/mojo/public/interfaces/application/application.mojom#newcode5 mojo/public/interfaces/application/application.mojom:5: [DartUriBase="mojo/public/interfaces/application"] On 2015/04/09 22:02:34, jamesr wrote: > why can't ...
5 years, 8 months ago (2015-04-09 22:20:55 UTC) #12
jamesr
The paths can be correctly computed from the canonical repository, so couldn't you record the ...
5 years, 8 months ago (2015-04-09 22:23:35 UTC) #13
viettrungluu
It seems to me that we're doing something wrong. In my, regardless of the language, ...
5 years, 8 months ago (2015-04-10 01:03:52 UTC) #14
qsr
On 2015/04/10 01:03:52, viettrungluu wrote: > It seems to me that we're doing something wrong. ...
5 years, 8 months ago (2015-04-10 08:04:33 UTC) #15
blundell
On 2015/04/10 01:03:52, viettrungluu wrote: > It seems to me that we're doing something wrong. ...
5 years, 8 months ago (2015-04-10 10:33:47 UTC) #16
zra
On 2015/04/10 10:33:47, blundell wrote: > I would be happy to have the dart package ...
5 years, 8 months ago (2015-04-10 23:36:23 UTC) #17
blundell
The symlinking seems unfortunate but I don't have a better solution: We want to be ...
5 years, 8 months ago (2015-04-13 15:18:14 UTC) #18
blundell
The CL description needs to be updated now.
5 years, 8 months ago (2015-04-13 15:18:27 UTC) #19
zra
On 2015/04/13 15:18:27, blundell wrote: > The CL description needs to be updated now. Done. ...
5 years, 8 months ago (2015-04-13 16:58:46 UTC) #20
zra
+johnmccutchan John, I had to change Dart embedder package creation a bit to account for ...
5 years, 8 months ago (2015-04-13 22:46:39 UTC) #22
Cutch
On 2015/04/13 22:46:39, zra wrote: > +johnmccutchan > > John, I had to change Dart ...
5 years, 8 months ago (2015-04-13 22:57:57 UTC) #23
zra
@tonyg: Does this look like it will be okay for consumers of Mojo?
5 years, 8 months ago (2015-04-14 21:53:18 UTC) #24
tonyg
lgtm I like this approach much more :)
5 years, 8 months ago (2015-04-15 01:22:40 UTC) #25
blundell
jamesr/vtl, do you want to re-review?
5 years, 8 months ago (2015-04-15 09:01:38 UTC) #26
jamesr
I'll defer to Tony on this
5 years, 8 months ago (2015-04-16 00:09:03 UTC) #27
zra
Committed patchset #6 (id:100001) manually as c34c0ab8bbbcb454bd3f4183be9b1c7abdc677b3 (presubmit successful).
5 years, 8 months ago (2015-04-16 14:50:53 UTC) #28
eseidel
5 years, 8 months ago (2015-04-16 19:39:18 UTC) #30
Message was sent while issue was closed.
Doesn't this collapse the namespaces between pub and mojo modules?

Should we be using a differnet scheme other than "package:" since these aren't
actually pub packages?

Powered by Google App Engine
This is Rietveld 408576698