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 path confusion on Windows #1065

Merged
merged 2 commits into from
Mar 18, 2022
Merged

Fix path confusion on Windows #1065

merged 2 commits into from
Mar 18, 2022

Conversation

elopez
Copy link
Member

@elopez elopez commented Feb 19, 2022

This fixes an issue when running Slither on Windows. Some paths were
being stringified as Windows-style (backslash) while the rest of the
paths use POSIX-style (forward slash). This caused an issue when
trying to match paths, and Slither would complain the dependency
was not present.

After this change, the paths for import directives are always expressed
as POSIX-style paths when stringified.

See the following issue for a complete error message:

https://github.com/crytic/crytic-compile/issues/241

This fixes an issue when running Slither on Windows. Some paths were
being stringified as Windows-style (backslash) while the rest of the
paths use POSIX-style (forward slash). This caused an issue when
trying to match paths, and Slither would complain the dependency
was not present.

After this change, the paths for import directives are always expressed
as POSIX-style paths when stringified.

See the following issue for a complete error message:

    crytic/crytic-compile#241
@montyly
Copy link
Member

montyly commented Mar 18, 2022

Is this still a draft, or is it ready for review? (it seems ready to review)
If we merge dev into this branch, the CI should pass

@elopez
Copy link
Member Author

elopez commented Mar 18, 2022

This fixed the issue I was seeing but I did not perform more extensive testing on other platforms. If you think it's safe to merge then go ahead 👍

Maybe setting up CI on Windows would be a good thing for the future 🤔

@elopez elopez marked this pull request as ready for review March 18, 2022 12:20
@montyly montyly merged commit c079206 into crytic:dev Mar 18, 2022
@elopez elopez deleted the fix-windows-paths branch March 18, 2022 18:48
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.

2 participants