Skip to content

Commit ec7e8e1

Browse files
joevin-slq-doctojoevin-slq-docto
authored
feat: check core.hooksPath when lefthook install (#1292)
1 parent 47b8f5c commit ec7e8e1

File tree

3 files changed

+301
-3
lines changed

3 files changed

+301
-3
lines changed

‎cmd/install.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,15 @@ func install() *cli.Command {
1919
Flags: []cli.Flag{
2020
&cli.BoolFlag{
2121
Name: "force",
22-
Usage: "overwrite .old files",
22+
Usage: "proceed with installation even if core.hooksPath is configured (overwrite .old files)",
2323
Aliases: []string{"f"},
2424
Destination: &args.Force,
2525
},
26+
&cli.BoolFlag{
27+
Name: "reset-hooks-path",
28+
Usage: "automatically unset core.hooksPath configuration",
29+
Destination: &args.ResetHooksPath,
30+
},
2631
&cli.BoolFlag{
2732
Name: "verbose",
2833
Aliases: []string{"v"},

‎internal/command/install.go‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,18 @@ var (
3636
)
3737

3838
type InstallArgs struct {
39-
Force bool
39+
Force bool
40+
ResetHooksPath bool
4041
}
4142

4243
func (l *Lefthook) Install(ctx context.Context, args InstallArgs, hooks []string) error {
44+
// Check for core.hooksPath configuration
45+
localHooksPath, globalHooksPath := l.getHooksPathConfig()
46+
47+
if err := l.ensureNoHooksPath(localHooksPath, globalHooksPath, args.Force, args.ResetHooksPath); err != nil {
48+
return err
49+
}
50+
4351
cfg, err := l.readOrCreateConfig()
4452
if err != nil {
4553
return err
@@ -461,3 +469,98 @@ func (l *Lefthook) ensureHooksDirExists() error {
461469

462470
return nil
463471
}
472+
473+
// getHooksPathConfig checks if core.hooksPath is configured locally or globally.
474+
func (l *Lefthook) getHooksPathConfig() (local, global string) {
475+
local, _ = l.repo.Git.Cmd([]string{"git", "config", "--local", "core.hooksPath"})
476+
global, _ = l.repo.Git.Cmd([]string{"git", "config", "--global", "core.hooksPath"})
477+
return
478+
}
479+
480+
// ensureNoHooksPath ensures core.hooksPath is not configured.
481+
func (l *Lefthook) ensureNoHooksPath(local, global string, force, resetHooksPath bool) error {
482+
hasLocal := len(local) > 0
483+
hasGlobal := len(global) > 0
484+
485+
if !hasLocal && !hasGlobal {
486+
return nil
487+
}
488+
489+
// If neither force nor resetHooksPath, returns an error with instructions.
490+
if !force && !resetHooksPath {
491+
return formatHooksPathError(local, global)
492+
}
493+
494+
// Warn about configured core.hooksPath
495+
if hasLocal {
496+
log.Warnf("core.hooksPath is set locally to '%s'.", local)
497+
}
498+
if hasGlobal {
499+
log.Warnf("core.hooksPath is set globally to '%s'.", global)
500+
}
501+
502+
// If resetHooksPath is true, unset the conflicting configurations.
503+
if resetHooksPath {
504+
return l.unsetHooksPathConfig(local, global)
505+
}
506+
507+
// If force is true, proceed with installation anyway (without unsetting).
508+
// Determine path: use global path if only global is defined, otherwise use local path
509+
path := local
510+
if !hasLocal && hasGlobal {
511+
path = global
512+
}
513+
log.Warnf("Installing lefthook anyway in '%s'.", path)
514+
515+
return nil
516+
}
517+
518+
// formatHooksPathError formats an error message for core.hooksPath conflicts.
519+
func formatHooksPathError(local, global string) error {
520+
var errMsg strings.Builder
521+
var hints []string
522+
hasLocal := len(local) > 0
523+
hasGlobal := len(global) > 0
524+
525+
if hasLocal {
526+
errMsg.WriteString(fmt.Sprintf("core.hooksPath is set locally to '%s'.\n", local))
527+
hints = append(hints, "hint: git config --unset-all --local core.hooksPath")
528+
}
529+
if hasGlobal {
530+
errMsg.WriteString(fmt.Sprintf("core.hooksPath is set globally to '%s'.\n", global))
531+
hints = append(hints, "hint: git config --unset-all --global core.hooksPath")
532+
}
533+
534+
errMsg.WriteString("hint: Run these commands to remove it:\n")
535+
errMsg.WriteString(strings.Join(hints, "\n"))
536+
errMsg.WriteString("\nhint: Or run lefthook with --reset-hooks-path to automatically unset it:\n")
537+
errMsg.WriteString("hint: lefthook install --reset-hooks-path\n")
538+
539+
// Determine path: use global path if only global is defined, otherwise use local path
540+
path := local
541+
if !hasLocal && hasGlobal {
542+
path = global
543+
}
544+
errMsg.WriteString(fmt.Sprintf("hint: Use option --force to install lefthook it anyway in '%s'.", path))
545+
546+
return errors.New(errMsg.String())
547+
}
548+
549+
// unsetHooksPathConfig removes core.hooksPath configuration.
550+
func (l *Lefthook) unsetHooksPathConfig(local, global string) error {
551+
if len(local) > 0 {
552+
if _, err := l.repo.Git.Cmd([]string{"git", "config", "--local", "--unset-all", "core.hooksPath"}); err != nil {
553+
return fmt.Errorf("failed to unset local core.hooksPath: %w", err)
554+
}
555+
log.Warn("local core.hooksPath has been unset.")
556+
}
557+
558+
if len(global) > 0 {
559+
if _, err := l.repo.Git.Cmd([]string{"git", "config", "--global", "--unset-all", "core.hooksPath"}); err != nil {
560+
return fmt.Errorf("failed to unset global core.hooksPath: %w", err)
561+
}
562+
log.Warn("global core.hooksPath has been unset.")
563+
}
564+
565+
return nil
566+
}

‎internal/command/install_test.go‎

Lines changed: 191 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,20 @@ remotes:
317317
t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) {
318318
assert := assert.New(t)
319319

320+
// Prepend git config commands required by getHooksPathConfig() in install.go.
321+
// These commands are always called at the start of Install() to detect core.hooksPath conflicts.
322+
gitCmds := tt.git
323+
if len(gitCmds) == 0 || gitCmds[0].Command != "git config --local core.hooksPath" {
324+
gitCmds = append([]cmdtest.Out{
325+
{Command: "git config --local core.hooksPath"},
326+
{Command: "git config --global core.hooksPath"},
327+
}, gitCmds...)
328+
}
329+
320330
repo := gittest.NewRepositoryBuilder().
321331
Root(root).
322332
Fs(fs).
323-
Cmd(cmdtest.NewOrdered(t, tt.git)).
333+
Cmd(cmdtest.NewOrdered(t, gitCmds)).
324334
Build()
325335
lefthook := &Lefthook{
326336
fs: fs,
@@ -708,3 +718,183 @@ remotes:
708718
})
709719
}
710720
}
721+
722+
func TestLefthookInstallWithCoreHooksPath(t *testing.T) {
723+
root, err := filepath.Abs("src")
724+
assert.NoError(t, err)
725+
726+
configPath := filepath.Join(root, "lefthook.yml")
727+
728+
hookPath := func(hook string) string {
729+
return filepath.Join(gittest.GitPath(root), "hooks", hook)
730+
}
731+
732+
infoPath := func(file string) string {
733+
return filepath.Join(gittest.GitPath(root), "info", file)
734+
}
735+
736+
configContent := `
737+
pre-commit:
738+
commands:
739+
tests:
740+
run: yarn test
741+
`
742+
743+
for n, tt := range [...]struct {
744+
name string
745+
force bool
746+
resetHooksPath bool
747+
git []cmdtest.Out
748+
wantError bool
749+
wantErrorMsg string
750+
wantExist []string
751+
}{
752+
{
753+
name: "with local and global core.hooksPath without flags",
754+
force: false,
755+
resetHooksPath: false,
756+
git: []cmdtest.Out{
757+
{
758+
Command: "git config --local core.hooksPath",
759+
Output: ".custom-hooks",
760+
},
761+
{
762+
Command: "git config --global core.hooksPath",
763+
Output: "/usr/local/hooks",
764+
},
765+
},
766+
wantError: true,
767+
wantErrorMsg: "core.hooksPath",
768+
},
769+
{
770+
name: "with local and global core.hooksPath with --force",
771+
force: true,
772+
resetHooksPath: false,
773+
git: []cmdtest.Out{
774+
{
775+
Command: "git config --local core.hooksPath",
776+
Output: ".custom-hooks",
777+
},
778+
{
779+
Command: "git config --global core.hooksPath",
780+
Output: "/usr/local/hooks",
781+
},
782+
},
783+
wantError: false,
784+
wantExist: []string{
785+
configPath,
786+
hookPath("pre-commit"),
787+
infoPath(config.ChecksumFileName),
788+
},
789+
},
790+
{
791+
name: "with local and global core.hooksPath with --reset-hooks-path",
792+
force: false,
793+
resetHooksPath: true,
794+
git: []cmdtest.Out{
795+
{
796+
Command: "git config --local core.hooksPath",
797+
Output: ".custom-hooks",
798+
},
799+
{
800+
Command: "git config --global core.hooksPath",
801+
Output: "/usr/local/hooks",
802+
},
803+
{
804+
Command: "git config --local --unset-all core.hooksPath",
805+
},
806+
{
807+
Command: "git config --global --unset-all core.hooksPath",
808+
},
809+
},
810+
wantError: false,
811+
wantExist: []string{
812+
configPath,
813+
hookPath("pre-commit"),
814+
infoPath(config.ChecksumFileName),
815+
},
816+
},
817+
{
818+
name: "with only global core.hooksPath with --force",
819+
force: true,
820+
resetHooksPath: false,
821+
git: []cmdtest.Out{
822+
{
823+
Command: "git config --local core.hooksPath",
824+
Output: "",
825+
},
826+
{
827+
Command: "git config --global core.hooksPath",
828+
Output: "/usr/local/hooks",
829+
},
830+
},
831+
wantError: false,
832+
wantExist: []string{
833+
configPath,
834+
hookPath("pre-commit"),
835+
infoPath(config.ChecksumFileName),
836+
},
837+
},
838+
{
839+
name: "with only local core.hooksPath with --reset-hooks-path",
840+
force: false,
841+
resetHooksPath: true,
842+
git: []cmdtest.Out{
843+
{
844+
Command: "git config --local core.hooksPath",
845+
Output: ".custom-hooks",
846+
},
847+
{
848+
Command: "git config --global core.hooksPath",
849+
Output: "",
850+
},
851+
{
852+
Command: "git config --local --unset-all core.hooksPath",
853+
},
854+
},
855+
wantError: false,
856+
wantExist: []string{
857+
configPath,
858+
hookPath("pre-commit"),
859+
infoPath(config.ChecksumFileName),
860+
},
861+
},
862+
} {
863+
fs := afero.NewMemMapFs()
864+
865+
t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) {
866+
assert := assert.New(t)
867+
868+
repo := gittest.NewRepositoryBuilder().
869+
Root(root).
870+
Fs(fs).
871+
Cmd(cmdtest.NewOrdered(t, tt.git)).
872+
Build()
873+
lefthook := &Lefthook{
874+
fs: fs,
875+
repo: repo,
876+
}
877+
878+
// Create configuration file
879+
assert.NoError(afero.WriteFile(fs, configPath, []byte(configContent), 0o644))
880+
timestamp := time.Date(2022, time.June, 22, 10, 40, 10, 1, time.UTC)
881+
assert.NoError(fs.Chtimes(configPath, timestamp, timestamp))
882+
883+
// Do install
884+
err := lefthook.Install(t.Context(), InstallArgs{Force: tt.force, ResetHooksPath: tt.resetHooksPath}, nil)
885+
if tt.wantError {
886+
if assert.Error(err) && tt.wantErrorMsg != "" {
887+
assert.Contains(err.Error(), tt.wantErrorMsg)
888+
}
889+
} else {
890+
assert.NoError(err)
891+
// Test files that should exist
892+
for _, file := range tt.wantExist {
893+
ok, err := afero.Exists(fs, file)
894+
assert.NoError(err)
895+
assert.True(ok)
896+
}
897+
}
898+
})
899+
}
900+
}

0 commit comments

Comments
 (0)