Skip to content

Commit

Permalink
Run docker as the current user (#1630)
Browse files Browse the repository at this point in the history
If you have real Docker (not Podman) then by default it will run everything as root, so your build directory is owned by root and the Makefile will not work at all because it tries to modify the build directory.

This adds a flag so that Asciidoctor runs as the current user in Docker instead of root.
  • Loading branch information
Timmmm committed Sep 10, 2024
1 parent 698e64a commit 3539eff
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,18 @@ ifneq ($(SKIP_DOCKER),true)
$(shell ! docker -v 2>&1 | grep podman >/dev/null ; echo $$?)
ifeq "$(DOCKER_IS_PODMAN)" "1"
DOCKER_VOL_SUFFIX = :z
else
# Real Docker requires this flag so that the files it creates
# are owned by the current user instead of root. Podman does
# that by default so this flag isn't necessary.
DOCKER_USER_ARG := --user $(shell id -u)
endif

DOCKER_CMD = \
docker run --rm \
-v ${PWD}/$@.workdir:/build${DOCKER_VOL_SUFFIX} \
-w /build \
$(DOCKER_USER_ARG) \
${DOCKER_IMG} \
/bin/sh -c
DOCKER_QUOTE := "
Expand Down

5 comments on commit 3539eff

@wmat
Copy link
Collaborator

@wmat wmat commented on 3539eff Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Timmmm any idea why this change breaks my ISA build on Fedora 40? I'm now getting numerous errors like the following:

asciidoctor: ERROR: images/bytefield/misareg.edn: line 2: Failed to generate image: Permission denied @ dir_s_mkdir - /build/images

Note that if I revert this change, the docs build fine.

@aswaterman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wmat Did you try a fresh checkout of the repo? (The directory might have already been created with the wrong permissions.) If that doesn't work, I'll leave it to you and @Timmmm to figure out.

@wmat
Copy link
Collaborator

@wmat wmat commented on 3539eff Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aswaterman yep, tried a fresh checkout and have the same problem.

@wmat
Copy link
Collaborator

@wmat wmat commented on 3539eff Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this change also broke the "Release new ISA when merging a PR" workflow as well, as far as I can tell, as it hasn't successfully ran since this change was applied.

@aswaterman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wmat I think you should revert the PR to unblock you and the release flow, then @Timmmm can resubmit a new version if he can figure out the problem.

Please sign in to comment.