Yarn should automatically install node-gyp if needed

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
The bundled .js version of Yarn (which is now used in all the other installers) doesn't include node-gyp, resulting in errors like this:

error C:\temp\license-test\node_modules\libalac: Command failed.
Exit code: 1
Command: C:\WINDOWS\system32\cmd.exe
Arguments: /d /s /c node-gyp rebuild
Directory: C:\temp\license-test\node_modules\libalac
Output:
'node-gyp' is not recognized as an internal or external command,
operable program or batch file.

This was already an issue with the standalone .js build, but it'll be an issue with other builds now that we use the standalone .js file everywhere.

Rather than bundling node-gyp with Yarn (like we did before), we should probably run yarn global add node-gyp to install it when needed.

Repro:

yarn init
yarn add nodetunes

@bestander

@bestander - Looks like we do indeed need node-gyp and my original testing (where it worked fine) was wrong somehow. To me, the best solution is to run yarn global add node-gyp automatically if needed. What do you think?

Let's get this fixed before the next RC.

Yeah, let's do this.

…

On Wed, 12 Apr 2017 at 06:24, Daniel Lo Nigro ***@***.***> wrote: @bestander <https://github.com/bestander> - Looks like we do indeed need node-gyp and my original testing (where it worked fine) was wrong somehow. To me, the best solution is to run yarn global add node-gyp automatically if needed. What do you think? Let's get this fixed before the next RC. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACBdWNT8ZWW9FztJjpPJWGF22RD9W4fHks5rvGAlgaJpZM4M69ya> .

By the way, I think I worked out why the other package I tested worked with no error: If a package declares node-gyp as a dependency, it should work fine. It's just packages that need node-gyp but don't declare it as a dependency (ie. they implicitly expect it to be available) that break.

…

-- Regards, Daniel Lo Nigro http://dan.cx/ | Twitter <http://twitter.com/Daniel15> | Facebook <http://www.facebook.com/daaniel> On Wed, Apr 12, 2017 at 12:16 AM, Konstantin Raev <notifications@github.com> wrote:

Yeah, let's do this. On Wed, 12 Apr 2017 at 06:24, Daniel Lo Nigro ***@***.***> wrote: > @bestander <https://github.com/bestander> - Looks like we do indeed need > node-gyp and my original testing (where it worked fine) was wrong > somehow. To me, the best solution is to run yarn global add node-gyp > automatically if needed. What do you think? > > Let's get this fixed before the next RC. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#3114 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ ACBdWNT8ZWW9FztJjpPJWGF22RD9W4fHks5rvGAlgaJpZM4M69ya> > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFnHT0Z4ZnGJ40iwC9z-xr8s2VvTYbCks5rvHpjgaJpZM4M69ya> .

My only concern if users will be prompted for elevates permissions when installing node-gyp globally. On Wed, 12 Apr 2017 at 08:18, Daniel Lo Nigro <notifications@github.com> wrote:

…

By the way, I think I worked out why the other package I tested worked with no error: If a package declares node-gyp as a dependency, it should work fine. It's just packages that need node-gyp but don't declare it as a dependency (ie. they implicitly expect it to be available) that break. -- Regards, Daniel Lo Nigro http://dan.cx/ | Twitter <http://twitter.com/Daniel15> | Facebook <http://www.facebook.com/daaniel> On Wed, Apr 12, 2017 at 12:16 AM, Konstantin Raev < ***@***.***> wrote: > Yeah, let's do this. > > > On Wed, 12 Apr 2017 at 06:24, Daniel Lo Nigro ***@***.***> > wrote: > > > @bestander <https://github.com/bestander> - Looks like we do indeed need > > node-gyp and my original testing (where it worked fine) was wrong > > somehow. To me, the best solution is to run yarn global add node-gyp > > automatically if needed. What do you think? > > > > Let's get this fixed before the next RC. > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > <#3114 (comment)>, > or mute > > the thread > > <https://github.com/notifications/unsubscribe-auth/ > ACBdWNT8ZWW9FztJjpPJWGF22RD9W4fHks5rvGAlgaJpZM4M69ya> > > . > > > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#3114 (comment)>, or mute > the thread > < https://github.com/notifications/unsubscribe-auth/AAFnHT0Z4ZnGJ40iwC9z-xr8s2VvTYbCks5rvHpjgaJpZM4M69ya > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACBdWF7pJOKReGxRr6IqwFe-IItYijjSks5rvHrggaJpZM4M69ya> .

Elevation shouldn't be needed as global packages should always be installed in a directory they're able to access. I know there's currently a bug on Windows where it tries to install global packages into the Program Files\Node.js directory, but that's a bug that needs to be fixed :) Sent from my phone.

…

