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

fix(solc): convert source paths on windows #1540

Merged

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jul 31, 2022

Motivation

this fixes yet another path issue on windows which can result in duplicate (src\\File.sol and src/File.sol) artifacts

Ref https://github.com/foundry-rs/foundry/runs/7586758672?check_suite_focus=true

solc uses unix style paths /, if it tries to resolve an import src/File.sol it checks the VFS for a corresponding source, src\\File.sol does not necessarily match so it will resolve from disk which results in duplicated entries in the output.

Solution

convert source paths to / on windows before invoking solc

foundry tests: foundry-rs/foundry#2531

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@gakonst
Copy link
Owner

gakonst commented Jul 31, 2022

gotta love windows. i wonder if we've been doing things the wrong way all this time, or if it's just full of footguns? how can we avoid these in the future?

@mattsse
Copy link
Collaborator Author

mattsse commented Jul 31, 2022

I guess the main problem is/was I've never worked with windows and just didn't know any better lol

since windows also supports / I think we can just convert all of them when ethers-solc spits out the Artifact set only just only work with slashed paths

trying to sort things out here foundry-rs/foundry#2531 and make sure everything passes now, getting some weird failures that only happen in the cross compile tests...

@gakonst
Copy link
Owner

gakonst commented Aug 1, 2022

smol lint issue otherwise merging

@mattsse
Copy link
Collaborator Author

mattsse commented Aug 1, 2022

lint issue is fixed here #1541

I'd like to keep this open until foundry-rs/foundry#2531 passes

this is incredible tedious lol for some reason a random fuzzing test keeps failing randomly...

@mattsse mattsse force-pushed the matt/convert-slashes-in-inputs-on-windows branch from 1f8084b to ef4fab1 Compare August 1, 2022 04:00
@mattsse
Copy link
Collaborator Author

mattsse commented Aug 1, 2022

@gakonst this is ready now, it will spit out / paths on windows now

@gakonst gakonst merged commit e62c84d into gakonst:master Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants