Proposal: add git hooks to the repo

[am11]

git hooks are useful to automate validation of certain common practices and conventions used by the repo, which maybe specified in the docs, but still (unintentionally) violated in changes being committed.

git hooks scripts are cross-platform (hint: git-win has sh.exe/bash.exe and so on) and thus have access to everything as the executing environment.

In dotnet/runtime, we can use them to validate:

• code being checked in inside eng/common, which is the mirror directory sourced from arcade.
• native dependencies (brotli, libunwind etc.) code is being updated without updating their corresponding {name}-version.txt file.
• format of commit message first (subject) line exceeding certain threshold.
• jit code is updated and jit-format's output is not clean.
• C# code is update, dotnet-format is available, but git show --name-only --pretty="" | xargs dotnet-format --include is not clean.
• change has invalid trailing whitespaces.
• change has new file without proper Unix-y EOF.
• more?

git hooks can be installed as part of bootstrap, e.g. ./install-hooks script which can be ran manually as well as invoked by top-level ./build script.

cc @janvorli, @jkotas, @danmosemsft

[jkotas]

Which git action would you suggest attaching the list of validations to?

This is fairly long list and executing all of it would take minutes if not longer. I think it would be better to encapsulate these validations as explicitly triggered tests.

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

[stephentoub]

change has invalid trailing whitespaces

FYI, for C# code specifically, we already run an analyzer that results in build failures when this occurs.

[am11]

I think pre-commit hook would be the most suitable action.

The shell script checks will have to be ran in no more than few seconds to detect potential issues. For example, dotnet-format (which is likely to take most time in the list above) on Windows 10 took 13 seconds to format 1000 unformatted files, and six seconds when I re-ran it on just-formatted thousand files.

When a commit, with change which is logically correct but not following the repo conventions (e..g. #37891), is pushed to a PR branch, it triggers 10s of jobs and wastes both time and resource. This would be the best effort to communicate it to the contributor about the convention via automation, in addition to docs.

[danmoseley]

I assume the commit hook would run locally, so unless it takes <1 sec it would be noticeable to folks who like to make lots of local commits as they work. Is there a way to add a PR validation check instead of a commit hook? Or a "validate" script that could be run explicitly?

Would the script be Windows specific? We know most of our contributors use Windows currently, but not all.

[am11]

Yes, the hook will run locally on git commit. I agree that the checks for git hook would have to be cheap, otherwise it would be a no-go. The purpose is to run quick sanity checks locally based on what has changed, the tools that are applicable to the changes and are available in user environment (sed and grep are available, jitutils is restored in default location and src/coreclr/src/jit/*.{cpp,hpp,h} are modified, dotnet-format is installed and *.cs are modified etc.) or bail out with an info message.

Would the script be Windows specific?

It is typically a POSIX shell or bash script which works on Windows (git-for-windows installs sh and bash and uses them internally) and Unix consistently. We can invoke other scripts from it (e.g. validate.sh, jit-format.py etc.).

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.