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

fix: issue where dependencies used local project's remappings instead of their own #151

Merged
merged 4 commits into from
Aug 19, 2024

Conversation

antazoey
Copy link
Member

What I did

Somehow an unnoticed bug (until recently) where remappings were resolving to paths from the local projects listings rather than looking at config from the dependency's themselves.

How I did it

Refactored all import remap stuff into its own module and relied on pydantic for some of the leg work.

How to verify it

Easy. Make a project with OZ 4.5 and yearn vaults. Ensure yearn vaults imports resolve using OZ 4.7.1 since it is a dependenc of a dependency (different version of OZ). Lemme know if you need help, I can add more info.

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

@antazoey antazoey changed the title fix: issue where dependencies-of-dependencies uses remappings of local project instead of their own fix: issue where dependencies-of-dependencies used remappings of local project instead of their own Aug 13, 2024
@antazoey
Copy link
Member Author

Bonus massive performance gain!

Before

--------------------------------------------------- benchmark: 1 tests --------------------------------------------------
Name (time in ms)                 Min       Max      Mean  StdDev    Median     IQR  Outliers     OPS  Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------
test_compile_performance     967.7907  967.7907  967.7907  0.0000  967.7907  0.0000       0;0  1.0333       1           1
-------------------------------------------------------------------------------------------------------------------------

After:

--------------------------------------------------- benchmark: 1 tests --------------------------------------------------
Name (time in ms)                 Min       Max      Mean  StdDev    Median     IQR  Outliers     OPS  Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------
test_compile_performance     189.7997  189.7997  189.7997  0.0000  189.7997  0.0000       0;0  5.2687       1           1
-------------------------------------------------------------------------------------------------------------------------

@antazoey antazoey changed the title fix: issue where dependencies-of-dependencies used remappings of local project instead of their own fix: issue where dependencies used remappings of local project instead of their own Aug 14, 2024
@antazoey antazoey changed the title fix: issue where dependencies used remappings of local project instead of their own fix: issue where dependencies used local project's remappings instead of their own Aug 14, 2024
@antazoey antazoey marked this pull request as ready for review August 14, 2024 15:47
# with project defining this dependency, for separate pieces of data.
# (need base project for relative .cache folder location and need dependency
# for configuration).
import_remapping = self.solidity._import_remapping_cache[config_project]
Copy link
Member Author

Choose a reason for hiding this comment

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

basically this logic here fixed the bug

Copy link

@bitwise-constructs bitwise-constructs left a comment

Choose a reason for hiding this comment

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

Looking good to me

@antazoey antazoey merged commit 961ca03 into ApeWorX:main Aug 19, 2024
13 checks passed
@antazoey antazoey deleted the fix/missing-sources-refactor branch August 19, 2024 18:58
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.

2 participants