Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot force command to run with --all-files unless using {files} #554

Closed
sanmai-NL opened this issue Sep 23, 2023 · 5 comments
Closed

Cannot force command to run with --all-files unless using {files} #554

sanmai-NL opened this issue Sep 23, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@sanmai-NL
Copy link
Contributor

🔧 Summary

Lefthook version

1.5.0

Steps to reproduce

.lefthook.yml:

---
pre-push:
  commands: &pre-push
    pdm-lock:
      files: git ls-files pyproject.toml
      run: pdm lock --group ':all' --refresh
  parallel: true

Expected results

When invoked manually, commands are always run, regardless of template occurrence in the command line.

Actual results

  1. The pre-push hook needlessly checks whether there is something to push, even when invoked manually.
  2. When a {files} template is used in the command line, the check whether there is something to push is suddenly skipped and the command is run, however.

Possible Solution

When invoked manually, commands are always run, regardless of template occurrence in the command line.

Logs / Screenshots

$ lefthook run -v pre-push --commands pdm-lock --all-files
│ [lefthook] cmd: [git rev-parse --show-toplevel]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: /Users/sanderhan/devel/gitlab.com/han-aim/tools
                                                               
│ [lefthook] cmd: [git rev-parse --git-path hooks]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/hooks
                          
│ [lefthook] cmd: [git rev-parse --git-path info]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/info
                         
│ [lefthook] cmd: [git rev-parse --git-dir]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git
                    
╭────────────────────────────────────╮
│ 🥊 lefthook v1.5.0  hook: pre-push │
╰────────────────────────────────────╯
│ [git-lfs] executing hook: git lfs pre-push 
│ [git-lfs] out: This should be run through Git's pre-push hook.  Run `git lfs update` to install it.
│ [git-lfs] err: exit status 1
│ [lefthook] cmd: [sh -c git ls-files pyproject.toml]
│ [lefthook] dir: /Users/myuser/repo
│ [lefthook] err: <nil>
│ [lefthook] out: pyproject.toml
                              
│ [lefthook] files before filters:
[pyproject.toml]                
│ [lefthook] files after filters:
[pyproject.toml]               
│ [lefthook] cmd: [sh -c git diff --name-only HEAD @{push}]
│ [lefthook] dir: /Users/myuser/repo
│ [lefthook] err: <nil>
│ [lefthook] out: 
│  pdm-lock (skip) no matching push files
                                      
  ─────────────────────────

When a (dummy) {files} template is used, the command is run, however.

# ...
run: pdm lock --group ':all' --group '{files}' --refresh
# ...
$ lefthook run -v pre-push --commands pdm-lock --all-files
│ [lefthook] cmd: [git rev-parse --show-toplevel]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: /Users/myuser/repo
                                                               
│ [lefthook] cmd: [git rev-parse --git-path hooks]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/hooks
                          
│ [lefthook] cmd: [git rev-parse --git-path info]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/info
                         
│ [lefthook] cmd: [git rev-parse --git-dir]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git
                    
╭────────────────────────────────────╮
│ 🥊 lefthook v1.5.0  hook: pre-push │
╰────────────────────────────────────╯
│ [git-lfs] executing hook: git lfs pre-push 
│ [git-lfs] out: This should be run through Git's pre-push hook.  Run `git lfs update` to install it.
│ [git-lfs] err: exit status 1
│ [lefthook] cmd: [sh -c git ls-files pyproject.toml]
│ [lefthook] dir: /Users/myuser/repo
│ [lefthook] err: <nil>
│ [lefthook] out: pyproject.toml
                              
│ [lefthook] files before filters:
[pyproject.toml]                
│ [lefthook] files after filters:
[pyproject.toml]               
│ [lefthook] files after escaping:
[pyproject.toml]                
│ [lefthook] executing: pdm lock --group ':all' --group 'pyproject.toml' --refresh
┃  pdm-lock ❯ 

Ignoring non-existing groups: pyproject.toml
Changes are written to pdm.lock.

                                      
  ────────────────────────────────────
summary: (done in 2.90 seconds)       
✔️  pdm-lock
@sanmai-NL sanmai-NL added the bug Something isn't working label Sep 23, 2023
@mrexox
Copy link
Member

mrexox commented Oct 7, 2023

Hey, in v1.5.1 I've added a --force flag, so you can use lefthook run pre-push --force if you don't want the command to be skipped.

I think it would be a bad design to distinguish between manual run and hook-triggered run. However it is not impossible.

@sanmai-NL
Copy link
Contributor Author

sanmai-NL commented Oct 10, 2023

@mrexox --force now runs commands with empty file lists if nothing is staged. Furthermore --force with --all-files runs all commands with all files, irrespective of if they match the command. I don‘t see how how these behaviors are ever useful.

I requested to be able to run a hook with either all files or the staged files, per the hook configuration, regardless of the name of the hook (pre-push now behaves differently from foo).

@mrexox
Copy link
Member

mrexox commented Oct 11, 2023

@sanmai-NL , I understand your case. There is an implicit check in pre-commit and pre-push hooks that skip execution if {staged_files} or {push_files} are empty respectively. This behavior was added for convenience, so people could omit using {files} with filters when they actually mean staged files and pushed files.

It looks like you want to use lefthook for manual hooks without those tweaks. I can suggest you to use the YAML anchors:

checks:
  files: <a custom command for files whether it is staged files or push files >
  commands: &checks
    pdm-lock:
      glob: <here you can specify any filters>
      run: pdm lock --group ':all' --refresh
    # ...

pre-push:
  commands: *checks # here push files will be used implicitly

pre-commit:
  commands: *checks # here staged files will be used implicitly

So, you use git hooks when calling lefthook implicitly and custom hooks when calling explicitly via lefthook run. Does it work for you?

@sanmai-NL
Copy link
Contributor Author

@mrexox That doesn't pick up other settings of the hook, like parallel.

I don't really see the logic of that convenience. Was there a problem for users?

@mrexox
Copy link
Member

mrexox commented Oct 12, 2023

There were the following issues:

That doesn't pick up other settings of the hook, like parallel.

Could you share your requirements, so I could think out a proper configuration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants