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

dev: bump cairo 1 compiler dep to 2.4 #1530

Merged
merged 5 commits into from
Jan 12, 2024

Conversation

greged93
Copy link
Contributor

Bump cairo compiler

Description

Bumps the dependency of the cairo 1 compiler to 2.4.

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.

fmoletta
fmoletta previously approved these changes Jan 3, 2024
@Robertorosmaninho
Copy link

Looking forward to this feature!!

@greged93
Copy link
Contributor Author

I'm not sure why the CI keeps failing, if you have an idea I can look into it?

@Oppen
Copy link
Contributor

Oppen commented Jan 10, 2024

OK, I think I found the issue. The Makefile has not been updated to fetch the right corelib in the deps target. I think then it uses the older stdlib where the prelude doesn't yet exist, but core brings everything instead. 2.4.3 programs try to import prelude and fail.

@Oppen
Copy link
Contributor

Oppen commented Jan 10, 2024

Try with this:

diff --git a/cairo1-run/Makefile b/cairo1-run/Makefile
index 337cf3a53..8c39b51ae 100644
--- a/cairo1-run/Makefile
+++ b/cairo1-run/Makefile
@@ -15,7 +15,7 @@ MEMORY:=$(patsubst $(CAIRO_1_FOLDER)/%.cairo, $(CAIRO_1_FOLDER)/%.memory, $(CAIR
 deps:
 	git clone https://github.com/starkware-libs/cairo.git \
 	&& cd cairo \
-	&& git checkout v2.3.1 \
+	&& git checkout v2.4.3 \
 	&& cd .. \
 	&& mv cairo/corelib/ . \
 	&& rm -rf cairo/

@greged93
Copy link
Contributor Author

Ahhhh missed this! Thanks @Oppen !

@Oppen
Copy link
Contributor

Oppen commented Jan 11, 2024

@greged93 most tests already pass, but there seems to be something broken with tests::cairo_1_run_from_entrypoint_tests::test_uint256_div_mod_hint.

@greged93
Copy link
Contributor Author

Yes found the issue, will update

@greged93
Copy link
Contributor Author

works for me now locally @Oppen

Copy link
Contributor

@Oppen Oppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Oppen
Copy link
Contributor

Oppen commented Jan 11, 2024

Let's ping @pefontana and @fmoletta for their reviews :)

@pefontana pefontana added this pull request to the merge queue Jan 12, 2024
Merged via the queue into lambdaclass:main with commit ef0ee41 Jan 12, 2024
46 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants