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

IsADirectoryError on first nodejs deploy #228

Closed
chr15m opened this issue Feb 21, 2022 · 8 comments · Fixed by #230
Closed

IsADirectoryError on first nodejs deploy #228

chr15m opened this issue Feb 21, 2022 · 8 comments · Fixed by #230
Assignees

Comments

@chr15m
Copy link
Contributor

chr15m commented Feb 21, 2022

On #195 and on #225 they mentioned this bug with first time nodejs deploys. For some reason the second time you deploy it works (I guess because the directory exists already).

sample-nodejs-app$ git push piku master
Counting objects: 24, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (18/18), done.
Writing objects: 100% (24/24), 6.40 KiB | 6.40 MiB/s, done.
Total 24 (delta 2), reused 23 (delta 2)
remote: -----> Creating app 'sample_nodejs_app'
remote: -----> Deploying app 'sample_nodejs_app'
remote: HEAD is now at 7efa857 Create dependabot.yml
remote: -----> Node app detected.
remote: -----> Checking requirements: ['nodejs', 'npm']
remote: [None, '/usr/bin/npm']
remote: -----> Checking requirements: ['node', 'npm']
remote: ['/usr/bin/node', '/usr/bin/npm']
remote: -----> Creating node_modules for 'sample_nodejs_app'
remote: -----> Checking requirements: ['nodeenv']
remote: [None]
remote: -----> Checking requirements: ['npm']
remote: ['/usr/bin/npm']
remote: -----> Running npm for 'sample_nodejs_app'
remote: npm WARN reify Removing non-directory /home/piku/.piku/apps/sample_nodejs_app/node_modules
remote: 
remote: added 54 packages, and audited 55 packages in 6s
remote: 
remote: 2 packages are looking for funding
remote:   run `npm fund` for details
remote: 
remote: found 0 vulnerabilities
remote: npm notice 
remote: npm notice New minor version of npm available! 8.1.2 -> 8.3.2
remote: npm notice Changelog: <https://github.com/npm/cli/releases/tag/v8.3.2>
remote: npm notice Run `npm install -g npm@8.3.2` to update!
remote: npm notice 
remote: Traceback (most recent call last):
remote:   File "/home/piku/piku.py", line 1530, in <module>
remote:     cli()
remote:   File "/usr/lib/python3/dist-packages/click/core.py", line 764, in __call__
remote:     return self.main(*args, **kwargs)
remote:   File "/usr/lib/python3/dist-packages/click/core.py", line 717, in main
remote:     rv = self.invoke(ctx)
remote:   File "/usr/lib/python3/dist-packages/click/core.py", line 1137, in invoke
remote:     return _process_result(sub_ctx.command.invoke(sub_ctx))
remote:   File "/usr/lib/python3/dist-packages/click/core.py", line 956, in invoke
remote:     return ctx.invoke(self.callback, **ctx.params)
remote:   File "/usr/lib/python3/dist-packages/click/core.py", line 555, in invoke
remote:     return callback(*args, **kwargs)
remote:   File "/home/piku/piku.py", line 1445, in cmd_git_hook
remote:     do_deploy(app, newrev=newrev)
remote:   File "/home/piku/piku.py", line 364, in do_deploy
remote:     settings.update(deploy_node(app, deltas))
remote:   File "/home/piku/piku.py", line 584, in deploy_node
remote:     unlink(node_path_tmp)
remote: IsADirectoryError: [Errno 21] Is a directory: '/home/piku/.piku/apps/sample_nodejs_app/node_modules'
To your_server:sample_nodejs_app
 * [new branch]      master -> master

The fix should be this:

Edit piku.py and changed line 584 to be:

            import shutil
            shutil.rmtree(node_path_tmp)
@chr15m chr15m added the bug label Feb 21, 2022
@chr15m chr15m self-assigned this Feb 21, 2022
@chr15m
Copy link
Contributor Author

chr15m commented Feb 24, 2022

Researching this some more, it looks like something changed in npm to start causing this (npm/cli#3669) however that appears to be a workaround because npm install was not respecting NPM_CONFIG_PREFIX which I have filed a bug against in npm/cli#4467. I am preparing a workaround for both issues and to try to normalize all of the node_modules path issues once and for all.

@chr15m
Copy link
Contributor Author

chr15m commented Feb 25, 2022

This will also stop #159 from happening again since the fix (incoming) gets rid of the symlinked node_modules hack.

@chr15m
Copy link
Contributor Author

chr15m commented Feb 25, 2022

Can reliably replicate this on node 16.14.0. Have bumped the version in piku/sample-nodejs-app to use as a test case.

@chr15m
Copy link
Contributor Author

chr15m commented Feb 25, 2022

@rcarmo I wonder if we should just install node_modules into the working directory. 🤔 Sometimes people (myself included) set their projects up to expect node_modules to be at the top level of their project and symlink to files in there etc. In this situation having it in envs/APPNAME is a bit counter intuitive. I have run into that issue before and I'm not sure the cleanest thing to do here.

Another option would be: 1. install into envs/APPNAME/node_modules but also 2. create a symlink in the apps directory by default and 3. have an option NODE_NO_MODULES_SYMLINK to remove the symlink if users don't want it.

chr15m added a commit to chr15m/piku that referenced this issue Feb 25, 2022
This fixes piku#228 whilst retaining the existing behaviour of symlinking node_modules to the actual location in envs/APPNAME.
@asheroto
Copy link

asheroto commented Jul 4, 2022

+1 vote

@chr15m
Copy link
Contributor Author

chr15m commented Jul 4, 2022

@asheroto this should be fixed already - did you see this bug recently?

@chr15m chr15m reopened this Jul 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

This issue is stale because it has been open for 90 days with no activity.

@github-actions github-actions bot added the stale label Oct 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

This issue was closed because it has been inactive for 30 days since being marked as stale.

@github-actions github-actions bot closed this as completed Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants