no-commit-to-branch linter fails in CI

[gregbrowndev]

The no-commit-to-branch hook fails when run in a CI environment:

[//:ci] [//:lint] Error running hk util no-commit-to-branch
[//:ci] [//:lint] Error: Failed to get current git branch
[//:ci] [//:lint] 
[//:ci] [//:lint] Location:
[//:ci] [//:lint]     src/cli/util/no_commit_to_branch.rs:39:78
[//:ci] [//:lint] 
[//:ci] [//:lint] See /home/runner/.local/state/hk/output.log for full command output
[//:ci] [//:lint] ERROR task failed

I'm basically running hk check --all here. Sample hk.pkl:

amends "package://github.com/jdx/hk/releases/download/v1.39.0/hk@1.39.0#/Config.pkl"
import "package://github.com/jdx/hk/releases/download/v1.39.0/hk@1.39.0#/Builtins.pkl"

local linters = new Mapping<String, Step> {
  ["no-commit-to-branch"] = Builtins.no_commit_to_branch
}

hooks {
  ["pre-commit"] {
    fix = true
    stash = "git"
    steps = linters
  }
  ["pre-push"] {
    steps = linters
  }
  ["fix"] {
    fix = true
    steps = linters
  }
  ["check"] {
    steps = linters
  }
}

I'm attempting to fix by extending it with a condition so the hook doesn't run in CI. If I'm not wrong, it would be sensible to fix this upstream in the hook config.

Thanks 🙏🏻

[jdx]

I think it doesn't make sense to use this step outside pre-commit

[gregbrowndev]

Running the hook with hk check seems valid, as long as you're running it in your dev environment, just not CI. I've aliased mise run lint to hk check --all so it already caught a mistake when I was working directly on main.

Would something like this work?

local linters = new Mapping<String, Step> {
  ["no-commit-to-branch"] = (Builtins.no_commit_to_branch) {
    // FIX: hook breaks in CI env
    condition = "exec('if [ -n \"$CI\" ]; then exit 0; else exit 1; fi')"
  }
}

I've not quite got it to work yet. I also tried using $env from expr but not had any luck.

Alternatively, I guess I could just add the hook to pre-commit and spread the remaining linters as you suggested.

[jdx]

I would probably add a profile for ci

[astei]

I've run into this same problem, except I'm trying to run the pre-commit step locally (I otherwise would've used a profile).

Both hooks invoke git symbolic-ref HEAD (hk adds --short). However, this command can fail in some cases (i.e. if the user currently has a detached HEAD/not on a branch). pre-commit-hooks ignores the failure. hk handles it strictly.

This can actually trigger in some legitimate pre-commit scenarios, i.e. if one uses Graphite for PR stacking.

I can work around this by just using pre-commit-hooks, but I'd prefer to use the native mechanism.

[astei]

(To add some more precision: we could skip this hook if Graphite is being used, but then we'd accidentally permit changes against protected branches without running git symbolic-ref HEAD ourselves again, and in that case, why use the built-in?)

[gregbrowndev]

As suggested, I ended up going with:

amends "package://github.com/jdx/hk/releases/download/v1.39.0/hk@1.39.0#/Config.pkl"
import "package://github.com/jdx/hk/releases/download/v1.39.0/hk@1.39.0#/Builtins.pkl"

local linters = new Mapping<String, Step> {
  // other linters
}

hooks {
  ["pre-commit"] {
    fix = true
    stash = "git"
    steps {
      ["no-commit-to-branch"] = Builtins.no_commit_to_branch
      ...linters
    }
  }
  ["pre-push"] {
    steps = linters
  }
  ["fix"] {
    fix = true
    steps = linters
  }
  ["check"] {
    steps = linters
  }
}