Skip to content

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