Add vtargetrun (run executables with qemu) and vcompletion (install helper for shell completions); use them in rustup and starship #22785

ericonr · 2020-06-11T03:41:51Z

Inspired by #22761 . Pinging @sgn

Not revbumping because I don't think it's urgent. I could see the case for making this procedure part of the cargo build style, configurable with some variable like clap_completions=yes.

If you feel it adds too much complexity for little gain, I can close the PR.

ericonr · 2020-06-11T03:46:24Z

I think arduino-cli might get shell completion in the next release, so it would need a similar procedure if we were to have completion for all archs. It makes the case for whatever procedure is adopted, if any at all, to not be rust specific.

fosslinux · 2020-06-11T06:10:33Z

I think this is fine, and it doesn't add too much complexity but I do think a revbump would be good - its not like rustup is a very heavy package.

NB i am not sgn

jcgruenhage · 2020-06-13T02:01:18Z

So, just to make sure I'm reading this right: This installs rustup as a dependency on cross builds, which might give out of date completions, if the package for the source architecture isn't already available. I think outdated/wrong completions are potentially worse than no completions.

As an alternative approach, we could generate the as an INSTALL shell snippet. This would have the drawback of the completions files not being searchable with xlocate and such, but it would be a solution that works on all architectures and doesn't have potential race conditions.

ericonr · 2020-06-13T02:32:08Z

@jcgruenhage it wouldn't be out of date, because xbps-src will use the version of rustup that it can find in the git repo. If the x86_64 builders have already built it, it will download it, but there's a chance the aarch64 builder (for example) will build it by itself. It won't ever use an out of date version, though.

The issue with a INSTALL snippet is that rustup-init needs to be linked to a file with basename rustup in order to generate completions, which means that the INSTALL script will end up creating temp files and whatnot, which I really hate.

At this point, running the binary with qemu during the build step might make more sense.

jcgruenhage · 2020-06-13T02:42:45Z

Ah right, that makes sense. Thanks for clearing this up. In that case, I agree that this solution is definitely the better way to solve it.

ericonr · 2020-06-13T03:21:32Z

Great! If we were generating outdated completions it would definitely suck :p

sgn · 2020-06-13T06:39:11Z

@ericonr Use qemu-user-static is the better solutions +1 from my side.

ericonr · 2020-06-14T00:56:27Z

Ok, so I added vqemu to the qemu build-helper, and used it directly in the starship template as well. No starship revbump.

ericonr · 2020-06-14T00:59:56Z

In this case, I would be interested in adding an xlint for manual installation of shell completions as well.

fosslinux · 2020-06-14T03:53:56Z

The title of this PR really should be changed, lol.

How would the xlint work? I believe that that needs to go over in https://github.com/leahneukirchen/xtools, too (please do correct me if I am wrong).

ericonr · 2020-06-14T04:33:36Z

@fosslinux tried to do some renaming :p

And yeah, xlint changes go in leah's repo.

ericonr · 2020-06-14T04:43:22Z

I'm getting

=> rustup-1.21.1_4: running post_build ...
error: Unable parse configuration: Provided host 'arm-unknown-linux-musleabihf' couldn't be converted to partial triple

in the armv6l-musl build. There's a chance it's a broken package for that arch. Current rustup in my Pi:

$ rustup-init
error: Pre-checks for host and toolchain failed: Provided host 'arm-unknown-linux-musleabihf' couldn't be converted to partial triple
If you are unsure of suitable values, the 'stable' toolchain is the default.
Valid host triples look something like: arm-unknown-linux-musleabihf

The triple it complains about and the valid example it gives are the same, too, so it's kind of weird.

ericonr · 2020-06-14T04:50:01Z

It is indeed unsupported, they only ship std for the target. https://forge.rust-lang.org/release/platform-support.html

fosslinux · 2020-06-14T22:58:04Z

I don't think that means that rustup should be unavailable for those architectures, it just means that there are no precompiled binaries for it.

ericonr · 2020-06-15T01:09:06Z

Isn't rustup pretty much useless without precombiled binaries? If you don't have cargo and rustc compiled for that architecture, there isn't anything for rustup to download. Also, current rustup doesn't even know that the arm-unknown-linux-musleabihf target exists, see rust-lang/rustup#2376

fosslinux · 2020-06-15T06:14:23Z

Oh, I forgot that rustup-init requires rust precompiled binaries itself.... In that case, it is useless. You can add manual toolchains in rustup but if there's no way to bypass the rustup-init problem... then it sure is useless.

Chocimier

Nice, vcompletion is useful. I like cmd argument that unifies usage with all shells.

Chocimier · 2020-06-15T07:16:44Z

common/environment/setup/install.sh

+		return 1
+	fi
+
+	: "${file:=${cmd}.${shell}}"


Default value is specific for completions generated by these two packages. Currently most of completions are installed from some subdir, so maybe let's make file first argument, like other vinstall based functions.

Default value is specific for completions generated by these two packages. Currently most of completions are installed from some subdir, so maybe let's make file first argument, like other vinstall based functions.

Agree, then we could guess shell from filename, and cmd from pkgname

file="$1" cmd="${2:-$pkgname}" shell="${3:-${file##*.}}"

Should we guess cmd part from file as well?

cmd from pkgname

Sure, but then check for existing target file is needed to prevent overwriting by consecutive calls.

shell from filename
shell="${3:-${file##*.}}"

It would need to be more like *.fish => fish, *bash* => bash, _* => zsh, else error to fit real world, which is too unpredictable and matching nothing too often imo.

I left the order as file shell command, with only command being optional, since it can be taken from pkgname. As @Chocimier said, guessing the shell from the filename will be a bit of a pain for little gain.

common/environment/setup/install.sh

ericonr · 2020-06-15T12:54:23Z

@Chocimier thanks, I will get around to fixing those!

@fosslinux running something like

rustup-init --default-host x86_64-unknown-linux-gnu

can bypass the issue, but I don't think anyone is going around downloading binaries for other archs in an armv6l device. I might just disable completion for that arch instead, for now.

jcgruenhage · 2020-06-15T13:03:09Z

So, which are the platform triplets that void supports that rust doesn't ship cargo and rustc for? AFAICT it's these:

arm-unknown-linux-musleabihf (armv6l-musl)
armv7-unknown-linux-musleabihf (armv7l-musl)
aarch64-unknown-linux-musl (aarch64-musl)

IIRC, shipping cargo and rustc for those platform triplets is just a matter of enabling it in their CI, since we can build cargo and rustc for those ourselves without problems. I can't see how ARM+musl is the same support wise as fuchsia and android on x86 (as in, you're only expected to compile for it, not on it).

Manual.md

srcpkgs/starship/template

sgn · 2020-06-15T14:37:50Z

common/environment/setup/install.sh

+		return 1
+	fi
+
+	: "${file:=${cmd}.${shell}}"


Default value is specific for completions generated by these two packages. Currently most of completions are installed from some subdir, so maybe let's make file first argument, like other vinstall based functions.

Agree, then we could guess shell from filename, and cmd from pkgname

file="$1" cmd="${2:-$pkgname}" shell="${3:-${file##*.}}"

Should we guess cmd part from file as well?

ericonr · 2020-06-16T01:14:47Z

Gotta wait for the llvm update to propagate, I guess.

jcgruenhage · 2020-06-16T08:33:45Z

@ericonr tried aarch64-musl on my pinebook, it still breaks because the target is unavailable. I'd assume the same to be the case for armv7l-musl then.

ericonr · 2020-06-16T15:44:37Z

@jcgruenhage got it. Since it's not completely broken in any of those platforms and can be used to download for another arch, I say we should keep it. If you want to open an issue about it with upstream, we might be able to get them to ship more toolchains.

jcgruenhage · 2020-06-17T14:51:12Z

The thing is, I'm not sure where exactly those issues should be opened. Otherwise, I'd already have done it. Might be worth coordinating with the alpine people though, since they're also doing musl on arm things.

ericonr · 2020-06-17T16:16:39Z

I wouldn't ask to enable Rust for armv6l, that thing is slow as hell. For armv7l and aarch64 it makes much more sense. I couldn't find where to ask that, either.

jcgruenhage · 2020-06-17T20:24:36Z

They already have that when running on glibc though, so I don't see why they shouldn't enable it on musl libc too. There'll be that person that wants to compile something on a zerophone or something like that, and they'll be stuck without a compiler from rustup then, which IMO shouldn't be.

ericonr · 2020-06-17T20:46:20Z

Well, fair enough. They can take a rust learning course on their phone c: (they should really use the compiler we ship, though).

Just need to find where to report this.

ericonr · 2020-06-17T21:02:19Z

@jcgruenhage according to folks on ##rust on IRC, a good place might be the rust repo. https://github.com/rust-lang/rust/issues

I don't think this is a blocker for merging this, though.

jcgruenhage · 2020-06-18T05:07:43Z

Def not a blocker just related. I'll contact the rustup maintainer at alpine to get which targets they're missing and open an issue upstream afterwards.

jcgruenhage · 2020-06-18T05:10:33Z

Nevermind, alpine seems to only support rustup on x86.

ericonr · 2020-07-08T19:37:32Z

Rebased to remove conflicts.

jcgruenhage · 2020-07-08T22:17:51Z

@ericonr gonna need another rebase soon because of #23476

ericonr · 2020-07-20T04:45:50Z

@sgn, @Chocimier would you mind doing a final review?

fosslinux · 2020-07-20T05:15:51Z

Manual.md

@@ -963,7 +970,9 @@ additional paths to be searched when linking target binaries to be introspected.
 - `qemu` sets additional variables for the `cmake` and `meson` build styles to allow
 executing cross-compiled binaries inside qemu.
 It sets `CMAKE_CROSSCOMPILING_EMULATOR` for cmake and `exe_wrapper` for meson
-to `qemu-<target_arch>-static` and `QEMU_LD_PREFIX` to `XBPS_CROSS_BASE`
+to `qemu-<target_arch>-static` and `QEMU_LD_PREFIX` to `XBPS_CROSS_BASE`.
+It also creates the `vtargetrun` function to call the target architecture


Nitpicks: the target architecture -> the target architecture's?

If we're going to reword, "[...] function to call the qemu-<arch>-static executable for the target architecture." Possessive inanimate objects are weird.

Or maybe better, "[...] function to wrap commands in a call to qemu-<arch>-static for the the target architecture."

fosslinux · 2020-07-20T05:21:52Z

srcpkgs/rustup/template

@@ -13,19 +14,25 @@ homepage="https://www.rustup.rs"
 distfiles="https://github.com/rust-lang/${pkgname}/archive/${version}.tar.gz"
 checksum=ad46cc624f318a9493aa62fc9612a450564fe20ba93c689e0ad856bff3c64c5b

+post_build() {
+	# rustup errors out because it doesn't know about armv6l-musl
+	if [ "$XBPS_TARGET_MACHINE" != armv6l-musl ]; then


I think there are a bunch more than just armv6l-musl. Going off https://forge.rust-lang.org/release/platform-support.html:

armv5tel

armv5tel-musl

armv7l-musl (??)

aarch64-musl (??)

i686-musl

mips* (we don't have glibc variants for mips)

So, the problem here is that rustup doesn't even know that armv6l-musl exists, so it errors out with a rather cryptic error. For the others, we might not have upstream binaries, but rustup knows about them and can work normally. I can try to build it for mips / armv5 to check those too.

I wouldn't go too crazy here. If you try building for archs that don't have binary packages, you are going to go crazy building a bunch of stuff just to get to the point where you can test your package. The main idea should be to prevent issues with trying to build official binary packages. If somebody wants to try building rustup for mips, let the user with the problem get involved with resolving any problems that shake out.

Fair enough, both good points

srcpkgs/starship/template

Install function for installing shell completions in the appropriate place according to the shell used.

It's a function to call the qemu executable for the target arch, or a noop if the build isn't a cross build.

Use qemu build_helper and vcompletion. Completion generation is broken for armv6l-musl due to rustup not knowing about the architecture.

Use qemu build_helper and vcompletion.

ericonr · 2020-07-28T02:51:42Z

Ping? I am waiting on this one in order to update arduino-cli.

ericonr force-pushed the rustup branch from 39731b3 to 09c0a71 Compare June 11, 2020 04:26

sgn mentioned this pull request Jun 13, 2020

starship: update to 0.42.0, add completions. #22761

Closed

ericonr force-pushed the rustup branch from 09c0a71 to e1ea20c Compare June 14, 2020 00:54

ericonr changed the title ~~[RFC] rustup: add completions for all archs.~~ [RFC] rustup: add completions for all archs + simplify completion generation and installation Jun 14, 2020

ericonr force-pushed the rustup branch 2 times, most recently from cb54f1e to 48b88df Compare June 14, 2020 02:26

Chocimier reviewed Jun 15, 2020

View reviewed changes

sgn requested changes Jun 15, 2020

View reviewed changes

ericonr force-pushed the rustup branch from 48b88df to 80d5314 Compare June 16, 2020 00:57

ericonr force-pushed the rustup branch from 30f871e to 3450188 Compare June 16, 2020 03:17

ericonr force-pushed the rustup branch from 3450188 to 2098b7c Compare July 8, 2020 19:37

ericonr force-pushed the rustup branch 2 times, most recently from cd05974 to 4663597 Compare July 20, 2020 04:04

ahesford approved these changes Jul 20, 2020

View reviewed changes

fosslinux suggested changes Jul 20, 2020

View reviewed changes

ericonr force-pushed the rustup branch from 4663597 to 2233e7e Compare July 20, 2020 18:51

ahesford reviewed Jul 20, 2020

View reviewed changes

srcpkgs/starship/template Show resolved Hide resolved

ericonr added 4 commits July 27, 2020 23:43

common/environment/setup/install.sh: add vcompletion.

bd3f8f5

Install function for installing shell completions in the appropriate place according to the shell used.

common/build-helper/qemu.sh: add vtargetrun function.

727d913

It's a function to call the qemu executable for the target arch, or a noop if the build isn't a cross build.

rustup: add completions for all archs.

dd6cdeb

Use qemu build_helper and vcompletion. Completion generation is broken for armv6l-musl due to rustup not knowing about the architecture.

starship: simplify completion generation.

18ab51f

Use qemu build_helper and vcompletion.

ericonr force-pushed the rustup branch from 2233e7e to 18ab51f Compare July 28, 2020 02:43

sgn merged commit 45d572a into void-linux:master Jul 28, 2020

ericonr deleted the rustup branch August 17, 2020 02:31

github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021

Add vtargetrun (run executables with qemu) and vcompletion (install helper for shell completions); use them in rustup and starship #22785

Add vtargetrun (run executables with qemu) and vcompletion (install helper for shell completions); use them in rustup and starship #22785

Conversation

ericonr commented Jun 11, 2020

ericonr commented Jun 11, 2020

fosslinux commented Jun 11, 2020

jcgruenhage commented Jun 13, 2020

ericonr commented Jun 13, 2020

jcgruenhage commented Jun 13, 2020

ericonr commented Jun 13, 2020

sgn commented Jun 13, 2020

ericonr commented Jun 14, 2020 • edited Loading

ericonr commented Jun 14, 2020

fosslinux commented Jun 14, 2020

ericonr commented Jun 14, 2020

ericonr commented Jun 14, 2020

ericonr commented Jun 14, 2020

fosslinux commented Jun 14, 2020 • edited Loading

ericonr commented Jun 15, 2020

fosslinux commented Jun 15, 2020

Chocimier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgn Jun 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericonr commented Jun 15, 2020

jcgruenhage commented Jun 15, 2020 • edited Loading

sgn Jun 15, 2020 • edited Loading

Choose a reason for hiding this comment

ericonr commented Jun 16, 2020

jcgruenhage commented Jun 16, 2020

ericonr commented Jun 16, 2020

jcgruenhage commented Jun 17, 2020

ericonr commented Jun 17, 2020

jcgruenhage commented Jun 17, 2020

ericonr commented Jun 17, 2020

ericonr commented Jun 17, 2020

jcgruenhage commented Jun 18, 2020

jcgruenhage commented Jun 18, 2020

ericonr commented Jul 8, 2020

jcgruenhage commented Jul 8, 2020

ericonr commented Jul 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericonr commented Jul 28, 2020

ericonr commented Jun 14, 2020 •

edited

Loading

fosslinux commented Jun 14, 2020 •

edited

Loading

sgn Jun 15, 2020 •

edited

Loading

jcgruenhage commented Jun 15, 2020 •

edited

Loading

sgn Jun 15, 2020 •

edited

Loading