fix: use target to get config, drop toml for json #515
Conversation
crytic_compile/platform/foundry.py
Outdated
| .strip() | ||
| LOGGER.info("'forge config --json' running") | ||
| json_config = json.loads( | ||
| subprocess.run(["forge", "config", "--json"], stdout=subprocess.PIPE, check=True).stdout |
There was a problem hiding this comment.
This should run with cwd=target or something like that, right? Otherwise you may get the foundry config from the current directory, which may not work or may not correspond to your target. If target can be a file, then you may need to preprocess it a bit:
target = target if os.path.isdir(target) else os.path.dirname(target)
subprocess.run(..., cwd=target)There was a problem hiding this comment.
On a second look, I think target is always a directory here, so scratch that last part of my comment.
Unrelated to this PR, but I think the check here may be a bit off, wouldn't it make more sense to look for the foundry/etc project in the path of the file rather than in cwd? e.g. if the file is /a/b/c/d/contracts/foo.sol, I'd check to see if any of [/a/b/c/d/contracts, /a/b/c/d, /a/b/c, /a/b, /a, /] is a project. Most users are probably going to be running the tool with cwd=the project folder, so it may work as-is now, but it doesn't feel completely correct.
Fix #509
Replaces #510