Lint Dockerfile and squash.Dockerfile #233
Conversation
Dockerfile
Outdated
|
|
||
| # Required for non-glvnd setups. | ||
| ENV LD_LIBRARY_PATH /usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}:/usr/local/nvidia/lib:/usr/local/nvidia/lib64 | ||
| ENV LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}:/usr/local/nvidia/lib:/usr/local/nvidia/lib64 |
There was a problem hiding this comment.
WARNING: UndefinedVar - https://docs.docker.com/go/dockerfile/rule/undefined-var/
Usage of undefined variable '$LD_LIBRARY_PATH'
https:/github.com/glennvl/zwift-linux.git#cac7aaf9ff4bcb7d6ef78c272ceada365af15896/Dockerfile:55
--------------------
53 |
54 | # Required for non-glvnd setups.
55 | >>> ENV LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}:/usr/local/nvidia/lib:/usr/local/nvidia/lib64
56 |
57 | COPY pulse-client.conf /etc/pulse/client.conf
--------------------
Does this mean LD_LIBRARY_PATH starts out empty and we should make sure the existing value is passed in correctly and updated?
Where exactly is this required? Maybe it should be moved to one of the scripts?
There was a problem hiding this comment.
To see what's going on, I tried:
RUN /bin/bash -c "[[ -v LD_LIBRARY_PATH ]] && echo \"LD_LIBRARY_PATH exists with value $LD_LIBRARY_PATH\" || echo \"LD_LIBRARY_PATH does not exist\""
# Required for non-glvnd setups.
ENV LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}:/usr/local/nvidia/lib:/usr/local/nvidia/lib64
RUN /bin/bash -c "[[ -v LD_LIBRARY_PATH ]] && echo \"LD_LIBRARY_PATH exists with value $LD_LIBRARY_PATH\" || echo \"LD_LIBRARY_PATH does not exist\""The output I get is:
[2/3] STEP 14/20: RUN /bin/bash -c "[[ -v LD_LIBRARY_PATH ]] && echo \"LD_LIBRARY_PATH exists with value $LD_LIBRARY_PATH\" || echo \"LD_LIBRARY_PATH does not exist\""
LD_LIBRARY_PATH does not exist
--> 84dd6b702086
[2/3] STEP 15/20: ENV LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}:/usr/local/nvidia/lib:/usr/local/nvidia/lib64
--> fe277e25422e
[2/3] STEP 16/20: RUN /bin/bash -c "[[ -v LD_LIBRARY_PATH ]] && echo \"LD_LIBRARY_PATH exists with value $LD_LIBRARY_PATH\" || echo \"LD_LIBRARY_PATH does not exist\""
LD_LIBRARY_PATH exists with value /usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu:/usr/local/nvidia/lib:/usr/local/nvidia/lib64
--> 776508a90d4b
So LD_LIBRARY_PATH does not exist yet in the image we are building. Does it then make sense to try to append to it? Maybe we can just do
ENV LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu:/usr/local/nvidia/lib:/usr/local/nvidia/lib64Or if the intention is to use the host LD_LIBRARY_PATH, we need to pass it as build argument:
ARG LD_LIBRARY_PATH
ENV LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu:/usr/local/nvidia/lib:/usr/local/nvidia/lib64
There was a problem hiding this comment.
Long time since I worked on that, so yeah, not sure if it's required as part of the dockerfile or if it could be embedded in a script or if its really required anymore. The landscape has changed quite a bit since then. Your guess is as good as mine unfortunately.
There was a problem hiding this comment.
For now, overwrite LD_LIBRARY_PATH in dde40dd since I can't tell for sure if it's needed. Docker doesn't know about environment variables in the host system, so LD_LIBRARY_PATH always starts out empty (it is not declared as en ARG and passed in as build argument).
| COPY --from=build-runfromprocess /usr/src/target/x86_64-pc-windows-gnu/release/runfromprocess-rs.exe /bin/runfromprocess-rs.exe | ||
| RUN chmod +rx /bin/runfromprocess-rs.exe | ||
|
|
||
| ENTRYPOINT ["entrypoint"] |
There was a problem hiding this comment.
WARNING: JSONArgsRecommended - https://docs.docker.com/go/dockerfile/rule/json-args-recommended/
JSON arguments recommended for CMD to prevent unintended behavior related to OS signals
https:/github.com/glennvl/zwift-linux.git#cac7aaf9ff4bcb7d6ef78c272ceada365af15896/Dockerfile:101
--------------------
99 |
100 | ENTRYPOINT ["entrypoint"]
101 | >>> CMD [$@]
102 |
--------------------
Does this mean CMD [$@] actually does nothing and we can just remove it since there's already an entrypoint anyway? I tried removing that line and everything worked as expected, also when passing extra stuff to the docker run command or when using a custom entrypoint.
From the limited documentation I can find, $@ does not appear to exist in Dockerfile context?
There was a problem hiding this comment.
Really can't remember where i got this from, but in bash this forwards all input arguments. So i guess it's supposed to have the same meaning in Dockerfile. But unsure if it has ever done anything, as i would assume that not declaring CMD at all would have the same effect when passing arguments.
So yes, I would assume one could remove it. Im pretty sure it's being used by one or two of the zwift workflows for git. so if they start to fail, then we know this was needed afterall :)
There was a problem hiding this comment.
Removed CMD [$@] in 9dd4fd8.
Iin dockerfile $@ does not exist. It attempts to read a variable with the name @ which is empty. So the CMD is also empty and does nothing.
It starts out empty in the image, so it is safe to overwrite and avoids warnings about the variable not existing.
2 participants
Since we're linting the nix flake and bash scripts, we should also lint the dockerfiles. Docker build has a
--checkoption which does the linting, so it is easy to add a github workflow using existing actions.There's still two warnings in the Dockerfile I did not touch because I'm not entirely sure what the current behaviour is given the warnings.