-
Notifications
You must be signed in to change notification settings - Fork 190
Draft: Steel Merge #1763
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
Draft: Steel Merge #1763
Conversation
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.
Excellent, thanks so much for this @gurukamath!
I would suggest splitting this into 3 PRs that:
- Removes the
ethereum-executiondependency.- I think this could be insta-merged. Is there any reason we couldn't merge this to
maintoday? cc @marioevz
- I think this could be insta-merged. Is there any reason we couldn't merge this to
- Adds a new transition tool class for the
ethereum-spec-evmand keeps the resolver.- This can be insta-merged to
mainand would allow filling with./testslocated (via git submodule) in EELS (by explicitly specifying the EELS t8n entrypoint).
- This can be insta-merged to
- Removes
ethereum-spec-evm-resolverand makes EELS the default.- We can't remove this until the
./testsmove out of EEST.
- We can't remove this until the
Rationale tldr: This allows us to test EEST as a package with ./tests/ in EELS while maintaining filling functionality in EEST until that point.
This also helps avoid accumulating changes into 1 large PR and allows us to get these changes merged incrementally.
Small comment about uv.lock below.
| @@ -10,27 +9,27 @@ resolution-markers = [ | |||
| name = "annotated-types" | |||
| version = "0.7.0" | |||
| source = { registry = "https://pypi.org/simple" } | |||
| sdist = { url = "https://files.pythonhosted.org/packages/ee/67/531ea369ba64dcff5ec9c3402f9f51bf748cec26dde048a2f973a4eea7f5/annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89", size = 16081, upload-time = "2024-05-20T21:33:25.928Z" } | |||
| sdist = { url = "https://files.pythonhosted.org/packages/ee/67/531ea369ba64dcff5ec9c3402f9f51bf748cec26dde048a2f973a4eea7f5/annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89", size = 16081 } | |||
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.
Could you please bump your uv version and replay your upgrade commands to avoid removing the upload-time fields that will be added back by someone updating uv.lock with a newer uv?
If you curl-installed uv, you can do uv self update.
Explanation here:
| else: | ||
| # improve behavior of which by resolving the path: ~/relative paths don't work | ||
| resolved_path = Path(os.path.expanduser(binary)).resolve() | ||
| if resolved_path.exists(): | ||
| binary = resolved_path | ||
| return | ||
|
|
||
| # improve behavior of which by resolving the path: ~/relative paths don't work | ||
| resolved_path = Path(os.path.expanduser(binary)).resolve() | ||
| if resolved_path.exists(): | ||
| binary = resolved_path | ||
| binary = shutil.which(binary) # type: ignore | ||
| if not binary: | ||
| raise CLINotFoundInPathError(binary=binary) |
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.
This looks like it'll give bad ux/unexpected errors for non-eels tools that don't get found in the PATH. Would need to test.
|
Closing this since #1280 has been merged |
🗒️ Description
This PR has some of the changes needed to be able to perform the EEST and EELS merge. Has two distinct components to it
evm_binisn't explicitly provided in the command lineThis merge unlocks great developer experience features while debugging. It also removes the need for an intermediate eels resolver.
Related EELS PR
🔗 Related Issues
✅ Checklist
mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.