Add comments

kwankyu · Nov 20, 2024 · 7da5efe · 7da5efe
1 parent 39ebbe4
commit 7da5efe
Show file tree

Hide file tree

Showing 2 changed files with 120 additions and 71 deletions.
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
@@ -163,63 +163,80 @@ jobs:
           remove-docker-images: true
         continue-on-error: true
         if: inputs.free_disk_space
+
       - name: Check out SageMath
         uses: actions/checkout@v4
         with:
           repository: ${{ inputs.sage_repo }}
           ref: ${{ inputs.sage_ref }}
           fetch-depth: 10000
+
       - name: Download upstream artifact
         uses: actions/download-artifact@v4
         with:
           path: upstream
           name: ${{ inputs.upstream_artifact }}
         if: inputs.upstream_artifact
+
       - name: Install test prerequisites
         run: |
           sudo DEBIAN_FRONTEND=noninteractive apt-get update
           sudo DEBIAN_FRONTEND=noninteractive apt-get install tox
           sudo apt-get clean
           df -h
+
       - name: Update Sage packages from upstream artifact
-        # Handle both the old and new location of write-dockerfile.sh,
-        # because docker.yml is a reusable workflow.
         run: |
-          (export PATH=$(pwd)/build/bin:$PATH; (cd upstream && bash -x update-pkgs.sh) && sed -i.bak '/upstream/d' .dockerignore; for a in build/bin/write-dockerfile.sh .ci/write-dockerfile.sh; do if [ -r $a ]; then echo "/:toolchain:/i ADD upstream upstream" | sed -i.bak -f - $a; fi; done; git diff)
+          export PATH=$(pwd)/build/bin:$PATH
+          cd upstream && bash -x update-pkgs.sh
+          # Remove any lines containing 'upstream' from .dockerignore
+          sed -i.bak '/upstream/d' .dockerignore
+          # Handle both the old and new location of write-dockerfile.sh, because docker.yml is a reusable workflow
+          for script_path in build/bin/write-dockerfile.sh .ci/write-dockerfile.sh; do
+            if [ -r "$script_path" ]; then
+              # Add 'ADD upstream upstream' before the line containing ':toolchain:' in the script
+              echo "/:toolchain:/i ADD upstream upstream" | sed -i.bak -f - "$script_path"
+            fi
+          done
+          # Show the changes made to the repository for review
+          git diff
         if: inputs.upstream_artifact
 
       - name: Try to login to ghcr.io
         if: inputs.docker_push_repository != ''
         # https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable
         run: |
-            TOKEN="${{ secrets.DOCKER_PKG_GITHUB_TOKEN }}"
-            if [ -z "$TOKEN" ]; then
-              TOKEN="${{ secrets.GITHUB_TOKEN }}"
-            fi
-            if echo "$TOKEN" | docker login ghcr.io -u ${{ github.actor }} --password-stdin; then
-              echo "DOCKER_PUSH_REPOSITORY=$(echo ${{ inputs.docker_push_repository }} | tr "[:upper:]" "[:lower:]")" >> $GITHUB_ENV
-              echo "DOCKER_CONFIG_FILE=$HOME/.docker/config.json" >> $GITHUB_ENV
-            fi
+          TOKEN="${{ secrets.DOCKER_PKG_GITHUB_TOKEN }}"
+          if [ -z "$TOKEN" ]; then
+            TOKEN="${{ secrets.GITHUB_TOKEN }}"
+          fi
+          if echo "$TOKEN" | docker login ghcr.io -u ${{ github.actor }} --password-stdin; then
+            echo "DOCKER_PUSH_REPOSITORY=$(echo ${{ inputs.docker_push_repository }} | tr "[:upper:]" "[:lower:]")" >> $GITHUB_ENV
+            echo "DOCKER_CONFIG_FILE=$HOME/.docker/config.json" >> $GITHUB_ENV
+          fi
 
       - name: Determine Docker tags to use
         run: |
-            # This line needs to be run before the step "Merge CI fixes from sagemath/sage".
-            DOCKER_TAG="$(git describe --dirty --always)"
-            echo "DOCKER_TAG=$DOCKER_TAG" >> $GITHUB_ENV
-            # From the docker documentation via .ci/update-env.sh:
-            # "A tag name must be valid ASCII and may
-            # contain lowercase and uppercase letters, digits, underscores, periods and
-            # dashes. A tag name may not start with a period or a dash and may contain a
-            # maximum of 128 characters."
-            EXTRA_DOCKER_TAGS=`echo $GITHUB_REF_NAME | tr -d '[:space:]' | tr -c '[:alnum:]_.-' '-' | sed 's/^[-.]*//' | cut -c1-128`
-            shopt -s extglob
-            case "$GITHUB_REF_NAME" in
-              +([0-9]).+([0-9])?(.+([0-9])) )
-                EXTRA_DOCKER_TAGS="latest dev $EXTRA_DOCKER_TAGS";;
-              +([0-9]).+([0-9])?(.+([0-9]))?(.)@(a|alpha|b|beta|rc)+([0-9]) )
-                EXTRA_DOCKER_TAGS="dev $EXTRA_DOCKER_TAGS";;
-            esac
-            echo "EXTRA_DOCKER_TAGS=$EXTRA_DOCKER_TAGS" >> $GITHUB_ENV
+          # This line needs to be run before the step "Merge CI fixes from sagemath/sage".
+          #
+          DOCKER_TAG="$(git describe --dirty --always)"
+          echo "DOCKER_TAG=$DOCKER_TAG" >> $GITHUB_ENV
+          #
+          # From the docker documentation via .ci/update-env.sh:
+          # "A tag name must be valid ASCII and may
+          # contain lowercase and uppercase letters, digits, underscores, periods and
+          # dashes. A tag name may not start with a period or a dash and may contain a
+          # maximum of 128 characters."
+          #
+          EXTRA_DOCKER_TAGS=`echo $GITHUB_REF_NAME | tr -d '[:space:]' | tr -c '[:alnum:]_.-' '-' | sed 's/^[-.]*//' | cut -c1-128`
+          shopt -s extglob
+          case "$GITHUB_REF_NAME" in
+            +([0-9]).+([0-9])?(.+([0-9])) )
+              EXTRA_DOCKER_TAGS="latest dev $EXTRA_DOCKER_TAGS";;
+            +([0-9]).+([0-9])?(.+([0-9]))?(.)@(a|alpha|b|beta|rc)+([0-9]) )
+              EXTRA_DOCKER_TAGS="dev $EXTRA_DOCKER_TAGS";;
+          esac
+          echo "EXTRA_DOCKER_TAGS=$EXTRA_DOCKER_TAGS" >> $GITHUB_ENV
 
       - name: Merge CI fixes from sagemath/sage
         # This step needs to happen after the commit sha is put in DOCKER_TAG
@@ -234,42 +251,67 @@ jobs:
         run: |
           df -h
         if: inputs.free_disk_space
+
       - name: Configure and build Sage distribution within a Docker container
-        # The first command below is a self-destruct sequence,
-        # which preempts the GitHub Actions 6-hour job cancellation.
-        #
-        # Using "docker exec", we enter the temporary containers used by
-        # "docker build" and kill the "make" processes of the Sage distribution.
-        #
-        # The arcane "find" command is a replacement for "pkill make",
-        # which we use because pkill is not installed in the "minimal" package
-        # configuration on many platforms.
-        #
-        # The "sed" command strips away timestamps from "docker build" (buildkit)
-        # such as "#25 1211.0" at the beginning of each line. The timestamps are
-        # redundant because GH Actions provides timestamps for each line already.
-        # Stripping the timestamps from the beginnings of lines also allows
-        # GitHub Actions to recognize workflow commands such as ::error etc.
-        #
         run: |
-          (sleep ${{ inputs.timeout }}; for id in $(docker ps -q); do docker exec $id find /proc -maxdepth 2 -name cmdline -exec bash -c "grep -l [m][a][k][e] {} | cut -d/ -f3 | xargs --no-run-if-empty kill" \;; done) &
-          set -o pipefail; EXTRA_DOCKER_BUILD_ARGS="--build-arg NUMPROC=9 --build-arg USE_MAKEFLAGS=\"-k V=0 SAGE_NUM_THREADS=5\"" tox -e $TOX_ENV -- $TARGETS 2>&1 | sed -E --unbuffered "s/^#[0-9]+ [0-9]+[.][0-9]+ //;/^configure: notice:/s|^|::warning file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|;/^configure: warning:/s|^|::warning file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|;/^configure: error:/s|^|::error file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|;"
+          # The first command below is a self-destruct sequence,
+          # which preempts the GitHub Actions 6-hour job cancellation.
+          #
+          # Using "docker exec", we enter the temporary containers used by
+          # "docker build" and kill the "make" processes of the Sage distribution.
+          (
+            sleep ${{ inputs.timeout }}
+            for container_id in $(docker ps -q); do
+              #
+              # The arcane "find" command is a replacement for "pkill make",
+              # which we use because pkill is not installed in the "minimal" package
+              # configuration on many platforms.
+              #
+              docker exec "$container_id" find /proc -maxdepth 2 -name cmdline \
+                -exec bash -c 'grep -l "[m][a][k][e]" {} | cut -d/ -f3 | xargs --no-run-if-empty kill' \;
+            done
+          ) &
+          # Set pipeline to fail if any command fails
+          set -o pipefail
+          #
+          EXTRA_DOCKER_BUILD_ARGS="--build-arg NUMPROC=9 --build-arg USE_MAKEFLAGS=\"-k V=0 SAGE_NUM_THREADS=5\""
+          #
+          # The tox command starts to build and test Sage within a Docker container.
+          #
+          # Adter the build, the sed command strips away timestamps from
+          # "docker build" (buildkit) such as "#25 1211.0" at the beginning of
+          # each line. The timestamps are redundant because GH Actions provides
+          # timestamps for each line already. Stripping the timestamps from
+          # the beginnings of lines also allows GitHub Actions to recognize
+          # workflow commands such as ::error etc. The other sed commands
+          # annotate configuration notices/warnings/errors as GitHub
+          # ::warning/::warning/::error respectively.
+          #
+          tox -e $TOX_ENV -- $TARGETS 2>&1 | sed -E --unbuffered "
+            s/^#[0-9]+ [0-9]+[.][0-9]+ //
+            /^configure: notice:/s|^|::warning file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|
+            /^configure: warning:/s|^|::warning file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|
+            /^configure: error:/s|^|::error file=artifacts/$LOGS_ARTIFACT_NAME/config.log::|
+          "
       - name: Copy logs from the Docker image or build container
         run: |
           mkdir -p "artifacts/$LOGS_ARTIFACT_NAME"
           cp -r .tox/$TOX_ENV/* "artifacts/$LOGS_ARTIFACT_NAME"
           rm -rf "artifacts/$LOGS_ARTIFACT_NAME"/{bin,lib,pyvenv.cfg}
         if: always()
+
       - uses: actions/upload-artifact@v4
         with:
           path: artifacts
           name: ${{ env.LOGS_ARTIFACT_NAME }}
         if: always()
+
       - name: Print out logs for immediate inspection
         # and markup the output with GitHub Actions logging commands
         run: |
           .github/workflows/scan-logs.sh "artifacts/$LOGS_ARTIFACT_NAME"
         if: always()
+
       - name: List Docker images
         run: |
           if [ -n "$DOCKER_PUSH_REPOSITORY" -a -f .tox/$TOX_ENV/Dockertags.pushed ]; then

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
@@ -187,20 +187,29 @@ $(foreach pkgname,$(PIP_PACKAGES),\
 $(SAGE_LOCAL)/$(SPKG_INST_RELDIR)/.dummy:
 	touch $@
 
-
-# Filtered by installation tree
+# Define variables
+#
+# SAGE_LOCAL_INSTALLED_PACKAGE_INSTS
+# SAGE_VENV_INSTALLED_PACKAGE_INSTS
+# SAGE_DOCS_INSTALLED_PACKAGE_INSTS
+#
+# SAGE_LOCAL_UNINSTALLED_PACKAGE_UNINSTALLS
+# SAGE_VENV_UNINSTALLED_PACKAGE_UNINSTALLS
+# SAGE_DOCS_UNINSTALLED_PACKAGE_UNINSTALLS
+#
+# that list targets for packages to be installed/uninstalled into installation
+# tree SAGE_LOCAL/SAGE_VENV/SAGE_DOCS
 $(foreach tree,SAGE_LOCAL SAGE_VENV SAGE_DOCS, \
     $(eval $(tree)_INSTALLED_PACKAGE_INSTS = \
                $(foreach pkgname,$(INSTALLED_PACKAGES), \
                          $(if $(findstring $(tree),$(trees_$(pkgname))), \
                               $(inst_$(pkgname))))) \
     $(eval $(tree)_UNINSTALLED_PACKAGE_UNINSTALLS = \
-               $(foreach pkgname,$(INSTALLED_PACKAGES), \
+               $(foreach pkgname,$(UNINSTALLED_PACKAGES), \
                          $(if $(findstring $(tree),$(trees_$(pkgname))), \
-                              $(inst_$(pkgname))))))
-
+                              $($(pkgname)-uninstall)))))
 
-###############################################################################
+# ==============================================================================
 
 # Silent rules
 # https://www.gnu.org/software/automake/manual/html_node/Automake-Silent-Rules.html
@@ -277,8 +286,8 @@ base-toolchain: _clean-broken-gcc base
 
 # All targets except for the base packages and except the documentation
 all-sage: \
-		$(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_UNINSTALLS) \
-		$(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
+	$(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_UNINSTALLS) \
+	$(SAGE_VENV_INSTALLED_PACKAGE_INSTS) $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 # Same but filtered by installation trees:
 all-build-local: toolchain-deps
@@ -289,12 +298,12 @@ all-sage-local: $(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_P
 all-build-venv: toolchain-deps
 	+$(MAKE_REC) all-sage-venv
 
-all-sage-venv:  $(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
+all-sage-venv: $(SAGE_VENV_INSTALLED_PACKAGE_INSTS) $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 all-build-docs: toolchain-deps
 	+$(MAKE_REC) all-sage-docs
 
-all-sage-docs:  $(SAGE_DOCS_INSTALLED_PACKAGE_INSTS) $(SAGE_DOCS_UNINSTALLED_PACKAGES_UNINSTALLS)
+all-sage-docs: $(SAGE_DOCS_INSTALLED_PACKAGE_INSTS) $(SAGE_DOCS_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 # Download all packages which should be inside an sdist tarball (the -B
 # option to make forces all targets to be built unconditionally)
@@ -343,17 +352,15 @@ SAGERUNTIME = sagelib $(inst_ipython) $(inst_pexpect)
 all-sageruntime: toolchain-deps
 	+$(MAKE_REC) $(SAGERUNTIME)
 
-
-###############################################################################
+# ==============================================================================
 # Building the base system
 #
 # This consists of packages which are required for the Sage build system.
-###############################################################################
+# ==============================================================================
 base: $(inst_patch) $(inst_pkgconf)
 
-###############################################################################
+# ==============================================================================
 # Building the documentation
-###############################################################################
 
 # You can choose to have the built HTML version of the documentation link to
 # the PDF version. To do so, you need to build both the HTML and PDF versions.
@@ -365,7 +372,7 @@ base: $(inst_patch) $(inst_pkgconf)
 # For more information on the docbuild utility, do
 #
 # $ ./sage --docbuild -H
-
+# ==============================================================================
 doc: doc-html
 
 # All doc-building is delegated to the script packages
@@ -481,9 +488,9 @@ wheels:
 
 pypi-wheels-check: $(PYPI_WHEEL_PACKAGES:%=%-check)
 
-#==============================================================================
+# ==============================================================================
 # Setting SAGE_CHECK... variables
-#==============================================================================
+# ==============================================================================
 ifeq "$(origin SAGE_CHECK)" "undefined"
 SAGE_CHECK := no
 endif
@@ -525,10 +532,9 @@ $(foreach clause, $(SAGE_CHECK_PACKAGES_sep),					\
 debug-check:
 	@echo $(foreach pkgname, $(NORMAL_PACKAGES) $(SCRIPT_PACKAGES), SAGE_CHECK_$(pkgname) = $(SAGE_CHECK_$(pkgname)))
 
-
-#==============================================================================
+# ==============================================================================
 # Rules generated from pkgs/<package>/dependencies files
-#==============================================================================
+# ==============================================================================
 
 # Define a function for generating the list of a package's dependencies
 # as $(inst_<pkgname>) variables.  For example, takes:
@@ -595,7 +601,7 @@ pkg_deps = \
 # sage-spkg, and --keep-files to sage-spkg-uninstall since those packages can
 # have a recursive self-dependency, and should not be deleted while upgrading.
 # See Issue #25857
-
+#
 # Positional arguments:
 #     $(1): package name
 #     $(2): package version
@@ -657,7 +663,7 @@ $(foreach pkgname, $(NORMAL_PACKAGES),\
 	                                   $(call pkg_deps,$(pkgname)),$(tree)))))
 endif
 
-# ================================ pip packages ===============================
+# ================================ pip packages ====================================
 # Generate build rules for 'pip' packages; this template is used to generate
 # two rules in the form:
 #
@@ -666,10 +672,11 @@ endif
 #
 # <pkgname>-uninstall:
 #     -sage --pip uninstall -y ...
-
+#
 # Positional arguments:
 #     $(1): package name
 #     $(2): package dependencies
+
 define PIP_PACKAGE_templ
 $(1)-build-deps: $(2)
 
@@ -719,7 +726,7 @@ endif
 #         . '$SAGE_ROOT/build/bin/sage-build-env-config' && \\
 #         . '$SAGE_ROOT/build/bin/sage-build-env' && \\
 #         '$SAGE_ROOT/build/pkgs/$PKG_NAME/spkg-uninstall'
-
+#
 # Positional arguments:
 #     $(1): package name
 #     $(2): package version