-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Demo of using a root .dart_tool/package_config.json
instead of pubspec.yaml
#51782
Conversation
# vended directories or code that is copied by tooling. | ||
- prebuilts/ | ||
- third_party/ | ||
# Directories that have Dart code, but the Dart code isn't resolvable by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we already think we're analyzing some or all of this Dart code: https://github.com/flutter/engine/blob/main/ci/analyze.sh#L58 and below. Why are we not already hitting problems with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'd probably want some documentation to explain how to add/remove dependencies.
@@ -0,0 +1,143 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there are a tool or package other than pub
that knows how to generate a package_config.json
file with the right schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package_config
package has a toJson
method on PackageConfig
. I am not sure what the API for creating PackageConfig objects is like, but you could use that. Ultimately it would just be more complicated I think than maintaining this file.
The Dart SDK has a bespoke tool for it also which does more sort of auto-creation based on looking in specific directories for packages, which might be an option here.
Fyi @pq. The engine has multiple analysis_options.yaml files so this could be a good test for the changes you just enabled. |
@matanlurey Can we land this? |
We could certainly talk about it. My personal impression was that:
Is about as bad as the current workflow, so not a blocker.
This seems like a blocker. We could try to make The system fix is that |
Canonical issue: flutter/flutter#147883 |
# .dart_tool but allow the .dart_tool/package_config.json file. | ||
.dart_tool/** | ||
!.dart_tool/package_config.json | ||
# But ignore the .dart_tool directory in any subdirectories. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/But/and
…3539) ... and use it in `engine_tool` to prove everything is working, i.e. on CI and elsewhere. Partial work towards flutter/flutter#147883. Supersedes and closes flutter#51782 (which was a prototype). See also: - https://flutter.dev/go/pub-workspace, the design doc on the feature. - dart-lang/pub-dev#7762, an example PR. I'll probably end up moving the inline docs in `pubspec.yaml` to a `docs/*.md` once that's a thing.
In this PR, I:
tools/engine_tool/pubspec.yaml
(and artifacts like locks and package_config).dart_tool/package_config.json
with all dependenciesI am not sure we'd want to land this as-is, it has the following rough spots:
.dart_tool/package_config.json
files, or they are used instead of the root. I suppose we could automate this with some sort of git hook, but it's not done today.Feedback welcome.
/cc @jacob314 @keertip mono repo stars.