Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

starship: update to 0.42.0, add completions. #22761

Closed
wants to merge 1 commit into from

Conversation

ericonr
Copy link
Member

@ericonr ericonr commented Jun 10, 2020

  • Completions are generated like the ones for rustup.

if ! [ "$CROSS_BUILD" ]; then
# generate shell completions
ln -s target/${RUST_TARGET}/release/starship starship
./starship completions zsh > starship.zsh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can pull starship in hostdepends
for cross build and building completion for cross.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgn this is probably even more useful for something like rustup. It could make sense to add it there as well.

- Completions are generated similar to the ones for rustup.
Comment on lines +21 to +28
STARSHIP="target/${RUST_TARGET}/release/starship"
if [ "$CROSS_BUILD" ]; then
STARSHIP=starship
fi
# generate shell completions
${STARSHIP} completions zsh > starship.zsh
${STARSHIP} completions bash > starship.bash
${STARSHIP} completions fish > starship.fish
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favour of moving those completion generation to build stage.
post_build, perhaps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could split it up too, of course

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of this https://github.com/ericonr/void-packages/blob/vcompletions/common/environment/setup/install.sh#L262 ?

I don't know. A bit too much magic :)

Anyway, most packages ships the completion instead of depending on some generator.

Or is it the new trend, every packages started to ship completion generated from command?

Thinking about this a bit, I wonder why do we need to ship it.
We can generate the completions in post-update hook (INSTALL file),
this will avoid the whole pulling host version,
(some generated completion maybe wrong, let's say something similar to -march of gcc).
Yeah, I probably add a new xbps-trigger type, let's say completion generator, feed it a cmd name.

For you suggested function, (even if I don't like it), If I were you, I might go with:

test -f $cmd.bash && _vinstall ${cmd}.bash 0644 $_bash_completion_dir $cmd
test -f $cmd.fish && _vinstall ${cmd}.fish 0644 $_fish_completion_dir ${cmd}.fish
test -f $cmd.zsh && _vinstall ${cmd}.zsh 0644 $_zsh_completion_dir _${cmd}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm rushing for $dayjobs now, maybe I was plain wrong about above comment :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it the new trend, every packages started to ship completion generated from command?

Might be, since they are using frameworks for the command line stuff, which also generate the completions. I wish they could ship tarballs with pre generated ones, but that would be a bit hard in the current GiitHub release mechanism where you just tag a specific version.

I like the idea of a custom hook, but it has some issues:

  • some packages use completions, others completion, for the subcommand.
  • rustup-init (the binary shipped in rustup), for example, needs to be symlinked to rustup, otherwise it errors out when generating completions (yes, I hate it too).
  • some packages might not support all shells.

I really like your suggestion for my function, though! It would make stuff less magical, but not by much :p . Thanks :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can merge it as is after fixing test failures for now.
We'll take #22785 as discussion for the completion generator :)

@sgn
Copy link
Member

sgn commented Jun 11, 2020

Don't know what wrong with their tests.
A bunch of test failure including seemingly basic one: test directory::root_directory ... FAILED
I'll take a look into it tomorrow.

@ericonr
Copy link
Member Author

ericonr commented Jun 12, 2020

old version:

test result: ok. 175 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

test result: ok. 187 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Lots of failures in the last round: https://0x0.st/iVn7.txt

@sgn
Copy link
Member

sgn commented Jun 13, 2020

I'll push with this tomorrow (or earlier) if you agree.

diff --git a/srcpkgs/starship/template b/srcpkgs/starship/template
index ba484a5003..cc9d0ae620 100644
--- a/srcpkgs/starship/template
+++ b/srcpkgs/starship/template
@@ -6,6 +6,7 @@ build_style=cargo
 build_helper="rust"
 hostmakedepends="pkg-config"
 makedepends="libgit2-devel"
+checkdepends="git"
 short_desc="Minimal, fast and customizable cross-shell prompt"
 maintainer="Aluísio Augusto Silva Gonçalves <aluisio@aasg.name>"
 license="ISC"
@@ -14,18 +15,24 @@ distfiles="https://github.com/starship/starship/archive/v${version}.tar.gz"
 checksum=cf7b5848949bc1e61dc771ef2a429e4ccb8d339e1ca705bdf521fc4a4fc96309
 
 if [ "$CROSS_BUILD" ]; then
-	hostmakedepends+=" starship"
+	hostmakedepends+=" qemu-user-static"
 fi
 
+pre_check() {
+	[ -L target/debug ] && unlink target/debug
+	ln -s release target/debug
+}
+
 post_install() {
 	STARSHIP="target/${RUST_TARGET}/release/starship"
 	if [ "$CROSS_BUILD" ]; then
-		STARSHIP=starship
+		export QEMU_LD_PREFIX=${XBPS_CROSS_BASE}
+		STARSHIP="/usr/bin/qemu-${XBPS_TARGET_QEMU_MACHINE}-static ${STARSHIP}"
 	fi
-	# generate shell completions
-	${STARSHIP} completions zsh > starship.zsh
-	${STARSHIP} completions bash > starship.bash
-	${STARSHIP} completions fish > starship.fish
+
+	${STARSHIP} completions zsh  >starship.zsh
+	${STARSHIP} completions bash >starship.bash
+	${STARSHIP} completions fish >starship.fish
 
 	vinstall starship.zsh 0644 usr/share/zsh/site-functions/ _starship
 	vinstall starship.bash 0644 usr/share/bash-completion/completions/ starship

sgn pushed a commit to sgn/void-packages that referenced this pull request Jun 13, 2020
- Completions are generated similar to the ones for rustup.

Closes: void-linux#22761
@sgn sgn closed this in 701002f Jun 13, 2020
misuchiru03 pushed a commit to misuchiru03/void-packages that referenced this pull request Jun 14, 2020
- Completions are generated similar to the ones for rustup.

Closes: void-linux#22761
Co-Authored-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
@ericonr ericonr deleted the starship branch August 17, 2020 03:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants