feat: improve dependency installation experience #1298
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #1298 +/- ##
=======================================
Coverage 97.88% 97.88%
=======================================
Files 89 89
Lines 35473 35473
=======================================
Hits 34724 34724
Misses 749 749 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Benchmark Results for unmodified programs 🚀
|
MegaRedHand
force-pushed
the
better-installation-experience
branch
from
f58c378
to
3a52a42
Compare
MegaRedHand
requested review from
igaray,
Oppen,
fmoletta,
entropidelic,
juanbono,
pefontana and
Juan-M-V
as code owners
Just having PyEnv and make installed suffices for running `make deps`. I don't think we need to list cairo-lang as a dependency.
Oppen
approved these changes
Jun 30, 2023
|
A few questions:
|
It seems the install-python action doesn't support Debian containers anyways 😞 |
Oppen
approved these changes
Jul 4, 2023
pefontana
requested changes
Jul 4, 2023
| @@ -26,4 +26,3 @@ source ~/cairo_venv/bin/activate | |||
|
|
|||
| # Install cairo & its dependencies | |||
| pip3 install -r requirements.txt | |||
|
|
|||
There was a problem hiding this comment.
I think we should install pyenv install 3.9.15
There was a problem hiding this comment.
Also, this isn't duplicated? Since then install_macos runs make deps-macos
But we can leave it duplicate because make deps-macos won't fail.
What we need to check is to add the CFLAGS=-I/opt/homebrew/opt/gmp/include LDFLAGS=-L/opt/homebrew/opt/gmp/lib before the pip install
There was a problem hiding this comment.
Good catch! Removed the whole Python install 👍
pefontana
approved these changes
Jul 4, 2023
kariy
pushed a commit
to dojoengine/cairo-rs
that referenced
this pull request
Jul 25, 2023
* Add installation instructions to README
* Add compilation step to installation testing
* Add script for universal dependency installation
* Update changelog
* Enable test_install workflow only on main
* Update optional dependencies
Just having PyEnv and make installed suffices for running `make deps`.
I don't think we need to list cairo-lang as a dependency.
* Don't compile, only check
* Push again
* Use swatinem cache
* Cache all crates even on failure
* Fix debian rust cache
* Use manual cache
* Test cache
* Cache cairo compiler
* Add pyenv dependency caching
* Cache pip instead of pyenv
* Try fixing cache
* Use setup-python action
* Use ~ instead of ${HOME}
* Use manual cache in Debian container
* Add some comments
* Make cache owned by current user and add logs
* Remove python install from script
kariy
pushed a commit
to dojoengine/cairo-rs
that referenced
this pull request
Jul 25, 2023
* Add installation instructions to README
* Add compilation step to installation testing
* Add script for universal dependency installation
* Update changelog
* Enable test_install workflow only on main
* Update optional dependencies
Just having PyEnv and make installed suffices for running `make deps`.
I don't think we need to list cairo-lang as a dependency.
* Don't compile, only check
* Push again
* Use swatinem cache
* Cache all crates even on failure
* Fix debian rust cache
* Use manual cache
* Test cache
* Cache cairo compiler
* Add pyenv dependency caching
* Cache pip instead of pyenv
* Try fixing cache
* Use setup-python action
* Use ~ instead of ${HOME}
* Use manual cache in Debian container
* Add some comments
* Make cache owned by current user and add logs
* Remove python install from script
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR:
install.shscript, that detects the OS/distro and calls the corresponding installation script andmake depstest_installworkflow to use the script and to compile the whole project, to catch any missing dependenciestest_installworkflow, so it runs only on pushes to main branchChecklist