Skip to content
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

Update make deps #963

Merged
merged 11 commits into from
Apr 17, 2023
Merged

Update make deps #963

merged 11 commits into from
Apr 17, 2023

Conversation

pefontana
Copy link
Collaborator

@pefontana pefontana commented Apr 12, 2023

Update make deps

Description

  • Update make deps:
    • Update to python version to 3.9.15 and pypy3.9-7.3.9
    • Pin bitarray==2.7.3 vesion
    • Add cargo install --version 0.11.0 wasm-pack
  • Add make deps-macos
  • Update bench/run_benchmarks.sh layouts

Description of the pull request changes and motivation.

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.
    • CHANGELOG has been updated.

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Merging #963 (dbca2d4) into main (8e24668) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #963   +/-   ##
=======================================
  Coverage   97.76%   97.76%           
=======================================
  Files          75       75           
  Lines       31155    31155           
=======================================
  Hits        30459    30459           
  Misses        696      696           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Makefile Outdated Show resolved Hide resolved
@igaray
Copy link
Collaborator

igaray commented Apr 13, 2023

Small comment, but the README in the Getting Started section has pyenv in the Optional section, saying "These dependencies are only necessary in order to run the original VM and compile Cairo programs.". It should clarify that pyenv is required to run the tests.

Another nitpick is if some downloads the repo and runs make, it defaults to installing the deps and errors when pyenv is not found. The deps target should be separated into the base "running dependencies" which might be required by a plain user and "development/testing" dependencies which do in fact require python and pyenv.

Makefile Outdated Show resolved Hide resolved
@pefontana pefontana added the pipelines This PR/issue is exclusively about improving our CI label Apr 14, 2023
@pefontana
Copy link
Collaborator Author

Small comment, but the README in the Getting Started section has pyenv in the Optional section, saying "These dependencies are only necessary in order to run the original VM and compile Cairo programs.". It should clarify that pyenv is required to run the tests.

Another nitpick is if some downloads the repo and runs make, it defaults to installing the deps and errors when pyenv is not found. The deps target should be separated into the base "running dependencies" which might be required by a plain user and "development/testing" dependencies which do in fact require python and pyenv.

Thanks for the comment @igaray

I update the README.md so users know that they need pyenv to run the test.
Regarding the second point, the only thing a plain user can do without pyenv is to compile the project. If the user wants to do anything more it will need the dependencies in make deps, so it thinks it doesn't make sense to add an empty make deps for plain users.

Maybe instead it will be better to add the make deps step in the README.md

What do you think?

@pefontana pefontana requested review from Oppen and MegaRedHand April 14, 2023 19:48
Makefile Show resolved Hide resolved
@Oppen Oppen enabled auto-merge April 17, 2023 15:12
@Oppen Oppen added this pull request to the merge queue Apr 17, 2023
Merged via the queue into main with commit 4271436 Apr 17, 2023
@Oppen Oppen deleted the update-make-deps branch April 17, 2023 15:57
kariy pushed a commit to dojoengine/cairo-rs that referenced this pull request Jun 23, 2023
* Add make deps-macos && update make deps

* Add .python-version to .gitignore

* pin bitarray version

* Add pypy to deps

* Modify python versions in un_benchmarks.sh

* Update run_benchmarks.sh layouts

* Add virtual envs to make deps

* Update README.md

* add make cargo-deps

---------

Co-authored-by: Pedro Fontana <pedro.fontana@lamdaclass.com>
Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pipelines This PR/issue is exclusively about improving our CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants