PolymerLabs / arcs

Arcs
BSD 3-Clause "New" or "Revised" License
58 stars 35 forks source link

Repo with a space in the path trips up tools/sigh #2752

Open joerick opened 5 years ago

joerick commented 5 years ago

Hello arcs!

Minor thing, but just tripped me up while getting setup. I checked out the repo in a location that has a space in the path. When running tools/sigh I get the following message:

πŸ™‹ test 
RUN 1 STARTING [11:33:08]:
(node:71893) ExperimentalWarning: The ESM module loader is experimental.
    at startup (internal/bootstrap/node.js:182:17)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)
{ Error: Cannot find module /Users/joerick/Work/Nord/Google
    at search (internal/modules/esm/default_resolve.js:28:12)
    at Loader.resolve [as _resolve] (internal/modules/esm/default_resolve.js:64:11)
    at Loader.resolve (internal/modules/esm/loader.js:58:33)
    at Loader.getModuleJob (internal/modules/esm/loader.js:113:40)
    at Loader.import (internal/modules/esm/loader.js:99:28)
    at loaderPromise (internal/process/esm_loader.js:54:43)
    at Object.exports.setup (internal/process/esm_loader.js:61:5)
    at startup (internal/bootstrap/node.js:186:59)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3) code: 'MODULE_NOT_FOUND' }
1 runs completed. 1 runs failed.
Failed runs:  [ 1 ]

The path /Users/joerick/Work/Nord/Google is incomplete, but that's where the first space occurs.

I've worked around by moving the repo, but just wanted to flag in case it trips someone else up too!

sjmiles commented 5 years ago

Sorry for fumbling the close button.

I'm guessing there was one or more problematic import or require statements, can you show the exact syntax here?

joerick commented 5 years ago

I've tracked this down to https://github.com/PolymerLabs/arcs/blob/master/tools/sigh.ts#L591, which looks like

    const testResult = saneSpawn(
        `${coveragePrefix} node`,
        [
          '--experimental-modules',
          '--trace-warnings',
          '--no-deprecation',
          ...extraFlags,
          '--loader',
          fixPathForWindows(path.join(__dirname, '../tools/custom-loader.mjs')),
          '-r',
          'source-map-support/register.js',
          runner
        ],

The problem is that these commands are executed in a shell, and __dirname is the absolute path to this file, so spaces in that path are interpreted as separate arguments by the shell.

There are actually a few script calls like this, wherever the custom loader is used throughout the file.

I can think of a few ways to solve this-

My preference would be to execute the child_process calls without a shell, unless there are any reasons why that wouldn't work? Any preference for the above let me know!

shaper commented 5 years ago

I just hit this myself at ToT, error below, using Git Bash. This seems like it would impede folks ahead of #3969. Perhaps @csilvestrini has been lucky enough to not start out with a directory/username containing spaces, or moved his bits to c:\src or similar. My sample error below.

Walter Korman@LAPTOP-SNO8904C MINGW64 ~/workspace/arcs ((faa3ad06...))
$ ./tools/sigh
😌 default
πŸ™‹ check
πŸ™† check
πŸ™‹ peg
πŸ™† peg
πŸ™‹ railroad
πŸ™† railroad
πŸ™‹ build
Version 3.6.4

Files:           714
Lines:        179177
Nodes:        758939
Identifiers:  254767
Symbols:      221577
Types:         56308
Memory used: 358692K
I/O read:      2.19s
I/O write:     7.65s
Parse time:    4.69s
Bind time:     1.01s
Check time:    6.00s
Emit time:    11.64s
Total time:   23.34s

πŸ™† build
πŸ™‹ runTestsOrHealthOnCron
RUN 1 STARTING [3:02:15 AM]:
(node:3108) ExperimentalWarning: The ESM module loader is experimental.
{ Error: Cannot find module /C:/Users/Walter
    at search (internal/modules/esm/default_resolve.js:29:12)
    at Loader.resolve [as _resolve] (internal/modules/esm/default_resolve.js:65:11)
    at Loader.resolve (internal/modules/esm/loader.js:58:33)
    at Loader.getModuleJob (internal/modules/esm/loader.js:113:40)
    at Loader.import (internal/modules/esm/loader.js:99:28)
    at loaderPromise (internal/process/esm_loader.js:53:43)
    at Object.exports.setup (internal/process/esm_loader.js:60:5)
    at startup (internal/bootstrap/node.js:184:59)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3) code: 'MODULE_NOT_FOUND' }
1 runs completed. 1 runs failed
Failed runs:  [ 1 ]
πŸ™… runTestsOrHealthOnCron
😱 FAILURE

Walter Korman@LAPTOP-SNO8904C MINGW64 ~/workspace/arcs ((faa3ad06...))
$ pwd
/c/Users/Walter Korman/workspace/arcs
csilvestrini commented 5 years ago

Thanks for the update, I'll have a look at this too when I get time. Indeed my user on Windows doesn't have any spaces in the name, I'll change this to try to repro.