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

Soldeer generated remappings do infer contract directory #236

Open
2 tasks done
silasdavis opened this issue Dec 3, 2024 · 5 comments
Open
2 tasks done

Soldeer generated remappings do infer contract directory #236

silasdavis opened this issue Dec 3, 2024 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@silasdavis
Copy link

silasdavis commented Dec 3, 2024

I have checked the following:

  • I have searched the issues of this repository and believe that this is not a duplicate.
  • I have checked that the bug is reproducible with the latest version of Soldeer.

Soldeer Version

soldeer 0.5.2

What Happened?

Foundry Forge seems to be able to infer the appropriate contract directory to suffix to a remapping, for example forge remappings generates:

forge-std/=lib/openzeppelin-foundry-upgrades/lib/forge-std/src/

For a nested copy of forge-std.

However if I ask Soldeer to manage my remappings it generates something like:

forge-std/=dependencies/forge-std-1.9.4/

It's similar for other repositories where forge correctly detects a /contracts suffix.

Without these remappings mamy imports are made non canonical, which is particularly irksome for forge-std since you need to have imports like

import {Script} from "forge-std/src/Script.sol";

Which is ugly and doesn't match any of the documentation. The situation is similar for other dependencies.

Perhaps I am missing something, since it is surprising other people would not be reporting this issue.

I could manually maintain my remappings but then each time I update a dependency version I'd have to update it in two places since soldeer places a version suffix in the directory name.

Expected Behavior

Instead of remappings like:

remappings = [
    "forge-std/=dependencies/forge-std-1.9.4/",
    "openzeppelin-foundry-upgrades/=dependencies/openzeppelin-foundry-upgrades-0.3.6/",
]

I would expect remappings like:

remappings = [
    "forge-std/=dependencies/forge-std-1.9.4/src/",
    "openzeppelin-foundry-upgrades/=dependencies/openzeppelin-foundry-upgrades-0.3.6/src/",
]

Reproduction Steps

forge init
forge soldeer init
// Enabel remappings_generate and remappings_regenerate
forge install forge-std~1.9.4

Configuration

[soldeer]
# whether soldeer manages remappings
remappings_generate = true
# whether soldeer re-generates all remappings when installing, updating or uninstalling deps
remappings_regenerate = true
# whether to suffix the remapping with the version: `name-a.b.c`
remappings_version = false
# a prefix to add to the remappings ("@" would give `@name`)
remappings_prefix = ""
# where to store the remappings ("txt" for `remappings.txt` or "config" for `foundry.toml`)
# ignored when `soldeer.toml` is used as config (uses `remappings.txt`)
remappings_location = "config"
recursive_deps = true
@silasdavis silasdavis added the bug Something isn't working label Dec 3, 2024
@mario-eth
Copy link
Owner

Hey,

Thank you for reporting this, yea this is something we were somehow aware, will for sure look to implement the same detection as forge!

@mario-eth mario-eth added the good first issue Good for newcomers label Dec 3, 2024
@beeb
Copy link
Collaborator

beeb commented Dec 3, 2024

I could manually maintain my remappings but then each time I update a dependency version I'd have to update it in two places since soldeer places a version suffix in the directory name.

Hey! Just wanted to point out that if you add the src/ at the end of the remapping manually once, it will remain there even if you update the dependency to a different version through the update command. There is logic to keep the extra path components which come after the dependency folder name when updating the dependency.

@silasdavis
Copy link
Author

silasdavis commented Dec 4, 2024

@beeb oh great, that removes most of the pain, then.

I see the good first issue tag, so lemme see if I can give this a go.

I'm not sure what logic forge uses, but I was guessing that it's something like a breadth first search on dir tree to first directory that has solidity files but no sibling directories do, then rooting at that. So something like:

find deepest depth N such that (dir, N) contains solidity files, explore all dirs at depth N, if only (dir, N) contains solidity files at depth N then use dir otherwise use parent(dir)

Does that sound sensible?

@mario-eth
Copy link
Owner

let's try to explore that @silasdavis and see how it goes, also forge remappings might worth looking into as we said.

@mario-eth
Copy link
Owner

thank you for picking this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants