Skip to content

ci: fail a Dart release that leaves a dependent on pub.dev behind - #3977

Open
andrewkolos wants to merge 2 commits into
a2ui-project:mainfrom
andrewkolos:ci/dart-release-dependents
Open

andrewkolos wants to merge 2 commits into
a2ui-project:mainfrom
andrewkolos:ci/dart-release-dependents

Conversation

@andrewkolos

Copy link
Copy Markdown
Collaborator

CI resolves the Dart packages from source through the pub workspace, so it passes while the versions on pub.dev cannot be used together (#3942). This adds a check to pull requests that change a2ui_core's version.

For every other package under dart/ that constrains a2ui_core and is on pub.dev, the check fails unless the pull request releases that package too, or the latest version on pub.dev already accepts the new a2ui_core. The pull request releases a package when it changes the package's version: or the top heading of its CHANGELOG.md, leaves the two equal, and the version would become the latest on pub.dev. Packages that are not on pub.dev, such as a2ui_cli, are skipped. The failure message points at the release steps that #3973 adds.

Replayed on the last two a2ui_core releases, #3192 and #3695, it fails both, naming a2ui_agent. On #3695:

a2ui_core 0.3.0 -> 0.4.0
a2ui_cli: not on pub.dev, skipped
a2ui_flutter 0.0.1-wip003 on pub.dev accepts a2ui_core 0.4.0
error: a2ui_agent 0.0.1-wip004, the latest version on pub.dev, requires a2ui_core ^0.2.2, which excludes 0.4.0, and this pull request does not release a2ui_agent: this pull request changes neither its version (0.0.1-wip005) nor the top heading of its CHANGELOG.md. Release a2ui_agent in this pull request, as docs/contributing/release/release.md describes.

Part of #3945. The scheduled check that installs the published packages together comes after the next Dart release, since it fails until then.

Pre-launch Checklist

One time:

For this PR:

  • I have updated the relevant CHANGELOG.md file.
  • I updated/added relevant documentation.
  • My code changes (if any) have tests.
  • If my branch is on a fork, I have verified that scripts/e2e_test.sh passes.

ℹ️ Initial text drafted by agent

On a pull request that changes a2ui_core's version, check every other
package under dart/ that constrains a2ui_core and is on pub.dev. The
check fails unless the pull request releases that package too, or the
latest version on pub.dev already accepts the new a2ui_core.

⚠️ Agent-generated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new Python script, check_dart_dependents.py, along with its test suite, to ensure that Dart package releases do not leave dependent packages on pub.dev incompatible. The feedback highlights a critical bug in the version constraint resolution logic: caret constraints on 0.0.x versions (where both major and minor are 0) are incorrectly treated like 0.y.z versions. In Dart's pub package manager, ^0.0.x should only allow that exact patch version. Suggestions are provided to correct this logic in the script and update the corresponding test assertions to reflect the proper behavior.

Comment on lines +109 to +112
if start.major > 0:
end = Version(start.major + 1, 0, 0)
else:
end = Version(0, start.minor + 1, 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

In Dart's pub package manager, caret constraints on 0.0.x versions (where both major and minor are 0) behave differently than other 0.y.z versions. Specifically, ^0.0.x is equivalent to >=0.0.x <0.0.(x+1), meaning it only allows that exact patch version (and its prereleases), rather than stopping at the next minor version 0.1.0.

Currently, the implementation treats ^0.0.x the same as ^0.y.z and sets the upper bound to 0.1.0, which incorrectly allows incompatible patch versions (e.g., treating ^0.0.3 as allowing 0.0.4). Updating this to correctly increment the patch version when both major and minor are 0 ensures alignment with Dart's version resolution rules.

Suggested change
if start.major > 0:
end = Version(start.major + 1, 0, 0)
else:
end = Version(0, start.minor + 1, 0)
if start.major > 0:
end = Version(start.major + 1, 0, 0)
elif start.minor > 0:
end = Version(0, start.minor + 1, 0)
else:
end = Version(0, 0, start.patch + 1)

Comment on lines +69 to +73
def test_caret_on_zero_minor_also_stops_at_next_minor(self):
self.assertTrue(cdd.allows("^0.0.3", V("0.0.4")))
self.assertTrue(cdd.allows("^0.0.1-wip002", V("0.0.2")))
self.assertFalse(cdd.allows("^0.0.3", V("0.1.0-0")))
self.assertFalse(cdd.allows("^0.0.3", V("0.1.0")))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Update the test assertions to reflect the correct Dart pub caret constraint behavior for 0.0.x versions. Since ^0.0.3 is equivalent to >=0.0.3 <0.0.4, it should not allow 0.0.4. Similarly, ^0.0.1-wip002 should not allow 0.0.2.

Suggested change
def test_caret_on_zero_minor_also_stops_at_next_minor(self):
self.assertTrue(cdd.allows("^0.0.3", V("0.0.4")))
self.assertTrue(cdd.allows("^0.0.1-wip002", V("0.0.2")))
self.assertFalse(cdd.allows("^0.0.3", V("0.1.0-0")))
self.assertFalse(cdd.allows("^0.0.3", V("0.1.0")))
def test_caret_on_zero_major_and_minor_stops_at_next_patch(self):
self.assertFalse(cdd.allows("^0.0.3", V("0.0.4")))
self.assertFalse(cdd.allows("^0.0.1-wip002", V("0.0.2")))
self.assertTrue(cdd.allows("^0.0.3", V("0.0.3")))

This branch has not been deployed

No deployments
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.

1 participant