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

bug(remappings): unable to import Solidity file using auto-remappings, missing src/ suffix #9247

Closed
2 tasks done
Tracked by #9157
acuarica opened this issue Nov 1, 2024 · 10 comments
Closed
2 tasks done
Tracked by #9157
Assignees
Labels
A-remappings Area: remappings T-bug Type: bug T-to-investigate Type: to investigate

Comments

@acuarica
Copy link

acuarica commented Nov 1, 2024

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 (17e0981 2024-11-01T00:27:49.655450000Z)

What command(s) is the bug in?

forge remappings, forge build

Operating System

macOS (Apple Silicon)

Describe the bug

When installing some dependencies, the auto-remappings (and build command) do not resolve the import path properly.

Steps to reproduce

Create an empty Foundry project and install the Surl library

forge init foundry-example
cd foundry-example
forge install memester-xyz/surl

then

$ forge remappings
ds-test/=lib/surl/lib/forge-std/lib/ds-test/src/
forge-std/=lib/forge-std/src/
solidity-stringutils/=lib/surl/lib/solidity-stringutils/
surl/=lib/surl/

The last remapping surl/=lib/surl/ seems to be missing the src/ suffix (according to how the library is configured).

If I add the following import into, for example, test/Counter.t.sol

import {Surl} from "surl/Surl.sol";

I get an error when running build (which is consistent with the remappings mentioned above)

$ forge build
[⠃] Compiling...2024-11-01T16:30:19.505013Z ERROR foundry_compilers_artifacts_solc::sources: error="/<path>/foundry-example/lib/surl/Surl.sol": No such file or directory (os error 2)
[⠊] Compiling...
Error: failed to resolve file: "/<path>/foundry-example/lib/surl/Surl.sol": No such file or directory (os error 2); check configured remappings
	--> /<path>/foundry-example/test/Counter.t.sol
	surl/Surl.sol

Using the foundry.toml provided by the template project

[profile.default]
src = "src"
out = "out"
libs = ["lib"]

Workaround

By changing the import as shown below solves this problem

import {Surl} from "surl/src/Surl.sol";

Additional context

However, if I use a .. in the lib path resolving to the same folder as above, both forge remappings (and forge build) commands work as expected

[profile.default]
src = "src"
out = "out"
libs = ["lib/../lib"]
$ forge remappings
ds-test/=lib/surl/lib/forge-std/lib/ds-test/src/
forge-std/=lib/forge-std/src/
solidity-stringutils/=lib/surl/lib/solidity-stringutils/src/
surl/=lib/surl/src/

Related issues

As mentioned in some of the issues below, this does not happen for every dependency (for example installing solmate works fine).

@acuarica acuarica added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Nov 1, 2024
@zerosnacks zerosnacks added A-remappings Area: remappings and removed T-needs-triage Type: this issue needs to be labelled labels Nov 1, 2024
@zerosnacks zerosnacks changed the title Unable to import Solidity file using auto-remappings bug(remappings): unable to import Solidity file using auto-remappings, missing src/ suffix Nov 1, 2024
@zerosnacks zerosnacks added the T-to-investigate Type: to investigate label Nov 1, 2024
@grandizzy
Copy link
Collaborator

grandizzy commented Nov 1, 2024

@acuarica I don't think there's anything wrong here, we're inheriting remappings of module, so the right way to do is to import as

import {Surl} from "surl/src/Surl.sol";

or add a remapping surl/=lib/surl/src in root project to be able to imports as

import {Surl} from "surl/Surl.sol";

Can you clarify why would you expect surl/Surl.sol import to work out of the box? thank you!

@acuarica
Copy link
Author

acuarica commented Nov 1, 2024

@grandizzy thanks for your reply. Sure.

The Surl documentation mentions how to import their contract and it's without the src/ part. So I assumed the import "surl/Surl.sol" worked at some point.

Moreover, if we replace Surl by solmate (for example) as a dependency, the remappings output is different. But there is no specific remapping configuration (why are they different?)

$ forge init foundry-example-solmate && cd foundry-example-solmate
$ forge install transmissions11/solmate
$ forge remappings
ds-test/=lib/solmate/lib/ds-test/src/
forge-std/=lib/forge-std/src/
solmate/=lib/solmate/src

Finally, whether I set my project using libs = ["lib"], or libs = ["lib/../lib"] should lead to the same remappings. Or at least I assumed that. Of course, provided both paths resolve to the same actual folder. But the former results in a remapping without the src/ and the latter with src/ suffix. I assume there is a problem here, because whatever the right answer should it be, the fact of having different results looks like a problem.

Hope it's a bit more clear now. Thanks for the support.

@grandizzy
Copy link
Collaborator

grandizzy commented Nov 1, 2024

yeah, but solmate doesn't have test nor scripts in root so you could only import from src. Surl has both so if remappings to src then there'll be no way to import and use something from test or script (Granted I am still digging the remappings mechanism.)

@grandizzy
Copy link
Collaborator

grandizzy commented Nov 4, 2024

checked the code and that's the intended behavior, if you want to make it solmate like and have remappings to src directly then remove script dirs from root lib project. pls reopen this if you still think it's an issue, thank you!

@grandizzy grandizzy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2024
@acuarica
Copy link
Author

acuarica commented Nov 4, 2024

Thanks for taking a look at this and for clarifying what the intended behavior should be. However, the only thing that hints something is not working properly is the following

Additional context

However, if I use a .. in the lib path resolving to the same folder as above, both forge remappings (and forge build) commands work as expected

[profile.default]
src = "src"
out = "out"
libs = ["lib/../lib"]
$ forge remappings
ds-test/=lib/surl/lib/forge-std/lib/ds-test/src/
forge-std/=lib/forge-std/src/
solidity-stringutils/=lib/surl/lib/solidity-stringutils/src/
surl/=lib/surl/src/

@grandizzy
Copy link
Collaborator

grandizzy commented Nov 4, 2024

ah, yes, worth checking some more, reopening. Thank you!

@grandizzy grandizzy reopened this Nov 4, 2024
@grandizzy grandizzy self-assigned this Nov 4, 2024
@grandizzy
Copy link
Collaborator

similar behavior to investigate https://github.com/StackOverflowExcept1on/frost-secp256k1-evm

@grandizzy
Copy link
Collaborator

@acuarica ack this is an issue, similar with #4717 (comment) going to close this one as a dupe and track fix there. thank you!

@grandizzy grandizzy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2024
@acuarica
Copy link
Author

acuarica commented Nov 5, 2024

That's great, thanks for confirming this.

@grandizzy
Copy link
Collaborator

That's great, thanks for confirming this.

np, pending fix in #9263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-remappings Area: remappings T-bug Type: bug T-to-investigate Type: to investigate
Projects
None yet
Development

No branches or pull requests

3 participants