Remove build.rs file #543

GuillaumeGomez · 2023-01-06T15:37:13Z

Fixes #489.

This is another take on #490. Since Rust 1.60, cfg(target_has_atomic = "...") is stable so we can now use it. However, it'll change the MSRV to 1.60 as well.

I used target_has_atomic = "ptr" (and not target_has_atomic = "8" for example) because that's what is used in the std for AtomicUsize. You can see it in the macro declaring the AtomicUsize here.

cc @nnethercote (since you're the one who suggested this change)

EDIT: Just saw that this solution was mentioned by @KodrAus in the linked issue.

Thomasdezeeuw

I'm generally in favour of this. But I don't know if should bump our MSRV without bumping our minor version. Similar discussions are going in the libc crate: rust-lang/libs-team#72 and the time crate: time-rs/time#535.

What ever we decide on this, we should try a pick a consistent strategy with the other crates.

GuillaumeGomez · 2023-01-06T16:50:47Z

Once it's figured out, we can then decide what to do with this PR. No hurry. ;)

KodrAus · 2023-04-10T08:18:06Z

1.60.0 is quite a jump, but is still just over 12 months old. I personally think the removal of a build.rs makes it worth it. I don't think it's worth doing a minor bump for this purely because we couldn't shim an 0.5.x build in 0.4.x without also bumping the MSRV of 0.4.x anyways, and wouldn't want to issue a breaking release for log that didn't automatically forward 0.4.x through 0.5.x.

Thomasdezeeuw · 2023-04-10T11:43:52Z

We could do a release with a bump to 1.60 and be prepared to yank it if people complain... Not an ideal solution, but maybe we're making a bigger deal of the MSRV bump than needed.

KodrAus · 2023-04-10T21:16:14Z

In this case, I think we've got something really tangible to offer people for the bump too; one less dependency running arbitrary code at build-time.

In our release notes we can mention why we couldn't do this in a minor bump, because the log macros for multiple versions need to forward through the same global logger. We can't even pull just this mechanism out into its own crate because that's the bit that needs the MSRV bump.

KodrAus

Thanks @GuillaumeGomez!

This looks good to me. If @Thomasdezeeuw is also happy I say we should get it merged and look at putting a release together 🙂

GuillaumeGomez · 2023-04-11T08:21:29Z

Thanks everyone!

The `log` crate decided to break support for rustc 1.48 (Debian bullseye) the day the next release of Debian comes out, obviously before anyone has had a chance to upgrade to the new Debian bookworm (at rust-lang/log#543). Thus, we have to manually pin it back to the previous release. Sadly, the `log` crate is a transitive dependency of `tokio` until 0.20, which requires rustc 1.49. Luckily at least we won't have to deal with this again, as `log` won't be a dependency of ours anymore soon.

* Avoid potential panicking in `args::parse` by using `Vec::get` instead of indexing * Refactor: Simplify `opts::{resolve, install}` API Many parameters can be shared and put into `opts::Options` intead and that would also avoid a few `Arc<Path>`. * Optimize `get_install_path`: Avoid cloning `install_path` * Optimize `LazyJobserverClient`: Un`Arc` & remove `Clone` impl to avoid additional boxing * Optimize `find_version`: Avoid cloning `semver::Version` * Optimize `GhCrateMeta::launch_baseline_find_tasks` return `impl Iterator<Item = impl Future<Output = ...>>` instead of `impl Iterator<Item = AutoAbortJoinHandle<...>>` to avoid unnecessary spawning. Each task spawned has to be boxed and then polled by tokio runtime. They might also be moved. While they increase parallelism, spawning these futures does not justify the costs because: - Each `Future` only calls `remote_exists` - Each `remote_exists` call send requests to the same domain, which is likely to share the same http2 connection. Since the conn is shared anyway, spawning does not speedup anything but merely add communication overhead. - Plus the tokio runtime spawning cost * Optimize `install_crates`: Destruct `Args` before any `.await` point to reduce size of the future * Refactor `logging`: Replace param `arg` with `log_level` & `json_output` to avoid dep on `Args` * Add dep strum & strum_macros to crates/bin * Derive `strum_macros::EnumCount` for `Strategy` * Optimize strategies parsing in `install_crates` * Fix panic in `install_crates` when `Compile` is not the last strategy specified * Optimize: Take `Vec<Self>` instead of slice in `CrateName::dedup` * Refactor: Extract new fn `compute_resolvers` * Refactor: Extract new fn `compute_paths_and_load_manifests` * Refactor: Extract new fn `filter_out_installed_crates` * Reorder `install_crates`: Only run target detection if args are valid and there are some crates to be installed. * Optimize `filter_out_installed_crates`: Avoid allocation by returning an `Iterator` * Fix user_agent of `remote::Client`: Let user specify it * Refactor: Replace `UIThread` with `ui::confirm` which is much simpler. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>

The `log` crate decided to break support for rustc 1.48 (Debian bullseye) the day the next release of Debian comes out, obviously before anyone has had a chance to upgrade to the new Debian bookworm (at rust-lang/log#543). Thus, we have to manually pin it back to the previous release. Sadly, the `log` crate is a transitive dependency of `tokio` until 0.20, which requires rustc 1.49. Luckily at least we won't have to deal with this again, as `log` won't be a dependency of ours anymore soon.

GuillaumeGomez added 2 commits January 6, 2023 16:28

Remove build.rs file

7f09d72

Update minimum supported Rust version to 1.60.0

0fe18a4

Thomasdezeeuw reviewed Jan 6, 2023

View reviewed changes

KodrAus approved these changes Apr 10, 2023

View reviewed changes

Thomasdezeeuw approved these changes Apr 10, 2023

View reviewed changes

KodrAus mentioned this pull request Apr 10, 2023

Add the structured-logger crate to the README #548

Merged

KodrAus merged commit 474699a into rust-lang:master Apr 10, 2023

KodrAus mentioned this pull request Apr 11, 2023

Prepare for 0.4.19 release #552

Merged

GuillaumeGomez deleted the rm-build-rs branch April 11, 2023 08:21

KodrAus mentioned this pull request Apr 13, 2023

Use target_has_atomic instead of the old atomic_cas cfg #555

Merged

est31 mentioned this pull request Apr 15, 2023

Put MSRV into Cargo.toml #557

Merged

KodrAus mentioned this pull request May 28, 2023

Revert MSRV bump for patch release #565

Merged

DMRobertson mentioned this pull request Jun 12, 2023

Bump log from 0.4.18 to 0.4.19 matrix-org/synapse#15761

Merged

michalbiesek mentioned this pull request Jun 12, 2023

Fix alpine integration test build criblio/appscope#1518

Merged

TheBlueMatt mentioned this pull request Jun 12, 2023

Fix build on supported platforms due to log breakage lightningdevkit/rust-lightning#2353

Merged

psychon mentioned this pull request Jul 1, 2023

log v0.4.19 increased its MSRV to Rust 1.60 psychon/x11rb#850

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove build.rs file #543

Remove build.rs file #543

GuillaumeGomez commented Jan 6, 2023 •

edited

Loading

Thomasdezeeuw left a comment

GuillaumeGomez commented Jan 6, 2023

KodrAus commented Apr 10, 2023

Thomasdezeeuw commented Apr 10, 2023

KodrAus commented Apr 10, 2023

KodrAus left a comment

GuillaumeGomez commented Apr 11, 2023

Remove build.rs file #543

Remove build.rs file #543

Conversation

GuillaumeGomez commented Jan 6, 2023 • edited Loading

Thomasdezeeuw left a comment

Choose a reason for hiding this comment

GuillaumeGomez commented Jan 6, 2023

KodrAus commented Apr 10, 2023

Thomasdezeeuw commented Apr 10, 2023

KodrAus commented Apr 10, 2023

KodrAus left a comment

Choose a reason for hiding this comment

GuillaumeGomez commented Apr 11, 2023

GuillaumeGomez commented Jan 6, 2023 •

edited

Loading