feat: remapping contexts #5397

onbjerg · 2023-07-14T20:33:26Z

Motivation

Dependencies in projects using Foundry can mess with how imports are resolved through the project because remappings from dependencies are just merged with remappings defined in the root folder.

To solve this, we make use of solc's remapping contexts.

Solution

This PR adds support for specifying remapping contexts if necessary, but ideally it should never be. Instead, this PR also automatically scopes each remapping defined in dependencies to files in the dependency itself.

Global remappings (context: None) are still automatically generated for each installed dependency, making it possible to import them as usual.

This PR in other words just makes the following user-facing change: remappings defined in dependencies/libraries can no longer override remappings defined in foundry.toml/remappings.txt, and it should no longer be necessary to copy over remappings from dependencies to foundry.toml/remappings.txt for dependencies with weird remappings.

This PR also adds a --pretty flag to forge remappings that groups remappings by context. Global remappings apply to every file (including dependencies), and remappings with a context only apply to the paths defined by the context. The default is to not pretty print since a common use case of forge remappings is to pipe the output to a file or other command.

Todo

Add tests
Merge ethers PR
Remove patch

Closes #3440, closes #1855

Depends on gakonst/ethers-rs#2509

gakonst · 2023-07-15T15:29:14Z

Ethers 2.0.8 is out with the fix.

aathan · 2023-07-17T16:28:03Z

Some comments/questions:
(1) How does this actually operate? are the remappings loaded from dependencies internally contextualized (with context:) for a given forge run, or are you re-writing the remappings files written to the lib/ projects when those are deposited? I assume the former.
(2) is there an opinion as to whether there should be an option to also scope the top level/global remappings so that they can optionally only apply to the "main" project files, and never to any libs that may have some import that happens to match? E.g., is it possible for a dependent project to itself depend on the same other project you also depend on, and in that case, could the global remappings end up affecting the dependency's imports?
(3) does someone know definitively, to tell me, whether the way solc resolves type names may cause structs of the same name, declared at the file-scope in various dependencies and/or the root project, to collide? If the solc devs need a poke about that, I can go open a feature request.

Evalir

So this seems to work well, and considering this will fix master i'm leaning to merging soon—some comments:

Evalir · 2023-07-17T18:04:44Z

