Add support for creating a .packages file when the plugin is in a Bazel workspace

Add support for creating a .packages file when the plugin is in a Bazel workspace

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/0f9707073daa6bb2342ca4b125d3d0bba9ee99f5

[Brian Wilkerson, 2017-07-24 15:09:01]

brianwilkerson@google.com changed reviewers:
+ mfairhurst@google.com, scheglov@google.com

[scheglov, 2017-07-24 15:35:34]

LGTM

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/plugin/plugin_manager.dart (right):

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/plugin/plugin_manager.dart:693: YamlDocument
document = loadYamlDocument(pubspecFile.readAsStringSync(),
Maybe read in try/catch?
Or do we catch exceptions on a higher level to fail plugin loading?

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/test/sr...
File pkg/analysis_server/test/src/plugin/plugin_manager_test.dart (right):

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/test/sr...
pkg/analysis_server/test/src/plugin/plugin_manager_test.dart:408:
resourceProvider.newFile(pluginFilePath, '');
MemoryResourceProvider.newFile() creates all enclosing folders.
We don't need to create them manually in each test.

[Brian Wilkerson, 2017-07-24 15:57:43]

ptal

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/plugin/plugin_manager.dart (right):

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/plugin/plugin_manager.dart:693: YamlDocument
document = loadYamlDocument(pubspecFile.readAsStringSync(),
I had intended to catch it at a higher level, but missed it. Thanks! I've added
the try-catch.

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/test/sr...
File pkg/analysis_server/test/src/plugin/plugin_manager_test.dart (right):

https://codereview.chromium.org/2988673002/diff/1/pkg/analysis_server/test/sr...
pkg/analysis_server/test/src/plugin/plugin_manager_test.dart:408:
resourceProvider.newFile(pluginFilePath, '');
Done

[scheglov, 2017-07-24 16:08:00]

LGTM

[Brian Wilkerson, 2017-07-24 17:00:09]

Description was changed from

==========
Add support for creating a .packages file when the plugin is in a Bazel
workspace
==========

to

==========
Add support for creating a .packages file when the plugin is in a Bazel
workspace

R=scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/0f9707073daa6bb2342ca4b125d3d0bba9ee99f5
==========

[Brian Wilkerson, 2017-07-24 17:00:10]

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

[mfairhurst, 2017-07-24 18:00:07]

LGTM, some thoughts from the g3 perspective (which may already be on your mind
and just be too soon, but still worth syncing)

https://codereview.chromium.org/2988673002/diff/20001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/plugin/plugin_manager.dart (right):

https://codereview.chromium.org/2988673002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/plugin/plugin_manager.dart:431: // because we won't
be running pub.
This will prevent the bazel support from working without a pubspec file, no?

Edit: I see, looks like bazel support requires pubspec files?

Any change we could do like, bazel queries instead?

https://codereview.chromium.org/2988673002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/plugin/plugin_manager.dart:676: return null;
should log the exception too though

https://codereview.chromium.org/2988673002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/plugin/plugin_manager.dart:703: YamlNode
dependencies = contents['dependencies'];
Is there any way we can get this info by running pub? This skips
dev_dependencies; would be nice if pub could encaspulate that behavior so it
doesn't have to be perfectly recreated here.