Remove problematic comments between package name in apt install #3060
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
hmm... still I got another error because of insufficient permissions. error |
musaprg
force-pushed
the
fix-devcontainer-issue
branch
from
0a7daa4 to
8de549f
Compare
|
I've tried the following configuration, but it didn't work. diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
index d2945b19..60ad6e20 100644
--- a/.devcontainer/devcontainer.json
+++ b/.devcontainer/devcontainer.json
@@ -31,5 +31,8 @@
],
"build": {
"dockerfile": "Dockerfile"
- }
+ },
+ "remoteUser": "vscode",
+ "containerUser": "vscode",
+ "postAttachCommand": "sudo chown -R vscode /workspaces"
}
\ No newline at end of file |
|
The permission problem was resolved by changing |
YJDoc2
added
the
kind/cleanup
|
Hey, so the the comments need to be removed that is correct. I am personally fine with simply removing the comments without splitting the command into two, a small issue with it that it might generate two docker image layers instead of one, but should be ok either way. As for the perm error, that is sensible, because the apt install needs the root perms to install deps. However I think there is a way to do that without having to make the remote user as root, I'll try to check and get back. Thanks for noticing this and letting us know! |
|
Hey, So I simply removed the comments from the dockerfile apt command, and no changes in the devcontainer.json , it seems to work for me, the showed list of appropriate packages. Can you try this, and check if that and |
My change won't increase the total number of layes of the built image because they're inside a single |
|
Hey @musaprg , may I ask you to update the PR so that the comments are removed and no changes in dev container json? |
|
hmm... I'm not sure why the build of |
musaprg
force-pushed
the
fix-devcontainer-issue
branch
2 times, most recently
from
ab857db to
36d5ac2
Compare
|
@YJDoc2 Hi. I've reverted my change on |
|
CI is failing, but that should be fixed after #3086 is merged. |
Signed-off-by: Kotaro Inoue <k.musaino@gmail.com>
musaprg
force-pushed
the
fix-devcontainer-issue
branch
from
36d5ac2 to
0957dcf
Compare
YJDoc2
approved these changes
Mar 3, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes an issue where several packages are not installed properly because of syntax error by unintended comments.
Type of Change
Testing
just cleanandcargo cleanjust lintand check it completes properly.Related Issues
In devcontainer, we cannot build youki because of the following error.
Build error when executing `just lint`
After I looked into the container, I've found that required packages were not installed properly even though they're specified in the Dockerfile.
This seems probably because of the comments in the middle of the heredoc part.
execution result of the problematic part
The built image should be equivalent even after splitting package installation into two lines as they're in the single heredoc. This PR splits a heredoc part including installation commands so that we can easily see which packages would be intended to be installed for what.
Additional Context