bytecodealliance / rustix

Safe Rust bindings to POSIX-ish APIs
Other
1.4k stars 139 forks source link

Fix publish to Artifactory crates registry #1072

Closed alloncm closed 3 weeks ago

alloncm commented 1 month ago

The core and alloc aliases causes problems in Artifactory crates registry and prevents rustix from being discovered (and as a result downloaded) by cargo. This has been introduced in version 0.38.10 when the dep: syntax for Cargo.toml was first introduced.

My guess is that the usage of the core and alloc keywords caused issues while uploading and indexing the crate.

Some background:

I have been crating a crates.io mirror registry using Artifactory (version 7.77.9) as the registry and after uploading rustix 0.38.34 cargo couldn't find it on the registry (not during the end of the publish process and not afterwards).

At first I couldn't understand why but after 2 days of debugging this I have noticed that version 0.38.9 is discoverable while 0.38.10 is not. I started to check the diff between the 2 versions which lead me to those crates and after renaming the aliases it solved it and made it discoverable (and downloadable) by cargo.

I know it is a fairly niche problem that could be worked around in various ways but I believe it could be solved rather trivially by simply removing those aliases (or even renaming them to something else).

And of-course thank you for all your hard work supporting the Rust ecosystem with those wonderful crates!

sunfishcode commented 1 month ago

Unfortunately, this patch breaks the use case for rustc-dep-of-std, which is using rustix as a dependency of std.

Using this branch of rust that depends on rustix, modified to use the branch in this PR, I get this error:

$ ./x.py check library/std
Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.00s
Checking stage0 library artifacts {std} (x86_64-unknown-linux-gnu)
    Checking rustix v0.38.34 (/home/me/rustix)
error[E0463]: can't find crate for `core`

For more information about this error, try `rustc --explain E0463`.
error: could not compile `rustix` (lib) due to 1 previous error
Build completed unsuccessfully in 0:00:00

Do you have any ideas for other ways we might fix or work around this?

alloncm commented 1 month ago

Thanks for the quick replay!

I now realize I kind of missed the whole purpose of this feature.

I'm now learning about those special crates and how to use them properly and Ill update once Ill figure out an appropriate fix or work around for the issue.

alloncm commented 1 month ago

I have noticed that rustc-std-workspace-alloc is never used in the rust fork you linked so removing this dependency along with the removal of the dep: syntax should make this work.

In case alloc is needed to work I did found a work around for the crate name alias in Cargo.toml (since the crate has a feature called alloc there is a name collision without the dep: keyword I'm trying to avoid) by adding this code to lib.rs:

#[cfg(feature = "rustc-dep-of-std")]
extern crate rustc_std_workspace_alloc as alloc;

I was able to turn on the alloc feature in your rust fork (for some reason it didn't work for core, maybe cause it contains fundamental types for the language?).

This is currently not in the PR since it errors on unused extern crate statements (and since alloc is not used it wont compile with this):

error: unused extern crate
   --> /home/alloncm/dev/OtherRepos/rustix/src/lib.rs:131:1
    |
130 | / #[cfg(feature = "rustc-dep-of-std")]
131 | | extern crate rustc_std_workspace_alloc as alloc;
    | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
    | |________________________________________________|
    |                                                  help: remove it

Please let me know if this is an acceptable fix for the issue.

I'm keeping this PR as draft in order to make some more tests against Artifactory to verify there are no new problems that arise from this suggestion.

Thanks!

alloncm commented 1 month ago

Just realized I can conditionally add the rustc-std-workspace-alloc alias in lib.rs with the alloc feature so I restored the support for alloc even though its not currently used.

sunfishcode commented 3 weeks ago

This looks like it'll work. Thanks!