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

Rust: other crates in workspace are not updated #1207

Closed
vladbat00 opened this issue Jun 10, 2019 · 11 comments
Closed

Rust: other crates in workspace are not updated #1207

vladbat00 opened this issue Jun 10, 2019 · 11 comments

Comments

@vladbat00
Copy link

vladbat00 commented Jun 10, 2019

Let's say both root Cargo.toml and libs/somelib/Cargo.toml have dependency A. When dependency A is updated, I get a pull request only for the root crate (and no PR for somelib). That often leads to having different dependency versions. If some struct instances of the library A are shared between the crates, that will definitely lead to a build failure.

Here's a simplified example of a project structure I'm working on:

/Catgo.toml
/Cargo.lock
/libs/somelib/Cargo.toml

My .dependabot/config.yml:

version: 1
update_configs:

  - package_manager: "rust:cargo"
    directory: "/"
    update_schedule: "live"

  - package_manager: "rust:cargo"
    directory: "libs/somelib/"
    update_schedule: "live"

^ I hope I haven't misconfigured anything, please, correct me if the issue is just on my side. At least I can see all the specified crates in dependabot dashboard, so I assume my configuration is correct...

I believe #1190 can help making this workflow more convenient, but for now just several separate PRs would be nice to have.

@greysteil
Copy link
Contributor

Hmm, Dependabot should already be dealing with Rust workspaces just fine - can you tag @dependabot in a PR which has this issue? It's hard to debug in the abstract.

@vladbat00
Copy link
Author

@greysteil I've just tagged @dependabot in the PR. It's in a private repo, I assume that shouldn't be an issue for you to be able to check it?

@greysteil
Copy link
Contributor

Great, thanks - I’ll check it out.

@greysteil
Copy link
Contributor

I've just taken a look at the files Dependabot is processing from that repo. It looks like you have a [workspace] line in the top-level Cargo.toml, but you don't specify any members of that workspace. Then you specify a bunch of path dependencies, which I think you want to have as members of the workspace, but because they're not Dependabot treats them as though they were being vendored.

You can fix the above by adding a members = [...] line to your top-level Cargo.toml. There are some docs here on how that works.

Does the above make sense? I'll mark this as resolved if so.

@vladbat00
Copy link
Author

vladbat00 commented Jun 12, 2019

@greysteil, thanks for looking into it!

Yeah, I thought the problem would be with [workspace] too.
The thing is that cargo automatically treats all the path dependencies as workspace members, so you don't have to specify them manually. I think that dependabot should do it as well, if it sees the [workspace] section. Is it possible to implement the same functionality for it? That seems to be the right way to do it.

And in order to bring the full support for the manifest file, exclude has to be taken into account as well...

Here's the full documentation in The Cargo Book: https://doc.rust-lang.org/cargo/reference/manifest.html#the-workspace-section

@vladbat00
Copy link
Author

Oh.. And another thing: I have all the workspace members specified in the .dependabot/config.yml. It doesn't seem to be completely ignored.. Though for some dependencies I receive PRs for workspace members, and for others I do not. My first assumption is that I will not receive PRs for members if the same dependency is specified in the root Cargo.toml.

@greysteil
Copy link
Contributor

The thing is that cargo automatically treats all the path dependencies as workspace members, so you don't have to specify them manually.

Oh interesting, we should definitely do that! I'll get that fixed.

@greysteil
Copy link
Contributor

Fixed in 9a08f39 which I'll deploy now. You'll want to remove the config file entries for everything other than the top-level Cargo.toml, as everything else will now be picked up automatically.

@ghost
Copy link

ghost commented Nov 13, 2020

@greysteil That's not a general fix. I have a Cargo.toml like this:

[package]
# ...
[workspace]
members = ["not-a-path-dependency"]
[dependencies]
workspace-member = { path = "path-dependency" }

Both not-a-path-dependency and path-dependency should be workspace member, but path-dependency/Cargo.toml is not monitored (not shown in the "Monitored dependency files" list). (I'm using GitHub-native Dependabot.)

@greysteil
Copy link
Contributor

Thanks for the report @hyd-dev. I don't work on Dependabot directly anymore, but if you or anyone else is up for submitting a pull request I'm sure the team would be delighted. GitHub-native Dependabot uses this library under the hood, so you're in the right place! :octocat:

@ghost
Copy link

ghost commented Nov 13, 2020

I wish to submit a PR but I don't know Ruby at all. However the logic is simple: just appending path_dependency_paths_from_file(file) to the Cargo.toml array unconditionally.

HadrienG2 added a commit to HadrienG2/crofiler that referenced this issue Jan 2, 2024
According to dependabot/dependabot-core#1207 , other workspace crates should be picked up automagically, and they do error out as currently set up...
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

No branches or pull requests

2 participants