Talk:Rust package guidelines

Using cargo install in package()

The cargo install command seems invalid, it contains --root twice ? It seems cargo install --locked --root "${pkgdir}"/usr --path "${srcdir}/${pkgname}-${pkgver}" is closer to what we need, but it creates a .creates.toml file in /usr/bin (see https://github.com/rust-lang/cargo/issues/3316)

Nicoulaj (talk) 08:34, 24 June 2019 (UTC)

That PR actually provides a solution: --no-metadata. So I guess the command now should be cargo install --locked --no-metadata --root "${pkgdir}"/usr --path "${srcdir}/${pkgname}-${pkgver}" ? Renyuneyun (talk) 18:41, 5 November 2019 (UTC)

A few concerns

--target "$CARCH-unknown-linux-gnu" is not required, as the host architecture is the default value.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

This is empirically not the case. Honestly it looks like the Rust docs are wrong here. The host architecture is the default only for build, not for fetch. For fetch the default is re download dependencies for all possible targets. You can see this pretty plainly if you check the actual downloads. For example take the starship package and build it yourself after adding a `du -sh ~cargo;exit 1` to the end of the prepare() function. You should get a dependency cache of 182M. Now delete the target argument and try again. Now you'll get 326M. This is why the argument is worthwhile for a huge portion of Rust packages: by default somewhere down the dependency chain they will have libraries for other platforms.

Alerque (talk) 11:49, 13 August 2021 (UTC)

In fact closer inspection shows the docs contradict themselves. In the options block it has the text copied from cargo build (which is wrong for cargo fetch) but just above that in the description block it does have the right thing documented: "If --target is not specified, then all target dependencies are fetched." I'm working on a PR to send upstream.

Alerque (talk) 12:32, 13 August 2021 (UTC)

Fantastic! Glad to hear that upstream documentation will (hopefully) be amended.

Grawlinson (talk) 01:01, 14 August 2021 (UTC)

This seems to cause problems on e.g. RaspberryPi 4 where CARCH=armv7h but cargo expects --target armv7-unknown-linux-gnueabihf

Christoph.gysin (talk) 20:28, 7 October 2021 (UTC)

RUSTUP_TOOLCHAIN=stable is not used when rust is installed, as per the documentation. This also causes builds to not be reproducible due to "stable" being the latest stable version, not a pinned version.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

No, this logic only applies when the specific package rust is installed, it does not apply when something else that provides rust is installed. This argument is specifically document as only being suggested for AUR packages not repo builds. There are quite a few packages including rustup that provide=(rust) that can be configured to default to other tool chains such as nightly.

Alerque (talk) 11:49, 13 August 2021 (UTC)

Using rustup with toolchain=stable does result in non-reproducible builds.

Grawlinson (talk) 01:30, 14 August 2021 (UTC)

While sort of true, this is irrelevant on two counts: ① the variable in question is noted only for inclusion in AUR builds where reproduciblity isn't exactly a front and center goal while not breaking is, and ② the makedepends should never use rustup, only either cargo or cargo-nightly. There are other things that can provide cargo-nightly besides rustup that may come in to play for some users, and only in the event that nightly is is reproduciblity a problem. In which case the variable in question won't be set to stable anyway!

Alerque (talk) 20:41, 14 August 2021 (UTC)

CARGO_TARGET_DIR, or --target or --target-dir cause Cargo to run in a different mode. The default value is sane enough.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

No it doesn't. And the docs don't say this. They say this for the --target argument but not for the CARGO_TARGET_DIR environment variable. And the default value is sane of course, the issue we are trying to fix is people who have user environments that override the default. We need the default back because it is generally assumed in the package() function where the builds will be.

Alerque (talk) 11:49, 13 August 2021 (UTC)

As it is, this page seems to be targeted at AUR users, rather than packagers. Providing both methods on this page suggests that packagers are happy with the page is it is.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

It includes tips for both with specific documentation for when, why, and where each argument is useful. I don't understand what the issue is. If you'd prefer the notes be rewritten the other way (notes for AUR instead of notes for not-AUR) we could do that, but the examples and formatting would get more cluttered — which is why I picked this way while editing it.

Alerque (talk) 11:49, 13 August 2021 (UTC)

Note Almost all of these concerns boil down to "is it okay to spell out good practices for AUR packages here, or is this strictly for repo. My edits (and many before me) have been done with the idea that good practices for AUR package are topical here but should be called out as such and explain why they are slightly different from repo packages (coping with user environments).

Alerque (talk) 11:49, 13 August 2021 (UTC)

A new thought on this topic ... since the real issues here is user env vars leaking into the build, maybe it would be better to recommend explicitly unsetting them instead of setting them back to defaults: e.g. unset CARGO_TARGET_DIR. I think this would be a better pointer towards the purpose of setting this in the first place.

Alerque (talk) 13:02, 13 August 2021 (UTC)

The usage of export explicitly overrides the variable for the session of the shell. An in-line environment variable override temporarily sets that variable for the scope of the command preceding it. They are two completely separate behaviours, setting/unsetting variables with export/unset gets messy fast.

Grawlinson (talk) 01:22, 14 August 2021 (UTC)

So? That's excatly what we want, if we could trust the functions would all be run we could just do it once, but since there are ways to skip some steps we need to export in each function to be safe. We want these to last for the duration of the build session. If this is such an anti-pattern maybe first work on fixing it for all the perl and other language prototype PKGBUILDs that come packaged in pacman. I don't think it's messy at all when the result is exactly the behaviour you want in the first place.

Alerque (talk) 20:41, 14 August 2021 (UTC)

Follow up idea: How about recommending local FOO instead of export FOO? This will prevent the scope leakage you were concerned about while preserving the attributes I was worried about.

Alerque (talk) 12:56, 18 August 2021 (UTC)

To ensure builds are reproducible, either rust or rustup should be specified as a build dependency instead of cargo. Rustup should *only* be used if upstream has explicitly stated this, *and* provides a toolchain file with an exact version.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

This isn't the way it works. rustup should never be specified as a dependency. Users may choose to meet the depends clause with it because it provides both cargo and cargo-nightly, but PKGBUILDs should always depend on either cargo or cargo-nightly and let those resolve te either the direct packages (as will always be the case for repository packages) or whatever the user has that provides them (as will sometimes be different for AUR builds). There is not point in depending an rust instead of cargo as far as reproducibility goes as they are currently provided by the same thing, and given that the actual tool being used by the build is cargo that is the most future-prof thing to require for now.

Alerque (talk) 11:49, 13 August 2021 (UTC)

vaultwarden uses rustup as a dependency because that is what's required, so there is a point in using rust/rustup for explicit package dependencies.

Grawlinson (talk) 01:28, 14 August 2021 (UTC)

As an additional followup on this point, see this IRC conversation with the rust package maintainer: https://paste.rs/NWZ

Alerque (talk) 09:51, 13 September 2021 (UTC)

You have completely misrepresented my point regarding reproducibility. Reproducibility is questionable when rustup is used along with the version environment variable, as there is no guarantee that rustup will pull the same version of cargo on later rebuilds.

It doesn't matter, these are simply guidelines and any packages that I maintain will be using rust as a make dependency because I would rather be explicit when there are multiple providers of cargo.

Grawlinson (talk) 06:13, 14 September 2021 (UTC)

The usage of export is completely unnecessary when only one command will pick up the environment variables, or when CLI flags/options are available.

Grawlinson (talk) 07:38, 13 August 2021 (UTC)

Again this is not quite true. First, the argument variants are not equivalent. They do cause slightly different things to happen. Also there is an order of precedence issue. Because user environment variables are what we are countering, fixing them as environment variables introduces the least surface area for possible precedence confusion. Beyond that it is mostly a style thing. It is a lot easier to compare/diff/grep across different packages if these interventions are on their own line. One way is easier to transform AUR needs into repo needs and vise versa.

Alerque (talk) 11:49, 13 August 2021 (UTC)

Noone actually mentioned that the target also can be overridden by user. And build process will be messed up.
We probably want to specify --target for every command.
Or maybe using the env variable CARGO_BUILD_TARGET="${CARCH}-unknown-linux-gnu" will be more clean.

Also, why we specify env variables in every function, instead of doing it globally for the entire PKGBUILD? I mean, export leaks them outside of functions anyway.

Hanabishi (talk) 17:29, 14 November 2023 (UTC)

Export leaks them outside of functions only if the functions are *run*, not if the file is just sourced. See top level Arch packaging guidelines, I believe I've seen this addressed elsewhere. Alerque (talk) 07:46, 19 February 2024 (UTC)

And that's exactly why this is a problem. If you put them only into prepare() for example, it will break by running makepkg with --noextract/--noprepare.

Duplicating exports into every function? But what's the point?

Arch package guidelines#Package etiquette discourages it because "these could possibly conflict with variables and functions used in makepkg itself". But I doubt that rust-specific-long-named variables have a possibility to ever conflict with anything.

Hanabishi (talk) 10:14, 19 February 2024 (UTC)

export or not ?

if a project wants rustflags, should they be exported as well, and why? i saw you do export CARGO_TARGET_DIR. --Soloturn (talk) 17:01, 15 February 2023 (UTC)

Users can configure RUSTFLAGS via makepkg.conf(5). It is not good to override user flags.

CARGO_TARGET_DIR=target is exported for a reason. This is default value for cargo and most PKGBUILDs do something like

install "target/executable_name" -t "${pkgdir}/usr/bin"

But it obviously will break if user set custom CARGO_TARGET_DIR location. So it is a safety measure.

Hanabishi (talk) 18:21, 15 February 2023 (UTC)

Why not set RUSTFLAGS to RUSTFLAGS="${RUSTFLAGS:---remap-path-prefix=\"$srcdir\"=src}" in PKGBUILD for instance?

Therefore the user's RUSTFLAGS would be taken into account while still setting a default.

HarmfulBreeze (talk) 14:23, 28 March 2023 (UTC)

mold linker

can we use the mold linker by default, or this would be bad practice? --Soloturn (talk) 17:01, 15 February 2023 (UTC)

Again, this requires overriding of user flags, which is not good. Users should be able use whatever linker they want.

And mold itself does not provide any quality improvements, it is more about performance tips. Also considering that it is not Rust exclusive, it is better to be adviced in e.g. Makepkg#Improving compile times imo.

Hanabishi (talk) 18:44, 15 February 2023 (UTC)

RUSTFLAGS link-time optimization

Currently enabling lto in PKGBUILD does not enable link-time optimization in Rust i.e. RUSTFLAGS+=' -C lto=true '

Is this intended behaviour? If true, should this be changed upstream in makepkg?

--ENV25 (talk) 21:40, 4 September 2023 (UTC)

Actually, reading https://doc.rust-lang.org/cargo/reference/profiles.html#lto, perhaps it should be RUSTFLAGS+=' -C linker-plugin-lto=true '.

https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-plugin-lto

ENV25 (talk) 21:45, 4 September 2023 (UTC)

Yes, makepkg currently does not handle Rust LTO. [1]

linker-plugin-lto will not work out of the box because Arch Linux defaults to GCC. For it to work clang and some additional setup is required.

Hanabishi (talk) 12:05, 5 September 2023 (UTC)

I actually were wrong. Rust does work with GCC ld lto. In fact Cargo applies linker-plugin-lto automatically in certain cases.

There is also a bug related to that. Hanabishi (talk) 13:31, 21 February 2024 (UTC)

I independently discovered this issue and created a pacman bug for it. Adding it here in case someone else *does* discover this discussion first before filing another bug in the future. --VorpalGun (talk) 21:35, 3 January 2024 (UTC)

cargo dependency checks - audit, deny, crev, auditable

It seems like it would be desirable to suggest or even require a library check for rust dependencies. Perhaps a minimum is to run cargo audit the step before cargo build to check for known security issues. Even suggesting to use cargo auditable such that the binaries themselves can be checked on a later date for security issues. Arch-audit failed the cargo audit test as an example.

i.e.

cargo audit

cargo auditable build --frozen --release --all-features

--Skunark (talk) 22:27, 2 April 2024 (UTC)