Talk:Rust package guidelines

From ArchWiki

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)