Skip to content
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

Make package configuration loading use the correct function. #3355

Merged
merged 2 commits into from Aug 17, 2022

Conversation

lrhn
Copy link
Member

@lrhn lrhn commented Aug 17, 2022

When you have the URI of a package configuration file, you should load it directly.
It's not safe to use discovery, which assumes you start with a Dart script file.

Fixes #3354

When you have the URI of a package configuration file, you don't need to
use package configuration discovery.
@lrhn lrhn requested review from jensjoha and mit-mit August 17, 2022 11:48
@mit-mit
Copy link
Member

mit-mit commented Aug 17, 2022

Please update changelog and pubspec too.

Copy link

@jensjoha jensjoha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Though "you don't need to use package configuration discovery." is misleading.
It's wrong to do it, and we've seen how it can crash depending on filenames.

@lrhn
Copy link
Member Author

lrhn commented Aug 17, 2022

Pubspec is at 2.1.6-dev. A bug-fix should be a safe addition to that release number, no need for a minor version increment.

If we want to release, we have to remove the -dev, but probably also other things, like references to this package from other packages in the same mono-repo. I'd prefer to leave that to people who know the repo.

CHANGELOG entry added.

@jakemac53 jakemac53 merged commit 231750a into master Aug 17, 2022
@jakemac53 jakemac53 deleted the fix-package-config-load branch August 17, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace findPackageConfigUri with loadPackageConfigUri in package_reader.dart
4 participants