-
Notifications
You must be signed in to change notification settings - Fork 26
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
Utils gatherSources
fails if path has spaces
#176
Comments
FWIW, we have fixed this replacing all calls to import isUrl from 'is-url';
function resolvePath(workingDir: string, relativePath: string): string {
// If the working dir is an URL (a file imported from an URL location)
// or the relative path is an URL (an URL imported from a local file)
// use url.resolve, which will work in both cases.
// > url.resolve('/local/folder/', 'http://example.com/myfile.sol')
// 'http://example.com/myfile.sol'
// > url.resolve('http://example.com/', 'myfile.sol')
// 'http://example.com/myfile.sol'
//
// If not, use path.resolve, since using url.resolve will escape certain
// charaters (e.g. a space as an %20), breaking the path
// > url.resolve('/local/path/with spaces/', 'myfile.sol')
// '/local/path/with%20spaces/myfile.sol'
return isUrl(workingDir) || isUrl(relativePath)
? urlSys.resolve(workingDir, relativePath)
: pathSys.resolve(path.dirname(workingDir), relativePath);
} |
@spalladino Heads up, doing just this makes the gatherSources tests break, see #187 for details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
gatherSources
script relies onurl.resolve
to resolve absolute paths. This changes any spaces in a local filesystem path to%20
, which then fails to find the file.resolver-engine/packages/imports/src/utils.ts
Line 108 in aa15ba1
The text was updated successfully, but these errors were encountered: