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

Unsupported operation: Cannot add to an unmodifiable list in build.dart scripts #168

Closed
dcharkes opened this issue Oct 27, 2023 · 4 comments · Fixed by #169
Closed

Unsupported operation: Cannot add to an unmodifiable list in build.dart scripts #168

dcharkes opened this issue Oct 27, 2023 · 4 comments · Fixed by #169
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_cli

Comments

@dcharkes
Copy link
Collaborator

  Unsupported operation: Cannot add to an unmodifiable list
  dart:_internal                                                UnmodifiableListMixin.addAll
  package:native_toolchain_c/src/cbuilder/cbuilder.dart 276:45  CBuilder.run
  ===== asynchronous gap ===========================
  test/cbuilder/cbuilder_cross_android_test.dart 132:3          buildLib
  ===== asynchronous gap ===========================
  test/cbuilder/cbuilder_cross_android_test.dart 46:24          main.<fn>

@parlough it seems we're running into unmodifiable list errors being thrown in various places because the default values for arguments are now not modifiable. Could you please submit a PR to make the defaults modifiable again (and test it with unit tests).

I'm not sure why this was not caught by the CI earlier. 🙈

Originally posted by @dcharkes in #161 (comment)

Also failing on roll into Dart SDK: https://ci.chromium.org/ui/p/dart/builders/ci.sandbox/pkg-linux-debug/18536/overview

Unsupported operation: Cannot add to an unmodifiable list
#0      UnmodifiableListMixin.addAll (dart:_internal/list.dart:129:5)
#1      CBuilder.run (package:native_toolchain_c/src/cbuilder/cbuilder.dart:276:45)
<asynchronous suspension>
#2      main (file:///b/s/w/ir/x/t/NRYGUQ/native_add/build.dart:21:3)
<asynchronous suspension>
@dcharkes dcharkes added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Oct 27, 2023
@dcharkes
Copy link
Collaborator Author

I have retracted bad version https://pub.dev/packages/native_assets_cli/versions/0.3.1

Please bump the version to v0.3.2 when addressing this @parlough.

@parlough
Copy link
Member

Sorry about that. I'm taking a look now.

@parlough
Copy link
Member

The issue 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.

For extra validation, it might make sense to run the tests for native_toolchain_c and native_assets_builder with a version of native_assets_cli from path, at least before a release. This will only work when there aren't intentional breaking changes though. I'm not sure the best way to do that though, or if it makes sense at all. I'll give it some more thought.

@dcharkes
Copy link
Collaborator Author

The issue 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.

For extra validation, it might make sense to run the tests for native_toolchain_c and native_assets_builder with a version of native_assets_cli from path, at least before a release. This will only work when there aren't intentional breaking changes though. I'm not sure the best way to do that though, or if it makes sense at all. I'll give it some more thought.

Ah yes, this is a known issue:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_cli
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants