-
Notifications
You must be signed in to change notification settings - Fork 1.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
Kernel package fails pkgbuild tests that run pub get #27937
Comments
BUG=#27937 R=asgerf@google.com Review URL: https://codereview.chromium.org/2543473003 .
Pub does not allow you to publish a package that contains path dependencies or dependency overrides specifically to avoid this case. The versions of kernel and front_end on pub.dartlang.org don't have any dependency overrides. I'm guessing whomever published them deleted that section of the pubspec locally before publishing? |
I think @nex3 changed that not too long ago to allow publishing with |
You can specifically publish packages that override dev dependencies. Normal dependencies are still forbidden. |
I think we are moving toward the consensus that we don't want test that "pub get" works correctly on these packages. Right now, this test has started passing again, but I will change its status to "SkipByDesign" instead of "Pass" or "Pass, Fail" |
BUG=#27937 BUG=#28243 R=kmillikin@google.com Review-Url: https://codereview.chromium.org/2602323003 .
Like analyzer, front_end, and dev_compiler, pkg/kernel has hardcoded paths in its pubspec.yaml dependency_overrides.
This means that the pkgbuild tests, that copy a package to a new directory and run pub get on it, fail.
I think it is fine that we say that these tests should fail, and running pub get on these tightly integrated packages is not supported. So I'm adding kernel to the list of skipped packages, in pkg/pkgbuild.status.
But it is weird that they are published to pub then, and that people can write third party packages depending on them, but can't check them out.
I would like someone to take ownership of this entire issue: of what the package.yaml of these packages should be, and whether they should be published to pub and usable by third parties, and what pub get should do on them in that case. Someone who totally understands pub specs and dependency overrides, and the problems we have with working on the -alpha version in the repository when the pub specs don't support it. It would be good to have the policy (i.e. the working fix) written up, so people understand why the pubspecs for these packages are the way they are, and how to update them. If this is the right way to handle incompatible version numbers while working on prerelease versions of a package, then that is fine. But unless that is documented (even just with a comment or reference to an issue), it is unclear if it is a hack found by trial and error, or if it is the right solution.
It seems like one reason people run pub get on these packages in their existing directories is so that the intellij or atom plugins work, and can navigate the source.
Why can't these plugins be told to use the .packages file at the root of the checkout? - that is the correct one, and the one that is used for all of our building and testing.
For my part, I can just suppress tests in the status files, and add new testing if it is suggested, for whatever we decide the setup should be, and what should be tested and enforced.
Related issues:
#27654 #27655
@munificent @nex3 @Asger @bwilkerson
The text was updated successfully, but these errors were encountered: