From a6b548ff534b5d9697d386817b0572bfa3e13074 Mon Sep 17 00:00:00 2001 From: Reid Priedhorsky <1682574+reidpr@users.noreply.github.com> Date: Tue, 19 Jan 2021 16:46:07 -0700 Subject: [PATCH] PR #949: fix "sudo make install" with non-trivial umask (also closes #598) --- .github/PERUSEME | 5 ++++ .github/workflows/main.yml | 19 +++++++++++-- configure.ac | 20 ++++++++++++++ doc/Makefile.am | 56 ++++++++++++++++++++++++-------------- test/Makefile.am | 2 +- 5 files changed, 77 insertions(+), 25 deletions(-) diff --git a/.github/PERUSEME b/.github/PERUSEME index 1837659e6..d59b06a35 100644 --- a/.github/PERUSEME +++ b/.github/PERUSEME @@ -57,6 +57,11 @@ Miscellaneous notes and gotchas: [1]: https://github.com/actions/virtual-environments/blob/ubuntu20/20201116.1/images/linux/Ubuntu2004-README.md + * The default shell (Bash) does not read any init files [1], so you cannot + configure it with e.g. .bashrc. + + [1]: https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell + * GitHub doesn’t seem to notice our setup if .github is a symlink. :( * GitHub seems to want us to encapsulate some of the steps that are now just diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 15ed0ccd8..ca933d4eb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,6 +44,15 @@ jobs: done echo "path_new=$path_new" echo "PATH=$path_new" >> $GITHUB_ENV + # Set sudo umask to something quite restrictive. The default is + # 0022, but we had a "make install" bug (issue #947) that was + # tickled by 0027, which is a better setting. For reasons I don't + # understand, this only affects sudo, but since that's what we want, + # I left it. Note there are a few dependency installs below that + # have similar permissions bugs; these relax the umask on a + # case-by-case basis. + sudo sed -i -E 's/^UMASK\s+022/UMASK 0077/' /etc/login.defs + fgrep UMASK /etc/login.defs - name: print starting environment run: | @@ -51,6 +60,10 @@ jobs: env | egrep '^(ch|CH)_' [[ $PATH != */usr/local/sbin* ]] # verify sbin removal; see above id + printf 'umask for %s: ' $USER && umask + printf 'umask under sudo: ' && sudo sh -c umask + [[ $(umask) = 0022 ]] + [[ $(sudo sh -c umask) = 0077 ]] pwd getconf _NPROCESSORS_ONLN free -m @@ -70,7 +83,7 @@ jobs: cd /usr/local/src git clone --depth 1 --branch v0.4.0 https://github.com/sstephenson/bats.git cd bats - sudo ./install.sh /usr/local + sudo sh -c 'umask 0022 && ./install.sh /usr/local' command -v bats bats --version [[ $(command -v bats) == /usr/local/bin/bats ]] @@ -83,14 +96,14 @@ jobs: # configure does tell us about these. sudo apt-get install squashfs-tools squashfuse # Track newest Sphinx in case it breaks things. - sudo pip3 install sphinx sphinx-rtd-theme + sudo su -c 'umask 0022 && pip3 install sphinx sphinx-rtd-theme' - name: install/configure dependencies, ch-image if: ${{ matrix.builder == 'ch-image' }} run: | # Use most current Lark rather than the one in Ubuntu b/c new # versions sometimes break things. - sudo pip3 install lark-parser + sudo su -c 'umask 0022 && pip3 install lark-parser' - name: install/configure dependencies, all Buildah if: ${{ startsWith(matrix.builder, 'buildah') }} diff --git a/configure.ac b/configure.ac index b4c925227..d1a9d7f66 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +21,26 @@ AS_CASE([$host_os], [*], [AC_MSG_ERROR([Linux is the only supported OS; see issue @%:@42.])] ) +# By default, Autotools honors umask for directories but not files. Thus, if +# you "sudo make install" with a umask more restrictive than 0022, the result +# is an installation unavailable to most users (issue #947). This appears to +# be a somewhat common complaint. +# +# Our workaround is to set the "mkdir -p" command [1]. (Note those +# instructions also mention a different variable ac_cv_path_mkdir, but I +# couldn't figure out how to set it.) This needs to be before AM_INIT_AUTOMAKE +# because that macro does something with the value. We use "install -d" rather +# than "mkdir -m" because the latter still uses only umask for intermediate +# directories [2]. +# +# This can still be overridden on the configure command line; for example, to +# restore the previous behavior, use "./configure MKDIR_P='mkdir -p'" [3]. +# +# [1]: https://unix.stackexchange.com/a/436000 +# [2]: http://gnu-automake.7480.n7.nabble.com/bug-12130-sudo-make-install-applies-umask-to-new-directories-tp18545p18548.html +# [3]: https://lists.gnu.org/archive/html/automake/2004-01/msg00013.html +MKDIR_P=${MKDIR_P:-install -d -m 0755} + AM_INIT_AUTOMAKE([1.13 -Wall -Werror foreign subdir-objects]) AC_CONFIG_HEADERS([bin/config.h]) diff --git a/doc/Makefile.am b/doc/Makefile.am index 50e0d612a..f53594f46 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -3,6 +3,21 @@ # Automake-ification, I stripped out most of the boilderplate and left only # the targets that we use. +# We turn off parallel build in doc: +# +# 1. Sphinx handles building the whole documentation internally already, as +# a unit, so we shouldn't call sphinx-build more than once for different +# output files at all, let alone in parallel. +# +# 2. Serial build is plenty fast. +# +# 3. There is a race condition in Sphinx < 1.6.6 that's triggered when two +# instances (e.g., for html and man targets) try to "mkdir doctrees" +# simultaneously. See issue #115. +# +# This special target was introduced in GNU Make 3.79, in April 2000. +.NOTPARALLEL: + EXTRA_DIST = \ bugs.rst \ charliecloud.rst \ @@ -137,37 +152,36 @@ BUILDDIR = . # Internal variables. ALLSPHINXOPTS = -d $(BUILDDIR)/doctrees $(SPHINXOPTS) . -# This target works around a race condition in Sphinx that's triggered when -# two instances (e.g., for html and man targets) try to "mkdir doctrees" -# simultaneously. It is temporary and can be removed when Sphinx >= 1.6.6 is -# an appropriate dependency. See issue #115. -.PHONY: mkdir_issue115 - mkdir -p $(BUILDDIR)/doctrees - _deps.rst: ../config.log make-deps-overview cat $< | ./make-deps-overview > $@ -# This called "html-local" because Automake has a Texinfo target called "html" -# and gripes if we override it. -$(nobase_html_DATA): html-local -html-local: mkdir_issue115 ../lib/version.txt _deps.rst +# Since we're not doing anything in parallel anyway, just put the HTML and the +# man pages in the same target, with conditionals. Gotchas: +# +# 1. If we build both, the HTML needs to go first otherwise it doesn't get +# curly quotes. ¯\_(ツ)_/¯ +# +# 2. This not a "grouped target" but rather an "independent target" [1], +# because the former came in GNU Make 4.3 which is quite new. However it +# does seem to get run only once. +# +# [1]: https://www.gnu.org/software/make/manual/html_node/Multiple-Targets.html +$(nobase_html_DATA) $(man_MANS): ../lib/version.txt _deps.rst $(EXTRA_DIST) +if ENABLE_HTML $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html -# avoid GitHub messing things up with Jekyll +# Avoid GitHub messing things up with Jekyll. touch html/.nojekyll -# remove unused files that Sphinx made +# Some output files are copies with same timestamp as source; fix. Note +# we need all the HTML output files, not just the one picked in $@. + touch --no-create $(nobase_html_DATA) +# remove unused files that Sphinx made rm -f $(BUILDDIR)/html/_deps.html \ $(BUILDDIR)/html/charliecloud.html \ $(BUILDDIR)/html/ch-*.html \ $(BUILDDIR)/html/bugs.html \ $(BUILDDIR)/html/objects.inv \ $(BUILDDIR)/html/see_also.html - -# FIXME: If Sphinx builds the man pages first, the HTML docs don't get curly -# quotes. ¯\_(ツ)_/¯ Force HTML to go first. -if ENABLE_HTML -HTML_FIRST = html endif - -$(man_MANS): man -man: mkdir_issue115 ../lib/version.txt _deps.rst $(HTML_FIRST) +if ENABLE_MAN $(SPHINXBUILD) -b man $(ALLSPHINXOPTS) $(BUILDDIR)/man +endif diff --git a/test/Makefile.am b/test/Makefile.am index 166cd5c42..735784b62 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -72,7 +72,7 @@ all-local: clean-local: rm -f fixtures/symlink-to-tmp install-data-hook: - mkdir -p $(DESTDIR)$(testdir)/fixtures + $(MKDIR_P) $(DESTDIR)$(testdir)/fixtures ln -fTs /tmp $(DESTDIR)$(testdir)/fixtures/symlink-to-tmp uninstall-hook: rm -f $(DESTDIR)$(testdir)/fixtures/symlink-to-tmp