Skip to content

Fix: skip buffered uploads for files scheduled for deletion (fixes #655) - #677

Open
RedAlex wants to merge 1 commit into
git-ftp:masterfrom
RedAlex:master
Open

RedAlex wants to merge 1 commit into
git-ftp:masterfrom
RedAlex:master

Conversation

@RedAlex

@RedAlex RedAlex commented Mar 12, 2026

Copy link
Copy Markdown

Prevent uploading files that are both in the upload buffer and the delete list (occurs when a file is created then deleted between commits)
Fix: After sorting change lists, remove entries from the upload list that also appear in the delete list so the upload buffer never tries to send deleted files

#655

@mkllnk mkllnk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for this contribution. I think that it can be simplified. I know that it has been a really long time now but would you like to try to improve it? Covering this case with a test would be great as well.

Comment thread git-ftp
Comment on lines +826 to +835
# Remove files from the upload list that are also scheduled for delete
if [ -s "$TMP_GITFTP_DELETE" ] && [ -s "$TMP_GITFTP_UPLOAD" ]; then
local TMP_GITFTP_FILTERED=$(mktemp)
while IFS= read -r -d '' FILE_NAME; do
if ! grep -zF -- "$FILE_NAME" "$TMP_GITFTP_DELETE" >/dev/null 2>&1; then
printf '%s\0' "$FILE_NAME" >> "$TMP_GITFTP_FILTERED"
fi
done < "$TMP_GITFTP_UPLOAD"
mv "$TMP_GITFTP_FILTERED" "$TMP_GITFTP_UPLOAD"
fi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This probably works but grep can do most of this for you. Here's an example:

# given lines a, b and c in file abc and
# given lnes b c and d in file bcd
$ grep -v -F -f abc bcd 
d

So I think that this logic should work here:

grep -v -F -f "$TMP_GITFTP_DELETE" "$TMP_GITFTP_UPLOAD" > "$TMP_GITFTP_FILTERED"

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants