Looks like this could help us. https://github.com/pre-commit/action

Thanks!

I've seen that before, but running all other pre-commit hooks with GitHub Actions might not be a good idea.

Maybe it would be better to manually implement uv lock separately.

I had a go in #4165, think that seems like a reasonable solution?

Accidentally closed it. It should be fine now.

Oh man, this is way more complicated that I figured.

I'll put my thoughts here, but first I should double check I understand what this is doing:

• On a push event, it runs the lock and commits the updated lock file
• On a PR, it checks if the PR is mergable, if if not, it fails. If so, it checks out the merge commit, runs uv lock there, then commits the result back to the HEAD of the PR.

The first part feels fine, but I'm not so sure about the second. If something has changed in the base branch, that means that the lock file committed in the PR might be out of sync from the PR itself (but that everything would be fine once it got merged.) I think that just running the lock against the HEAD of the PR might be simpler and more desirable. It might end up causing a merge conflict if there were also dep changes in the base branch, but I don't think that's really a problem. (Is that the worst potential outcome?) Would something simpler without all the scripting (like I was thinking in #4165) work if we just wanted to run on the HEAD of the PR?

Sorry I waited for you to do all this work to ask these questions, I didn't fully understand your plan until seeing this.

I'll put my thoughts here, but first I should double check I understand what this is doing:

Your understanding of this PR is correct.

but I'm not so sure about the second. If something has changed in the base branch, that means that the lock file committed in the PR might be out of sync from the PR itself (but that everything would be fine once it got merged.) I think that just running the lock against the HEAD of the PR might be simpler and more desirable. It might end up causing a merge conflict if there were also dep changes in the base branch, but I don't think that's really a problem. (Is that the worst potential outcome?)

I had previously considered whether uv lock should run on the merge commit or the HEAD commit of the PR branch, but after careful thought, I believe it should run on the merge commit for the following reasons:

1. The behavior of this PR is consistent with that of pre-commit.ci. pre-commit.ci runs on the merge commit and then modifies the PR branch files. If pre-commit.ci provides network connectivity in the future and we enable the uv-lock pre-commit hook, it will behave the same as this PR.

2. The purpose of running tests for a pull request is to ensure that there are no issues after merging, not before it. This can be demonstrated by the fact that workflows triggered by the pull_request event run on the merge commit, rather than the HEAD commit of a PR branch.

   This PR can absolutely guarantee that if the tests running on the pull request pass, the merge will succeed. However, if uv lock runs on the HEAD commit of the PR branch, it cannot guarantee this.

   For example, if the PR branch was split off from the main branch a long time ago and has modified its dependencies, there is a high likelihood of dependency conflicts after the merge. Another example from our project is when all tests for this PR passed, but the tests failed after the merge.

3. There is no issue with the discrepancy between the uv.lock and pyproject.yaml in the PR branch. The PR author doesn't need to pull the autofix commit. Even if they do, it won't cause any problems because all uv commands will automatically update the uv.lock file (unless --frozen argument is provided).

Would something simpler without all the scripting (like I was thinking in #4165) work if we just wanted to run on the HEAD of the PR?

The following lines can be replaced with tox-dev/action-pre-commit-uv:

- name: Install uv
  uses: astral-sh/setup-uv@887a942a15af3a7626099df99e897a18d9e5ab3a # v5
  with:
    enable-cache: true
    python-version: ${{ env.PYTHON_VERSION }}
    version: ${{ env.UV_VERSION }}
- id: push
  run: |
    if [[ ${{github.event_name}} == "push" ]]
    then
      uv lock
    else
      uv lock --directory merge
      mv merge/uv.lock uv.lock
    fi

However, in order to ensure that the uv version and Python version are consistent with other workflows, and to share the cache from other workflows, I believe it should not be replaced with:

- uses: tox-dev/action-pre-commit-uv@v1
  with:
    extra_args: uv-lock

The only lines of code that can be replaced with stefanzweifel/git-auto-commit-action are the ones below. I don't think it's necessary to introduce a new dependency just for these few lines of code, as the uncertainty in the behavior of stefanzweifel/git-auto-commit-action might actually increase the difficulty of understanding and maintaining the code.

if ! git diff --exit-code; then
  git config user.email github-actions[bot]@users.noreply.github.com
  git config user.name github-actions[bot]
  git add uv.lock
  git commit -m "Autofix: update uv.lock"
  git push
fi

These are just my thoughts. If you firmly hold a different opinion, I am willing to make changes.

Okay, you've convinced me, let's go for it. It looks like we might be able to let pre-commit.ci handle this again soon, there are rumblings that uv lock will no longer need the build backend with a few more changes.