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.

Sign up for GitHub

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

Jump to bottom

dart pub unpack fails with dependency published from a workspace #4393

Closed
Luckey-Elijah opened this issue Sep 23, 2024 · 4 comments · Fixed by #4400
Closed

dart pub unpack fails with dependency published from a workspace #4393

Luckey-Elijah opened this issue Sep 23, 2024 · 4 comments · Fixed by #4400
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@Luckey-Elijah
Copy link

Environment

Dart SDK version: 3.5.3 (stable) (Wed Sep 11 16:22:47 2024 +0000) on "linux_x64"

Problem

unpack command runs dart pub get in dependency that is a workspace and fails.

Expected behavior

unpack should work "out of the box" with resolution: workspace packages

dart pub unpack binary_serializable should succeed.

Actual behavior

$ dart pub unpack binary_serializable
Downloading binary_serializable 0.3.0 to `./binary_serializable-0.3.0`... 
To explore type: cd ./binary_serializable-0.3.0
Found a pubspec.yaml at /home/elijah/Development/binary_serializable-0.3.0. But it has resolution `workspace`.
But found no workspace root including it in parent directories.

See https://dart.dev/go/pub-workspaces for more information.

The exit code is 66

I am able to resolve by removing the resolution: workspace entry in the pubspec.yaml and running dart pub get by hand.
@Luckey-Elijah Luckey-Elijah changed the title dart pub unpack fails with workspaces dart pub unpack fails with dependency published from a workspace Sep 23, 2024
@sigurdm sigurdm added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Sep 24, 2024
@sigurdm
Copy link
Contributor

sigurdm commented Sep 24, 2024

Yeah - this is a bit unfortunate.

This speaks for having a resolution mode that ignores the resolution: workspace... or we could explicitly remove the resolution: workspace when unpacking.

I don't really like any of those solutions. unpack changing the files seem surprising. Resolving dependencies in a non-standard mode also seems surprising.

We could have a command dart pub workspace eject that removes the package from the workspace, and suggest that command when the resolution fails.

@jonasfj what do you think...

@jonasfj
Copy link
Member

jonasfj commented Sep 24, 2024

Nothing is fully thought through.

what do you think...

Off the top my head, it makes me wonder if adding resolution: workspace was necessary.
Maybe we could just have searched upwards until we found nothing or a pubspec.yaml with a workspace property.

Since we ended up doing the .dart_tool/workspace_ref.json (or something like that), for caching purposes anyways.

But in a world where we don't undo this, I agree, it's tempting to make a mode that ignores resolution: workspace.

But I fear that it would poorly as opening in the IDE often triggers a pub get (certainly editing pubspec.yaml does), as does dart run or dart test.

In general: We want dart pub get to be safe to run implicitly.

If dart pub get is always safe to run, then it's hard to have modes. Unless we persist the mode in pubspec.lock. But then we're introducing an entirely new concept. And it's unclear to me that persisting mode in pubspec.lock won't cause confusing and weird scenarios where users don't understand the behavior.

I don't really like any of those solutions. unpack changing the files seem surprising. Resolving dependencies in a non-standard mode also seems surprising.

Agreed, but doing so in order to do dart pub get might be okay, if it prints a warning line.

It's certainly an option to consider.

Could we do it by adding a pubspec_override.yaml ? (are pubspec_override.yaml files ever published?)

@sigurdm
Copy link
Contributor

sigurdm commented Sep 25, 2024

Could we do it by adding a pubspec_override.yaml ? (are pubspec_override.yaml files ever published?)

Yeah - that is not a bad idea!

We could let the resolution and workspace fields be overridable from pubspec_overrides.yaml...

@sigurdm
Copy link
Contributor

sigurdm commented Sep 25, 2024

We probably don't need to override the workspace field (but the ability should be there).

And we could do the same for dependency_overrides (override it with an empty field).

@sigurdm sigurdm mentioned this issue Sep 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow overriding resolution and workspace fields in pubspec_overrides. sigurdm/pub
3 participants
@jonasfj @sigurdm @Luckey-Elijah