Skip to content

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Feb 10, 2025

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)

@camilamacedo86 camilamacedo86 requested a review from a team as a code owner February 10, 2025 10:42
Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 075a8fe
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/67a9dc2bc6732400084c8f3b
😎 Deploy Preview https://deploy-preview-1733--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Feb 10, 2025

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.

@grokspawn
Copy link
Contributor

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)
Copy link
Contributor Author

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.

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor

@perdasilva perdasilva left a comment

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!

@camilamacedo86 camilamacedo86 added this pull request to the merge queue Feb 10, 2025
Merged via the queue into operator-framework:main with commit 1cf5261 Feb 10, 2025
23 checks passed
@camilamacedo86 camilamacedo86 deleted the fix-mac-move-out branch February 10, 2025 16:45
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