chore: Makefile tweaks #841
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
oracle-contributor-agreement
bot
added
the
OCA Required
tinvaan
changed the title
| # install SLSA verifier binary, download mvnw, and gradlew. | ||
| .PHONY: setup | ||
| setup: force-upgrade setup-go setup-binaries setup-schemastore | ||
| ifeq ($(PRE_COMMIT_HOOKS),false) |
There was a problem hiding this comment.
What's the reason to make pre-commit optional? Our development environment requires pre-commit hooks.
There was a problem hiding this comment.
This is purely for dev convenience & is also related to the 3rd party(Souffle) build break.
make setup triggers the pre-commit hooks and because tests fail (as expected), build/setup step is blocked as a result.
| souffle: | ||
| if ! command -v souffle; then \ | ||
| ifeq ($(INSTALL_SOUFFLE),false) | ||
| $(warning "Skipping Souffle installation. Set INSTALL_SOUFFLE=true to install.") |
There was a problem hiding this comment.
I assume you are trying to optionally disable the Souffle installation, but the implementation below should have handled that. Did you get this error on your macOS?
There was a problem hiding this comment.
No, the brew command failed for me instead. AFAIK, command -v <tool> only checks if <tool> exists in the system path. My issue is that Souffle's homebrew package's M* OSX build is broken.
| @if [ -f .venv/upgraded-on ]; then \ | ||
| rm -f .venv/upgraded-on; \ | ||
| else \ | ||
| rm -f ${VIRTUAL_ENV}/upgraded-on; \ |
There was a problem hiding this comment.
Can you please provide more details? What was the original issue?
There was a problem hiding this comment.
I'll add more details in the PR description but TL;DR - if a user is using a Python version manager such as pyenv to manage virtual envs, the make script fails because it's looking for a hardcoded file that's probably specific to venv.
pyenv for instance sets the VIRTUAL_ENV variable so appending the upgraded-on file to it's path fixes the breakage.
$ echo $VIRTUAL_ENV
... <nothing here> ...
$ pyenv activate oracle@macaron
... <nothing here> ...
(oracle@macaron) $ echo $VIRTUAL_ENV
/Users/harish/.pyenv/versions/3.11.9/envs/oracle@macaron
|
@behnazh-w I triggered a CI build on my fork which looks green. Let me know if there are any failures here. |
|
Weirdly, I needed to explicitly set-up |
|
Closing this PR as the development checks can be skipped using |
pyenv)souffleinstallation (soufflebuilds for OSX are broken: https://github.com/souffle-lang/homebrew-souffle/issues)