-
Notifications
You must be signed in to change notification settings - Fork 208
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
Do not export env variables, just assign to them. #2044
Conversation
The commit message made me first think all the variables are environment variables, but only CXXFLAGS is here. To distinguish the two you could make the non-constant/environment variables lowercase, but that's a cosmetic for another change. |
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 the cleanup. I agree that the commit message could have a summary of the changes below the subject line.
tests/oss-fuzz/build.sh
Outdated
if [ "$SANITIZER" != "coverage" ] && [ "$SANITIZER" != "introspector" ] | ||
then | ||
export DAV1D_EXTRA_FLAGS="-Db_sanitize=$SANITIZER -Db_lundef=false" | ||
DAV1D_EXTRA_FLAGS="-Db_sanitize=$SANITIZER -Db_lundef=false" |
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.
Optional: Since DAV1D_EXTRA_FLAGS
has been initialized an empty string, we can append to it here.
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.
Done
tests/oss-fuzz/build.sh
Outdated
export CXXFLAGS="${CXXFLAGS} -DFUZZTEST_COMPATIBILITY_MODE" | ||
export EXTRA_CMAKE_FLAGS="-DAVIF_ENABLE_FUZZTEST=ON -DFUZZTEST_COMPATIBILITY_MODE=libfuzzer" | ||
CXXFLAGS="${CXXFLAGS} -DFUZZTEST_COMPATIBILITY_MODE" | ||
EXTRA_CMAKE_FLAGS="-DAVIF_ENABLE_FUZZTEST=ON -DFUZZTEST_COMPATIBILITY_MODE=libfuzzer" |
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.
Optional: We can also initialize EXTRA_CMAKE_FLAGS
to an empty string before the if statement, and append to it here. This will allow us to get rid of the else branch.
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.
Done
Have DAV1D_EXTRA_FLAGS and EXTRA_CMAKE_FLAGS be normal non-environment variables.
BUG=oss-fuzz:67122