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: yul remapping issue #182

Merged
merged 4 commits into from
May 6, 2024
Merged

fix: yul remapping issue #182

merged 4 commits into from
May 6, 2024

Conversation

gzeoneth
Copy link
Member

@gzeoneth gzeoneth commented May 6, 2024

eljobe and others added 3 commits May 6, 2024 16:11
For some reason, the master branch of nitro won't build the contracts
if there are any remappings specified for a yul build.

This change removes the remappings for the yul build.
@cla-bot cla-bot bot added the s label May 6, 2024
eljobe
eljobe previously approved these changes May 6, 2024
foundry.toml Outdated Show resolved Hide resolved
@eljobe
Copy link
Member

eljobe commented May 6, 2024

Oh, one other thing. I noticed when I was trouble-shooting this, that:

forge config

Output considerably more remappings than the four which were listed in remappings.txt.

So, is it possible that it was augmenting that list through some sort of auto-detection and that hardcoding these 4 in the configuration file directly might have some adverse consequences? (Totally just being paranoid.)

@gzeoneth
Copy link
Member Author

gzeoneth commented May 6, 2024

Oh, one other thing. I noticed when I was trouble-shooting this, that:

forge config

Output considerably more remappings than the four which were listed in remappings.txt.

So, is it possible that it was augmenting that list through some sort of auto-detection and that hardcoding these 4 in the configuration file directly might have some adverse consequences? (Totally just being paranoid.)

forge config

Thanks, I believe auto_detect_remappings has always been enabled and it will add entries to remappings, but those in the static list would always be included. This PR should keep the expected behavior. (For YUL, remapping are irrelevant)

@gzeoneth gzeoneth merged commit a00d2fa into develop May 6, 2024
9 of 11 checks passed
@gzeoneth gzeoneth deleted the fix-ci branch May 6, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants