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

Remappings break CI builds using foundry > e649e62 #9375

Closed
2 tasks done
davidlaprade opened this issue Nov 21, 2024 · 8 comments · Fixed by #9379
Closed
2 tasks done

Remappings break CI builds using foundry > e649e62 #9375

davidlaprade opened this issue Nov 21, 2024 · 8 comments · Fixed by #9379
Labels
T-bug Type: bug T-needs-triage Type: this issue needs to be labelled

Comments

@davidlaprade
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (25cc1ac 2024-11-21T00:22:12.782328000Z)

What command(s) is the bug in?

forge test

Operating System

Linux

Describe the bug

TLDR minimal reproduction here

I commented on this in another issue yesterday.

We started seeing CI builds break for flex-voting (as well as other internal projects) a few days ago. The errors happened during the forge test step:

Run forge test
2024-11-19T15:32:25.463715Z ERROR foundry_compilers_artifacts_solc::sources: error="/home/runner/work/flexible-voting/flexible-voting/lib/openzeppelin-contracts/contracts/utils/math/SafeCast.sol": No such file or directory (os error 2)
Error: failed to resolve file: "/home/runner/work/flexible-voting/flexible-voting/lib/openzeppelin-contracts/contracts/utils/math/SafeCast.sol": No such file or directory (os error 2); check configured remappings
	--> /home/runner/work/flexible-voting/flexible-voting/src/FlexVotingClient.sol
	@openzeppelin/contracts/utils/math/SafeCast.sol

Weirdly, we were never able to reproduce this locally. However, we resolved it in CI by pinning foundry to e649e62 in our workflow yaml, as seen here.

This morning we tried to reproduce the issue in a clean repo. We found that the issue requires:

If the remapping is removed from the foundry.toml the issue is fixed -- forge test is able to run again in CI.

Open questions at this point:

  1. How is the code able to compile if there are no remappings? import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; should not compile without a remapping, but it does.
  2. Why are the remappings breaking CI?
  3. What changed in foundry to cause this to happen? Nothing obvious stands out in the diff.

Thanks for your help!

@davidlaprade davidlaprade added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Nov 21, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Nov 21, 2024
@davidlaprade
Copy link
Author

Operating system is linux because our CI uses ubuntu, and that's the only environment where this happened

@grandizzy
Copy link
Collaborator

@davidlaprade thank you for the detailed report and repro! the issue should be fix in tomorrow nightly build

How is the code able to compile if there are no remappings? import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; should not compile without a remapping, but it does.
Why are the remappings breaking CI?

There wasn't an issue with remappings but with project deps that were not installed in certain conditions (like CI), hence build failed since sources weren't found

What changed in foundry to cause this to happen? Nothing obvious stands out in the diff.

This was related to the newly added feature, see #9379 (comment)

@davidlaprade
Copy link
Author

Thanks @grandizzy and @yash-atreya!

Unfortunately #9379 didn't fix the issue for either flex voting or the minimal repro. CI still fails with remappings on current nightly.

There wasn't an issue with remappings but with project deps that were not installed in certain conditions

Got it, that explains why CI was broken. But I still don't understand how the project is able to compile locally without remappings. @openzeppelin/contracts/utils/math/SafeCast.sol is not a real path. How does forge know where to look for this file when remappings are removed from foundry.toml?

@grandizzy
Copy link
Collaborator

@davidlaprade the fix wasn't merged in time for the nightly build to pick it / include it https://github.com/foundry-rs/foundry/releases You can check it works locally, and will be solved tomorrow on CI as well

@davidlaprade
Copy link
Author

davidlaprade commented Nov 22, 2024

Oh wait, I'm just realizing that @klkvr's (76a2cb0) wasn't in the current nightly (41b4359). So ignore my previous message about it not being fixed.

@davidlaprade
Copy link
Author

Still curious about this part, though:

I still don't understand how the project is able to compile locally without remappings. @openzeppelin/contracts/utils/math/SafeCast.sol is not a real path. How does forge know where to look for this file when remappings are removed from foundry.toml?

@klkvr
Copy link
Member

klkvr commented Nov 22, 2024

Still curious about this part, though:

I still don't understand how the project is able to compile locally without remappings. @openzeppelin/contracts/utils/math/SafeCast.sol is not a real path. How does forge know where to look for this file when remappings are removed from foundry.toml?

This is happening because there's a lib/openzeppelin-contracts/remappings.txt which specifies @openzeppelin/contracts/=contracts/. So when finding remappings we find it and infer this remappings without requiring user to specify it in the root of the actual project.

@davidlaprade
Copy link
Author

@klkvr Nice, I didn't know projects could do that. That's great to know and answers my question, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug T-needs-triage Type: this issue needs to be labelled
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants