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.

Sign up for GitHub

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

Jump to bottom

This repositories devcontainer setup deletes all your other devcontainers #53129

Closed
alcuadrado opened this issue May 24, 2024 · 2 comments · Fixed by #53137
Closed

This repositories devcontainer setup deletes all your other devcontainers #53129

alcuadrado opened this issue May 24, 2024 · 2 comments · Fixed by #53137
Labels
meta Issues and PRs related to the general management of the project.

Comments

@alcuadrado
Copy link

If you clone this repository in a devcontainer, it will delete all your other devcontainers.

This is extremely bad, as it ruins your development environment. This may not be as problematic for people who use this repository often, but it is for the casual contributor who uses devcontainer.

I don't know why docker system prune -f -a" was added as initializeCommand in #40825, but I think it should be removed.

(Sorry for not using an issue template. This is not related to node itself, but rather to its repository)
@alcuadrado alcuadrado mentioned this issue May 24, 2024
Closed
@mattaereal
Copy link

mattaereal commented May 24, 2024 •
edited

This is bad practice, the initializeCommand runs on your host machine.

From containers.dev documentation
initializeCommand:

A command string or list of command arguments to run on the host machine during initialization, including during container creation and on subsequent starts. The command may run more than once during a given session.

⚠️ The command is run wherever the source code is located on the host. For cloud services, this is in the cloud.

Note that the array syntax will execute the command without a shell. You can learn more about formatting string vs array vs object properties.

I don't know the specific reason why they do this but generally is due to the need to avoid polluting the docker system with new builds every time a new change is deployed. Nevertheless, this will undoubtedly destroy entirely all your other containers :/

Edited to fix a typo.

@RedYetiDev RedYetiDev added the meta Issues and PRs related to the general management of the project. label May 24, 2024
@RedYetiDev
Copy link
Member

CC @bnb (Added the file)

@RedYetiDev RedYetiDev mentioned this issue May 24, 2024
Merged
alcuadrado added a commit to alcuadrado/node that referenced this issue May 24, 2024
@alcuadrado
Remove initializeCommand from devcontainer.json
3b8bb77 
Fixes nodejs#53129
@alcuadrado alcuadrado mentioned this issue May 24, 2024
Closed
nodejs-github-bot pushed a commit that referenced this issue May 26, 2024
@RedYetiDev
meta: remove initializeCommand from devcontainer
dc609f5 
PR-URL: #53137
Fixes: #53129
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jun 1, 2024
@RedYetiDev @targos
meta: remove initializeCommand from devcontainer
e079967 
PR-URL: #53137
Fixes: #53129
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this issue Jun 1, 2024
@RedYetiDev @lukins-cz
meta: remove initializeCommand from devcontainer
34169f4 
PR-URL: nodejs#53137
Fixes: nodejs#53129
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
sophoniie pushed a commit to sophoniie/node that referenced this issue Jun 20, 2024
@RedYetiDev @sophoniie
meta: remove initializeCommand from devcontainer
657baf3 
PR-URL: nodejs#53137
Fixes: nodejs#53129
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
@RedYetiDev @bmeck
meta: remove initializeCommand from devcontainer
c9774aa 
PR-URL: nodejs#53137
Fixes: nodejs#53129
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
3 participants
@alcuadrado @mattaereal @RedYetiDev