Skip to content

Fix import resolution on Windows: normalise path separators#493

Open
sgraphics wants to merge 1 commit into
EYBlockchain:masterfrom
sgraphics:fix/windows-import-path-separator
Open

Fix import resolution on Windows: normalise path separators#493
sgraphics wants to merge 1 commit into
EYBlockchain:masterfrom
sgraphics:fix/windows-import-path-separator

Conversation

@sgraphics
Copy link
Copy Markdown

Problem

On Windows, every contract with a relative import fails to compile with We couldn't find the import ... — for example CharityPot, Escrow, Swap, and Assign-constructor2. Import-free contracts are unaffected, which is why CI (Ubuntu) stays green.

Cause

buildSources in src/solc.ts keys the sources map with the output of path.relative(). solc's findImports callback always looks those keys up with forward slashes, but on Windows path.relative returns the OS-native separator (a backslash), so the lookup never matches and compilation aborts.

Fix

Extract the key-building into a small toImportKey() helper that normalises the separators to forward slashes. This is a no-op on POSIX (path.sep is already /) and only changes behaviour on Windows.

Tests

Adds test/solc-import-path.test.js with cross-platform cases driven by path.posix and path.win32 explicitly, so the Windows case fails without the fix even when run on a Linux CI runner. With the fix, npm test goes from 0 to 9 passing on Windows and is unchanged on Linux/Mac.

solc's findImports callback always receives '/'-separated import paths,
but buildSources keyed the sources map with path.relative()'s output,
which uses '\' on Windows. The mismatch made every contract with a
relative import fail on Windows with "We couldn't find the import ..."
(e.g. CharityPot, Escrow, Swap, Assign-constructor2).

Extract the key-building into an injectable toImportKey() helper that
normalises separators to '/', and add cross-platform unit tests in
test/solc-import-path.test.js covering both path.posix and path.win32.
The Windows case is driven with path.win32 explicitly, so it fails
without the fix even on a Linux CI runner.

No-op on POSIX (path.sep is '/'); takes `npm test` from 0 -> 9 passing
on Windows.
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.

1 participant