Skip to content

Commit

Permalink
gh-38026: Do not create spkg-*.log, spkg-*.time files
Browse files Browse the repository at this point in the history
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes #12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes #12345". -->

- Clean up after #37391, which used `sage-logger` for prefixing output;
generating the log/time files was a side effect, which has not proven to
be useful
- Fixes #37887 @jhpalmieri

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - #12345: short description why this is a dependency -->
<!-- - #34567: ... -->

- Depends on #37840 (merged here)
    
URL: #38026
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
  • Loading branch information
Release Manager committed May 31, 2024
2 parents 2e4039c + bfb292a commit efc613a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 113 deletions.
47 changes: 35 additions & 12 deletions build/bin/sage-logger
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
#!/usr/bin/env bash
#
# sage-logger [-p] COMMAND LOGFILE
# sage-logger [-p] [-P PREFIX] COMMAND [LOGFILE]
#
# Evaluate shell command COMMAND while logging stdout and stderr to
# LOGFILE. If either the command or the logging failed, return a
# LOGFILE (if given). If either the command or the logging failed, return a
# non-zero exit status.
#
# If the -p argument is given, each line printed to stdout is prefixed
# with the name of the log file.
#
# If the -P PREFIX argument is given, each line printed to stdout is
# prefixed with PREFIX.
#
# AUTHOR:
#
# - Jeroen Demeyer (2015-07-26): initial version based on old pipestatus
Expand All @@ -24,22 +27,26 @@
#*****************************************************************************

use_prefix=false
prefix=""

if [[ "$1" = -p ]]; then
use_prefix=true
shift
fi
case "$1" in
-p) use_prefix=true
shift
;;
-P) use_prefix=true
prefix="[$2] "
shift 2
;;
esac

cmd="$1"
logfile="$2"
export SAGE_LOGFILE="$logfile"
logname="$(basename $logfile .log)"
logdir=`dirname "$logfile"`

if [[ $use_prefix = true ]]; then
if [[ $use_prefix = true && -z "$prefix" ]]; then
prefix="[${logname}] "
else
prefix=""
fi

# Use sed option to reduce buffering, to make the output appear more
Expand All @@ -61,15 +68,31 @@ else
SED=cat
fi

if [ -z "$logfile" ]; then
# Just prefix, nothing else
( exec 2>&1; sh -c "$cmd" ) | \
( trap '' SIGINT; if [ -n "$GITHUB_ACTIONS" -a -n "$prefix" ]; then echo "::group::${logname}"; fi; $SED; if [ -n "$GITHUB_ACTIONS" -a -n "$prefix" ]; then echo "::endgroup::"; fi )

pipestatus=(${PIPESTATUS[*]})

if [ ${pipestatus[1]} -ne 0 ]; then
exit ${pipestatus[1]}
else
exit ${pipestatus[0]}
fi
fi

# Remainder of the script is with logging to "$logfile" and timing.

timefile="$logdir/$logname.time"
rm -f "$timefile"
time_cmd() {
local max=$(($SECONDS+10))
exec 3>&1 4>&2
local out
TIME_OUTPUT=$((time 1>&3 2>&4 sh -c "$1") 2>&1)
local time_output
time_output=$((time 1>&3 2>&4 sh -c "$1") 2>&1)
local retstat=$?
[ "$SECONDS" -lt "$max" ] || echo "$TIME_OUTPUT" > "$timefile"
[ "$SECONDS" -lt "$max" ] || echo "$time_output" > "$timefile"
return $retstat
}
Expand Down
130 changes: 30 additions & 100 deletions build/bin/sage-spkg
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ cat >&2 <<MESSAGE
MESSAGE
}

exit_with_error_msg()
{
error_msg "$@"
exit 1
}

lookup_param()
{
local param=$1
Expand Down Expand Up @@ -185,20 +191,9 @@ fi
# Since this is sourced, it returns a non-zero value on errors rather
# than exiting. Using dot suggested by W. Cheung.
. sage-env-config
. sage-env

if [ $? -ne 0 ]; then
error_msg "Error setting environment variables by sourcing sage-env"
exit 1
fi

. sage-env || exit_with_error_msg "Error setting environment variables by sourcing sage-env"
. sage-build-env-config
. sage-build-env

if [ $? -ne 0 ]; then
error_msg "Error setting environment variables by sourcing sage-build-env"
exit 1
fi
. sage-build-env || exit_with_error_msg "Error setting environment variables by sourcing sage-build-env"

# Remove '.' from PYTHONPATH, to avoid trouble with setuptools / easy_install
# (cf. #10192, #10176):
Expand Down Expand Up @@ -375,11 +370,7 @@ ensure_pkg_src() { ###############################################
if [ ! -f "$PKG_SRC" ]; then
if [ -n "$PKG_NAME_UPSTREAM" ]; then
# Normal or wheel package
PKG_SRC=$(sage-package download $SAGE_DOWNLOAD_FILE_OPTIONS $PKG_BASE)
if [ $? != 0 ]; then
error_msg "Error downloading tarball of $PKG_BASE"
exit 1
fi
PKG_SRC=$(sage-package download $SAGE_DOWNLOAD_FILE_OPTIONS $PKG_BASE) || exit_with_error_msg "Error downloading tarball of $PKG_BASE"
# Do a final check that PKG_SRC is a file with an absolute path
cd /
if [ ! -f "$PKG_SRC" ]; then
Expand All @@ -399,34 +390,27 @@ cd "$SAGE_ROOT" || exit $?
if [ -n "$SAGE_SPKG_COPY_UPSTREAM" -a -n "$PKG_NAME_UPSTREAM" ]; then
mkdir -p "$SAGE_SPKG_COPY_UPSTREAM" && cp -p "$PKG_SRC" "$SAGE_SPKG_COPY_UPSTREAM"
if [ $? -ne 0 ]; then
error_msg "Error copying upstream tarball to directory '$SAGE_SPKG_COPY_UPSTREAM'"
exit 1
exit_with_error_msg "Error copying upstream tarball to directory '$SAGE_SPKG_COPY_UPSTREAM'"
fi
fi
} ################################################# ensure_pkg_src

setup_directories() { ############################################

for dir in "$SAGE_SPKG_INST" "$SAGE_SPKG_SCRIPTS" "$SAGE_BUILD_DIR"; do
mkdir -p "$dir"
if [ $? -ne 0 ]; then
error_msg "Error creating directory $dir"
exit 1
fi
mkdir -p "$dir" || exit_with_error_msg "Error creating directory $dir"
done

# Issue #5852: check write permissions
if [ ! -w "$SAGE_BUILD_DIR" ]; then
error_msg "Error: no write access to build directory $SAGE_BUILD_DIR"
exit 1
exit_with_error_msg "Error: no write access to build directory $SAGE_BUILD_DIR"
fi
if [ ! -d "$SAGE_INST_LOCAL" ]; then
# If you just unpack Sage and run "sage -p <pkg>" then local does not yet exist
mkdir "$SAGE_INST_LOCAL"
fi
if [ ! -w "$SAGE_INST_LOCAL" ]; then
error_msg "Error: no write access to installation directory $SAGE_INST_LOCAL"
exit 1
exit_with_error_msg "Error: no write access to installation directory $SAGE_INST_LOCAL"
fi

# Make absolutely sure that we are in the build directory before doing
Expand All @@ -439,19 +423,14 @@ if [ "x$SAGE_KEEP_BUILT_SPKGS" != "xyes" ]; then
else
if [ -e "$PKG_NAME" ]; then
echo "Moving old directory $PKG_NAME to $SAGE_BUILD_DIR/old..."
mkdir -p old
if [ $? -ne 0 ]; then
error_msg "Error creating directory $SAGE_BUILD_DIR/old"
exit 1
fi
mkdir -p old || exit_with_error_msg "Error creating directory $SAGE_BUILD_DIR/old"
rm -rf old/"$PKG_NAME"
mv "$PKG_NAME" old/
fi
fi

if [ -e "$PKG_NAME" ]; then
error_msg "Error (re)moving $PKG_NAME"
exit 1
exit_with_error_msg "Error (re)moving $PKG_NAME"
fi
} ############################################## setup_directories

Expand Down Expand Up @@ -491,16 +470,9 @@ else
;;
*)
# Source tarball
sage-uncompress-spkg -d src "$PKG_SRC"
if [ $? -ne 0 ]; then
error_msg "Error: failed to extract $PKG_SRC"
exit 1
fi
sage-uncompress-spkg -d src "$PKG_SRC" || exit_with_error_msg "Error: failed to extract $PKG_SRC"
cd src
if ! sage-apply-patches; then
error_msg "Error applying patches"
exit 1
fi
sage-apply-patches || exit_with_error_msg "Error applying patches"
cd ..
;;
esac
Expand Down Expand Up @@ -646,40 +618,24 @@ unset V
# First uninstall the previous version of this package, if any
if [ "$KEEP_EXISTING" != "yes" ]; then
sage-spkg-uninstall "$PKG_BASE" "$SAGE_INST_LOCAL" --log-directory .
sage-spkg-uninstall "$PKG_BASE" "$SAGE_INST_LOCAL"
fi
# To work around #26996: Create lib and set a symlink so that writes into lib64/ end up in lib/
(mkdir -p "$SAGE_DESTDIR_LOCAL/lib" && cd "$SAGE_DESTDIR_LOCAL" && ln -sf lib lib64)
# Run the pre-install script, if any
if [ -f spkg-preinst ]; then
sage-logger -p "$SAGE_SUDO ./spkg-preinst" spkg-preinst.log
if [ $? -ne 0 ]; then
error_msg "Error running the preinst script for $PKG_NAME."
exit 1
fi
sage-logger -P spkg-preinst "$SAGE_SUDO ./spkg-preinst" || exit_with_error_msg "Error running the preinst script for $PKG_NAME."
fi
if [ -f spkg-build ]; then
# Package has both spkg-build and spkg-install; execute the latter with SAGE_SUDO
sage-logger -p "./spkg-build" spkg-build.log
if [ $? -ne 0 ]; then
error_msg "Error building package $PKG_NAME" "make"
exit 1
fi
sage-logger -p "$SAGE_SUDO ./spkg-install" spkg-install.log
if [ $? -ne 0 ]; then
error_msg "Error installing package $PKG_NAME" "make"
exit 1
fi
sage-logger -P spkg-build "./spkg-build" || exit_with_error_msg "Error building package $PKG_NAME" "make"
sage-logger -P spkg-install "$SAGE_SUDO ./spkg-install" || exit_with_error_msg "Error installing package $PKG_NAME" "make"
else
# Package only has spkg-install
sage-logger -p "./spkg-install" spkg-install.log
if [ $? -ne 0 ]; then
error_msg "Error installing package $PKG_NAME" "make"
exit 1
fi
sage-logger -P spkg-install "./spkg-install" || exit_with_error_msg "Error installing package $PKG_NAME" "make"
fi
} ##################################### actually_build_and_install
Expand All @@ -697,21 +653,13 @@ if [ -d "$SAGE_DESTDIR" ]; then
# any trailing slash is removed; https://github.com/sagemath/sage/issues/26013
PREFIX="${SAGE_DESTDIR_LOCAL%/}"
rm -f "$PREFIX"/lib/*.la
if [ $? -ne 0 ]; then
error_msg "Error deleting unnecessary libtool archive files"
exit 1
fi
rm -f "$PREFIX"/lib/*.la || exit_with_error_msg "Error deleting unnecessary libtool archive files"
# Generate installed file manifest
FILE_LIST="$(cd "$PREFIX" && find . -type f -o -type l | sed 's|^\./||' | sort)"
# Copy files into $SAGE_INST_LOCAL
$SAGE_SUDO cp -Rp "$PREFIX/." "$SAGE_INST_LOCAL"
if [ $? -ne 0 ]; then
error_msg "Error moving files for $PKG_NAME."
exit 1
fi
$SAGE_SUDO cp -Rp "$PREFIX/." "$SAGE_INST_LOCAL" || exit_with_error_msg "Error moving files for $PKG_NAME."
# Remove the $SAGE_DESTDIR entirely once all files have been moved to their
# final location.
Expand Down Expand Up @@ -742,17 +690,8 @@ for script in $INSTALLED_SCRIPTS; do
script="spkg-$script"
if [ -f "$script" ]; then
mkdir -p "$INSTALLED_SCRIPTS_DEST"
if [ $? -ne 0 ]; then
error_msg "Error creating the spkg scripts directory $INSTALLED_SCRIPTS_DEST."
exit 1
fi
cp -a "$script" "$INSTALLED_SCRIPTS_DEST"
if [ $? -ne 0 ]; then
error_msg "Error copying the $script script to $INSTALLED_SCRIPTS_DEST."
exit 1
fi
mkdir -p "$INSTALLED_SCRIPTS_DEST" || exit_with_error_msg "Error creating the spkg scripts directory $INSTALLED_SCRIPTS_DEST."
cp -a "$script" "$INSTALLED_SCRIPTS_DEST" || exit_with_error_msg "Error copying the $script script to $INSTALLED_SCRIPTS_DEST."
fi
done
} ################################################ install_scripts
Expand All @@ -761,18 +700,10 @@ post_install() { #################################################
# Run the post-install script, if any
# But first complete the delayed installation of wheels.
if [ -f spkg-pipinst ]; then
sage-logger -p "$SAGE_SUDO ./spkg-pipinst" spkg-pipinst.log
if [ $? -ne 0 ]; then
error_msg "Error running the pipinst script for $PKG_NAME."
exit 1
fi
sage-logger -P spkg-pipinst "$SAGE_SUDO ./spkg-pipinst" || exit_with_error_msg "Error running the pipinst script for $PKG_NAME."
fi
if [ -f spkg-postinst ]; then
sage-logger -p "$SAGE_SUDO ./spkg-postinst" spkg-postinst.log
if [ $? -ne 0 ]; then
error_msg "Error running the postinst script for $PKG_NAME."
exit 1
fi
sage-logger -P spkg-postinst "$SAGE_SUDO ./spkg-postinst" || exit_with_error_msg "Error running the postinst script for $PKG_NAME."
fi
} ################################################### post_install
Expand All @@ -788,16 +719,15 @@ run_test_suite() { ###############################################
if [ -f "$INSTALLED_SCRIPTS_DEST"/spkg-check ]; then
echo "Running the test suite for $PKG_NAME..."
export PKG_BASE
sage-logger -p "$INSTALLED_SCRIPTS_DEST"/spkg-check spkg-check.log
sage-logger -P spkg-check "$INSTALLED_SCRIPTS_DEST"/spkg-check
if [ $? -ne 0 ]; then
TEST_SUITE_RESULT="failed"
if [ "$SAGE_CHECK" = "warn" ]; then
# The following warning message must be consistent
# with SAGE_ROOT/build/make/install (see #32781)
error_msg "Warning: Failures testing package $PKG_NAME (ignored)" "make check"
else
error_msg "Error testing package $PKG_NAME" "make check"
exit 1
exit_with_error_msg "Error testing package $PKG_NAME" "make check"
fi
else
TEST_SUITE_RESULT="passed"
Expand Down
2 changes: 1 addition & 1 deletion build/sage_bootstrap/uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def run_spkg_script(spkg_name, path, script_name,
log_file = pth.join(log_directory, script + '.log')
subprocess.check_call(['sage-logger', '-p', script, log_file])
else:
subprocess.check_call([script])
subprocess.check_call(['sage-logger', '-P', script_name, script])
elif if_does_not_exist == 'ignore':
pass
elif if_does_not_exist == 'log':
Expand Down

0 comments on commit efc613a

Please sign in to comment.