cli/src/cmd/forge/remappings.rs

 }
 impl_figment_convert_basic!(RemappingArgs);

 impl Cmd for RemappingArgs {
    type Output = ();

+    // TODO: Do people use `forge remappings >> file`?


How are we planning to test this? Wasn't this what you tweeted the other day?

Evalir · 2023-07-17T18:06:10Z

config/src/providers/remappings.rs

@@ -83,11 +90,13 @@ impl<'a> RemappingsProvider<'a> {
        new_remappings.extend(remappings);

        // scan all library dirs and autodetect remappings
+        // todo: if a lib specifies contexts for remappings manually, we need to figure out how to


do we plan to do this now or later?

Evalir · 2023-07-17T18:06:23Z

config/src/providers/remappings.rs

+                    .into_iter()
+                    .map(Remapping::from)
+                    .map(|mut r| {
+                        // todo: windows/mac/linux


mattsse

cool, this looks less invasive than I expected, and since context is optional, this should work as before

smol nit re deps, otherwise lgtm

mattsse · 2023-07-17T20:49:28Z

Cargo.toml

+ethers = { version = "2.0.8", default-features = false }
+ethers-addressbook = { version = "2.0.8", default-features = false }
+ethers-core = { version = "2.0.8", default-features = false }
+ethers-contract = { version = "2.0.8", default-features = false }
+ethers-contract-abigen = { version = "2.0.8", default-features = false }
+ethers-providers = { version = "2.0.8", default-features = false }
+ethers-signers = { version = "2.0.8", default-features = false }
+ethers-middleware = { version = "2.0.8", default-features = false }
+ethers-etherscan = { version = "2.0.8", default-features = false }
+ethers-solc = { version = "2.0.8", default-features = false }


now this is on ethers master, we should continue using git deps

fixed in d92bd23 — we can switch to master deps whenever we plan on publishing crates 😄

mattsse · 2023-07-17T20:50:28Z

cli/src/cmd/forge/remappings.rs

 }
 impl_figment_convert_basic!(RemappingArgs);

 impl Cmd for RemappingArgs {
    type Output = ();

+    // TODO: Do people use `forge remappings >> file`?


they definitely do,
I also used this a few times at least

mattsse · 2023-07-17T20:52:09Z

config/src/providers/remappings.rs

-            match mappings.entry(key) {
+        /// grouped by remapping context
+        fn insert_closest(
+            mappings: &mut HashMap<Option<String>, HashMap<String, PathBuf>>,


I think this should work,
so the previous hashmap is now just the None key

and this also supports remappings with context extracted from nested deps

onbjerg added 2 commits July 14, 2023 21:29

wip

e735b88

wip

e2acf2f

onbjerg changed the title ~~Onbjerg/remapping context~~ feat: remapping contexts Jul 14, 2023

onbjerg added 2 commits July 14, 2023 22:40

wip

694e476

fix: top level lib auto-remappings

1dea694

onbjerg mentioned this pull request Jul 14, 2023

Support context aware remappings from nested foundry imports #1855

Closed

1 task

onbjerg added T-feature Type: feature C-forge Command: forge Cmd-forge-build Command: forge build labels Jul 14, 2023

onbjerg added 2 commits July 15, 2023 00:31

fix test

9bd56df

hide new remappings cmd behind flag

84e51f1

onbjerg marked this pull request as ready for review July 15, 2023 01:03

chore: update cargo.lock

23f00c8

Evalir requested review from mattsse and Evalir July 15, 2023 01:27

Evalir added 2 commits July 15, 2023 14:13

chore(deps): re-pin ethers to master and update

9bc9b70

chore(deps): pin to released version

684d394

Evalir mentioned this pull request Jul 15, 2023

cast lookup-address and resolve-name fail on names with unicode characters #5386

Closed

2 tasks

mattsse mentioned this pull request Jul 16, 2023

master - build broken #5409

Closed

2 tasks

This was referenced Jul 17, 2023

EVM: Remove dependency on the foundry fork Sovereign-Labs/sovereign-sdk#498

Closed

pin ethers-rs #5418

Closed

onbjerg mentioned this pull request Jul 17, 2023

bug(config): file paths not resolved if submodule specifies root path remappings #3440

Closed

Evalir reviewed Jul 17, 2023

View reviewed changes

mattsse approved these changes Jul 17, 2023

View reviewed changes

chore: keep git deps for ethers

d92bd23

Evalir merged commit 8e365be into master Jul 17, 2023

Evalir deleted the onbjerg/remapping-context branch July 17, 2023 21:56

PaulRBerg mentioned this pull request Jul 21, 2023

bug(forge): overlapping nested remappings are not resolved #5447

Closed

2 tasks

mds1 mentioned this pull request Jul 25, 2023

bug: regression introduced in PR#5397 (remapping contexts) #5475

Closed

2 tasks

ColePBryan mentioned this pull request Aug 30, 2023

JSON Outputs Contain Invalid Syntax #5751

Closed

2 tasks

0xalpharush mentioned this pull request Oct 9, 2023

import path canonicalization results in file paths that are not on disk gakonst/ethers-rs#2609

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remapping contexts #5397

feat: remapping contexts #5397

onbjerg commented Jul 14, 2023 •

edited by Evalir

Loading

gakonst commented Jul 15, 2023

aathan commented Jul 17, 2023

Evalir left a comment

Evalir Jul 17, 2023

Evalir Jul 17, 2023

Evalir Jul 17, 2023

mattsse left a comment

mattsse Jul 17, 2023

Evalir Jul 17, 2023

mattsse Jul 17, 2023

mattsse Jul 17, 2023

feat: remapping contexts #5397

feat: remapping contexts #5397

Conversation

onbjerg commented Jul 14, 2023 • edited by Evalir Loading

Motivation

Solution

Todo

gakonst commented Jul 15, 2023

aathan commented Jul 17, 2023

Evalir left a comment

Choose a reason for hiding this comment

Evalir Jul 17, 2023

Choose a reason for hiding this comment

Evalir Jul 17, 2023

Choose a reason for hiding this comment

Evalir Jul 17, 2023

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

mattsse Jul 17, 2023

Choose a reason for hiding this comment

Evalir Jul 17, 2023

Choose a reason for hiding this comment

mattsse Jul 17, 2023

Choose a reason for hiding this comment

mattsse Jul 17, 2023

Choose a reason for hiding this comment

onbjerg commented Jul 14, 2023 •

edited by Evalir

Loading