[Infrastructure] Respect partially staged changes on pre-commit hook

[javiercn]

Make sure unstaged bits of files don't get staged on partial commits

Addresses #29768

[SteveSandersonMS]

I might be reading it incorrectly, but it looks like this will pop from the stash list even if it didn't push onto it (because there were no unstaged changes). This will mess up the stash history and add unexpected changes to the working copy.

Do you need to be checking if the push actually did something? See https://stackoverflow.com/questions/34114700/git-stash-pop-only-if-successfully-stashed-before

Even so this is a bit sketchy because it works by actually changing the working copy, which will unexpectedly trigger any file watchers even though a normal git commit does not.

[javiercn]

@SteveSandersonMS this is the recommended way I read how to do it.

I can't think of a different way to achieve this unless there's a way to programatically do git add to a partial hunk and we have a way to identify the hunks you have staged.

[SteveSandersonMS]

We definitely need not to corrupt the stash history and working copy :)

[adrianwright109]

Would something like this work ?

#!/bin/sh
LC_ALL=C

# unique timestamp for stash
t=timestamp-$(date +%s) 

# Stash unstaged changes with unique timestamp
r=$(git stash -q --keep-index --message $t)

# Select files to format
FILES=$(git diff --cached --name-only --pretty="")
[ -z "$FILES" ] && exit 0

# Format all selected files with dotnet-format
echo "dotnet-format: Formatting changed source files.."
echo "$FILES" | cat | xargs | sed -e 's/ /,/g' | xargs dotnet-format --folder . --include >/dev/null
echo "dotnet-format: $(git diff --cached --numstat | wc -l) file(s) formatted."

# Add files to staging
echo "$FILES" | xargs git add

# check to see if the stash exists with the unique timestamp
v=$(echo $r | grep $t)

if ["$v"] then 
	# Restore unstaged changes
	git stash pop -q
fi

exit 0

[javiercn]

@adrianwright109 I was reading about this too, seems like that's the missing piece.

[javiercn]

We've decided to change this to just completely skip partially staged files. We can detect these if we run git status -s as they show up with the MM moniker.

MM src/.................../Program.cs

[javiercn]

I'm not really able to figure out the bash syntax to get this to work, if someone wants to try. $FILES and $PARTIAL have what's needed to separate partially staged files from non partially staged files. (We might also need to filter out untracked files with /^?? /d)

#!/bin/sh
LC_ALL=C

# Select files to format
FILES=$(git status -s | sed '/^MM /d;s/^...//g')
[ -z "$FILES" ] && exit 0

# Check for partial files and validate that they don't need formatting.
# We don't handle formatting partially staged files in the pre-commit and we require them to be properly formatted
# before they are committed, so the command and the changes need to be run manually for partially staged files.
PARTIAL=$(git status -s | sed '/^MM /!d;s/^...//g')

if [ -n "$PARTIAL" ]; then
  if ! echo "$PARTIAL" | cat | xargs | sed -e 's/ /,/g' | xargs dotnet-format --folder . --check --include > /dev/null; then {
      echo "The following files need reformatting ${PARTIAL}";
      exit 1;
    }
  fi
fi

# Format all selected files with dotnet-format
echo "dotnet-format: Formatting changed source files.."
echo "$FILES" | cat | xargs | sed -e 's/ /,/g' | xargs dotnet-format --folder . --include >/dev/null
echo "dotnet-format: $($FILES | wc -l) file(s) formatted."
# Add files to staging
echo "$FILES" | xargs git add
exit 0

[javiercn]

Closing this PR for the time being