Skip to content

Block upload of packages with stable versions and pre-release lower SDK constraint #3557

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

Closed
mit-mit opened this issue Apr 23, 2020 · 7 comments · Fixed by #3607
Closed

Block upload of packages with stable versions and pre-release lower SDK constraint #3557

mit-mit opened this issue Apr 23, 2020 · 7 comments · Fixed by #3607
Milestone

Comments

@mit-mit
Copy link
Member

mit-mit commented Apr 23, 2020

In we #3375 discussed a number of options and decided to add warnings when uploading a stable version package with a pre-release SDK or package dependency. I'd like propose that we add a second check, and that we make this second check a hard error, not just warning:

Upload check: packages depending on a non-stable Dart SDK must themselves be a pre-release version. Not doing this is an error, and blocks the upload.

Specifically, if the lower SDK constraint references a non-stable (i.e. dev or beta) SDK, then package itself must use a pre-release version.

This ensures that you can clearly tell from a published package if it depends on a stable SDK or not, and that we encourage any feature use of non-stable SDK features to only end up as pre-release.

This by-design doesn't handle the case where a package in a stable version depends on a package which is in a pre-release version. It just ensures that any package with a non-stable SDK dep itself has proper versioning. Also, with this, a developer can run pub deps -s compact and see if they depend on any pre-release packages; if not, they know they don't transitively depend on a non-stable SDK.

@mit-mit
Copy link
Member Author

mit-mit commented Apr 23, 2020

cc @jonasfj @sigurdm @kevmoo @natebosch @jakemac53 @munificent for feedback

@kevmoo
Copy link
Member

kevmoo commented Apr 23, 2020

SGTM. We do publish packages today w/ -dev SDK constraints to keep things working as -dev SDKs are released w/ subtle changes.

We should talk about the impacts here.

Also: are we going to restrict things on the pub server side, too?

@sigurdm
Copy link
Contributor

sigurdm commented Apr 24, 2020

Also: are we going to restrict things on the pub server side, too?

For blocking uploads this will be a server side check. Otherwise old/fake clients could circumvent it.

@mit-mit
Copy link
Member Author

mit-mit commented Apr 24, 2020

We do publish packages today w/ -dev SDK constraints to keep things working as

For whom / for what use case do we think this is important?

I think some run CI against dev "to catch things early", but given we have no breaking change policy for dev, I wonder if we need to point developers to the new beta channel for that kind of testing.

@jonasfj
Copy link
Member

jonasfj commented Apr 24, 2020

We should talk about the impacts here.

When shipping a release with new language features, we often want to ship stable releases of packages compatible with said release the same time. For example, we want to ship a package using extension methods at the same time as the SDK with extension methods is shipping.

However, if we do this, we can't release the new package before the new SDK is ready. This makes the release process more lock-step, but a fair counter argument is shipping a package before the compatible SDK is ready is a bad idea.

@jakemac53
Copy link
Contributor

I thought we had agreed to make this a warning but not an error. There are valid cases where this would unnecessarily (imo) restrict things. While there are no guarantees around stability in dev sdks it is very rare for new features to be reverted in a later dev release in my experience.

As a concrete example, consider a package which is broken by an upcoming change in the SDK. The fix requires some thing that only exists in the dev sdk where the breakage occurred (lets say its catching some new exception).

Today you could release your new (stable) version of your package with a lower bound on the dev sdk where the exception was added, and that works for all users with a pub upgrade. They don't have to know this breaking change happened regardless of whether they are on stable, dev, or beta sdks. If instead they have to release a pre-release version of their package, then their users on the dev/beta channels will have to change their contraint on this package to a pre-release version, which they will most likely not realize on their own. It also means we have to do an additional release when the stable sdk drops, and in the interim all users on stable will be broken (even if they pub upgrade).

What value does making this an error provide compared to the pain it causes for cases like this?

@mit-mit
Copy link
Member Author

mit-mit commented Apr 27, 2020

I thought we had agreed to make this a warning but not an error.

We had, but I fell uneasy about this decision.

There are valid cases where this would unnecessarily (imo) restrict things.

What are those (beyond the example given below)?

While there are no guarantees around stability in dev sdks it is very rare for new features to be reverted in a later dev release in my experience.

I think this is getting increasingly likely, and speculate it will happen soon. I see us taking a feature that's under a flag, making that available without a flag in one dev release, to then discover a major issue and put it back under a flag again. And I see us first target a feature for one language version in one dev build to then later move it forward to a future language version because we determine it's not ready yet.

As a concrete example, consider a package which is broken by an upcoming change in the SDK. The fix requires some thing that only exists in the dev sdk where the breakage occurred (lets say its catching some new exception).

I don't think targeting dev channel for this is appropriate, as there are no guarantees that dev channel is a good reflection of an upcoming stable build.

I'd feel a bit more easy about supporting using beta channel for this, but would really prefer if packages targeted stable.

What value does making this an error provide compared to the pain it causes for cases like this?

It prevents something I'm pretty sure will cause a lot of pain once we start making more changes to what's supported in dev channel.

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 a pull request may close this issue.

6 participants