-
Notifications
You must be signed in to change notification settings - Fork 92
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
Refactor to avoid unnecessary multi-stage build #109
Conversation
This takes advantage of `apk add --virtual` to install packages temporarily so they can be cleanly/easily removed after the build is complete. It also cleans up / avoids extra files being created by Yarn / Node.js during build.
I'd be happy to also remove |
I got again the error |
How are you building? If you |
For example, if you build test via (Doing |
I used Maybe it's safe to leave the |
I was never getting |
I'm using Podman instead of Docker. Can the error be related to a git setting? |
Edit -- Oh, sorry, I misspoke, the copy is on the host machine. Maybe it is a git setting? But I don't think there is a risk of this impacting others if it's passing the test on the official image repo right? |
Or maybe is related to the OS (I'm using Windows 11 + WSL) |
I think WSL should work as long as you are not trying to build from an NTFS path. I think WSL reads all files as 777 (rwxrwxrwx) and I'm not sure what git would do in that case. What do you see when you do an ls -l? If you are using an NTFS path, can you try a path that isn't? |
I have files (Dockerfile, git cloned repos etc) on Windows and then use a Podman client (it communicates with Podman installed in a WSL distro (podman-machine-default)) to use Podman commands in command line. containers/podman#15299 it's the same problem? Edit: another issue containers/podman#18093 |
If you cloned from windows then I'm pretty sure the permissions are not going to work. Can you try to clone from the command line in your WSL home folder? Permissions should work correctly there. You should be able to see what I mean if you do ls -l on /mnt/c/ vs $HOME -- everything in /mnt/c is a translated filesystem and all files will show 777. At least that's the way it worked the last time I had tried WSL, it was probably over a year ago. I haven't had access to a Windows device in a while. |
@rtritto do you think it's okay to merge this as-is? Or would you prefer that we keep the |
We should probably keep PR's small if possible, I can make a new one for that after this gets merged. I am curious why alpine3.17 isn't working though... it should still be supported right? |
Inside the WSL distro, I did git clone + podman build + podman run and it works.
Yes, this PR is LGTM. The Can these warnings during build be ignored? ...
+ apk del --no-network .me-fetch-deps
WARNING: opening from cache https://dl-cdn.alpinelinux.org/alpine/v3.18/main: No such file or directory
WARNING: opening from cache https://dl-cdn.alpinelinux.org/alpine/v3.18/community: No such file or directory
... |
I think so. It looks like it's just a warning that the cache for these repos cannot be found, but that's expected since we always install with |
That was dropped by |
This takes advantage of
apk add --virtual
to install packages temporarily so they can be cleanly/easily removed after the build is complete.It also cleans up / avoids extra files being created by Yarn / Node.js during build.