-
Notifications
You must be signed in to change notification settings - Fork 349
[stable-v2.2] cherry-pick docker and other upgrade from main branch #7551
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
Changes from all commits
26d632a
4d33ae4
c02aa82
475f193
f0715da
252eda4
834387c
be64732
ac9a487
06eb7fc
e32deb4
6fadce3
a15fcde
75f6ae2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,12 +25,16 @@ if tty --quiet; then | |
| SOF_DOCKER_RUN="$SOF_DOCKER_RUN --tty" | ||
| fi | ||
|
|
||
| # Not fatal, just a warning to allow other "creative" solutions. | ||
| # TODO: fix this with 'adduser' like in zephyr/docker-build.sh | ||
| test "$(id -n)" = 1001 || | ||
| >&2 printf "Warning: this script should be run as user ID 1001 to match the container\n" | ||
| # The --user option below can cause the command to run as a user who | ||
| # does not exist in the container. So far so good but in case something | ||
| # ever goes wrong try replacing --user with the newer | ||
| # scripts/sudo-cwd.sh script. | ||
| test "$(id -u)" = 1000 || | ||
| >&2 printf "Warning: this script should be run as user ID 1000 to match the container's account\n" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, what changed the user from 1001 to 1000?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bug fix, I got the ID wrong. See commit message. |
||
|
|
||
| set -x | ||
| # FIXME: During the transition to sudo-cwd.sh, the tag will be "latest_ubuntu22.04". | ||
| # Later it will be back to latest | ||
| docker run -i -v "${SOF_TOP}":/home/sof/work/sof.git \ | ||
| -v "${SOF_TOP}":/home/sof/work/sof-bind-mount-DO-NOT-DELETE \ | ||
| --env CMAKE_BUILD_TYPE \ | ||
|
|
@@ -40,6 +44,5 @@ docker run -i -v "${SOF_TOP}":/home/sof/work/sof.git \ | |
| --env VERBOSE \ | ||
| --env http_proxy="$http_proxy" \ | ||
| --env https_proxy="$https_proxy" \ | ||
| --user "$(id -u)" \ | ||
| $SOF_DOCKER_RUN \ | ||
| thesofproject/sof "$@" | ||
| thesofproject/sof:latest_ubuntu22.04 ./scripts/sudo-cwd.sh "$@" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| #!/bin/sh | ||
| # SPDX-License-Identifier: BSD-3-Clause | ||
| # Copyright(c) 2022 Intel Corporation. All rights reserved. | ||
|
|
||
| # This is a "brute force" solution to filesystem permission issues: | ||
| # | ||
| # If the current user does not own the current directory then this | ||
| # wrapper script switches to the user who does own the current directory | ||
| # before running the given command. | ||
|
|
||
| # If no user owns the current directory, a user who does gets created | ||
| # first! | ||
|
|
||
| # The main use case is to run this first thing inside a container to | ||
| # solve file ownership mismatches. | ||
|
|
||
| # `docker run --user=$(id -un) ...` achieves something very similar | ||
| # without any code except the resulting user many not exist inside the | ||
| # container. Some commands may not like that. | ||
| # | ||
| # To understand more about the Docker problem solved here take a look at | ||
| # https://stackoverflow.com/questions/35291520/docker-and-userns-remap-how-to-manage-volume-permissions-to-share-data-betwee | ||
| # and many other similar questions. | ||
|
|
||
| # TODO: replace sudo with gosu? | ||
|
|
||
| set -e | ||
| set -x | ||
|
|
||
| main() | ||
| { | ||
| cwd_uid="$(stat --printf='%u' .)" | ||
| local current_uid; current_uid="$(id -u)" | ||
| if test "$current_uid" = "$cwd_uid"; then | ||
| exec "$@" | ||
| else | ||
| exec_as_cwd_uid "$@" | ||
| fi | ||
| } | ||
|
|
||
| exec_as_cwd_uid() | ||
| { | ||
| # If missing, add new user owning the current directory | ||
| local cwd_user; cwd_user="$(id "$cwd_uid")" || { | ||
| cwd_user='cwd_user' | ||
|
|
||
| local cwd_guid; cwd_guid="$(stat --printf='%g' .)" | ||
|
|
||
| getent group "$cwd_guid" || | ||
| sudo groupadd -g "$cwd_guid" 'cwd_group' | ||
|
|
||
| sudo useradd -m -u "$cwd_uid" -g "$cwd_guid" "$cwd_user" | ||
|
|
||
| local current_user; current_user="$(id -un)" | ||
|
|
||
| # Copy sudo permissions just in case the build needs it | ||
| if test -e /etc/sudoers.d/"$current_user"; then | ||
| sudo sed -e "s/$current_user/$cwd_user/" /etc/sudoers.d/"$current_user" | | ||
| sudo tee -a /etc/sudoers.d/"$cwd_user" | ||
| sudo chmod --reference=/etc/sudoers.d/"$current_user" \ | ||
| /etc/sudoers.d/"$cwd_user" | ||
| fi | ||
| } | ||
|
|
||
| # Double sudo to work around some funny restriction in | ||
| # zephyr-build:/etc/sudoers: 'user' can do anything but... only as | ||
| # root. | ||
| # Passing empty http[s]_proxy is OK | ||
| # shellcheck disable=SC2154 | ||
| sudo sudo -u "$cwd_user" REAL_CC="$REAL_CC" \ | ||
| http_proxy="$http_proxy" https_proxy="$https_proxy" \ | ||
| "$@" | ||
|
|
||
| exit "$?" | ||
| } | ||
|
|
||
| main "$@" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,7 +52,7 @@ exec_as_sof_uid() | |
| # Double sudo to work around some funny restriction in | ||
| # zephyr-build:/etc/sudoers: 'user' can do anything but... only as | ||
| # root. | ||
| sudo sudo -u "$sof_user" "$0" "$@" | ||
| sudo sudo -u "$sof_user" http_proxy="$http_proxy" https_proxy="$https_proxy" "$0" "$@" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this all works and has been tested, but where does $http_proxy and $https_proxy come from here?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a shell script so they come from your personal environment (and they're optional) |
||
| exit "$?" | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are explicitly changing the behavior here? Changing from the "moving target" of "latest" to a hard-coded version? Does that mean someone has to make sure to keep this up-to-date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes because this is a stable branch, see commit message.