🌱 fix demo-update script to support mac os envs by camilamacedo86 · Pull Request #1733 · operator-framework/operator-controller
Conversation
Description
Currently, the script does not work for mac env.
The change here ensure that it can work for linux and mac
See:
View the recording at:
https://asciinema.org/a/R4bFB4cTQaJSytvePHqLl8pNu
This asciinema CLI hasn't been linked to any asciinema.org account.
Recordings uploaded from unrecognized systems, such as this one, are automatically
deleted 7 days after upload.
If you want to preserve all recordings uploaded from this machine,
authenticate this CLI with your asciinema.org account by opening the following link:
https://asciinema.org/connect/054dd205-c559-4711-80ae-a01034074c79
Before the change made:
$ make demo-update
hack/scripts/generate-asciidemo.sh
hack/scripts/generate-asciidemo.sh: line 40: [: /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/d.EqBWI4NlpY: binary operator expected
unable to find prerequisite: asciinema
hack/scripts/generate-asciidemo.sh: line 17: [: /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/d.EqBWI4NlpY: binary operator expected
make: *** [demo-update] Error 1
Reviewer Checklist
- API Go Documentation
- Tests: Unit Tests (and E2E Tests, if appropriate)
- Comprehensive Commit Messages
- Links to related GitHub Issue(s)
Codecov Report
All modified and coverable lines are covered by tests ✅
Project coverage is 68.24%. Comparing base (
d72e551) to head (075a8fe).
Report is 4 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@ ## main #1733 +/- ## ======================================= Coverage 68.24% 68.24% ======================================= Files 58 58 Lines 4988 4988 ======================================= Hits 3404 3404 Misses 1342 1342 Partials 242 242
| Flag | Coverage Δ | |
|---|---|---|
| e2e | 52.84% <ø> (-0.09%) |
⬇️ |
| unit | 55.45% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.
Hey @camilamacedo86 have you done this setup?
Having done it, it works because /opt/homebrew/opt/coreutils/libexec/gnubin/mktemp is earlier in the PATH than the old mac default.
| set -u | ||
|
|
||
| WKDIR=$(mktemp -td generate-asciidemo.XXXXX) | ||
| WKDIR=$(mktemp -d -t generate-asciidemo.XXXXX) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @grokspawn ^ it is the problem with mac os.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See:
Hey @camilamacedo86 have you done this setup?
See:
$ brew install bash gnu-tar gsed
==> Downloading https://formulae.brew.sh/api/formula.jws.json
==> Downloading https://formulae.brew.sh/api/cask.jws.json
Warning: bash 5.2.37 is already installed and up-to-date.
To reinstall 5.2.37, run:
brew reinstall bash
Warning: gnu-sed 4.9 is already installed and up-to-date.
To reinstall 4.9, run:
brew reinstall gnu-sed
==> Downloading https://ghcr.io/v2/homebrew/core/gnu-tar/manifests/1.35-1
######################################################################### 100.0%
==> Fetching gnu-tar
==> Downloading https://ghcr.io/v2/homebrew/core/gnu-tar/blobs/sha256:83fe22ea67
######################################################################### 100.0%
==> Pouring gnu-tar--1.35.arm64_sonoma.bottle.1.tar.gz
==> Caveats
GNU "tar" has been installed as "gtar".
If you need to use it as "tar", you can add a "gnubin" directory
to your PATH from your bashrc like:
PATH="/opt/homebrew/opt/gnu-tar/libexec/gnubin:$PATH"
==> Summary
🍺 /opt/homebrew/Cellar/gnu-tar/1.35: 17 files, 1.8MB
==> Running `brew cleanup gnu-tar`...
Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
Then:
we will still facing issues with the mktemp -td
$ make demo-update
stat /Users/camilam/go/src/github/operator-framework/operator-controller/catalogd/internal/version: directory not found
hack/scripts/generate-asciidemo.sh
hack/scripts/generate-asciidemo.sh: line 40: [: /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/d.PREDMYraHv: binary operator expected
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 5481 100 5481 0 0 87177 0 --:--:-- --:--:-- --:--:-- 88403
curl: (6) Could not resolve host: generate-asciidemo.UzLDl
chmod: generate-asciidemo.UzLDl/asciinema-rec_script: Not a directory
/var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/d.PREDMYraHv: line 155: generate-asciidemo.UzLDl/asciinema-rec_script: Not a directory
asciinema: /Users/camilam/go/src/github/operator-framework/operator-controller/catalogd/hack/scripts/demo-script.sh already exists, aborting
asciinema: use --overwrite option if you want to overwrite existing recording
asciinema: use --append option if you want to append to existing recording
usage: asciinema [-h] [--version] {rec,play,cat,upload,auth} ...
asciinema: error: unrecognized arguments: generate-asciidemo.UzLDl/catalogd-demo.cast
hack/scripts/generate-asciidemo.sh: line 17: [: /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/d.PREDMYraHv: binary operator expected
make: *** [demo-update] Error 2
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the issue is because the docs don't also brew install coreutils which is where the updated mktemp binary lives.
If we updated the docs, we wouldn't need this PR.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - seems to still work on lunix!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters