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

Temporarily allow modification of a BuildOutput's raw dependencies #169

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

parlough
Copy link
Member

@parlough parlough commented Oct 27, 2023

The issue in #168 was not caught due to the packages depending on each through through pub rather than as path dependencies. It was the native_toolchain_c tests that failed which earlier on CI depended on a version of native_assets_cli during test.

The core issue here is that Dependencies should likely be properly immutable (as indicated by its const constructor that's always been there) and its dependencies property should instead be exposed as an Iterable or not at all directly.

Since making those changes is a breaking change, I will instead follow-up with those changes as part of a 0.4.0 release. This fix removes the const lists/maps that the depending packages are not currently expecting.

Fixes #168

@parlough
Copy link
Member Author

parlough commented Oct 27, 2023

For extra validation I locally modified native_assets_builder and native_toolchain_c to use a path dependency on the updated native_assets_cli and ran their complete test suites.

As I mentioned in #168 (comment), it might be a good idea to explore how to do this in CI (if/when it makes sense?).

Copy link
Collaborator

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

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

Thanks Parker!

pkgs/native_assets_cli/CHANGELOG.md Outdated Show resolved Hide resolved
pkgs/native_assets_cli/test/model/build_output_test.dart Outdated Show resolved Hide resolved
@coveralls
Copy link

Coverage Status

coverage: 98.079%. first build when pulling ab90f30 on parlough:fix/168 into 762b4da on dart-lang:main.

@dcharkes dcharkes merged commit 9629a55 into dart-lang:main Oct 27, 2023
17 checks passed
@parlough parlough deleted the fix/168 branch October 27, 2023 19:50
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.1.0 to 3.3.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@93ea575...ac59398)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.1.0 to 3.3.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@93ea575...ac59398)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsupported operation: Cannot add to an unmodifiable list in build.dart scripts
3 participants