On Apr 12, 2017 2:37 AM, "Konstantin Raev" ***@***.***> wrote: My only concern if users will be prompted for elevates permissions when installing node-gyp globally. On Wed, 12 Apr 2017 at 08:18, Daniel Lo Nigro ***@***.***> wrote: > By the way, I think I worked out why the other package I tested worked with > no error: If a package declares node-gyp as a dependency, it should work > fine. It's just packages that need node-gyp but don't declare it as a > dependency (ie. they implicitly expect it to be available) that break. > > -- > Regards, > Daniel Lo Nigro > http://dan.cx/ | Twitter <http://twitter.com/Daniel15> | Facebook > <http://www.facebook.com/daaniel> > > On Wed, Apr 12, 2017 at 12:16 AM, Konstantin Raev < > ***@***.***> > wrote: > > > Yeah, let's do this. > > > > > > On Wed, 12 Apr 2017 at 06:24, Daniel Lo Nigro < ***@***.***> > > wrote: > > > > > @bestander <https://github.com/bestander> - Looks like we do indeed > need > > > node-gyp and my original testing (where it worked fine) was wrong > > > somehow. To me, the best solution is to run yarn global add node-gyp > > > automatically if needed. What do you think? > > > > > > Let's get this fixed before the next RC. > > > > > > — > > > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > > > <#3114 (comment) >, > > or mute > > > the thread > > > <https://github.com/notifications/unsubscribe-auth/ > > ACBdWNT8ZWW9FztJjpPJWGF22RD9W4fHks5rvGAlgaJpZM4M69ya> > > > . > > > > > > > — > > You are receiving this because you authored the thread. > > Reply to this email directly, view it on GitHub > > <#3114 (comment)>, > or mute > > the thread > > < > https://github.com/notifications/unsubscribe-auth/AAFnHT0Z4ZnGJ40iwC9z- xr8s2VvTYbCks5rvHpjgaJpZM4M69ya > > > > . > > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#3114 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe- auth/ACBdWF7pJOKReGxRr6IqwFe-IItYijjSks5rvHrggaJpZM4M69ya> > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFnHRDPjGkQHndru_R5WEOfXfEBLcmPks5rvJtvgaJpZM4M69ya> .

What about when people need to install node-gyp globally for another reason? Could it interfere with Yarn's needs? What if node-gyp has breaking changes and we need to upgrade it? I feel uncomfortable with pieces of Yarn being in isolated global places.

…

On 12 April 2017 at 16:23, Daniel Lo Nigro ***@***.***> wrote: Elevation shouldn't be needed as global packages should always be installed in a directory they're able to access. I know there's currently a bug on Windows where it tries to install global packages into the Program Files\Node.js directory, but that's a bug that needs to be fixed :) Sent from my phone. On Apr 12, 2017 2:37 AM, "Konstantin Raev" ***@***.***> wrote: > My only concern if users will be prompted for elevates permissions when > installing node-gyp globally. > > On Wed, 12 Apr 2017 at 08:18, Daniel Lo Nigro ***@***.***> > wrote: > > > By the way, I think I worked out why the other package I tested worked > with > > no error: If a package declares node-gyp as a dependency, it should work > > fine. It's just packages that need node-gyp but don't declare it as a > > dependency (ie. they implicitly expect it to be available) that break. > > > > -- > > Regards, > > Daniel Lo Nigro > > http://dan.cx/ | Twitter <http://twitter.com/Daniel15> | Facebook > > <http://www.facebook.com/daaniel> > > > > On Wed, Apr 12, 2017 at 12:16 AM, Konstantin Raev < > > ***@***.***> > > wrote: > > > > > Yeah, let's do this. > > > > > > > > > On Wed, 12 Apr 2017 at 06:24, Daniel Lo Nigro < > ***@***.***> > > > wrote: > > > > > > > @bestander <https://github.com/bestander> - Looks like we do indeed > > need > > > > node-gyp and my original testing (where it worked fine) was wrong > > > > somehow. To me, the best solution is to run yarn global add node-gyp > > > > automatically if needed. What do you think? > > > > > > > > Let's get this fixed before the next RC. > > > > > > > > — > > > > You are receiving this because you were mentioned. > > > > Reply to this email directly, view it on GitHub > > > > <#3114 (comment)- 293477171 > >, > > > or mute > > > > the thread > > > > <https://github.com/notifications/unsubscribe-auth/ > > > ACBdWNT8ZWW9FztJjpPJWGF22RD9W4fHks5rvGAlgaJpZM4M69ya> > > > > . > > > > > > > > > > — > > > You are receiving this because you authored the thread. > > > Reply to this email directly, view it on GitHub > > > <#3114 (comment) >, > > or mute > > > the thread > > > < > > https://github.com/notifications/unsubscribe- auth/AAFnHT0Z4ZnGJ40iwC9z- > xr8s2VvTYbCks5rvHpjgaJpZM4M69ya > > > > > > . > > > > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > <#3114 (comment)>, > or mute > > the thread > > <https://github.com/notifications/unsubscribe- > auth/ACBdWF7pJOKReGxRr6IqwFe-IItYijjSks5rvHrggaJpZM4M69ya> > > . > > > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#3114 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AAFnHRDPjGkQHndru_ R5WEOfXfEBLcmPks5rvJtvgaJpZM4M69ya> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACBdWNiD2E0LOnWhIDbNU-K8xAD1NUxLks5rvOxvgaJpZM4M69ya> .

Navigation Menu

Repository navigation

Yarn should automatically install node-gyp if needed #3114

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Yarn should automatically install node-gyp if needed #3114

Description

Activity

Daniel15 commented on Apr 12, 2017

bestander commented on Apr 12, 2017

Daniel15 commented on Apr 12, 2017

bestander commented on Apr 12, 2017

Daniel15 commented on Apr 12, 2017

bestander commented on Apr 12, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions