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

GetRequiredPlugins may hang if CWD contains large numbers of files or has circularities #2928

Open
Tracked by #11598
pgavlin opened this issue Jul 12, 2019 · 4 comments
Labels
area/language-host Runtime that executes user programs impact/performance Something is slower than expected kind/enhancement Improvements or new features language/javascript

Comments

@pgavlin
Copy link
Member

pgavlin commented Jul 12, 2019

When using the Node SDK, this call crawls the CWD looking for node_modules folders to find required plugins and SxS versions of @pulumi/pulumi. If the number of files rooted at the CWD is significant, this can cause the call to hang (potentially indefinitely if there are circularities in symlinks).

@ellismg
Copy link
Contributor

ellismg commented Jul 12, 2019

Symlinks aside (and that's something we should look into), I do wonder if instead of crawling every folder from the root, we should instead just look for a top level node_modules folder in the CWD of the language host and walk under just that. I think that gets us 99.9999% of the way there. GetRequiredPlugins is allowed to return an incomplete set of information, so if we do happen to miss something it would just mean the user would get an actionable error message later telling them to install a plugin.

@lukehoban lukehoban added this to the 0.26 milestone Jul 25, 2019
@ellismg ellismg removed the feature/q3 label Aug 5, 2019
@ellismg ellismg removed this from the 0.26 milestone Aug 5, 2019
@gsuess
Copy link

gsuess commented May 7, 2020

I am not sure if this is related, but I get it to hang on:

$ pulumi up --logtostderr -v=9
I0507 13:47:42.755386   28275 backend.go:410] found username for access token
I0507 13:47:43.697486   28275 backend.go:410] found username for access token
Previewing update (dev):
I0507 13:47:44.093878   28275 backend.go:410] found username for access token
I0507 13:47:44.093702   28275 backend.go:887] Stack dev being updated to version 1
I0507 13:47:44.476166   28275 plugins.go:76] gatherPluginsFromProgram(): gathering plugins from language host
I0507 13:47:44.476447   28275 plugins.go:426] GetPluginPath(language, nodejs, <nil>): found on $PATH /home/gsuess/.pulumi/bin/pulumi-language-nodejs
I0507 13:47:44.476491   28275 plugin.go:83] Launching plugin 'nodejs' from '/home/gsuess/.pulumi/bin/pulumi-language-nodejs' with args: 127.0.0.1:39367
I0507 13:47:44.553950   28275 langruntime_plugin.go:170] langhost[nodejs].GetPluginInfo() executing
I0507 13:47:44.554275   28275 langruntime_plugin.go:83] langhost[nodejs].GetRequiredPlugins(proj=leaderboard,pwd=/home/gsuess/meteor-leaderboard,program=.) executing
     Type                 Name             Plan     
     pulumi:pulumi:Stack  leaderboard-dev           
System Messages
  ^C received; cancelling. If you would like to terminate immediately, press ^C again.
  ^C received; terminating

You can reproduce this by running: EDIT: Turns out its not reproducible, without some of my uncommitted local files after all.

curl https://install.meteor.com/ | sh
meteor create --minimal mymeteor-app
cd mymeteor-app
meteor npm install @empirica/meteor-deploy
npx meteor-deploy init

Then:

pulumi stack select

Then:

npx meteor-deploy stack configure aws-ecs-ec2 --instanceType t2.medium
pulumi up --logtostderr -v=9

@gsuess
Copy link

gsuess commented May 8, 2020

It would be great if there at least was some explicit control of which plugins are to be installed. This implicit GetRequiredPlugins that does some very opinionated parsing of package.json files and directory structures only works on two assumptions:

  1. There is no new breaking change in the standard npm works.
  2. There is no unrelated work-in-progress stuff that causes it to come to wrong conclusions. (As it happened in my case)

So I am favour of simplifying it and remove this routine that is always going to be error-prone.

I have lost many hours trying to find the issue for this. It might not be a frequent issue, but when it does strike, it is severe in the sense that it is quite hard to troubleshoot.

It would be much nicer to just explictly list the needed plugins in the Pulumi.yaml. It would also be much faster.

@mikhailshilkov mikhailshilkov added area/cli UX of using the CLI (args, output, logs) kind/enhancement Improvements or new features impact/performance Something is slower than expected labels Apr 1, 2022
@ghost
Copy link

ghost commented Jun 3, 2022

I just got bit by this. I had an odd setup like this:

tree -F -P 'package.json' -L 3
./
├── package.json
├── projects/
│   ├── my_project/
│   │   ├── node_modules/
│   │   ├── package.json
└─── node_modules/

In the root package.json I’ve listed @pulumi/pulumi and @pulumi/aws as dependencies.

In the project package.json I’ve listed ../.. as a dependency.

When I run npm install in my_project it creates in node_modules a symlink to the root dir, which is of course circular.

pulumi preview or pulumi up therefore hang on GetRequiredPlugins (I confirmed this with -v 9).

I’m not much of an expert with npm so maybe I’m doing something wrong here, but whatever it is, if I’ve done it, then it’s likely that someone else will, at some point. So it’d be good if GetRequiredPlugins could be disabled, or could somehow detect that it’s (effectively) hung and abort.

@justinvp justinvp added language/javascript area/language-host Runtime that executes user programs and removed area/cli UX of using the CLI (args, output, logs) labels Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/language-host Runtime that executes user programs impact/performance Something is slower than expected kind/enhancement Improvements or new features language/javascript
Projects
None yet
Development

No branches or pull requests

7 participants