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

cliui git dependency breaks lerna installation #4

Closed
richallanb opened this issue May 2, 2023 · 11 comments
Closed

cliui git dependency breaks lerna installation #4

richallanb opened this issue May 2, 2023 · 11 comments

Comments

@richallanb
Copy link

It appears the introduction of cliui using a git path for the dependency has broken installing lerna on npm 8 and 9.
https://github.com/isaacs/jackspeak/blob/main/package.json#L70

richard.bull@MSAN42AD2EED git % npm i -g lerna
npm WARN deprecated @npmcli/move-file@2.0.1: This functionality has been moved to @npmcli/fs
npm ERR! code 127
npm ERR! git dep preparation failed
npm ERR! command /Users/richard.bull/.nvm/versions/node/v16.16.0/bin/node /Users/richard.bull/.nvm/versions/node/v16.16.0/lib/node_modules/npm/bin/npm-cli.js install --force --cache=/Users/richard.bull/.npm --prefer-offline=false --prefer-online=false --offline=false --no-progress --no-save --no-audit --include=dev --include=peer --include=optional --no-package-lock-only --no-dry-run
npm ERR! npm WARN using --force Recommended protections disabled.
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /Users/richard.bull/.npm/_cacache/tmp/git-cloneFiix7g
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c npm run compile
npm ERR! npm ERR! > cliui@8.0.1 precompile
npm ERR! npm ERR! > rimraf build
npm ERR! npm ERR! npm WARN using --force Recommended protections disabled.
npm ERR! npm ERR! sh: rimraf: command not found
npm ERR!

Tested on node v16 and v18, with npm 9.6.5 and 8.19.4

@DavidAnson
Copy link

Agreed - @richallanb beat me by an hour (nice!), but I think I'm getting reports of the same problem for the markdownlint-cli project, see here: igorshubovych/markdownlint-cli#394. (Includes a link to a similar report on Stack Overflow.) It's only an issue when installing --global for reasons I don't yet know.

@DavidAnson
Copy link

Not sure you need receipts, but here's some supporting evidence:

node@7ed5037cb4c0:/home/workdir$ npm ls cliui
workdir@ /home/workdir
`-- markdownlint-cli@0.34.0
  `-- glob@10.2.2
    `-- jackspeak@2.1.4
      `-- cliui@8.0.1 (git+ssh://git@github.com/isaacs/cliui.git#9f97090165675fdda63a79c29bc36bb1033506b0)
npm http fetch GET 200 https://registry.npmjs.org/color-name 44ms (cache revalidated)
npm http fetch GET 200 https://codeload.github.com/isaacs/cliui/tar.gz/9f97090165675fdda63a79c29bc36bb1033506b0 445ms (cache revalidated)
npm verb stack Error: git dep preparation failed
npm verb stack     at ChildProcess.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/@npmcli/promise-spawn/lib/index.js:53:27)

@isaacs
Copy link
Owner

isaacs commented May 2, 2023

I can roll it back, but what's the problem with git deps? npm has supported git deps for ages, I've used them in published packages lots of times.

@isaacs
Copy link
Owner

isaacs commented May 2, 2023

I'll just publish it as a normal npm fork for now I guess. But cliui failing prepare seems like an npm bug to me.

@richallanb
Copy link
Author

I can roll it back, but what's the problem with git deps? npm has supported git deps for ages, I've used them in published packages lots of times.

@isaacs Yeah I think you're right. This actually worked perfectly fine in npm 6.

@6XGate
Copy link

6XGate commented May 2, 2023

It seems there is a circular dependencies between cliui -> rimraf -> glob -> jackspeak -> cliui

I started having this issue with npm-check-updates

@isaacs
Copy link
Owner

isaacs commented May 2, 2023

@6XGate one issue is enough, no need to repost this on everything that uses glob. (Everything uses glob, you'll be busy for some time.)

@isaacs
Copy link
Owner

isaacs commented May 2, 2023

But yes, the cycle is what I was missing. It can't build cliui without rimraf, which needs glob, which needs jackspeak, which needs cliui, so it tries to run the prepare before it has what it needs.

@6XGate
Copy link

6XGate commented May 2, 2023

It's weird you can install cliui without issue by itself.

@isaacs
Copy link
Owner

isaacs commented May 2, 2023

It's weird you can install cliui without issue by itself.

That's because cyclical deps are normally fine, npm handles them no problem. And in fact, they're usually fine when installing deps from git, as well.

Because a git repository typically doesn't have built artifacts, in order to install a git dep, npm has to do a little bit of extra work. It clones the repo, installs its dev dependencies, and then runs the prepare and prepublish scripts, the same as it would when publishing, so that the end result is the same sort of tarball artifact that you'd get from the registry.

Unfortunately, in this case, cliui depends on rimraf, and uses the rimraf bin in its prepare script. Rimraf depends on glob, which depends on jackspeak, which depends on that same git version of cliui.

npm usually is fairly clever about building dependencies before their dependents. However, once it encounters a cycle, it has to pick something, and the algorithm it uses it "upon a cycle, don't continue to recurse, just hope for the best". It's easy to see with our human brains that if it had chosen to build rimraf prior to building cliui, it would have worked, but what if rimraf had required glob to do its build? Then choosing to build rimraf first would have failed in the same way.

Bootstrap problems are tricky.

Anyway, published a new jackspeak that depends on @isaacs/cliui instead of the git fork. Whenever they resolve that string wrapping issue in esm, I'll deprecate it and move back to using main.

@isaacs isaacs closed this as completed May 2, 2023
Repository owner deleted a comment from Sweetog May 2, 2023
@ljharb
Copy link

ljharb commented May 2, 2023

Thanks for fixing; fyi the use of aliases also is still breaking installation of my dev dep that's pulling this in.

ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue May 3, 2023
ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue May 3, 2023
ljharb added a commit to michaelkirk/tape that referenced this issue Jun 15, 2023
ljharb added a commit to ljharb/npm-lockfile that referenced this issue Jun 21, 2023
ljharb added a commit to inspect-js/object-inspect that referenced this issue Jul 4, 2023
ljharb added a commit to inspect-js/object-inspect that referenced this issue Jul 5, 2023
ljharb added a commit to ljharb/shell-quote that referenced this issue Jul 26, 2023
ljharb added a commit to bmish/eslint-plugin-import that referenced this issue Aug 9, 2023
ljharb added a commit to jsx-eslint/eslint-plugin-jsx-a11y that referenced this issue Aug 11, 2023
ljharb added a commit to tape-testing/tape that referenced this issue Sep 21, 2023
ljharb added a commit to inspect-js/object-inspect that referenced this issue Oct 14, 2023
ljharb added a commit to inspect-js/object-inspect that referenced this issue Oct 14, 2023
ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue Nov 3, 2023
ljharb added a commit to import-js/eslint-plugin-import that referenced this issue Dec 19, 2023
ljharb added a commit to import-js/eslint-plugin-import that referenced this issue Dec 19, 2023
ljharb added a commit to ljharb/qs that referenced this issue Feb 28, 2024
ljharb added a commit to ljharb/es-abstract that referenced this issue Feb 28, 2024
…pectively depend on npm aliases, which kill the install process in npm < 6

See isaacs/jackspeak#4
ljharb added a commit to es-shims/es-arraybuffer-base64 that referenced this issue Feb 29, 2024
…pectively depend on npm aliases, which kill the install process in npm < 6

See isaacs/jackspeak#4
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

No branches or pull requests

5 participants