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

Legacy cml-command syntax broken on Windows #736

Closed
0x2b3bfa0 opened this issue Sep 22, 2021 · 3 comments · Fixed by #737
Closed

Legacy cml-command syntax broken on Windows #736

0x2b3bfa0 opened this issue Sep 22, 2021 · 3 comments · Fixed by #737
Assignees
Labels
bug Something isn't working testing Unit tests & debugging

Comments

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Sep 22, 2021

const [, base, command] = basename(file).match(/^(\w+)-(.+)$/);
                          ^

TypeError: basename is not iterable (cannot read property Symbol(Symbol.iterator))
    at Object.<anonymous> (D:\a\cml\cml\bin\legacy.js:12:27)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1101:10)
    at Module.load (internal/modules/cjs/loader.js:937:32)
    at Function.Module._load (internal/modules/cjs/loader.js:778:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47

NPM uses cmd-shim on Windows instead of creating symbolic links. This shim consists of a hardcoded script that calls the executables specified on package.json:bin with the node interpreter, and, because of the indirect call, $0 gets lost in the process.

cml/bin/legacy.js

Lines 11 to 12 in 7d664ae

const [, file, ...parameters] = process.argv;
const [, base, command] = basename(file).match(/^(\w+)-(.+)$/);

Because of this, basename(file) equals legacy.js instead of cml-command as expected. Therefore, /^(\w+)-(.+)$/ doesn't match anything (obviously) and we don't have any means to determine which legacy command was called.

@0x2b3bfa0
Copy link
Member Author

This only affects legacy commands on Windows, and doesn't have any easy realistic solution. Should we just ignore it?

@0x2b3bfa0
Copy link
Member Author

🔔 @iterative/cml, voting to close without changes through #737

@0x2b3bfa0 0x2b3bfa0 self-assigned this Sep 22, 2021
@0x2b3bfa0 0x2b3bfa0 added bug Something isn't working testing Unit tests & debugging labels Sep 22, 2021
@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 added a commit that referenced this issue Sep 23, 2021
* Remove multiplatform legacy command tests

Closes #736

* Apply @casperdcl's suggestion

Co-authored-by: Casper da Costa-Luis <[email protected]>

Co-authored-by: Casper da Costa-Luis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing Unit tests & debugging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant