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

docker build scripts assume user id to be 1000 (uid) #8261

Closed
SomberNight opened this issue Mar 18, 2023 · 6 comments · Fixed by #8267
Closed

docker build scripts assume user id to be 1000 (uid) #8261

SomberNight opened this issue Mar 18, 2023 · 6 comments · Fixed by #8267

Comments

@SomberNight
Copy link
Member

I recently realised that most of our build scripts only work when run with uid==1000.
I guess this just happens to be the case for everyone who cares to run them. :P

The issue is that we mount the local git clone with docker run -v and work on that directly:

docker run -it \
--name electrum-wine-builder-cont \
-v "$PROJECT_ROOT_OR_FRESHCLONE_ROOT":/opt/wine64/drive_c/electrum \

Inside the container, the local git clone will have numerically the same uid/gid owner.
The unix user inside the container is created via useradd, and this starts numbering uids from 1000.
RUN useradd --create-home --shell /bin/bash ${USER}

So due to the -v mount, the unix user inside the container needs matching user id with the host unix user, and due to the useradd, the container's unix user will have uid==1000, hence the host unix user also needs to have uid==1000.

I guess this is likely a regression from #7697.
Not clear how to fix cleanly.

related:
moby/moby#7198
https://stackoverflow.com/q/39397548
https://stackoverflow.com/a/45640469
coder/code-server#439

@accumulator
Copy link
Member

alternative solution: use uid from user building the containers as the build container's uid

https://github.com/accumulator/electrum/tree/build_uid_from

@accumulator
Copy link
Member

Note: Might be problematic w.r.t reproducibility

SomberNight added a commit to SomberNight/electrum that referenced this issue Mar 20, 2023
- repro builds to use fixed uid=1000 inside the container
  - in case the file permissions leak into the binaries, they are still reproducible
  - chown 1000:1000 fresh_clone
- repro builds to create fresh_clone dir outside git clone
  - otherwise the local dev build would still interact with the fresh_clone dir
    - due to e.g. recursive "find -exec touch",
    - and even the "docker build" cmd itself would try to stat/read it
      - see docker/for-linux#380
  - and "rm -rf fresh_clone" needs sudo if the host uid is not 1000
  - this way the local dev build does not need sudo

to recap:
- local dev builds use the host userid inside the container, directly operate on the project dir
  - does not need sudo
- repro builds create a fresh git clone, chown it to 1000, and use userid=1000 inside the container
  - if the host userid is 1000, does not need sudo
  - otherwise, needs sudo

closes spesmilo#8261
SomberNight added a commit to SomberNight/electrum that referenced this issue Mar 20, 2023
- repro builds to use fixed uid=1000 inside the container
  - in case the file permissions leak into the binaries, they are still reproducible
  - chown 1000:1000 fresh_clone
- repro builds to create fresh_clone dir outside git clone
  - otherwise the local dev build would still interact with the fresh_clone dir
    - due to e.g. recursive "find -exec touch",
    - and even the "docker build" cmd itself would try to stat/read it
      - see docker/for-linux#380
  - and "rm -rf fresh_clone" needs sudo if the host uid is not 1000
  - this way the local dev build does not need sudo

to recap:
- local dev builds use the host userid inside the container, directly operate on the project dir
  - does not need sudo
- repro builds create a fresh git clone, chown it to 1000, and use userid=1000 inside the container
  - if the host userid is 1000, does not need sudo
  - otherwise, needs sudo

closes spesmilo#8261
@SomberNight
Copy link
Member Author

use uid from user building the containers as the build container's uid

Great idea!

Might be problematic w.r.t reproducibility

Indeed. I was also thinking about that...

We can differentiate local dev builds and reproducible builds, and only use your idea in the former case though.
I cherry-picked your change and added some more changes, see #8267

@SomberNight
Copy link
Member Author

Btw, just as context, I bumped into this bug and want to fix it now as I have set up a server I would like to build on but am already using userid=1000 for other things.

SomberNight added a commit to SomberNight/electrum that referenced this issue Mar 20, 2023
- repro builds to use fixed uid=1000 inside the container
  - in case the file permissions leak into the binaries, they are still reproducible
  - chown 1000:1000 fresh_clone
- repro builds to create fresh_clone dir outside git clone
  - otherwise the local dev build would still interact with the fresh_clone dir
    - due to e.g. recursive "find -exec touch",
    - and even the "docker build" cmd itself would try to stat/read it
      - see docker/for-linux#380
  - and "rm -rf fresh_clone" needs sudo if the host uid is not 1000
  - this way the local dev build does not need sudo

to recap:
- local dev builds use the host userid inside the container, directly operate on the project dir
  - does not need sudo
- repro builds create a fresh git clone, chown it to 1000, and use userid=1000 inside the container
  - if the host userid is 1000, does not need sudo
  - otherwise, needs sudo

closes spesmilo#8261
@assorted-casuals
Copy link

not sure if related, but attempting a fresh build on (lubuntu 22.04) results in error.

current ID 1000,

$ ./build.sh                                                  
💬 INFO:  Found 12 CPUs, which we might use for building.
💬 INFO:  building docker image.
[+] Building 400.5s (10/14)                                   docker:default
 => [internal] load build definition from Dockerfile                    0.3s
 => => transferring dockerfile: 2.49kB                                  0.0s
 => [internal] load metadata for docker.io/library/debian:buster@sha25  6.1s
 => [internal] load .dockerignore                                       0.1s
 => => transferring context: 103B                                       0.0s 
 => [internal] load build context                                       0.1s 
 => => transferring context: 406B                                       0.0s 
 => [ 1/10] FROM docker.io/library/debian:buster@sha256:233c3bbc892229  0.0s 
 => CACHED [ 2/10] RUN apt update -qq > /dev/null && apt install -qq -  0.0s 
 => [ 3/10] COPY apt.sources.list /etc/apt/sources.list                 2.1s 
 => [ 4/10] COPY apt.preferences /etc/apt/preferences.d/snapshot        0.4s
 => [ 5/10] RUN apt-get update -q &&     apt-get install -qy --allow  385.1s
 => ERROR [ 6/10] RUN useradd --uid 0 --create-home --shell /bin/bash   6.0s 
------                                                                       
 > [ 6/10] RUN useradd --uid 0 --create-home --shell /bin/bash user:         
0.957 useradd: UID 0 is not unique                                           
------                                                                       
Dockerfile:81                                                                
--------------------
  79 |     ENV WORK_DIR="${HOME_DIR}/wspace" \
  80 |         PATH="${HOME_DIR}/.local/bin:${PATH}"
  81 | >>> RUN useradd --uid $UID --create-home --shell /bin/bash ${USER}
  82 |     RUN usermod -append --groups sudo ${USER}
  83 |     RUN echo "%sudo ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers
--------------------
ERROR: failed to solve: process "/bin/sh -c useradd --uid $UID --create-home --shell /bin/bash ${USER}" did not complete successfully: exit code: 4

@assorted-casuals
Copy link

assorted-casuals commented Nov 27, 2024

attempting again on fresh install of ubuntu 24, tag 4.5.8
seems to be fixed by adding

sudo chown -R 1000:1000 "$PROJECT_ROOT"

at line #59
https://github.com/spesmilo/electrum/blob/master/contrib/build-wine/build.sh

raised a new issue
#9323

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants