Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,15 @@ jobs:

- name: "Shellcheck"
shell: bash
working-directory: scripts
run: shellcheck -o all *.sh

- name: "Test check_filenames.sh"
shell: bash
working-directory: scripts
run: |
mkdir -p filename-dir
mkdir -p filename-dir/doc
cd filename-dir
mkdir -p doc
touch :q \\Users\\GillBates AUX COM².tar.gz period. space\ DOC \
PackageInfo.g doc/manual.pdf manual.pdf README readme.txt auxiliary .gaplint.yml
../check_filenames.sh > $RUNNER_TEMP/filenames_run.txt || true # suppress expected error here
Expand All @@ -89,6 +90,7 @@ jobs:

- name: "Test check_absolute_paths.sh"
shell: bash
working-directory: scripts
run: |
mkdir -p paths-dir/doc
cd paths-dir
Expand All @@ -101,3 +103,30 @@ jobs:
./doc/chap1_mj.html
./doc/chap2_mj.html
EOF

- name: "Test check_symlinks.sh"
shell: bash
working-directory: scripts
run: |
mkdir -p symlinks-dir
cd symlinks-dir
echo "THIS FILE IS NOT A SYMLINK" > actual.file
../check_symlinks.sh > $RUNNER_TEMP/symlinks_run.txt # should not give error
ln -s actual.file sym.link
../check_symlinks.sh > $RUNNER_TEMP/symlinks_run.txt || true # suppress expected error here
diff <(sort $RUNNER_TEMP/symlinks_run.txt) - <<EOF
./sym.link
EOF

- name: "Test check_date.sh"
shell: bash
working-directory: scripts
run: |
if ! ./check_date.sh "$(date +%Y-%m-%d)" \
|| ! ./check_date.sh "$(date -d "1 day ago" +%Y-%m-%d)" \
|| ! ./check_date.sh "$(date -d "1 day" +%Y-%m-%d)" \
|| ./check_date.sh "$(date -d "2 days ago" +%Y-%m-%d)" \
|| ./check_date.sh "$(date -d "2 days" +%Y-%m-%d)"
then
exit 1
fi
Comment on lines +125 to +132
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't GitHub run scripts with set -e anyway? Then we could just do this, and get better info about which check failed:

Suggested change
if ! ./check_date.sh "$(date +%Y-%m-%d)" \
|| ! ./check_date.sh "$(date -d "1 day ago" +%Y-%m-%d)" \
|| ! ./check_date.sh "$(date -d "1 day" +%Y-%m-%d)" \
|| ./check_date.sh "$(date -d "2 days ago" +%Y-%m-%d)" \
|| ./check_date.sh "$(date -d "2 days" +%Y-%m-%d)"
then
exit 1
fi
! ./check_date.sh "$(date +%Y-%m-%d)"
! ./check_date.sh "$(date -d "1 day ago" +%Y-%m-%d)"
! ./check_date.sh "$(date -d "1 day" +%Y-%m-%d)"
./check_date.sh "$(date -d "2 days ago" +%Y-%m-%d)"
./check_date.sh "$(date -d "2 days" +%Y-%m-%d)"

Copy link
Member

Choose a reason for hiding this comment

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

@fingolfin I prefer your syntax.
I think GitHub does, but its for testing, I suppose.

96 changes: 53 additions & 43 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ inputs:
runs:
using: "composite"
steps:
- name: "Validate input"
shell: bash
run: |
validate_boolean() {
local input=$1
local option_name=$2
if ! [[ "$input" =~ ^(true|false)$ ]]; then
echo "::error::Invalid value for option $option_name. Expected 'true' or 'false', but found '$input'"
exit 1;
fi
}

validate_boolean "${{ inputs.dry-run }}" dry-run
validate_boolean "${{ inputs.force }}" force

