build-gnu.sh: fix for /usr/bin/timeout on MacOS #5194
Conversation
util/build-gnu.sh
Outdated
| @@ -142,6 +151,9 @@ sed -i -e '/tests\/misc\/seq-precision.sh/ D' \ | |||
| sed -i '/INT_OFLOW/ D' tests/misc/printf.sh | |||
|
|
|||
| # Use the system coreutils where the test fails due to error in a util that is not the one being tested | |||
| # TODO : tests/tail-2/ does not appear to exist | |||
| # and have been moved to just tests/tail/ location | |||
| # Might need to update the section bvelow to reflect that | |||
There was a problem hiding this comment.
| # Might need to update the section bvelow to reflect that | |
| # Might need to update the section below to reflect that |
|
Does it maybe make sense to do the check one time to figure out the timeout command and reuse that? It's too bad that homebrew needs to be installed. Maybe another possibility is that we could make a cross-platform Python script like this to run on any platform: import subprocess
import sys
let res = subprocess.run(sys.argv[2:], timeout=float(sys.argv[1]))
print("===stdout===")
print(res.stdout)
print("===stderr===")
print(res.stderr)
sys.exit(res.returncode)This would be kind of interesting, because that would be the script to run a test, which we could then also expand. Edit: I'm not necessarily suggesting we do this now, it's more just something we could think about. |
…deleted test files that make script fail
it sure does :) ... refactored to check once and reuse that
On MacOS there is no way around it, homebrew has to be installed, otherwise most of this script (and other scripts and tools) would not work at all. It is not that only brew install coreutils |
Oof that's a long list. Thanks for checking that out. Sadly, I guess there's not much we can do there, because this script is provided by GNU. I guess we could document that list somewhere though. |
util/build-gnu.sh
Outdated
| # TODO: Might need to review and updated sections below | ||
| # TODO: As a result this script will fail when executed normally as 'bash util/build-gnu.hs' due to the 'set -e' set at the beginning | ||
| # TODO: The rest of the 'sed' commands after first failure in this scenario will not be executed as bash will exit on first error | ||
| # TODO: However, the behaviour might be different when running it via GitHub actions (GnuTests) |
There was a problem hiding this comment.
Yes, this has been changed after the last coreutils release. You should be able to fix it by checking out the tag of the latest release.
There was a problem hiding this comment.
interesting ... I have branched of from main that was already at release 0.0.20, admittedly not from the release tag, just HEAD on main:
Cargo.toml
[package]
name = "coreutils"
version = "0.0.20"
has there been a regression since that release (it looks like it is latest)?
There was a problem hiding this comment.
just checked out release by tag 0.0.20 ... the issues are still there starting at line 135 on build-gnu.sh
i.e. the script will stop executing here:
sed -i '/INT_OFLOW/ D' tests/misc/printf.sh
since tests/misc/printf.sh does not exist at main (HEAD) GNU coreutils at 'https://github.com/coreutils/coreutils.git'
what am I missing?
There was a problem hiding this comment.
ok, never mind :) what I am missing is that you were talking about checking out latest release tag of GNU coreutils, not this repo. Will update the PR.
On that note: would it make sense to modify the message to indicate that latest release tag needs to be checkout after cloning GNU coreutils?
`
Could not find GNU coreutils (expected at '/gnu')
Run the following to download into the expected path:
git clone --recurse-submodules https://github.com/coreutils/coreutils.git "/gnu"
`
There was a problem hiding this comment.
Updated PR to target correct GNU coreutlis release
There was a problem hiding this comment.
Yeah I meant GNU. Sorry gor the confusion 😄
There was a problem hiding this comment.
yep, thanks! Added GNU coreutlis release tag to the script
|
Considering #5221 PR is in progress to upgrade to GNU/coreutils v9.4 release - should this PR also target v9.4 ? Also, would it make sense to have targeted GNU/coreutils release value in one place only (i.e. only in |
|
Maybe we should fix the tests for 9.4 first and then rebase this PR? I think it would make sense to separate the two changes, but I don't really mind in which order we merge them. |
|
#5221 has been merged. Could you rebase this PR? |
|
@tertsdiepraam rebased and pointed to v9.4 |
|
@sylvestre thanks for catching that typo. Interestingly the Code Spell Checker extension for VSCode does not catch that by default - it has to be manually enabled for shell scripts. Should it be added to this PR or have a separate small PR for this? |
|
@tertsdiepraam @sylvestre with most recent changes - do you feel like it can be merged or there is more work to be done? |
|
yes, sorry about that :) |
Set to /usr/local/timeout if /usr/bin/timeout is not found.
On MacOS there is no system
/usr/bin/timeoutand trying to add it to /usr/bin (with symlink of copy binary) will fail unless system integrity protection is disabled (not ideal)ref: https://support.apple.com/en-us/102149
On MacOS the Homebrew coreutils could be installed and then
sudo ln -s /opt/homebrew/bin/timeout /usr/local/bin/timeoutfixes #5193