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

artifact-dependencies seems to build both cdylib and staticlib when only one is specified #12109

Closed
ehuss opened this issue May 8, 2023 · 3 comments · Fixed by #13842
Closed
Labels
C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. Z-bindeps Nightly: binary artifact dependencies

Comments

@ehuss
Copy link
Contributor

ehuss commented May 8, 2023

Problem

When an artifact dependency specifies artifact='staticlib', and the dependency declares its crate type as ["staticlib", "cdylib"], cargo will build both the staticlib and cdylib separately. I would expect it to only build the staticlib.

Steps

Create a project with an artifact dependency:

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
bar = {path = "bar", artifact = "staticlib"}

and in the bar dependency, specify it with multiple crate types:

[package]
name = "bar"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["staticlib", "cdylib"]

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.71.0-nightly (569b648b5 2023-05-05)
release: 1.71.0-nightly
commit-hash: 569b648b5831ae8a515e90c80843a5287c3304ef
commit-date: 2023-05-05
host: aarch64-apple-darwin
libgit2: 1.6.3 (sys:0.17.0 vendored)
libcurl: 7.87.0 (sys:0.4.61+curl-8.0.1 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1t  7 Feb 2023
os: Mac OS 13.3.1 [64-bit]
@ehuss ehuss added C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. Z-bindeps Nightly: binary artifact dependencies labels May 8, 2023
@skogseth
Copy link
Contributor

Hi! Lately I've been playing around with artifact dependencies at work, and I noticed this issue, so I thought I'd try to fix it. I seem to have something working now so I looked into the contribution process. From what I understand it seems that this won't be prioritized as of now since the label is "S-needs-mentor" and not "S-accepted"? Should I wait with my PR or do I still post it?

@ehuss
Copy link
Contributor Author

ehuss commented Apr 30, 2024

Thanks for checking in @skogseth! I think it depends on how complex of a change it looks like it might be. Unfortunately artifact-dependencies hasn't been a very high priority for us, and when reviewing PRs it can take a lot of time to bring back all the context of how it works since we aren't actively working on it. If it is a relatively simple change, feel free to open a PR and we can discuss it more. Just make sure it has a test, preferably as the first commit and then include the fix in the second commit to show how the test changes.

If it is a complex change, I'm reluctant to encourage anyone to put too much effort in since the PR might sit for a long while.

@skogseth
Copy link
Contributor

Okay, I see. It’s a very simple fix, I think it was less than 10 LoC, so I’ll open a PR. But I’ll make sure to include tests, and also try to include any relevant context in the description. It’s also fine by me if the PR ends up sitting unreviewed for a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. Z-bindeps Nightly: binary artifact dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants