[Dev] pre-commit rubocop hook attempts to install for non-Ruby file changes

[thisisnic]

Describe the bug, including details regarding any error messages, version, and platform.

The below description and proposed solution was generated by claude when I ran into this issue, so while I'm happy to make the change myself, please can someone verify this makes sense to do?

---

Describe the bug

The rubocop pre-commit hook attempts to install its environment even when committing changes that don't touch any Ruby files. This causes errors for developers who don't have Ruby installed, even when they're only working on R, Python, or other components.

Current behavior:

When committing changes to non-Ruby files (e.g., R package files), pre-commit tries to install the rubocop environment and fails if Ruby is not installed:

[INFO] Installing environment for https://github.com/rubocop/rubocop.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('gem', 'build', 'rubocop.gemspec')
return code: 1
stdout:
Executable gem not found

Root cause:

In .pre-commit-config.yaml, the rubocop hook (lines 247-258) has an exclude filter but is missing a files filter:

- repo: https://github.com/rubocop/rubocop
  rev: "v1.71.0"
  hooks:
    - id: rubocop
      name: Ruby Format
      alias: ruby
      args:
        - "--autocorrect"
      exclude: >-
        (
        ?^dev/tasks/homebrew-formulae/.*\.rb$|
        )

Without a files filter, pre-commit attempts to set up the hook environment for all commits, regardless of which files are being modified.

Proposed fix:

Add a files filter to only activate rubocop for Ruby files:

  - repo: https://github.com/rubocop/rubocop
    rev: "v1.71.0"
    hooks:
      - id: rubocop
        files: \.rb$  # <-- Add this line
        name: Ruby Format
        alias: ruby
        args:
          - "--autocorrect"
        exclude: >-
          (
          ?^dev/tasks/homebrew-formulae/.*\.rb$|
          )

This pattern is already used correctly in other hooks in the same config file.

Expected behavior:

The rubocop hook should only attempt to install/run when Ruby files are being committed.

Component(s)

Developer Tools

[kou]

I think that the Rubocob hook is initialized with files: \.rb$.
How about using nice pre-commit run --show-diff-on-failure --color=always --all-files r for R development?

[thisisnic]

Just for additional context, I was on a fresh Ubuntu install and whichever pre-commit command ran was activated by the RStudio IDE when I hit the commit button. I solved it by just installing it but it might trip up other contributors.

I've also had similar issues in VS Code - I think it's an "interacting with git in an IDE" problem.

If it's possible, I'd prefer to find a solution which doesn't require using a specific command as I think it'll be off-putting to new or occasional contributors.

[thisisnic]

I think that the Rubocob hook is initialized with files: .rb$.

I took a look and it doesn't look like it is, looking at lines 243-254 below.

https://github.com/apache/arrow/blob/main/.pre-commit-config.yaml

[thisisnic]

I had a bit more of a look into it, that wasn't the actual problem and it doesn't solve the issue. The rubocop hook itself is configured to run on only Ruby files, so including the files filter or not becomes redundant.

There is still the issue that it tries to install regardless of whether it's a Ruby PR or not.

See below for LLM summary of what I tried.

I'm going to remove the instructions to use pre-commit from the R docs as it creates a barrier for new contributors, but otherwise I think no changes needed here as none of the alternatives suggested seem like good options.

I feel this is going to create barriers for new contributors in other implementations if they try to use pre-commit. Maybe we want to update the docs to mention that it'll install everything?

CC @AlenkaF as well.

claude summary below

---

After investigating, the proposed fix (files: .rb$ filter) doesn't actually solve the issue. Here's why:

What we found:

• The files filter in pre-commit only controls which files are checked, not whether the environment is installed
• Pre-commit attempts to install ALL hook environments upfront, regardless of file filters
• The rubocop environment installation requires gem to build the rubocop.gemspec file
• Without Ruby/gem installed, installation fails with "Executable gem not found" before it even gets to the file filtering stage

Testing:

• Uninstalled Ruby completely
• Tested with and without files: .rb$ filter
• Both cases failed with the same error when pre-commit tried to initialize environments
• The error occurs even when committing only non-Ruby files

Why this is problematic:

• IDE git integration (RStudio, VS Code) runs pre-commit automatically
• Can't use workarounds like SKIP=rubocop environment variable from an IDE
• Can't control which pre-commit command the IDE runs

Possible solutions:

1. Make rubocop a local hook with a wrapper script that checks if Ruby exists first
2. Accept that Ruby needs to be installed as a dev dependency for anyone using pre-commit
3. Use a custom git hook wrapper that sets SKIP automatically
4. Remove rubocop from pre-commit entirely and run it separately

The fundamental issue is that pre-commit's design installs all environments upfront, and there's no way to make an environment optional based on what's available on the system.

[kou]

How about implementing auto-fix GitHub Actions for new contributors so that they don't need to run pre-commit on local?

I think that something like the following will work:

name: Autofix

on:
  push:

permissions:
  contents: write

jobs:
  process:
    name: Process
    if: >-
      github.repository != 'apache/arrow' &&
      github.ref_name != 'main'
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
      # Prepare pre-commit
      - name: Run pre-commit
        run: |
          pre-commit run --all-files --color=always --show-diff-on-failure
      - name: Push
        run: |
          if [ "$(git status --porcelain)" != "" ]; then
            git add --all
            git commit -m "Autofix"
            git push
          fi

[thisisnic]

I really like this idea, sounds good!

[AlenkaF]

Hm, I haven't ran into anything similar. In Python we use:

pre-commit run --show-diff-on-failure --color=always --all-files python

that is what @kou suggested, the command is included in the dev docs and it works very well.

Having auto-fix GitHub Actions for new contributors sounds cool too! Was there any similar discussion about GitHub Actions when implementing pre-commit @kou ?

[thisisnic]

@AlenkaF Absolutely; that works great from the command line, but not from an IDE where it'll try to run all of the pre-commit hooks and not just the Python one, which is where folks could get tripped up if they don't have Ruby (or R if not R folks!) installed, so perhaps worth us mentioning in the docs?

[AlenkaF]

Ah, got it. I am sure that is an issue in R, that's unfortunate.
In Python, though, I am not sure how many devs use IDE to run that? How does VS Code run pre-commit automatically?

[thisisnic]

Ah, got it. I am sure that is an issue in R, that's unfortunate. In Python, though, I am not sure how many devs use IDE to run that? How does VS Code run pre-commit automatically?

If the user has pre-commit installed/configured and then clicks on the button in the UI in VS Code to commit/push (rather than using the terminal) it will automatically run all hooks.

[AlenkaF]

Ah, good to know. Thanks!
I think it would be worth mentioning that in the docs, yes 👍

[kou]

Was there any similar discussion about GitHub Actions when implementing pre-commit?

No. But the "autotone" job may be related. We had the "autotune" job: #6932
But it didn't work. So it was removed: #44649

The "autotune" job runs on apache/arrow. But #47937 (comment) runs on fork. I think that we can fix the permission problem by this.