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

Consider simplify workspace - package [patch] table relation #4448

Closed
kayagokalp opened this issue Apr 17, 2023 · 1 comment · Fixed by #5092
Closed

Consider simplify workspace - package [patch] table relation #4448

kayagokalp opened this issue Apr 17, 2023 · 1 comment · Fixed by #5092
Assignees
Labels
enhancement New feature or request forc forc-pkg Everything related to the `forc-pkg` crate. good first issue Good for newcomers

Comments

@kayagokalp
Copy link
Member

With #4086, workspaces will have [patch] table support and the current relation between workspace and package [patch] table is the following:

'If the workspace manifest file contains a [patch] table, a member package's manifest file cannot contain any [patch] declarations.'

@mitchmindtree stated rust's approach is a little bit simpler which is (https://github.com/FuelLabs/sway/pull/4086/files#r1163520745):

i.e. "If workspace exists, patch table must go in workspace manifest. Otherwise patch table can go in package manifest."

@kayagokalp kayagokalp added enhancement New feature or request good first issue Good for newcomers forc forc-pkg Everything related to the `forc-pkg` crate. labels Apr 17, 2023
@cr-fuel cr-fuel self-assigned this Aug 30, 2023
@cr-fuel
Copy link
Contributor

cr-fuel commented Aug 31, 2023

cargo on rust will warn to the CLI. I will copy the same approach although it would be best to just error/panic and force the user to fix their mistake (move the patch to the workspace root)

This is cargo's output:

warning: patch for the non root package will be ignored, specify patch at the workspace root:
package:   /Users/cr-fuel/projects/sway/crates/types/Cargo.toml
workspace: /Users/cr-fuel/projects/sway/Cargo.toml
    Blocking waiting for file lock on package cache

cr-fuel added a commit that referenced this issue Sep 6, 2023
## Description

Take the cargo approach of ignoring patches of workspace members,
printing a warning, and only accepting patches defined at the workspace
root file.

Fixes #4448

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request forc forc-pkg Everything related to the `forc-pkg` crate. good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants