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

feat: Common dependency overrides #594

Closed

Conversation

Heinrich-vanNieuwenhuizen

Description:

This pull request addresses the challenges faced in projects involving multiple mono repositories that depend on each other during development. Our scenario unfolds where a sub-package is expanded by project-specific mono repositories.

For illustrative purposes, consider we have packages A, B, C, and D:

Package A imports B and C using path references.
Both packages B and C are utilized to expand D, ensuring they maintain path references to D.
Notably, package A lacks the directories B/Packages or C/Packages.
Package A encounters version resolution issues due to conflicting paths, and the YAML files of the sub-sub packages are non-existent. The current setup doesn't allow overriding the sub-sub package in A to affect B & C's references. This update facilitates the use of an override mechanism to address the daisy-chaining of packages, ensuring smoother dependency resolution and management.

Type of Change

  • [X ] ✨ feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
  • ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 refactor -- Code refactor
  • ci -- Build configuration change
  • 📝 docs -- Documentation
  • 🗑️ chore -- Chore

Solves issues for multiple mono repos depending on each other for development
@CLAassistant
Copy link

CLAassistant commented Oct 25, 2023

CLA assistant check
All committers have signed the CLA.

@Heinrich-vanNieuwenhuizen Heinrich-vanNieuwenhuizen changed the title Common dependency overrides feat: Common dependency overrides Oct 25, 2023
docs/commands/bootstrap.mdx Outdated Show resolved Hide resolved
Copy link
Collaborator

@spydon spydon left a comment

Choose a reason for hiding this comment

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

It looks good to me overall, however it will be a bit strange when removing a dependency_override from melos.yaml since the override then will still stay in all packages, but I don't think there is a good solution for that.

docs/commands/bootstrap.mdx Show resolved Hide resolved
docs/commands/bootstrap.mdx Outdated Show resolved Hide resolved
@Salakar Salakar self-requested a review November 21, 2023 16:04
@Salakar
Copy link
Member

Salakar commented Nov 21, 2023

It looks good to me overall, however it will be a bit strange when removing a dependency_override from melos.yaml since the override then will still stay in all packages, but I don't think there is a good solution for that.

I think you're right here, I'm not sold on this PR doing the same method of applying these overrides that dependencies/dev_dependencies does (mutating each package pubspec file to add them as committable changes) - especially since Pub also blocks packages being published that have overrides in the pubspec.yaml?

We have ways internally in Melos to apply overrides (it's how Melos generally works already) by instead mutating the generated pub files instead, (e.g. change the logic here; https://github.com/invertase/melos/blob/main/packages/melos/lib/src/commands/bootstrap.dart#L117 so these are reflected in pubspec_overrides.yaml changes only)

Is there a need for this PR to be mutating pubspecs directly?

@spydon
Copy link
Collaborator

spydon commented Nov 21, 2023

We have ways internally in Melos to apply overrides (it's how Melos generally works already) by instead mutating the generated pub files instead, (.dart_tool/* files).

It uses pubspec_overrides.yaml now right?

For the normal common dependencies I think it makes sense since it is a good way to get the same versions into all packages, but those are not temporary.

@Heinrich-vanNieuwenhuizen
Copy link
Author

It uses pubspec_overrides.yaml now right?

For the normal common dependencies I think it makes sense since it is a good way to get the same versions into all packages, but those are not temporary.

In our mono-repo setup, each repository is stored locally for better integration. In our structure, repository A includes B as a package, and B includes C, but A does not directly include C. This has led to a compilation issue for A, as it cannot resolve package C without modifications.

The specific error encountered is: 'pepkor_db depends on scotch_dev_real_time from a path that doesn't exist (package scotch_dev_real_time not found at "packages\scotch_dev_real-time"), resulting in version solving failure.

@spydon
Copy link
Collaborator

spydon commented Nov 22, 2023

In our mono-repo setup, each repository is stored locally for better integration.

Can you clarify this a bit? Why not just have all packages properly within the monorepo?

@Heinrich-vanNieuwenhuizen
Copy link
Author

In our mono-repo setup, each repository is stored locally for better integration.

Can you clarify this a bit? Why not just have all packages properly within the monorepo?

Hi @spydon ,

So we have many clients with their own different apps and different standards and separation of responsibilities. Some of our code bases are so large that it crashes the analyzer. Please see my drawing below

When a certification or bank rule changes it will be updated from a QA branch and re-iteratively worked back. So as an example, unique errors have to be defined and tested in QA PCI DSS, but all other apps should be aware of it. So it helps to have this as a mono repo to update the error. By having mini mono-repos it helps developing with the analyzer and keeps our code single principled.

No, we could publish for example Scotch dev error with every iteration but you might still have merge issues and will have to version until infinite. Some codes are shared with auditors/clients or kept private. Various QA repos might get different rules/laws then it will be great to have the mini mono repos and work our way back and then finally update the app.

Hope this makes sense. Open to suggestions if there is a better way.

@spydon spydon mentioned this pull request Dec 28, 2023
7 tasks
@tgrushka
Copy link

tgrushka commented Jan 8, 2024

Curious why adding the following to melos.yaml does not work. Why doesn't it just add the proper overrides to pubspec_overrides.yaml in each subproject?

dependency_overrides:
  protobuf:
    path: /Users/tom/better_protoc_plugin/protobuf.dart/protobuf

(Where the protobuf.dart/ folder is a git submodule but that's minimally relevant here -- should work with any valid override path, url, git, etc. as long as pub can resolve it.)

Is that what this PR is supposed to do? Why not just integrate the overrides into pubspec_overrides.yaml instead of pubspec.yaml as this PR originally suggests (which I agree is problematic)?

@spydon
Copy link
Collaborator

spydon commented Jan 9, 2024

Why not just integrate the overrides into pubspec_overrides.yaml instead of pubspec.yaml as this PR originally suggests (which I agree is problematic)?

Agree, that sounds like a better idea.

@spydon
Copy link
Collaborator

spydon commented Mar 7, 2024

@Heinrich-vanNieuwenhuizen would you like to continue working on this? Otherwise I'll close it.
The requirements for it to be merged is that the dependency_overrides should go into all the packages pubspec_overrides.yaml file instead of their pubspec.yaml file. And then when they are removed from melos.yaml they should be removed from the pubspec_overrides.yaml files when bootstrap runs again (that might already be how it works).

@Heinrich-vanNieuwenhuizen
Copy link
Author

@Heinrich-vanNieuwenhuizen would you like to continue working on this? Otherwise I'll close it. The requirements for it to be merged is that the dependency_overrides should go into all the packages pubspec_overrides.yaml file instead of their pubspec.yaml file. And then when they are removed from melos.yaml they should be removed from the pubspec_overrides.yaml files when bootstrap runs again (that might already be how it works).

Let me work on this I tried the latest melos version it still struggled I'll see if I cant take what I have done and add it to pubspec_overrides.yaml

@spydon
Copy link
Collaborator

spydon commented Jun 5, 2024

@Heinrich-vanNieuwenhuizen any progress? :)

@Heinrich-vanNieuwenhuizen
Copy link
Author

Hi @spydon , unfortunately not. I'll close it, maybe one day I'll get around to it or find an alternative way to have temporary packages included for development.

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.

5 participants