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

Fixes undefined 'USER_ID' usage #12

Closed
wants to merge 1 commit into from

Conversation

vpetersson
Copy link

@vpetersson vpetersson commented Nov 26, 2021

The variable $USER_ID was defined but then in the actual Docker run, only USER_ID was passed on, which broke the script.

(Also includes a minor white space cleanup)

@lmbarros
Copy link

Hi @vpetersson, thanks for looking into this. I believe, though, that the script is correct at that point. As per the Docker docs, "If the operator names an environment variable without specifying a value, then the current value of the named variable is propagated into the container’s environment".

@vpetersson
Copy link
Author

Hmm, it didn't work like that for me. Before this fix, I got the following:

Building tool, please wait...
[LOG]Unwraping balena image from the provided balena image flasher...
[LOG]Changing file permissions.
chown: cannot access '': No such file or directory

This was on:

$ docker --version
Docker version 20.10.7, build 20.10.7-0ubuntu5~20.04.2

@lmbarros
Copy link

The user ID is getting to the script running on the container. The error happens because the file name is missing; we are running something like chown 1234:1234 "", hence the "cannot access ''" message. With your change, I believe the USER_ID variable will be unset in the container. The error vanishes because of the if [ ! -z "$USER_ID" ] right before the point where we call chown.

(Now, of course you have hit a bug, which I believe is for different reasons: #13).

@vpetersson
Copy link
Author

I stand corrected. You're right @lmbarros. I wasn't aware of this feature. Good one!

@vpetersson vpetersson closed this Nov 30, 2021
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.

2 participants