- name: "Validate PackageInfo.g"
shell: bash
Expand All @@ -27,7 +41,7 @@ runs:
- name: "Check documentation for absolute paths"
shell: bash
run: |
$GITHUB_ACTION_PATH/check_absolute_paths.sh || {
$GITHUB_ACTION_PATH/scripts/check_absolute_paths.sh || {
echo "::error::Found absolute paths in HTML file(s)"
exit 1
}
Expand Down Expand Up @@ -58,10 +72,10 @@ runs:
- name: "Generate package-info.json"
shell: bash
run: |
gap -A -q $GITHUB_ACTION_PATH/pkginfo_to_json.g
gap -A -q $GITHUB_ACTION_PATH/scripts/pkginfo_to_json.g
if [ ! -f "package-info.json" ] ; then
echo "::error::Could not create package-info.json"
exit 1
echo "::error::Could not create package-info.json"
exit 1
fi
mv package-info.json $ASSETS

Expand All @@ -75,11 +89,11 @@ runs:
[ "${TMP[1]}" = "github.com" ] &&
[ "${TMP[4]}" = "releases" ] &&
[ "${TMP[5]}" = "download" ]; then
echo "TAG=${TMP[6]}" | tee -a "$GITHUB_ENV"
echo "BASENAME=${TMP[7]}" | tee -a "$GITHUB_ENV"
echo "TAG=${TMP[6]}" | tee -a "$GITHUB_ENV"
echo "BASENAME=${TMP[7]}" | tee -a "$GITHUB_ENV"
else
echo "::error::ArchiveURL in PackageInfo.g does not point to a GitHub release"
exit 1
echo "::error::ArchiveURL in PackageInfo.g does not point to a GitHub release"
exit 1
fi
echo "PKGNAME=$(cat package-info.json | jq -r '.PackageName')" | tee -a "$GITHUB_ENV"
echo "VERSION=$(cat package-info.json | jq -r '.Version')" | tee -a "$GITHUB_ENV"
Expand Down Expand Up @@ -113,19 +127,10 @@ runs:
shell: bash
if: ${{ inputs.force != 'true' }}
run: |
# Convert DD/MM/YYYY to YYYY-MM-DD
if [[ "$DATE" =~ ^[0-9]{2}/[0-9]{2}/[0-9]{4}$ ]]; then
DATE=$(echo "$DATE" | awk -F/ '{print $3"-"$2"-"$1}')
fi

TODAY=$(date +%Y-%m-%d)
YESTERDAY=$(date -d "yesterday" +%Y-%m-%d)
TOMORROW=$(date -d "tomorrow" +%Y-%m-%d)

if [[ "$DATE" != "$TODAY" && "$DATE" != "$YESTERDAY" && "$DATE" != "$TOMORROW" ]]; then
echo "::error::Date found in PackageInfo.g ($DATE) does not match current date ($TODAY) up to ±1 day."
$GITHUB_ACTION_PATH/scripts/check_date.sh $DATE || {
echo "::error::Date found in PackageInfo.g ($DATE) does not match current date ($(date +%Y-%m-%d)) up to ±1 day."
exit 1
fi
}

# We make a copy of the current directory to build the release from, because
# the next step will remove files/folders that may be necessary for subsequent
Expand All @@ -140,15 +145,16 @@ runs:
shell: bash
working-directory: ${{ runner.temp }}/${{ env.BASENAME }}
run: |
echo "::group::Cleanup git and github related files"
rm -rvf .git* .hg* .cvs* .circleci
echo "::group::Cleanup codecov, travis, azure-pipelines"
rm -fv .codecov.* .travis.* .appveyor.* azure-pipelines.*
rm -fv .gaplint.*
rm -fv requirements.txt
echo "::group::Cleanup git and github related files"
rm -rvf .git* .hg* .cvs* .circleci

echo "::group::Cleanup codecov, travis, azure-pipelines"
rm -fv .codecov.* .travis.* .appveyor.* azure-pipelines.*
rm -fv .gaplint.*
rm -fv requirements.txt

echo "::group::Cleanup macOS metadata"
find . -name .DS_Store -exec rm -f {} +
echo "::group::Cleanup macOS metadata"
find . -name .DS_Store -exec rm -f {} +

- name: "Run autogen.sh"
shell: bash
Expand All @@ -165,37 +171,41 @@ runs:
shell: bash
working-directory: ${{ runner.temp }}/${{ env.BASENAME }}
run : |
if find . -type l | grep -q .; then
$GITHUB_ACTION_PATH/scripts/check_symlinks.sh || {
echo "::error::Symlinks detected"
exit 1
fi
}

- name: "Reject bad filenames"
shell: bash
working-directory: ${{ runner.temp }}/${{ env.BASENAME }}
run: $GITHUB_ACTION_PATH/check_filenames.sh
run: |
$GITHUB_ACTION_PATH/scripts/check_filenames.sh || {
echo "::error::Found problematic filenames"
exit 1
}

- name: "Create archives"
shell: bash
working-directory: ${{ runner.temp }}
run: |
for EXT in $ARCHIVE_FORMATS ; do
ARCHIVENAME=$BASENAME$EXT
echo "Creating $ARCHIVENAME ..."
case $EXT in
ARCHIVENAME=$BASENAME$EXT
echo "Creating $ARCHIVENAME ..."
case $EXT in
.tar.gz) tar cf - "$BASENAME" | gzip -9c > "$ASSETS/$ARCHIVENAME" ;;
.tar.bz2) tar cf - "$BASENAME" | bzip2 -9c > "$ASSETS/$ARCHIVENAME" ;;
.zip) zip -r9 --quiet "$ASSETS/$ARCHIVENAME" "$BASENAME" ;;
*)
echo "::error::Unsupported archive format $EXT"
exit 1
;;
esac
if [ ! -f "$ASSETS/$ARCHIVENAME" ] ; then
echo "::error::Failed at creating "$ARCHIVENAME""
exit 1
fi
echo "Created $ARCHIVENAME"
echo "::error::Unsupported archive format $EXT"
exit 1
;;
esac
if [ ! -f "$ASSETS/$ARCHIVENAME" ] ; then
echo "::error::Failed at creating '$ARCHIVENAME'"
exit 1
fi
echo "Created $ARCHIVENAME"
done

- name: "Copy manual(s)"
Expand Down
File renamed without changes.
18 changes: 18 additions & 0 deletions scripts/check_date.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash
set -e
set -o pipefail

DATE="$1"

# Convert DD/MM/YYYY to YYYY-MM-DD
if [[ "${DATE}" =~ ^[0-9]{2}/[0-9]{2}/[0-9]{4}$ ]]; then
DATE=$(echo "${DATE}" | awk -F/ '{print $3"-"$2"-"$1}')
fi

TODAY=$(date +%Y-%m-%d)
YESTERDAY=$(date -d "yesterday" +%Y-%m-%d)
TOMORROW=$(date -d "tomorrow" +%Y-%m-%d)

if [[ "${DATE}" != "${TODAY}" && "${DATE}" != "${YESTERDAY}" && "${DATE}" != "${TOMORROW}" ]]; then
exit 1
fi
File renamed without changes.
7 changes: 7 additions & 0 deletions scripts/check_symlinks.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash
set -e
set -o pipefail

if find . -type l | grep .; then
exit 1
fi
11 changes: 6 additions & 5 deletions pkginfo_to_json.g → scripts/pkginfo_to_json.g
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ LoadPackage("json");
InstallMethod(RecNames, [IsRecord and IsInternalRep], x -> AsSSortedList(REC_NAMES(x)));

InstallMethod(_GapToJsonStreamInternal, [IsOutputStream, IsObject],
function(o, x)
function(o, x)
PrintTo(o, "null");
end);
end
);

Read("PackageInfo.g");
if not IsBound(GAPInfo.PackageInfoCurrent) then
Print("Reading PackageInfo.g failed\n");
FORCE_QUIT_GAP(2);
Exec("echo \"::error::Reading PackageInfo.g failed\"");
Copy link
Member

Choose a reason for hiding this comment

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

This file is a GAP "script", I mean, we can easily treat it as script with GAP as execution environment, but in general I'm not a big fan of including Actions logic into GAP.

Writing an abstraction layer could be helpful.

@fingolfin What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I guess we could have a file gh_actions.g which contains code that re-defines Error to do something else (e.g. call Exec like here), and then change e.g.

gap -A -q $GITHUB_ACTION_PATH/scripts/pkginfo_to_json.g

to

gap -A -q $GITHUB_ACTION_PATH/scripts/gh_actions.g $GITHUB_ACTION_PATH/scripts/pkginfo_to_json.g

But I am not sure it's worth the trouble? After all, you can run pkginfo_to_json.g locally even with that Exec for errors in it.

Copy link
Member

Choose a reason for hiding this comment

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

I.e. I am not sure what another abstraction layer would gain us, what benefit it would bring? Its downside is the usual: it adds complexity and thus opportunity for new bugs, and makes debugging harder.

(I am not saying there are no issues, just that I don't see them; happy to learn)

ForceQuitGap(2);
fi;
pkginfo := GAPInfo.PackageInfoCurrent;

Expand All @@ -19,7 +20,7 @@ if IsBound(pkginfo.PackageDoc) and not IsList(pkginfo.PackageDoc) then
pkginfo.PackageDoc := [pkginfo.PackageDoc];
fi;

output := OutputTextFile("package-info.json", false );
output := OutputTextFile("package-info.json", false);
GapToJsonStream(output, pkginfo);
CloseStream(output);

Expand Down