foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.12k stars 1.68k forks source link

feat: allow single file remappings #6706

Open arthurneuron opened 8 months ago

arthurneuron commented 8 months ago

Component

Forge

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

What version of Foundry are you on?

forge 0.2.0 (9e3ab9b 2024-01-04T00:18:01.892563000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

remappings.txt

Error:

remappings.txt

Error:

remappings.txt

Error:

mattsse commented 8 months ago

could you please provide a repro for this?

mattsse commented 8 months ago

@tash-2s I wonder if this is related to the recent path changes?

tash-2s commented 8 months ago

I'll look into this.

@arthurneuron Did the same setup and command work in previous versions of forge? (Is this an issue that occurred after updating forge?) It would be really helpful if you could provide a detailed reproduction of the error.

tash-2s commented 8 months ago

I believe this is the reproduction of the issue:

$ mkdir issues6706 && cd issues6706
$ mkdir src my-lib

$ cat << EOF > src/A.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "@my-lib/B.sol";
contract A is B {}
EOF

$ cat << EOF > my-lib/B.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
contract B {}
EOF

$ cat << EOF > remappings.txt
@my-lib/B.sol=my-lib/B.sol
EOF

$ forge build
[⠊] Compiling...
Error: 
failed to resolve file: "/Users/t/bench/issues6706/my-lib/B.sol/": Not a directory (os error 20); check configured remappings.
    --> "/Users/t/bench/issues6706/src/A.sol"
        "@my-lib/B.sol"

This change makes it work:

diff --git a/remappings.txt b/remappings.txt
index 134e3a5..ceb4244 100644
--- a/remappings.txt
+++ b/remappings.txt
@@ -1 +1 @@
-@my-lib/B.sol=my-lib/B.sol
+@my-lib/=my-lib/

This issue is not related to the recent changes (https://github.com/foundry-rs/compilers/pull/35, https://github.com/foundry-rs/compilers/pull/36). I can confirm the same behavior occurs with an older version of forge. (foundryup --commit 67ab8704476d55e47545cf6217e236553c427a80)

The current implementation of remapping resolution does not support this case. See: https://github.com/foundry-rs/compilers/blob/1cde7fe2559815f40255f7338c7998df4f55ca7d/src/config.rs#L344-L345

To simulate that code:

fn main() {
    use std::path::Path;

    let p1 = Path::new("@my-lib/A.sol").strip_prefix(Path::new("@my-lib/A.sol"));
    let p2 = Path::new("A.sol").join(Path::new(""));

    println!("{:?}, {:?}", p1, p2); // => Ok(""), "A.sol/"
}
onbjerg commented 8 months ago

This is correct - you cannot remap individual files, only directories. Not sure if we should count this as a bug or as a feature request, in any case, it might make sense to support this

drinkius commented 6 months ago

I've faced similar issue with dependency that relies on IERC721Metadata.sol. Definitely need some way to account for possible path changes

heeween commented 2 months ago

The reason is that from 3.4.2 openzepplin has change the path of ERC72Metadata.sol. so i solve it by install a low version of openzepplin. forge install github.com/OpenZeppelin/openzeppelin-contracts@v3.4.2 --no-commit.

zerosnacks commented 2 months ago

The reason is that from 3.4.2 openzepplin has change the path of ERC72Metadata.sol.

so i solve it by install a low version of openzepplin.

forge install github.com/OpenZeppelin/openzeppelin-contracts@v3.4.2 --no-commit.

Not directly related but it is not recommended to install older versions of OpenZeppelin as they may have (known) bugs and vulnerabilities.

zerosnacks commented 2 months ago

This is correct - you cannot remap individual files, only directories. Not sure if we should count this as a bug or as a feature request, in any case, it might make sense to support this

Updated the ticket to reflect that this is a feature request, not a bug

Related: https://github.com/foundry-rs/foundry/issues/7527