Repository navigation
gh-159452: Run the regen-configure container as the invoking user - #159454
Conversation
Tools/build/regen-configure.sh ran the autoconf container as root, so it left a root-owned autom4te.cache/ in the source tree. The invoking user could neither write into it (making a later `make autoconf` fail with "cannot open autom4te.cache/requests: Permission denied") nor delete it (making `git clean -xdf` warn for every file in it). Run the container as the invoking user, and purge autom4te.cache before and after the run from inside the container, so leftovers owned by root are also removed without needing sudo.
itamaro
left a comment
itamaro
left a comment
There was a problem hiding this comment.
thanks for the fix!
as is, this breaks rootless podman/docker. see inline comments.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again |
|
The |
clicked re-run |
itamaro
added
needs backport to 3.14
|
Thanks @sendaoYan for the PR, and @itamaro for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15. |
|
GH-161680 is a backport of this pull request to the 3.15 branch. |
|
GH-161681 is a backport of this pull request to the 3.14 branch. |
…ser (GH-159454) (#161681) gh-159452: Run the regen-configure container as the invoking user (GH-159454) `Tools/build/regen-configure.sh` ran the Autoconf container as root, so with a rootful Docker daemon the regenerated files and `autom4te.cache` ended up owned by root. The stale cache broke subsequent runs and couldn't be removed without sudo. Run the container as the invoking user instead: `--userns=keep-id` for Podman, `-u UID:GID` for rootful Docker. Rootless runtimes already map container root to the invoking user and need nothing. Also remove `autom4te.cache` before and after the run, using the same image, so an existing root-owned cache gets cleaned up too. (cherry picked from commit 7b1bc41) Co-authored-by: Sendao Yan <yansendao@126.com>
…ser (GH-159454) (#161680) gh-159452: Run the regen-configure container as the invoking user (GH-159454) `Tools/build/regen-configure.sh` ran the Autoconf container as root, so with a rootful Docker daemon the regenerated files and `autom4te.cache` ended up owned by root. The stale cache broke subsequent runs and couldn't be removed without sudo. Run the container as the invoking user instead: `--userns=keep-id` for Podman, `-u UID:GID` for rootful Docker. Rootless runtimes already map container root to the invoking user and need nothing. Also remove `autom4te.cache` before and after the run, using the same image, so an existing root-owned cache gets cleaned up too. (cherry picked from commit 7b1bc41) Co-authored-by: Sendao Yan <yansendao@126.com>
|
Thanks for the review and merge! |
Summary
Tools/build/regen-configure.sh ran the Autoconf container as root, so Autoconf left a root-owned autom4te.cache/ in the source tree. Being a directory owned by root and mode 0755, it cannot be written to or removed by the invoking user, which breaks make autoconf afterwards (autom4te: error: cannot open autom4te.cache/requests: Permission denied) and makes git clean -xdf fail with warning: failed to remove autom4te.cache/...: Permission denied. Recovering needs sudo, which not every developer has.
Changes
Run the container as the invoking user (-u "$(id -u):$(id -g)"), so the regenerated configure, aclocal.m4 and pyconfig.h.in are not left owned by root either.
Remove $WORK_DIR/autom4te.cache before the run and from an EXIT trap, running the cleanup inside the container so that a stale cache left by an earlier root run can be removed without sudo.
The cleanup reuses the same image via --entrypoint sh, so no extra dependency (e.g. busybox) is pulled in.
Why the cache cannot simply be rm'd from the script
The invoking user has no write permission on a root-owned autom4te.cache/, so it has to be deleted as root — i.e. from inside the container. Doing it before the run also matters: a stale root-owned cache makes even the corrected, non-root run fail immediately.
Why the cache cannot simply be rm'd from the script
The invoking user has no write permission on a root-owned autom4te.cache/, so it has to be deleted as root — i.e. from inside the container. Doing it before the run also matters: a stale root-owned cache makes even the corrected, non-root run fail immediately.
issue: #159452