Skip to content
This repository has been archived by the owner on Jun 16, 2024. It is now read-only.

More ways of obtaining gitlab-runner for container #15

Merged
merged 1 commit into from
Sep 3, 2021

Conversation

TpmKranz
Copy link
Contributor

@TpmKranz TpmKranz commented Sep 2, 2021

I'm using gitlab-runner in user mode and can't write to /usr{/local,}/bin, so my binary is stored in ~/bin, which prepare.sh doesn't care to look at.

I've generalized the logic to look at the shell's PATH locations as well and fall back to downloading if that's unsuccessful (about the same as it was before 4364e41).

The uname -m bit needs verification by someone who's more knowledgable than me about the architecture names, but x86_64 has already been verified to work.

Copy link
Owner

@jonasbb jonasbb left a comment

Choose a reason for hiding this comment

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

Hi, thanks, this looks great. I like the tiered approach to look at multiple places.

I only have x86_64 machines, so cannot test the uname code, but somebody can open a PR if it is wrong.

prepare.sh Outdated
if [ ! -x "${RUNNER_BINARY}" ]
then
# ... to temporary directory
RUNNER_BINARY_TMP=$(mktemp --directory --tmpdir="${CACHE_DIR}")
Copy link
Owner

Choose a reason for hiding this comment

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

Does this need quoting around the subshell? You have it on line 113 but not here.

Choose a reason for hiding this comment

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

One reason to use shellcheck as part of the CI

https://github.com/koalaman/shellcheck

Copy link
Owner

Choose a reason for hiding this comment

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

@JohnVillalovos Thanks for the tip. I am aware of shellcheck and use it already. shellcheck didn't report any errors, but I am not sure if that is a false negative or if quoting here really has no effect.

# shellcheck source=base.sh

I look into adding a couple of automatic checks.

Choose a reason for hiding this comment

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

I look into adding a couple of automatic checks.

Almost always a good idea 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are not needed in bash, but since I've been trying to make a habit of quoting everything in shell scripts, it's likely I've either plain forgotten or was thrown by the quotes inside the subshell.
Anyway, the quotes should've been there, I've tested that they don't interfere with the inner quotes and I'll force-push them right away.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for checking :)

@jonasbb jonasbb merged commit 2754017 into jonasbb:master Sep 3, 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

3 participants