It’s a nice idea to explain all these cryptic incantations. One of my pet peeves is mixing languages in an uncontrolled way, and it looks like Github Actions sort of encourages that. Sometimes it leads to code injection bugs, but it’s almost always hard to read, so I just avoid it all the time.
One way to avoid it here would be to start off the script with:
# initialize variables from Github Actions API
github_event_ref=${{ github.event.ref }}
github_token=${{ secrets.GITHUB_TOKEN }}
And then use normal shell substitution for the rest. Mixing in the ${{ }} syntax is too close to shell’s ${} and $() IMO.
This is similar to my recent comment here to avoid concatenation of shell code: https://lobste.rs/s/9yu5sl/after_discussion_here_i_created_lib_for#c_paq9ch (although unfortunately I added a caveat that it’s risky in bash and ksh derivatives, although it is more readable IMO. That use case is for security and this one isn’t though.)
This is not very actionable, but shells actually use temp files to implement here docs, including here strings, but Oil doesn’t. A regex would avoid that, although it may not be more readable. It’s crazy what people make do with in shell :)
Another nitpick: it seems weird to me to use IFS=/ to split by /, and then use awk -F/ to print the last field. It’s 2 different syntaxes and 2 different tools for essentially the same thing.
Both can be done with regexes (which also avoids temp files). Bash supports ERE regex syntax with capturing via [[ $x =~ $pat ]] and the $BASH_REMATCH array.
It’s not the prettiest code, but neither is using IFS or awk. And it has caveats like everything in bash – you should always make the regex a constant string, rather than putting it online, because they botched the quoting (and even the bash manual says this.)
Something like:
regex1='(.*)/(.*)'
if [[ $GITHUB_REPOSITORY =~ $regex1 ]]; then
owner=${BASH_REMATCH[1]}
repository=${BASH_REMATCH[2]}
fi
regex2='([^/]*)$'
if [[ $github_event_ref =~ $regex2 ]]; then
head_ref_name=${BASH_REMATCH[1]}
fi
Notes:
tested minimally
quotes aren’t required within [[ ]] because it’s part of the language. Not required after = either.
This might make a good blog post for rewrites in Eggex
I haven’t suggested to the original shell incantations’ author that they use this approach, but I do think it’s lovely. That said, I do also think that it’s less well known; while I’ve used Bash for a while, I’ve used it in “general shell mode” first, i.e. combining other commands and builtins, rather than dig into any specific Bash features like this. I think mostly because my general shell mode thinking has blinded me to the deeper and more detailed Bash possibilities. I wonder how many others are like me on this.
Thanks, these are great thoughts. In fact, I had the same initial thought when reading the code for the first time, but I guess I came to the conclusion that this was a quick and almost throw-away bit of code, but still worth pondering over. In fact, it did me a favour in a way because I could talk about the awk part as well as IFS. And I sometimes like the way that people write code like they speak, using idioms and glued together bits of wisdom or practices. I know it’s not ideal, but it’s real and has (to me) character. I also sort of left it to the reader to come to the same conclusion you came too as well, and let them decide whether it was weird or not.
To your observation about mixing languages, I know what you mean; I have a related issue in that while I enjoy Bash, writing it in the context of YAML makes it more unwieldy than it should be; my editor can’t give me the syntax highlighting that I like, and so on.
For similar situations in my own workflow definitions, I’ve been pondering organising even the smallest sets of shell incantations like this into small scripts, but then got stuck on where in the repo to put them … and as it’s not a pressing issue, it’s taken a back seat, except for the decision that I don’t want to store anything in .github/ as that “belongs” to GitHub; it makes more sense to me to have a .<companyname>/ or at least some other top level hidden directory.
The other thing to note is that the setting of the value for IFS is done “in the same breath” as the read command, on the same line. This means that the value assigned is temporary, just for the duration of the command or builtin that follows.
Note this does not work in all cases with IFS. Here’s a tricky little puzzle:
/ $ arr=(a b c)
/ $ (IFS=,; echo "${arr[*]}")
a,b,c
/ $ (IFS=, echo "${arr[*]}")
a b c
In the first, correct version, also note the use of the subshell (...) so that our IFS change won’t persist past the single command where we use it.
Thanks, yes - I like the use of the subshell like that; in fact, it was also mentioned in that SO answer that I linked to (which was indeed a very good one in helping me get my head around the whole area).
I enjoy reading posts like this, as I learn so much from so many people. So I wrote this post in that style - I’m convinced that there are folks who want to increase their terminal knowledge but are held back by the seemingly arcane incantations. I do think that there may be a gap forming in the programming/admin population where shell knowledge is being lost or at least not renewed or passed on. I’d love to know what you think on this, too.
About halfway down the page, when you’re explaining the difference between $(...) and `...`, the markdown engine lets you down and elides the backticks.
Noooo! Nice spot - I’d been distracted by a related issue (liquid variables swallowing the {{ … }} bits - and forgot about checking the backtick escapes. Fixed, with thanks.
It’s a nice idea to explain all these cryptic incantations. One of my pet peeves is mixing languages in an uncontrolled way, and it looks like Github Actions sort of encourages that. Sometimes it leads to code injection bugs, but it’s almost always hard to read, so I just avoid it all the time.
One way to avoid it here would be to start off the script with:
And then use normal shell substitution for the rest. Mixing in the
${{ }}syntax is too close to shell’s${}and$()IMO.This is similar to my recent comment here to avoid concatenation of shell code: https://lobste.rs/s/9yu5sl/after_discussion_here_i_created_lib_for#c_paq9ch (although unfortunately I added a caveat that it’s risky in bash and ksh derivatives, although it is more readable IMO. That use case is for security and this one isn’t though.)
This is not very actionable, but shells actually use temp files to implement here docs, including here strings, but Oil doesn’t. A regex would avoid that, although it may not be more readable. It’s crazy what people make do with in shell :)
Another nitpick: it seems weird to me to use
IFS=/to split by /, and then useawk -F/to print the last field. It’s 2 different syntaxes and 2 different tools for essentially the same thing.Both can be done with regexes (which also avoids temp files). Bash supports ERE regex syntax with capturing via
[[ $x =~ $pat ]]and the$BASH_REMATCHarray.It’s not the prettiest code, but neither is using IFS or awk. And it has caveats like everything in bash – you should always make the regex a constant string, rather than putting it online, because they botched the quoting (and even the bash manual says this.)
Something like:
Notes:
=either.This might make a good blog post for rewrites in Eggex
Could also get these values with bash parameter expansion
I haven’t suggested to the original shell incantations’ author that they use this approach, but I do think it’s lovely. That said, I do also think that it’s less well known; while I’ve used Bash for a while, I’ve used it in “general shell mode” first, i.e. combining other commands and builtins, rather than dig into any specific Bash features like this. I think mostly because my general shell mode thinking has blinded me to the deeper and more detailed Bash possibilities. I wonder how many others are like me on this.
Might as well do the eggex rewrite while I’m waiting for a job to finish :)
I think that is a lot cleaner than both bash and the
read -randawk -F/combo! Eggex also has named captures but they’re not fully implemented yet.You can also break it up for readability:
(I like this version better)
Thanks, these are great thoughts. In fact, I had the same initial thought when reading the code for the first time, but I guess I came to the conclusion that this was a quick and almost throw-away bit of code, but still worth pondering over. In fact, it did me a favour in a way because I could talk about the
awkpart as well asIFS. And I sometimes like the way that people write code like they speak, using idioms and glued together bits of wisdom or practices. I know it’s not ideal, but it’s real and has (to me) character. I also sort of left it to the reader to come to the same conclusion you came too as well, and let them decide whether it was weird or not.To your observation about mixing languages, I know what you mean; I have a related issue in that while I enjoy Bash, writing it in the context of YAML makes it more unwieldy than it should be; my editor can’t give me the syntax highlighting that I like, and so on.
For similar situations in my own workflow definitions, I’ve been pondering organising even the smallest sets of shell incantations like this into small scripts, but then got stuck on where in the repo to put them … and as it’s not a pressing issue, it’s taken a back seat, except for the decision that I don’t want to store anything in
.github/as that “belongs” to GitHub; it makes more sense to me to have a.<companyname>/or at least some other top level hidden directory.Note this does not work in all cases with
IFS. Here’s a tricky little puzzle:In the first, correct version, also note the use of the subshell
(...)so that ourIFSchange won’t persist past the single command where we use it.Thanks, yes - I like the use of the subshell like that; in fact, it was also mentioned in that SO answer that I linked to (which was indeed a very good one in helping me get my head around the whole area).
I enjoy reading posts like this, as I learn so much from so many people. So I wrote this post in that style - I’m convinced that there are folks who want to increase their terminal knowledge but are held back by the seemingly arcane incantations. I do think that there may be a gap forming in the programming/admin population where shell knowledge is being lost or at least not renewed or passed on. I’d love to know what you think on this, too.
From #lobsters (relaying because I think it’s true):
Thank you wink and iDon - fixed.
About halfway down the page, when you’re explaining the difference between
$(...)and`...`, the markdown engine lets you down and elides the backticks.Noooo! Nice spot - I’d been distracted by a related issue (liquid variables swallowing the {{ … }} bits - and forgot about checking the backtick escapes. Fixed, with thanks.