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

build: fix yarn errors upon npm install minidump #95

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jimbly
Copy link

@Jimbly Jimbly commented Dec 12, 2024

  • Only run git submodule update when running out of a git working copy
  • Don't require yarn for end-user installation (or any installation, since it's not listed as a dep)

Closes #93

This fixes the following 3 errors:

development without yarn installed globally

$ git clone [email protected]:electron/node-minidump.git && cd node-minidump
$ npm install
> [email protected] preinstall
> yarn submodule && node build.js

sh: 1: yarn: not found

end-user installing of minidump (or any package depending on it) without yarn installed locally or globally

$ mkdir newproj && cd newproj && echo {} > package.json
$ npm i minidump
npm ERR! command failed
npm ERR! command sh -c yarn submodule && node build.js
npm ERR! sh: 1: yarn: not found

end-user installing of minidump (or any package depending on it) in a non-git-controlled working directory:

$ mkdir newproj && cd newproj && echo {} > package.json
$ npm i yarn
$ npm i minidump
npm ERR! command failed
npm ERR! command sh -c yarn submodule && node build.js
npm ERR! yarn run v1.22.22
npm ERR! $ git submodule update --init --recursive
npm ERR! info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
npm ERR! warning ../../package.json: No license field
npm ERR! fatal: not a git repository (or any of the parent directories): .git

Note: simply reverting 4be7737 will solve the last two end-user problems, presumably a situation like the first one was what that commit was attempting to solve (and it did solve it, as long as you have yarn installed globally, which, as far as I know, is not generally recommended), hopefully my addition to build.js will solve more elegantly solve the development case without needing to add external (non-dev) dependencies.

@Jimbly Jimbly requested a review from a team as a code owner December 12, 2024 20:33
- Only run `git submodule update` when running out of a git working copy
- Don't require `yarn` for end-user installation
@Jimbly
Copy link
Author

Jimbly commented Dec 12, 2024

Updated (and force-pushed) to remove trailing command that was causing a style check failure.

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.

Missing yarn dependency?
2 participants