-
Notifications
You must be signed in to change notification settings - Fork 656
Fixes for bash scripts and cmake #1145
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
base: master
Are you sure you want to change the base?
Conversation
It's `python` on Windows and `python3` on Linux and macOS fix lightvector#1142 (cherry picked from commit 945d1a5)
fix lightvector#1028 (cherry picked from commit 0d0bc6c)
…pload_model_for_selfplay.sh` To fix "find: -printf: unknown primary or operator" on macOS fix lightvector#1144 (cherry picked from commit a10ff0d)
fix lightvector#1143 (cherry picked from commit 261b427)
532faa4 to
51775cc
Compare
lightvector
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. Left some questions!
| if command -v python3 >/dev/null 2>&1; then | ||
| PYTHON=python3 | ||
| else | ||
| PYTHON=python | ||
| fi | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good way to guard against the case where "python3" doesn't exist and "python" is "python2"? In that case it would probably be best to fail rather than attempt to run python-3 scripts.
| mkdir "$TMPDST" | ||
|
|
||
| TOBEZIPPED="$TMPDST"/"$RUNNAME"-"$NAME" | ||
| TOBEZIPPED="$TMPDST/$RUNNAME-$NAME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason for changing the quoting style on this and subsequent lines? Just a style thing or is there some other reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason, just a style thing (suggested by Junie AI assistance). I can rollback to make the changes minimal.
| #to newer ones as they get generated. | ||
| echo "Cleaning up any old dirs" | ||
| find "$BASEDIR"/shuffleddata/ -mindepth 1 -maxdepth 1 -type d -mmin +120 | sort | head -n -5 | xargs --no-run-if-empty rm -r | ||
| find "$BASEDIR"/shuffleddata/ -mindepth 1 -maxdepth 1 -type d -mmin +120 -print0 | sort -z | head -z -n -5 | xargs -0 --no-run-if-empty rm -r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does "-z" exist in MacOS as a flag for these commands? https://ss64.com/mac/sort.html doesn't list it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed it looks like macOS doesn't support the -z argument. However, I was able to run the script without problems on macOS, it's strange.
It's especially useful for new users that aren't aware a lot about bash and cmake.