rust icon indicating copy to clipboard operation
rust copied to clipboard

Turn `stdarch` into a Josh subtree

Open Kobzol opened this issue 7 months ago • 18 comments

In a similar vein as https://github.com/rust-lang/rust/pull/141229, this PR makes the stdarch repository a Josh subtree (it was previously a submodule). The initial commit of stdarch upon this is based is b6e2249e388f520627544812649b77b0944e1a2e, which is the previous commit SHA of the stdarch submodule. The sync was performed according to https://hackmd.io/7pOuxnkdQDaL1Y1FQr65xg.

This was decided in https://github.com/rust-lang/stdarch/issues/1655.

Test pull PR on my fork: https://github.com/Kobzol/stdarch/pull/1 Test push PR on my fork: https://github.com/Kobzol/rust/pull/59

I plan to use the same Rust (miri-inspired) tooling that we use for rustc-dev-guide to enable pulls/pushes on stdarch.

Note that this repository currently doesn't have any stdarch-specific tests, so before that, the subtree should only be modified through this repository only when dealing with changes that contain "cyclical dependencies" between stdarch and rustc. The long term vision is to integrate stdarch into rust-lang/rust completely.

CC @Amanieu

try-job: aarch64-apple try-job: aarch64-gnu try-job: x86_64-msvc-* try-job: x86_64-gnu try-job: x86_64-gnu-aux

Kobzol avatar Jun 02 '25 11:06 Kobzol

Thanks a lot! However, I feel like this should be approved by t-libs... maybe r? @Amanieu ?

Also it my be a good idea to wait for compiler-builtins having their first successful syncs in both directions to ensure there's no hidden pitfalls in the setup.

How hard would it be to add a smoke test that at least checks that the crates build by themselves (i.e., outside of libcore)? That should be feasible wrt CI time and should catch some of the obvious issues -- the lib.rs file is otherwise entirely unused on the rustc side.

RalfJung avatar Jun 03 '25 06:06 RalfJung

Also it my be a good idea to wait for compiler-builtins having their first successful syncs in both directions to ensure there's no hidden pitfalls in the setup.

Definitely agreed.

How hard would it be to add a smoke test that at least checks that the crates build by themselves (i.e., outside of libcore)? That should be feasible wrt CI time and should catch some of the obvious issues -- the lib.rs file is otherwise entirely unused on the rustc side.

Yeah, I think we should do at least some smoke test. I guess that we could do it in bootstrap, I'll take a look.

Kobzol avatar Jun 03 '25 06:06 Kobzol

:umbrella: The latest upstream changes (presumably #141229) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Jun 04 '25 00:06 bors

This is going to conflict with #141964 which updates the submodule.

Amanieu avatar Jun 04 '25 10:06 Amanieu

How hard would it be to add a smoke test that at least checks that the crates build by themselves (i.e., outside of libcore)? That should be feasible wrt CI time and should catch some of the obvious issues -- the lib.rs file is otherwise entirely unused on the rustc side.

Is it required to build them using the in-tree rustc? Or is it enough to use stage0/beta compiler? Is it enough to just run cargo build in library/stdarch as a smoke test?

Kobzol avatar Jun 09 '25 08:06 Kobzol

Well, the goal is to avoid multi-PR transitions for things like renaming an intrinsic that stdarch uses. So ideally the in-tree rustc is the only one that we require to be able to build stdarch.

RalfJung avatar Jun 09 '25 08:06 RalfJung

@bors2 try

Kobzol avatar Jun 09 '25 09:06 Kobzol

:hourglass: Trying commit b644ddc619661d56d25d5b55e8a696daa613cb5e with merge 171601d2588b8197e034b978ef02d1f65c6170c9…

To cancel the try build, run the command @bors2 try cancel.

rust-bors[bot] avatar Jun 09 '25 09:06 rust-bors[bot]

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[TIMING] core::build_steps::test::RustdocJson { compiler: Compiler { stage: 2, host: aarch64-unknown-linux-gnu, forced_compiler: false }, target: aarch64-unknown-linux-gnu } -- 0.000
Skipping Set({test::src/tools/rust-installer}) because it is excluded
Testing stdarch stage1 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
    Updating crates.io index
error: the lock file /checkout/library/stdarch/Cargo.lock needs to be updated but --locked was passed to prevent this
If you want to try to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:51:07
  local time: Mon Jun  9 10:34:03 UTC 2025
  network time: Mon, 09 Jun 2025 10:34:04 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

rust-log-analyzer avatar Jun 09 '25 10:06 rust-log-analyzer

:broken_heart: Test failed

rust-bors[bot] avatar Jun 09 '25 10:06 rust-bors[bot]

error: the lock file /checkout/library/stdarch/Cargo.lock needs to be updated but --locked was passed to prevent this If you want to try to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.

Hmm. What to do about stdarchs lockfile? Should we always update it in the rustc repo? I wonder how this works for miri and other tools, it seems like it also has its own lockfile.

Kobzol avatar Jun 09 '25 17:06 Kobzol

For Miri, we basically never change the lockfile in the rustc repo. I do manual cargo updates in the Miri repo every now and then.

RalfJung avatar Jun 09 '25 17:06 RalfJung

Oh, nevermind, stdarch actually doesn't have a lockfile, that's what was causing the issue. Sorry! The same issue actually happens also for https://github.com/rust-lang/compiler-builtins, as it also misses a lockfile, and it also causes bootstrap problems.

Kobzol avatar Jun 09 '25 17:06 Kobzol

@bors2 try

Kobzol avatar Jun 10 '25 05:06 Kobzol

:hourglass: Trying commit 11d63c5812fe37e407481c1c36cf09b40129aca9 with merge 594fc6f8fae7090415c27845d859b6444bf8f4a1…

To cancel the try build, run the command @bors2 try cancel.

rust-bors[bot] avatar Jun 10 '25 05:06 rust-bors[bot]

:broken_heart: Test failed

rust-bors[bot] avatar Jun 10 '25 06:06 rust-bors[bot]

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] core_arch test:false 4.803
error: could not compile `core_arch` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
[RUSTC-TIMING] syn test:false 5.130
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:50:53
  local time: Tue Jun 10 06:58:32 UTC 2025
  network time: Tue, 10 Jun 2025 06:58:32 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

rust-log-analyzer avatar Jun 10 '25 06:06 rust-log-analyzer

This will need a stdarch bump with the avx512 features removed.

Kobzol avatar Jun 10 '25 08:06 Kobzol

:umbrella: The latest upstream changes (presumably #142443) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Jun 13 '25 21:06 bors

Rebased the PR on top of latest master, which included a stdarch update that now makes the bootstrap smoke test work. This should be ready now.

Kobzol avatar Jun 16 '25 08:06 Kobzol

:warning: Warning :warning:

  • There are issue links (such as #123) in the commit messages of the following commits. Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

    • ab098c6be61cb5a60d84192404df8f65755a1c29
    • 6e4ad9cc18591731af2afabafe5b8ccb96e9e57e
    • e61df091c1a54588f0ea6a58a646f030ea0e5ef5
    • 68c0308cbe90eb49a7f4643b86d482fa0e27a859
  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    • f59d0727f42ddecfd9ea2602fe8df9af6a646b7f

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

rustbot avatar Jun 16 '25 08:06 rustbot

@bors2 try

Kobzol avatar Jun 16 '25 08:06 Kobzol

:hourglass: Trying commit d97a99a932f47462c1e5f62c7a4dc378bdc19781 with merge f871e6e94be3a3682a7f6ea5726341a6f2404047…

To cancel the try build, run the command @bors2 try cancel.

rust-bors[bot] avatar Jun 16 '25 08:06 rust-bors[bot]

Did we have successful compiler-builtin syncs in both directions, to confirm that the basic approach works?

RalfJung avatar Jun 16 '25 10:06 RalfJung

I believe so (https://github.com/rust-lang/compiler-builtins/pull/946, https://github.com/rust-lang/rust/pull/142489). Not sure why it's so important though (what's the "basic approach"?). We're using josh in the same way here and for compiler-builtins as we have been using it for rustc-dev-guide, and that has been working for a couple of months already.

The approach definitely works; we can always screw up when setting the initial PR for each subtree though :)

Kobzol avatar Jun 16 '25 10:06 Kobzol

:sunny: Try build successful (CI) Build commit: f871e6e94be3a3682a7f6ea5726341a6f2404047 (f871e6e94be3a3682a7f6ea5726341a6f2404047, parent: 68ac5abb067806a88464ddbfbd3c7eec877b488d)

rust-bors[bot] avatar Jun 16 '25 11:06 rust-bors[bot]

Okay cool :)

With earlier josh-ifications sometimes there were issues with the initial import that only surfaced on a sync, so I became a bit paranoid here.

RalfJung avatar Jun 16 '25 13:06 RalfJung

:warning: Warning :warning:

  • There are issue links (such as #123) in the commit messages of the following commits. Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

    • ab098c6be61cb5a60d84192404df8f65755a1c29
    • 6e4ad9cc18591731af2afabafe5b8ccb96e9e57e
    • e61df091c1a54588f0ea6a58a646f030ea0e5ef5
    • 68c0308cbe90eb49a7f4643b86d482fa0e27a859
    • 8fb78f3f6991caa6305f7442c38d4eaf391be011
  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    • e43310188291c19d469189b76fa55486c3dce2e3

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

rustbot avatar Jun 23 '25 15:06 rustbot

This PR is quite bitrotty, and every stdarch submodule update requires me to redo it from scratch. @RalfJung @Amanieu do you want me to prepare/check anything else before merging this?

Kobzol avatar Jun 23 '25 15:06 Kobzol

No, I think this is fine to merge.

r=me

Amanieu avatar Jun 23 '25 16:06 Amanieu