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

No need to chown. Use npm cache. #3

Merged
merged 3 commits into from
Aug 1, 2024
Merged

No need to chown. Use npm cache. #3

merged 3 commits into from
Aug 1, 2024

Conversation

ygerasimov
Copy link
Contributor

Addresses comments from #2

Comment on lines 22 to 23
- DDEV_UID
- DDEV_GID

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two variables are also no longer needed inside the container. (These were used for the post-start hook only.)

Suggested change
- DDEV_UID
- DDEV_GID

@stasadev
Copy link

@ygerasimov before merging this:

I tested this add-on and prepared a patch that fixes the open issues.

git checkout 2-npm-cache-chown
wget https://github.com/user-attachments/files/16445076/diffy.patch.txt
git apply diffy.patch.txt
rm -f diffy.patch.txt

diffy.patch.txt

@rfay
Copy link

rfay commented Jul 31, 2024

Just a note here... If using ddev-global-cache, then the npm operations do in fact need to run with the same UID as the host-side user. So my suggestion is invalid if they don't run with that UID. The username doesn't matter.

@ygerasimov ygerasimov merged commit fd3ed9b into main Aug 1, 2024
0 of 2 checks passed
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.

3 participants