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

Fix scripted tests on Mac: use portable sed -i syntax #181

Conversation

rtyley
Copy link
Contributor

@rtyley rtyley commented Oct 17, 2023

On Mac OS (which is BSD-based), the following sbt scripted tests would fail:

  • sbt-version-policy/example-sbt-ci-release
  • sbt-version-policy/example-sbt-dynver

...with an error message like this:

sed: 1: "build.sbt": undefined label 'uild.sbt'
sbt.internal.scripted.TestException: {line 8}  Command failed

The failing test line would be like this:

$ exec sed -i 's/versionPolicyIntention := .*/versionPolicyIntention := Compatibility.None/' build.sbt

Unfortunately there's variation between the ways the 'optional' backup-file-extension argument for sed's change-in-place flag (-i) is handled on Linux (GNU) vs Mac (BSD). GNU sed allows you to completely omit the backup-file-extension argument, as in the sed line above - so GitHub CI, running with Linux, passes fine!

However, with BSD sed, you have to provide some argument to the -i flag, even if it's just '' to indicate you want no backup at all. Consequently, on Mac the above line was interpreted as specifying a backup-file-extension of s/versionPolicyIntention .../ (obviously very wrong), and the parser then treated build.sbt as the transformation program for sed - starting with b, which is sed's 'branching' command, and then uild.sbt as the label to branch to, thus giving the undefined label error - it's a mess.

Fix

The simplest way to get a portable sed command that works on both Mac OS and Linux is to just supply a backup-file-extension argument (eg .bak), and then gitignore the resulting build.sbt.bak files.

https://www.thegeekstuff.com/2009/12/unix-sed-tutorial-6-examples-for-sed-branching-operation/ https://unix.stackexchange.com/a/92907/46453
https://stackoverflow.com/a/22084103/438886

With this change, the sbt scripted suite passes on Mac OS.

On Mac OS (which is BSD-based), the following sbt `scripted` tests
would fail:

* sbt-version-policy/example-sbt-ci-release
* sbt-version-policy/example-sbt-dynver

...with an error message like this:

```
sed: 1: "build.sbt": undefined label 'uild.sbt'
sbt.internal.scripted.TestException: {line 8}  Command failed
```

The failing line would be like this:

https://github.com/scalacenter/sbt-version-policy/blob/b7fc360d8b55a78e018a45d970a959ffee4d357b/sbt-version-policy/src/sbt-test/sbt-version-policy/example-sbt-ci-release/test#L8C1-L8C103
```
$ exec sed -i 's/versionPolicyIntention := .*/versionPolicyIntention := Compatibility.None/' build.sbt
```

Unfortunately there's variation between the ways the 'optional'
backup-file-extension argument for `sed`'s change-in-place flag (`-i`) is
handled on Linux (GNU) vs Mac (BSD). GNU `sed` allows you to completely
omit the backup-file-extension argument, as in the `sed` line above - so
GitHub CI, running with Linux, passes fine!

However, with BSD `sed`, you have to provide _some_ argument to the `-i`
flag, even if it's just `''` to indicate you want no backup at all.
Consequently, on Mac the above line was interpreted as specifying a
backup-file-extension of `s/versionPolicyIntention .../` (obviously very
wrong), and the parser then treated `build.sbt` as the transformation
program for `sed` - starting with `b`, which is `sed`'s 'branching' command,
and then `uild.sbt` as the label to branch to, thus giving the `undefined label`
error - it's a mess.

The simplest way to get a portable `sed` command that works on both Mac OS and
Linux is to just supply a backup-file-extension argument (eg `.bak`), and then
gitignore the resulting `build.sbt.bak` files.

https://www.thegeekstuff.com/2009/12/unix-sed-tutorial-6-examples-for-sed-branching-operation/
https://unix.stackexchange.com/a/92907/46453
https://stackoverflow.com/a/22084103/438886

With this change, the `sbt scripted` suite passes on Mac OS.
@rtyley rtyley changed the title Use portable sed inline syntax for Mac OS & Linux Fix scripted tests on Mac: use portable sed -i syntax Oct 17, 2023
@rtyley rtyley marked this pull request as ready for review October 17, 2023 14:07
Copy link
Collaborator

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

Thank you @rtyley for the investigation and the fix!

@sjrd sjrd merged commit 322da0c into scalacenter:main Oct 17, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants