Skip to content

Only include files modified by the PR in tarball #1119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 11, 2025
Merged

Only include files modified by the PR in tarball #1119

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
437fa0b
Make sure all scripts are actually changed by the PR
ocaisa Jun 11, 2025
5c62557
Make pr_diff file available
ocaisa Jun 11, 2025
0d0ff5d
Update install_scripts.sh
ocaisa Jun 11, 2025
330416f
Modify eb_hooks.py to test new functionality
ocaisa Jun 11, 2025
19999e3
Revert eb_hooks.py change
ocaisa Jun 11, 2025
5e6ddc1
Remove unused variable, make sure required envvar is set
ocaisa Jun 11, 2025
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
9 changes: 5 additions & 4 deletions EESSI-install-software.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ fi
# order is important: these are needed to install a full CUDA SDK in host_injections
# for now, this just reinstalls all scripts. Note the most elegant, but works

# the install_scripts.sh script relies on knowing the location of the PR diff
# assume there's only one diff file that corresponds to the PR patch file
pr_diff=$(ls [0-9]*.diff | head -1)
export PR_DIFF="$PWD/$pr_diff"

# Only run install_scripts.sh if not in dev.eessi.io for security
if [[ -z ${EESSI_DEV_PROJECT} ]]; then
${TOPDIR}/install_scripts.sh --prefix ${EESSI_PREFIX}
Expand Down Expand Up @@ -341,10 +346,6 @@ else
echo_green ">> MODULEPATH set up: ${MODULEPATH}"
fi

# assume there's only one diff file that corresponds to the PR patch file
pr_diff=$(ls [0-9]*.diff | head -1)


# use PR patch file to determine in which easystack files stuff was added
changed_easystacks=$(cat ${pr_diff} | grep '^+++' | cut -f2 -d' ' | sed 's@^[a-z]/@@g' | grep 'easystacks/.*yml$' | egrep -v 'known-issues|missing')
if [ -z "${changed_easystacks}" ]; then
Expand Down
45 changes: 43 additions & 2 deletions install_scripts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,35 @@ display_help() {
echo " -h | --help - display this usage information"
}

file_changed_in_pr() {
local full_path="$1"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also check that full_path is not empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that kind of done at line 16?

Copy link
Contributor

Choose a reason for hiding this comment

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

test -f (without argument) exits with zero, so I'd say no

Copy link
Contributor

Choose a reason for hiding this comment

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

Interestingly, test -f "" does fail with non-zero exit code, so it's fine as is I guess, don't want to nitpick here...

Copy link
Member Author

Choose a reason for hiding this comment

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

You had me sweating but

ocaisa@~$ export full_path="$PWD/file"; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ touch file
ocaisa@~$ export full_path="$PWD/file"; [[ -f "$full_path" ]] || echo Fail
ocaisa@~$ export full_path=""; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ export full_path= ; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ unset full_path
ocaisa@~$ [[ -f "$full_path" ]] || echo Fail
Fail

so the check below is indeed enough

local base_branch="${2:-origin/2023.06-software.eessi.io}" # Default to origin/2023.06-software.eessi.io

# Make sure file exists
[[ -f "$full_path" ]] || return 1

# Check if the file is in a Git repo (it should be)
local repo_root
repo_root=$(git -C "$(dirname "$full_path")" rev-parse --show-toplevel 2>/dev/null)
if [[ -z "$repo_root" ]]; then
return 2 # Not in a git repository
fi

# Compute relative path to the repo root
local rel_path
rel_path=$(realpath --relative-to="$repo_root" "$full_path")

# Check if the file changed in the PR diff file that we have
(
cd "$repo_root" || return 2
if [ -f "$PR_DIFF" ]; then # PR_DIFF should be set by the calling script
grep -q "b/$rel_path" "$PR_DIFF" # Add b/ to match diff patterns
else
return 3
fi
) && return 0 || return 1
}

compare_and_copy() {
if [ "$#" -ne 2 ]; then
echo "Usage of function: compare_and_copy <source_file> <destination_file>"
Expand All @@ -18,8 +47,20 @@ compare_and_copy() {
destination_file="$2"

if [ ! -f "$destination_file" ] || ! diff -q "$source_file" "$destination_file" ; then
cp "$source_file" "$destination_file"
echo "File $1 copied to $2"
echo "Files $source_file and $destination_file differ, checking if we should copy or not"
# We only copy if the file is part of the PR
if file_changed_in_pr "$source_file"; then
echo "File has changed in the PR"
cp "$source_file" "$destination_file"
echo "File $source_file copied to $destination_file"
else
case $? in
1) echo "❌ File has NOT changed in PR" ;;
2) echo "🚫 Not in Git repository" ;;
3) echo "🚫 No PR diff file found" ;;
*) echo "⚠️ Unknown error" ;;
esac
fi
else
echo "Files $1 and $2 are identical. No copy needed."
fi
Expand Down