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

I still get links errors despite #4978 landing #7310

Closed
Eh2406 opened this issue Apr 3, 2018 · 19 comments
Closed

I still get links errors despite #4978 landing #7310

Eh2406 opened this issue Apr 3, 2018 · 19 comments

Comments

@Eh2406
Copy link
Contributor

Eh2406 commented Apr 3, 2018

I maintain a -sys crate but my users keep getting

error: multiple packages link to native library `z`, but a native library can be linked only once

I thought that had been fixed in rust-lang/cargo#4978 but it doesn't seem to be working. What can I do?


In fact this is mostly a place for me to link people to, to unify the discussion. I will try to keep this up to date with any discussion below.

it doesn't seem to be working

Here is why rust-lang/cargo#4978 doesn't seem to be working for you.
rust-lang/cargo#4978 only stops that error for crate with the links attribute in the index. That attribute only gets into the index if you publish the crate with a recent nightly/beta. So no protection for the older versions of your crate.

What can I do?

You can manually make a PR to the index to add the attribute to older versions.

Someone could scan all crates to find all the missing links attributes and add them.

It is not yet clear what the cargo team recommends at this time.

@klausi
Copy link

klausi commented Apr 7, 2018

Thanks for the info! My current plan is to build a cargo subcommand to update the links attribute for a single crate:

I tried to search the cargo source for an API how to interact with the registry and read versions, but did not find anything useful. So I think I will use https://github.com/nabijaczleweli/cargo-update which has helpers such as get_index_path() and find_package_data().

@klausi
Copy link

klausi commented Apr 7, 2018

First prototype: https://github.com/klausi/cargo_fix_links/blob/master/src/main.rs
I have hard-coded everything to get the update for libz-sys working. Let's see if that pull request gets accepted and then we can refine the script from there.

@alexcrichton
Copy link
Member

I'm personally not super enthusiastic about retroactively modifying the index in that I'm hoping that this issue will eventually solve itself as newer versions of crates are published. I think we may first want to send out a post recommending publishing new versions (and perhaps a tool detecting the crate you need to publish with newer versions of Cargo). After that's baked for a bit if it's still a big problem I think we could consider changing the index, but until that point we may want to hold off just yet

@Eh2406
Copy link
Contributor Author

Eh2406 commented Apr 7, 2018

"Just wait for it to solve it self" Is a reasonable decision for what "the cargo team recommends". It has the disadvantage of slowing the time before -Z minimal-versions can be used effectively. As adding a new version does not resolve the issue of old versions not having the attribute. Eventually all the versions of the library that you are compatible with will be published with a new enough cargo, but that could be years if the library does not have a lot of breaking changes and you don't want to bump version just for the the attribute being in the index.

@alexcrichton
Copy link
Member

Nah yeah I definitely agree that if we really want to push on -Z minimal-version we'd need to prioritize fixing this, probably through an index update. At this point though I don't think we're necessarily trying to get that flag to work too super smoothly, though.

@klausi
Copy link

klausi commented Apr 8, 2018

OK, so then my alternative plan is to maintain a fork of the crates.io index at https://github.com/klausi/crates.io-index that has the links attributes fixed where necessary. That way I can now test compilation of minimal dependency versions with this:

CARGO_REGISTRY_INDEX=https://github.com/klausi/crates.io-index cargo build -Z minimal-versions

That at least helps with fixing crates manually such as https://github.com/alexcrichton/git2-rs/pull/307 . I think this is not super ideal to be added to CI configs, but a starting point that people can experiment with.

@alexcrichton
Copy link
Member

Sounds like a plausible solution to me though! (albeit definitely transitionary)

@epage
Copy link

epage commented Oct 18, 2023

@Turbo87 have you all done a links backfill of the index?

@Turbo87
Copy link
Member

Turbo87 commented Oct 18, 2023

not that I am aware of. do we have crates with differences between manifest and index?

@epage
Copy link

epage commented Oct 18, 2023

@Turbo87 from Eh2406's description, it sounds like this attribute existed in packages and was later added to the index and we were telling people to publish with newer cargo or manually backfill if they needed it. This was 5 years ago, so likely people have published new versions since then or manually backfilled, so it would be a relatively low priority to do an automated backfill.

I'll transfer this issue over to the crates.io repo for you to decide whether its worth fixing or to close as "good enough".

@epage epage transferred this issue from rust-lang/cargo Oct 18, 2023
@Turbo87
Copy link
Member

Turbo87 commented Oct 18, 2023

I'll keep it in mind for the next round of backfilling, but I agree that it is probably low priority :)

@Turbo87
Copy link
Member

Turbo87 commented Jun 25, 2024

FWIW the original issue has been fixed. https://github.com/rust-lang/crates.io-index/blob/master/li/bz/libz-sys now includes links: z for the relevant crate versions. once a new version is published we automatically sync the whole index file for the crate with the database which fixes the issue.

if you maintain a -sys crate and experience a similar issue: publish a new version and then the issue should be fixed for the older versions as well.

@Turbo87 Turbo87 closed this as completed Jun 25, 2024
@epage
Copy link

epage commented Jun 26, 2024

I'm a bit confused by that explanation. To double check my understanding: when a new package is published, you re-parse all previous manifests and write their index entries from scratch? That seems quite expensive which is why I suspect my understanding is incorrect. If that isn't it, how does publishing a new version help existing index entries?

@Turbo87
Copy link
Member

Turbo87 commented Jun 26, 2024

when a new package is published, you re-parse all previous manifests and write their index entries from scratch?

no, when a new package is published we parse the manifest and save the relevant stuff in our database. the database is then used to regenerate the index file for all existing versions, not just the newly published version. this allowed us to get rid of various inconsistencies in the index last year that had snuck in over time.

@epage
Copy link

epage commented Jun 26, 2024

The index is always re-generated from the database but what is updating the database so that past entries have the links field?

@Turbo87
Copy link
Member

Turbo87 commented Jun 26, 2024

the database was backfilled from the git index (see #5112). we were previously storing a subset of the data in the index and a subset in the database. with the linked PR and some of the related ones we converted the database to be the source of truth and changed the index to be derived from the data in the database.

@epage
Copy link

epage commented Jun 26, 2024

The question posed was about backfilling from Cargo.toml, like we did with package.rust-version, and not from the database or the git index.

Like I said, enough time has passed that this might not be worth fixing (or was already fixed). I just want to make sure we are on the same page on the problem and closing this out for the right reasons.

@Turbo87
Copy link
Member

Turbo87 commented Jun 26, 2024

ah, I see what you mean. the issue was in the index, we backfilled the database from the index, so the issue got into the database as well and so creating the index files from the database doesn't help... 😅

@Turbo87
Copy link
Member

Turbo87 commented Jun 28, 2024

@Turbo87 Turbo87 closed this as completed Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants