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

request: pub workspaces #747

Closed
1 task done
lishaduck opened this issue Aug 7, 2024 · 29 comments · Fixed by #816
Closed
1 task done

request: pub workspaces #747

lishaduck opened this issue Aug 7, 2024 · 29 comments · Fixed by #816

Comments

@lishaduck
Copy link

lishaduck commented Aug 7, 2024

Is there an existing feature request for this?

  • I have searched the existing issues.

Command

No response

Description

Once Pub Workspaces land in stable, it would be great if Melos replaced dependency_overrides generation in favor of Pub Workspaces.
I believe this is planned, but I didn't see an issue.

Reasoning

Pub Workspaces enable a single analyzer scope, which both improves performance and reduces the maintenance burden on the Melos team.

Additional context and comments

Thanks for melos! It works great!

@spydon
Copy link
Collaborator

spydon commented Aug 7, 2024

It is indeed planned, thanks for opening an issue so that we can track it!
I'll link in the Flutter design doc for it is here too: https://flutter.dev/go/pub-workspace

@spydon
Copy link
Collaborator

spydon commented Sep 10, 2024

Related: dart-lang/pub#4376

@spydon
Copy link
Collaborator

spydon commented Sep 18, 2024

Related (globbing in workspace definition, which would make future migrations easier): dart-lang/pub#4391

@spydon spydon self-assigned this Sep 19, 2024
@Leptopoda
Copy link

I can see how globs are nice to have for this use case, but I wouldn't consider it a blocker.
I've migrated some repos to workspaces by using melos list -rp.

I'd love to use melos + workspaces sooner rather than waiting for this that isn't even such a big inconvenience.

@spydon
Copy link
Collaborator

spydon commented Nov 27, 2024

@Leptopoda I agree, it's not a blocker, possibly we could even write a simple migration script for it.

@xaethos
Copy link

xaethos commented Dec 4, 2024

Do Melos and Pub workspaces work together right now? Has anybody gotten a repo migrated by hand?

A few weeks back I took a stab at migrating a repo and couldn't get everything to work. I cannot remember for sure, but I think my blocker ultimately was the Flutter toolchain not handling Pub workspaces, though believe some Melos commands were confused by the dep overrides being elsewhere.

Is there a sense of what needs to be done for Melos and Pub workspaces to coexist?

@spydon
Copy link
Collaborator

spydon commented Dec 4, 2024

@xaethos it shouldn't be a problem for them to co-exist already afaik. With the workspaces we can simplify a lot of our code in Melos and remove the packages config from melos.yaml though, and that needs some work.

@Leptopoda
Copy link

What I have done so far:

  • remove dep overrides
  • add every package to the workspace in the root pubspec
  • change the dependency resolution for every package to "workspace"

Et voilà.
You MUST NOT use melos bootstrap as it will try to generate the overrides again. I suggest to just use dart pub get and melos for all the other commands (format, run scripts, publish, ...).

The current stable flutter version is broken for workspaces and last time I checked master it was also not working. I found 3.26.0-0.1.preto be working in some testing (just specify it in fvm) although your use case might forbid unreleased versions. A stable release should be right around the corner.

@SAGARSURI
Copy link
Contributor

finally workspace is released to stable version 3.27:

https://dart.dev/tools/pub/workspaces

@SAGARSURI
Copy link
Contributor

SAGARSURI commented Dec 12, 2024

I hope melos bootstrap will not be removed but tweaked to handle the new workspace changes.

@spydon
Copy link
Collaborator

spydon commented Dec 12, 2024

I hope melos bootstrap will not be removed but tweaked to handle the new workspace changes.

* This way we can still use `hooks` pre/post `bootstrap` command execution.

* Also the variable `MELOS_PACKAGES` might get effected which we use it in our pipeline to run test on changed packages because of this problem [fix:  melos overrides the applied filter for dependents/dependencies #675](https://github.com/invertase/melos/issues/675)

Yeah, that's true. We should keep bootstrap, but as an optional step.

I'm pretty busy currently, so if there is anyone else that would like to be assigned to this issue, give me a ping!
I don't think it is thaaat much to do, mostly removing a lot of stuff that we have in melos that is now superfluous.

@lishaduck
Copy link
Author

If anyone is looking for a working example of a migration, I've got this PR: lishaduck/legacy_checks#38.

@SalihCanBinboga
Copy link

If anyone is looking for a working example of a migration, I've got this PR: lishaduck/legacy_checks#38.

“melos bootstrap” command cannot be executed by you, right? (“melos bootstrap” command creates a ‘pubspec_overrides.yaml’ file for each module. Because of this, pub workspaces do not function properly)

@lishaduck
Copy link
Author

“melos bootstrap” command cannot be executed by you, right? (“melos bootstrap” command creates a ‘pubspec_overrides.yaml’ file for each module. Because of this, pub workspaces do not function properly)

Right. You call dart pub get from the root instead. See here: lishaduck/legacy_checks#38 (comment).

@SalihCanBinboga
Copy link

“melos bootstrap” command cannot be executed by you, right? (“melos bootstrap” command creates a ‘pubspec_overrides.yaml’ file for each module. Because of this, pub workspaces do not function properly)

Right. You call dart pub get from the root instead. See here: lishaduck/legacy_checks#38 (comment).

Thanks @lishaduck but I used the 'melos bootstrap' command because it also generates *.iml files specific to modules in IntelliJ. Additionally, when I run build_runner in the root directory, it still doesn't detect the modules - I need to go to the relevant module directory and run build_runner there. Hopefully this will be resolved soon..

@lishaduck
Copy link
Author

Thanks @lishaduck but I used the 'melos bootstrap' command because it also generates *.iml files specific to modules in IntelliJ.

Yeah, I use VS Code, which doesn't need them. What'd I'd probably do is run bootstrap, run rm -rf .dart_tool **/pubspec_overrides.yaml, and then run dart pub get.

Additionally, when I run build_runner in the root directory, it still doesn't detect the modules - I need to go to the relevant module directory and run build_runner there. Hopefully this will be resolved soon..

I'd think that's the expected behavior? Just make a melos task to run it all concurrently.

@SalihCanBinboga
Copy link

SalihCanBinboga commented Jan 3, 2025

Thanks @lishaduck but I used the 'melos bootstrap' command because it also generates *.iml files specific to modules in IntelliJ.

Yeah, I use VS Code, which doesn't need them. What'd I'd probably do is run bootstrap, run rm -rf .dart_tool **/pubspec_overrides.yaml, and then run dart pub get.

Additionally, when I run build_runner in the root directory, it still doesn't detect the modules - I need to go to the relevant module directory and run build_runner there. Hopefully this will be resolved soon..

I'd think that's the expected behavior? Just make a melos task to run it all concurrently.

Actually, I already have the melos run command as shown below, but due to bootstrap pub workspaces, this command cannot be executed, so it doesn't apply to all modules. I'm in a dilemma in such a situation :)

 gen:all:
    name: gen
    description: Generate all files
    run: "dart run build_runner build -d --low-resources-mode"
    packageFilters:
      dependsOn:
        - "build_runner"
    exec:
      concurrency: 4
      failFast: true

@lishaduck
Copy link
Author

Actually, I already have the melos run command as shown below, but due to bootstrap pub workspaces, this command cannot be executed, so it doesn't apply to all modules. I'm in a dilemma in such a situation :)

What's the error? (I haven't migrated my build_runner-dependent packages because custom_lint support is still in-progress)

@SalihCanBinboga
Copy link

Actually, I already have the melos run command as shown below, but due to bootstrap pub workspaces, this command cannot be executed, so it doesn't apply to all modules. I'm in a dilemma in such a situation :)

What's the error? (I haven't migrated my build_runner-dependent packages because custom_lint support is still in-progress)

Problem: Due to the issue mentioned here, Melos does not recognize my modules, so I cannot run my Melos scripts for my modules.

@lishaduck
Copy link
Author

Problem: Due to the issue mentioned here, Melos does not recognize my modules, so I cannot run my Melos scripts for my modules.

Can you just copy+paste the error message? That message doesn't provide an error nor does it mention Melos failing to recognize modules.

@SalihCanBinboga
Copy link

Problem: Due to the issue mentioned here, Melos does not recognize my modules, so I cannot run my Melos scripts for my modules.

Can you just copy+paste the error message? That message doesn't provide an error nor does it mention Melos failing to recognize modules.

Melos needs to recognize the modules, which requires running the melos bootstrap method, right? Since I cannot run the melos bootstrap method due to the use of the pub workspace, the modules are not recognized. Therefore, when I run melos gen:all, it does not give any errors, but it does not recognize any of the 60 modules.

@lishaduck
Copy link
Author

Melos needs to recognize the modules, which requires running the melos bootstrap method, right?

I don't think so... it just uses a glob in the melos.yaml, right?

@spydon
Copy link
Collaborator

spydon commented Jan 3, 2025

I don't think so... it just uses a glob in the melos.yaml, right?

Correct, there is no caching of the package list when bootstrapping.

@SalihCanBinboga
Copy link

SalihCanBinboga commented Jan 3, 2025

@lishaduck @spydon You are right; I noticed the issue was different. After enabling pub workspaces, I removed the build_runner dependency from the modules because it is sufficient to use it only in the root workspace. I realized that the gen:all script doesn’t recognize the modules because it depends on build_runner..

@spydon
Copy link
Collaborator

spydon commented Jan 6, 2025

The PR to migrate Melos to Pub Workspaces is now out, feel free to review it! (#816)

Tomorrow I'll create a pre-release so that you can try it out in your projects.

@spydon
Copy link
Collaborator

spydon commented Jan 7, 2025

I just published Melos v7.0.0-dev.1 enabling the Pub Workspaces feature, please try it out so that we can iron out any potential problems! 🙂
https://pub.dev/packages/melos

@spydon
Copy link
Collaborator

spydon commented Jan 7, 2025

If you want to add resolution: workspace to all of your pubspec.yaml files you can do this:

find . -type f -name "pubspec.yaml" -exec sed -i '/^name:/a resolution: workspace' {} +

and to get all your pubspec.yaml paths to add to the root pubspec's workspace section, you can do this:

find . -name "pubspec.yaml" -exec dirname {} + | sed 's|^\./||'

Remember that the workspace section doesn't support globs yet, so you'll most likely have quite a long list, you can give a thumbs up to the issue suggesting to add globs to it here: dart-lang/pub#4391

@spydon
Copy link
Collaborator

spydon commented Jan 7, 2025

If anyone is curious how a migration might look like, this is what it looked like for the Flame monorepo:
flame-engine/flame#3433

@lishaduck
Copy link
Author

Oh, and in case anyone is using custom_lint: don't upgrade 😢
custom_lint is essentially unmaintained right now, as the Dart team is actively working on shipping analyzer plugins built-in... which means Remi isn't going to put in the effort to get custom_lint working in Pub Workspaces.
Alternatively, turn off custom_lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants