-
Notifications
You must be signed in to change notification settings - Fork 1
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
Commit test output #131
Commit test output #131
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
24afa27
to
3166d0c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
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.
Great! This will make tracking changes much easier.
3166d0c
to
0225fbd
Compare
@@ -6,6 +6,10 @@ CHARON ?= $(CHARON_HOME)/bin/charon | |||
BROKEN_TESTS = step_by where_clauses chunks mutable_slice_range closure issue_49 issue_37 issue_105 | |||
TEST_DIRS = $(filter-out $(BROKEN_TESTS),$(subst test/,,$(shell find test -maxdepth 1 -mindepth 1 -type d))) | |||
|
|||
# Enable `foo/**` glob syntax | |||
SHELL := bash -O globstar |
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.
@Nadrieril Son started reporting issues with this:
bash: line 0: globstar: invalid shell option name
bash: line 0: globstar: invalid shell option name
@sonmarcho is possibly on OSX where the default version of make is rather old -- we forbid this in some other projects https://github.com/hacl-star/hacl-star/blob/main/Makefile#L46-L49
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.
hm, that looks like a bash
difference, not a make
difference
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 an OSX difference: https://apple.stackexchange.com/questions/291287/globstar-invalid-shell-option-name-on-macos-even-with-bash-4-x . I guess I should avoid the glob and do a find | xargs
dance? I can't believe I can't even rely on having a decently modern bash though :'(.
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.
sorry I meant bash -- this is the same issue, OSX ships a very outdated bash for GPLv3 reasons
this is the bash version that is bad in OSX:
jonathan@absinthe:~/Code/noise-star/src (main) $ /bin/bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin24)
if you can detect that and error out with a suggestion to brew install bash
, then I'm fine with it (I personally run with brew bash and everyone else should)
jonathan@absinthe:~/Code/noise-star/src (main) $ /bin/bash
bash: complete: nosort: invalid option name
The default interactive shell is now zsh.
To update your account to use zsh, please run `chsh -s /bin/zsh`.
For more details, please visit https://support.apple.com/kb/HT208050.
jonathan@absinthe:~/Code/noise-star/src (main) $ echo $BASH_VERSION
3.2.57(1)-release
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.
Do you know a reliable way to detect that?
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.
since Apple's bash is frozen in time I think it's ok to check for that exact version string
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.
Do you know how to do such a check in a makefile? I'd rather not run it on every test
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.
I would do something like
SHELL=/bin/env bash
_ := $(shell bash -c '(( ${BASH_VERSION%%.*} >= 4 ))')
ifneq ($(.SHELLSTATUS),0)
_: $(error "bash version too old")
endif
(sketch)
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.
probably need to use $$ though to prevent make from attempting variable expansion
As discussed privately, this PR commits the
.c
and.h
files generated for our tests. This will make it easier to spot behavior changes. CI will ensure the committed test output matches what's being generated.