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

[check] Fixes to scripts to run on OSX #4595

Closed
wants to merge 1 commit into from

Conversation

burblebee
Copy link
Contributor

Patches include:

WIP - more fixes to come...

@tkoeppe
Copy link
Contributor

tkoeppe commented Apr 30, 2021

Note to self: extend the precommit scripts to run on Mac.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 7, 2021

@burblebee: that change alone isn't enough to get the script to run on MacOS. I set up a MacOS presubmit on the beta branch, see here: https://github.com/cplusplus/draft/runs/2766096602

If you'd like to experiment further, feel free to just push more changes into that branch. When it all works, we can apply the full change to master.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 30, 2021

I added the missing semicolon in awk. The other semicolons don't seem to be necessary.

That said, the bigger problem is that BSD-sed doesn't appear to allow nested commands, e.g. see https://unix.stackexchange.com/questions/13711/differences-between-sed-on-mac-osx-and-other-standard-sed. That breaks

sed -n '/^\\pnum/{h;:x;n;/^\\index/b x;/^\\\(constraints\|mandates\|expects\|effects\|sync\|ensures\|returns\|throws\|complexity\|remarks\|errors\)/{x;/\n/{x;=;p;};d;};/^\\pnum/D;H;b x;}' $f |
. I've not managed to rewrite that.

For the time being, we can install gnu-sed on MacOS (brew install gnu-sed); that's at least good enough for the GitHub unit tests.

I'll close this for now. Please reopen if you have found a way to rewrite the sed command.

@tkoeppe tkoeppe closed this Jun 30, 2021
@burblebee
Copy link
Contributor Author

That said, the bigger problem is that BSD-sed doesn't appear to allow nested commands

@tkoeppe Yup, that's what I got stuck on. Let's just document in the scripts and on github that gnu-sed is required. Thanks for looking into this!

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.

None yet

2 participants