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

Patch git dependency in a workspace or sub-directory #9624

Closed
ratijas opened this issue Jun 25, 2021 · 12 comments · Fixed by #13341
Closed

Patch git dependency in a workspace or sub-directory #9624

ratijas opened this issue Jun 25, 2021 · 12 comments · Fixed by #13341
Labels
A-crate-dependencies Area: [dependencies] of any kind A-documenting-cargo-itself Area: Cargo's documentation C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@ratijas
Copy link

ratijas commented Jun 25, 2021

Describe the problem you are trying to solve

Patch syntax allows replacing a dependency's git URL, but it doesn't seem allow to specify a directory in that repository. At least, it is not documented at https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html. Essentially, it is useless when patched crate is a part of a cargo workspace.

Describe the solution you'd like

Two solutions:

More integrated with cargo/rust: specify workspace member's package name, so that cargo will parse manifest and resolve to the required directory automatically.

More general: manually specify sub-directory of a git repository, so that cargo would look for a crate's Cargo.toml manifest in that dir.

Notes

I think, both proposed solutions are fine and can be implemented concurrently and without interfering each other.

Related: #6126

@ratijas ratijas added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 25, 2021
@ratijas
Copy link
Author

ratijas commented Jun 25, 2021

Because of this lack of functionality, I am currently forced to download the whole repo, store it somewhere, and override each dependency of a workspace in a patch section like this:

[patch.crates-io]
cpp        = { path = "../../rust-cpp/cpp" }
cpp_common = { path = "../../rust-cpp/cpp_common" }
cpp_macros = { path = "../../rust-cpp/cpp_macros" }
cpp_build  = { path = "../../rust-cpp/cpp_build" }

Instead, I'd like to be able to write something like this:

[patch.crates-io]
cpp        = { git = "https://github.com/mystor/rust-cpp", member = "cpp" }
cpp_common = { git = "https://github.com/mystor/rust-cpp", member = "cpp_common" }
cpp_macros = { git = "https://github.com/mystor/rust-cpp", member = "cpp_macros" }
cpp_build  = { git = "https://github.com/mystor/rust-cpp", member = "cpp_build" }

@ratijas ratijas changed the title Patch git dependency in a workspace /sub-directory Patch git dependency in a workspace or sub-directory Jun 25, 2021
@ehuss
Copy link
Contributor

ehuss commented Jun 25, 2021

Cargo should automatically find the package based on the name. That is, you should be able to write:

[patch.crates-io]
cpp        = { git = "https://github.com/mystor/rust-cpp" }
cpp_common = { git = "https://github.com/mystor/rust-cpp" }
cpp_macros = { git = "https://github.com/mystor/rust-cpp" }
cpp_build  = { git = "https://github.com/mystor/rust-cpp" }

Does that work for you?

@ratijas
Copy link
Author

ratijas commented Jun 25, 2021

Works on Linux now. How strange. I'm almost sure I tried exactly that, and it didn't work yesterday on Windows. I'll check Windows again soon.

@ratijas
Copy link
Author

ratijas commented Jun 26, 2021

Nevermind my stupidity. git patches work with workspaces on Windows too. Anyway, it probably worth mentioning in the documentation that workspaces are explicitly supported.

@ratijas ratijas closed this as completed Jun 26, 2021
@ratijas
Copy link
Author

ratijas commented Jun 26, 2021

However, path variant does not support workspaces. It requires a full path to the final directory with a specific crate.

No good:

qttypes          = { path = "../../qmetaobject-rs" }
qmetaobject      = { path = "../../qmetaobject-rs" }
qmetaobject_impl = { path = "../../qmetaobject-rs" }

Good:

qttypes          = { path = "../../qmetaobject-rs/qttypes" }
qmetaobject      = { path = "../../qmetaobject-rs/qmetaobject" }
qmetaobject_impl = { path = "../../qmetaobject-rs/qmetaobject_impl" }

@ratijas ratijas reopened this Jun 26, 2021
@ehuss
Copy link
Contributor

ehuss commented Jul 2, 2021

I think it is unlikely that we will change the path dependency to do a recursive search. Is there a problem with specifying the path where the package is located?

@ratijas
Copy link
Author

ratijas commented Jul 2, 2021 via email

@ehuss ehuss added the A-documenting-cargo-itself Area: Cargo's documentation label Aug 3, 2021
@weihanglo weihanglo added the A-crate-dependencies Area: [dependencies] of any kind label Mar 16, 2023
@weihanglo
Copy link
Member

If anyone wants to give a hand with this, here are some pointers for the possible enhancements:

  • Under Specifying dependencies from git repositories section, explain the auto-discovery functionality of git dependencies, such as

    Note that the package you depends on isn't required to be under the root of git dependency. Cargo is smart enough to traverse the repository and find the package matching the name specified in Cargo.toml. That said, Cargo will be confused if there are multiple package with the same name in a git repository.

    And if not overkill, probably provide a simple file tree graph to help understand that.

  • Under Specifying path dependencies, we could add something like

    Unlike git dependencies, Cargo doesn't do any traversal for path dependencies. You need to specify the path to the root of the package you depends on.

I am not good at writing so feel free to sumbit a PR with better wordings than this!

@weihanglo weihanglo added E-easy Experience: Easy E-mentor labels Mar 16, 2023
@wabisabia
Copy link

If no one else is working on this, I'd be happy to make this change!

@wabisabia
Copy link

@weihanglo I've noticed that when duplicate crates are discovered in a git repository, Cargo actually selects one of them, and a "Skipping duplicate package ..." warning is issued for each skipped crate.

It seems that the eventually selected crate is the one that was discovered first, but I'm not certain what determines the discovery order:

match all_packages.entry(pkg_id) {
Entry::Vacant(v) => {
v.insert(pkg);
}
Entry::Occupied(_) => {
// We can assume a package with publish = false isn't intended to be seen
// by users so we can hide the warning about those since the user is unlikely
// to care about those cases.
if pkg.publish().is_none() {
let _ = config.shell().warn(format!(
"skipping duplicate package `{}` found at `{}`",
pkg.name(),
path.display()
));
}
}
}

Would you have any guidance on whether/how this behaviour should be documented?

@weihanglo
Copy link
Member

Hi, @wabisabia. Sorry for the late response. I don't know why it is off my radar.

We can skip the skipping behavior for now. It's not reliable and is considered as a bug, see #10752 (comment). If you come up with any better idea or question, please share with us!

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed E-mentor labels Apr 19, 2023
@rimutaka
Copy link
Contributor

If this is still relevant I'd like to submit a PR.

Pages in question:

rimutaka added a commit to rimutaka/cargo that referenced this issue Jan 24, 2024
* minor rephrasing for readability
* added more examples
* added sub-headings
* rearranged a few paragraphs for readability
rimutaka added a commit to rimutaka/cargo that referenced this issue Mar 5, 2024
* rephrasing for readability
* added more examples
* added sub-headings
* rearranged paragraphs for readability
@bors bors closed this as completed in 1c01c62 Mar 5, 2024
charmitro pushed a commit to charmitro/cargo that referenced this issue Sep 13, 2024
* rephrasing for readability
* added more examples
* added sub-headings
* rearranged paragraphs for readability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-crate-dependencies Area: [dependencies] of any kind A-documenting-cargo-itself Area: Cargo's documentation C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants