Provide pre-compiled binaries on each release

[Veetaha]

It makes sense to compile cargo-marker on CI for different platforms and upload the compiled binaries to Github Releases. This will allow people to download them instead of compiling from source, and it will speed up CI that use cargo-marker for linting.

There are various approaches to building and shipping pre-compiled binaries. The simplest one would be to use the github action that does everything for you. Another is to do more manual work but gain more flexibility.

See examples of binary releases:

• cargo-deny (manual approach)
• taplo-cli (even more manual approach)
• typos (another manual approach)

It is best to cover as many architectures and OSes as possible. Beware that cross-compiling to ARM is a PITA if you have native (C/C++) dependencies. However, starting with x84-64 arch only is already something.

[xFrednet]

I've been thinking a bit about this issue and believe that this won't be as simple as the mentioned examples above. cargo-marker is a binary which we can compile and provide as a download. This should be simple enough and should even be possible to automate.

However, the rustc driver requires a specific toolchain, which is one of the reasons why cargo-marker exists. Just providing a download of the driver could result in a bunch of problems. The next thing is, that providing precompiled binaries for cargo-marker and marker_rustc_driver might only be half the battle, since the compilation of lint crates can take a significant amount of time.

I see two options how this can be addressed:

1. Provide a GH action, that has the right toolchain and contains the binaries in the right location.
2. Add support in cargo-marker to handle the download. If cargo-marker downloads the right driver into the right folder, we could avoid the headache for everyone. We can potentially also include the driver in the cargo-marker binary, if we compile it in our CI. Then the driver installation would become a simple copy operation.
   • In addition to this solution, it would probably be good, to also support the download of precompiled lint crates.

I believe we should definitely do option 1. at some point. That might already resolve the use case for CIs. Option two is also possible, I just fear that it might be more involved.

[Veetaha]

Having a single binary for cargo-marker and the marker_rustc_driver dispatched by the binary name (like busybox) should make the distribution easier definitely. I'll also think about how this should eventually be designed

[Veetaha]

I gave this problem more thought.

Preserve the binary split between cargo-marker and marker_rustc_driver

We can't easily combine cargo-marker and marker_rustc_driver into a single binary because marker_rustc_driver depends on the following.

ldd ./target/debug/marker_rustc_driver
        linux-vdso.so.1 (0x00007ffd01d9f000)
        librustc_driver-00c03654e9ecf095.so => not found
        libstd-19964a87984cce8a.so => not found
        libLLVM-17-rust-1.74.0-nightly.so => not found
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fd3c977e000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fd3c9697000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd3c946d000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fd3c998d000)

All those "not found" libraries are provided by the rustc toolchain, and they are quite large. The librustc_driver is 111 MB and libLLVM is 178MB. If we want to link all of that statically then it's going to be a monster binary, but first, we'll need to find or build the static versions of those libraries to be able to link to them... That's really not worth it. If we combined cargo-marker and marker_rustc_driver into a single binary, then users wouldn't be able to run cargo-marker without the required nightly Rust toolchain to be installed.

Rustc libs here also depend on glibc dynamically, making it harder to produce a portable marker_rustc_driver binary (or maybe I'm not understanding how dynamic linking works; I suppose dynamic libs always depend on libc dynamically, and we can't have the executable link the libc statically, and have other dynamic libs dependencies, right?). We'll just provide a pre-compiled binary for x86_64-unknown-linux-gnu, with a sufficiently small glibc version requirement. We'll have to use an older ubuntu20 CI runner for that just like it was done in nushell/nushell#7282.

cargo-marker should then be able to download the marker_rustc_driver from the GitHub releases page in marker's setup command. That command will continue to auto-install the required toolchain with rustup, but it will not build the marker_rustc_driver if it can download it as a pre-compiled binary.

For platforms where the pre-compiled binary of marker_rustc_driver can't be run we'll need to fallback to building it from sources, but it should generally be a really rare case, and I'll be interested to see if there will be people using some exotic platforms where our marker_rustc_driver binary won't run for them. So if cargo-marker can't download the binary due to the platform incompatibility, then it should return an error but suggest reporting that as an issue to this repo and using a special flag --allow-build-from-source to allow building if from sources for that platform to unblock the user.

The question of how to determine if marker_rustc_driver is compatible with the platform where the driver needs to be run is a tricky one. I'm not sure how that can be detected right now. I think this may be discovered when prototyping this feature.

Provide a statically linked binary for cargo-marker

On the other hand cargo-marker is a really neat Rust-only binary without any C dependencies and any other dynamic library dependencies. It is possible to build and statically link cargo-marker today already using this command.

cd cargo-marker
cargo rustc -- -C target-feature=+crt-static

or using the method suggested in this article.

RUSTFLAGS='-C target-feature=+crt-static' cargo build -p cargo_marker --target x86_64-unknown-linux-gnu

It's possible to link it statically to glibc (which is what happens in the examples above). Usually, when it comes to static linking I see people using musl for that. I didn't have much experience with static linking, I've seen issues with musl (rust-lang/rust#99740) that tell that it has bugs, or it doesn't have all functions implemented (leaving just stubs for them) or it's performing worse than glibc, which makes it preferable for linking to glibc. But glibc wasn't designed for static linking (StackOverflow) which is an argument against linking to it statically.

There also are licensing concerns when linking to glibc statically, as it uses LGPL. I am not an expert in this area, but since marker's code is already open-source under MIT/Apache, hopefully, this isn't an issue.

There was also an interesting analysis done in this issue OptiVorbis/OptiVorbis#32 (comment) regarding the comparison between musl and glibc. I am still not sure which of them would fit better in the use case of cargo-marker, but we can change the libc backend at any time we want, so it's not really important what to choose at this stage. Since most of the world uses musl for that, then it seems to be better to keep that trend.

Multiple architectures support

Unfortunately, GitHub doesn't provide ARM runners natively. We can only use AMD64 runners and thus we need to set up cross-compilation.

We should be able to leverage cargo-zigbuild to handle the cross-compilation to ARM. It should be easy to set it up for cargo-marker, but I'm not sure how easy it will go for marker_rustc_driver.

If cargo-zigbuild appears to be a huge PITA to set up for marker_rustc_driver then we may fallback to using docker buildx with qemu backend for building ARM binaries on AMD64 host. I have experience setting up this cross-compilation flow in my other project. I had to do that due the fair amount of pain when using cargo-zigbuild with the rust project that has C dependencies.

The downside of using docker + qemu is obviously the considerable compilation performance decrease due to the emulation layer. In my project with ~600 dependencies, it took around an hour to compile with that approach on a pretty powerful dedicated root server (16 cores, 64GB RAM). But the number of dependencies in marker_rustc_driver and its general code footprint is really small, so it may not be a big issue. Experiments will tell.

I am not sure what it will take to have ARM builds for windows and macos, but it will be interesting to look into that more when coming closer to implemening this issue.

CI organization

We should have a dedicated workflow for cutting a release that should be possible to trigger manually via GitHub UI. It should take the version number as input.

_{And maybe that workflow could be a scheduled workflow that requires approval before it can run (not sure that this is useful, probably not worth doing this initially)}

_{Maybe the workflow could provide patch/minor/major directives to remove the need for the maintainer to figure out the exact version when doing a release, but taking the exact version will be a simple good start}

The workflow should have the jobs to do the following.

• Build the cargo-marker static binary using musl with a matrix of (windows, linux, macos) + (amd64, arm64)
• Build the marker_rustc_driver dynamically-linked binary with the same matrix but on ubuntu-20 for linux for lower glibc version requirement
• Create a new GitHub release and upload all built artifacts
• Prepare the next minor version of marker by automatically bumping the versions in various files to the new 0.x-dev version and pushing that as a new git commit

Footnote

Regarding the problem of distributing pre-compiled lint crate dynamic libraries. This is a totally different issue, and it is definitely out of scope for this issue. This should definitely be implemented and I see that cargo-marker should provide utilities for distributing pre-compiled lint libraries for their authors, but it is a totally different story.

We need to decide how that works. Where the pre-compiled libs will be stored? Is it github releases of the repo that hosts the lint crate code? Is it some self-hosted binary artifacts registry? Could we use some existing solution for that? We'll return to that at some point, but not know

[Veetaha]

I've recently discovered the effort in another linter (reddit post).

The approach that was used there is to leverage RUSTC_BOOTSTRAP=1 and compile the driver with the stable released version of rustc. This way users are more likely to have the required toolchain installed.

I know RUSTC_BOOTSTRAP is an implementation detail of rustc and it may be gone at any point. But just like with any unstable feature that we use everything may break on the next rustc release, so it's probably not a big deal?

I've been considering this approach myself already, but the fact that @estebank used it in redpen makes me lean to this more.

[Veetaha]

I experimented with the workspace structure in this repository.

Multiple cargo workspaces

I thought that there should be at least two rust-toolchain.toml versions because the crates here have quite different toolchain targets.

For example, I tried this structure

• /bin
  • cargo-marker
  • marker_rustc_driver
  • marker_error
  • marker_lints
  • marker_uilints
  • Cargo.toml (workspace manifest)
  • rust-toolchain.toml (pins a specific nightly version or stable one but with RUSTC_BOOTSTRAP)
• /lib
  • marker_api
  • marker_utils
  • marker_uitest
  • Cargo.toml (workspace manifest)
  • rust-toolchain.toml (pins an older stable version like 1.66 according to the MSRV)

Pros of multiple cargo workspaces

• It is immediately obvious which crates are published to crates-io, and which are part of the pre-compiled binaries (lib vs bin directories)
• It is immediately obvious which crates need to support an older MSRV (lib crates)
• It is immediately obvious where we need to be more careful with the public API of the crate (lib), and where the public API of the crate isn't considered stable (bin crates)
• rust-analyzer will now run cargo check of the expected toolchain version. For example, it will underline you in the editor if you try to use a feature that isn't yet stable in the MSRV that is applied to a lib crate
• cargo clippy will no longer suggest you to use a feature that isn't yet stable in the MSRV that is applied to a lib crate. For example, I already stumbled with the problem that clippy complained that I don't use Option::is_some_and() in one of the lib crates. I had to re-write the code in a way that it doesn't detect that pattern so I didn't have to add an allow() for the lint that doesn't exist in the MSRV.

Cons of multiple cargo workspaces

• It is a pain in the ass to get around with cargo because cargo doesn't support multiple workspaces in a single repo. So you no longer can run cargo build at the top level folder of the repository. You have to explicitly cd into either bin or lib to build it. 99% of the time you already want to operate in the bin directory with cargo, and the fact that you need to cd into it every time trips me. This is the main problem that made me drop the idea of multiple cargo workspaces
• rust-analyzer supports multiple workspaces, but it does have compromises. For example "jump to definition" of the std library in the lib crates still forwards you to the nigthly version of std for some reason. It's not a big deal, but something to be aware of.
• We now have two target directories. But.. you will probably never run cargo build inside of lib crates, and will operate inside of bin, so what's the point of even doing this? I gave up on this idea at this point...

Compromise solution

Given the expreince of above I decided that we should keep a single cargo workspace. However, I like the idea of separating the "binary" self-compiled crates and the "libary" crates with the older MSRV restriction. This should make it easier to reason about the code in the repository.

Yes, we will still have a single rust-toolchain.toml, but CI will cover catching the MSRV violations. For sure, this means we are getting a really late feedback, but, hopefully, this won't be a problem big enough.

So this isn't something that isn't required for the feature of precompiled binaries, but rather and adjacent refactoring that I decided makes sense either way. @xFrednet what do you think about organizing the crates into two folders (but not creating multiple cargo workspaces) as I described?

Folders naming

I know the naming is a bit wrong here because not all bin crates have binary targets, but I couldn't come up with a succinct naming for these two dirs that could read better.

The other naming patterns that I considered are the following, and all of them have their flaws.

• stable/unstable
• oldest-toolchain/latest-toolchain
• public/internal
• published/compiled
• msrv/pinned
• distributed/self-compiled

[xFrednet]

I've recently discovered the effort in another linter (reddit post).

Thank you for pointing this out, I totally missed this project. My plan was to post on Reddit with v0.3.0. This update, I want to focus on making the API more usable.

The approach that was used there is to leverage RUSTC_BOOTSTRAP=1 and compile the driver with the stable released version of rustc. This way users are more likely to have the required toolchain installed.

I know RUSTC_BOOTSTRAP is an implementation detail of rustc and it may be gone at any point. But just like with any unstable feature that we use everything may break on the next rustc release, so it's probably not a big deal?

I've been considering this approach myself already, but the fact that @estebank used it in redpen makes me lean to this more.

If that project also uses it, it might be fine to leverage RUSTC_BOOTSTRAP=1 as well :)

[xFrednet]

My answers for your first comment. The past few weeks have been pretty full, that's why I'm only answering now 😅

cargo-marker should then be able to download the marker_rustc_driver from the GitHub releases page in marker's setup command. That command will continue to auto-install the required toolchain with rustup, but it will not build the marker_rustc_driver if it can download it as a pre-compiled binary.

I wouldn't default to downloading the driver from Github, at least not if cargo-marker was compiled from source. One reason is also the recent discussion when syn came with precompiled binaries. It should definitely be a selectable option though.

For platforms where the pre-compiled binary of marker_rustc_driver can't be run we'll need to fallback to building it from sources, but it should generally be a really rare case, and I'll be interested to see if there will be people using some exotic platforms where our marker_rustc_driver binary won't run for them.

This should be exceedingly rare. marker_adapter depends on the libloading crate which also restricts Marker to windows, macos and linux systems. But, we'll see.

The question of how to determine if marker_rustc_driver is compatible with the platform where the driver needs to be run is a tricky one. I'm not sure how that can be detected right now. I think this may be discovered when prototyping this feature.

Maybe we could also distinguish between pre-compiled binaries and build from source one. If someone downloads the GH binary it could include the driver, but if they build it from source, we also build the driver from source 🤔

Provide a statically linked binary for cargo-marker

I'm not an expert either, but your analysis sounds good so far

CI organization

This is also what I was imagining 👍

[xFrednet]

• cargo clippy will no longer suggest you to use a feature that isn't yet stable in the MSRV that is applied to a lib crate. For example, I already stumbled with the problem that clippy complained that I don't use Option::is_some_and() in one of the lib crates. I had to re-write the code in a way that it doesn't detect that pattern so I didn't have to add an allow() for the lint that doesn't exist in the MSRV.

We should still be able to set the msrv, even if we use a newer compiler. That would make Clippy not suggest invalid code :)

Yes, we will still have a single rust-toolchain.toml, but CI will cover catching the MSRV violations. For sure, this means we are getting a really late feedback, but, hopefully, this won't be a problem big enough.

I believe this should work fine. So far, it has worked okay, and the CI has only gotten better.

what do you think about organizing the crates into two folders (but not creating multiple cargo workspaces) as I described?

I don't really have a strong opinion either way. As you said, I think the lib and bin names are a bit confusing, if we want to separate between having a stable or unstable API. But we can adjust the names. It's hard to find a good naming convention, though. 🤔 Having everything in the repository root makes it a bit easier to find all crates for newcomers, but having the separation might help with the actual development of features for them. So if there are benefits to the separation, we can do it.

One downside I see, is that I often use auto-complete with the folder names, for cargo commands. Like cargo build -p car<tab> and then I remove the added / but that's just a tiny thing from my workflow, which shouldn't stop such a refactoring at all

[Veetaha]

We should still be able to set the msrv, even if we use a newer compiler

I didn't expect that clippy is so smart, but indeed. When I set the package.rust-version = "1.66.0" then the behavior of this lint changes.

Without the rust-version in Cargo.toml set:

-         let _ = Some(32).map(|val| val == 32).unwrap_or(false);
+         let _ = Some(32).is_some_and(|val| val == 32);

With the rust-version in Cargo.toml set:

-         let _ = Some(32).map(|val| val == 32).unwrap_or(false);
+         let _ = Some(32).map_or(false, |val| val == 32);

Even though I don't like map_or[_else] combinators, it's still good that clippy is MSRV-aware

---

I wouldn't default to downloading the driver from Github, at least not if cargo-marker was compiled from source.

Regarding the binary download behavior, I think the situation with proc macros is a bit different, but I'd still have the default preference for the precompiled binaries because 95% of use cases will be to use a pre-compiled binary I expect. Those 5% of security-aware people will be able to compile everything themselves, but we just need to make sure we don't make that exceptionally difficult; providing an option to compile from sources is fine, but we won't provide strong MSRV guarantees for that.

The dev flow for cargo-marker will under no circumstances be broken. The local builds will work by default in debug builds and the env var opt out will remain.

---

Another thing that I was thinking about is packaging cargo-marker and marker_rustc_driver into the same archive, or even embedding the marker_rustc_driver into cargo-marker with include_bytes!(), because marker_rustc_driver is so tiny (1MB).

---

There is also an option to use AppImage packaging that depends on FUSE, which is a linux-only feature and I believe there will be number of problems with it on different platforms due to compatibility and whatever.
For example it requires you to install libfuse2 which isn't available on ubuntu 22.04 by default, for example.

[xFrednet]

Clippy has quite a few config options, it's sad that they're not used more.

---

Regarding the binary download behavior, I think the situation with proc macros is a bit different [...]

After a bit more thought, I agree with you, that the default should be to download the binary. The thing which actually convinced me, was the potential usage of the RUSTC_BOOTSTRAP variable. Doing that ourselves, feels okay, but asking a user to do that (Or doing that for them) feels hacky (as it should). If we download the driver by default, it feels more okay for me, to use this hack :)

---

Another thing that I was thinking about is packaging cargo-marker and marker_rustc_driver into the same archive, or even embedding the marker_rustc_driver into cargo-marker with include_bytes!(), because marker_rustc_driver is so tiny (1MB).

Embedding the driver would be my preferred method right now. At least for cargo-marker binaries compiled in our CI. For binaries build from source, we obviously need to download the driver separately or compile it.

---

I've never worked with app images, for us, I think it's sufficient to have cargo-marker handle everything. At one point, I hope that we can be distributed via rustup, but I wouldn't count on that anytime soon.

[Veetaha]

One thing with RUSTC_BOOTSTRAP and using a stable toolchain this way is that we'll need to rely on the stable release schedule to update the toolchain version. I wonder if you'd consider that to be acceptable.

For example, consider this.

Suppose at some point marker pins the toolchain version 1.73 when it becomes stable, and 1.74 isn't released yet. Then we need to wait until 1.74 is available to prepare the next update. It will take some time to update to 1.74 once that is available. All this time 1.73 will be set in git.

IIRC the release cadance of marker is already tied to the release cadance of rustc toolchain. Will it be okay if marker lags behind by one version from what's in stable? How and when should the rust toolchain be updated according to your vision?