PR #949: fix "sudo make install" with non-trivial umask (also closes #…

…598)

.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

.github/workflows/main.yml

@@ -44,13 +44,26 @@ 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: |
           env | egrep '^(PATH|USER)='
           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') }}

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])

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

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