Skip to content

Commit

Permalink
fix: some small improvements and deprecation logs
Browse files Browse the repository at this point in the history
  • Loading branch information
mrexox committed Jan 18, 2024
1 parent 79defa8 commit 1bf3f79
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 89 deletions.
36 changes: 18 additions & 18 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -376,21 +376,21 @@ remote:

> :test_tube: This feature is in **Beta** version

You can provide multiple remotes configs if you want to share yours lefthook configurations across many projects. Lefthook will automatically download and merge configurations into your local `lefthook.yml`.
You can provide multiple remote configs if you want to share yours lefthook configurations across many projects. Lefthook will automatically download and merge configurations into your local `lefthook.yml`.

You can use [`extends`](#extends) related to the config file (not absolute paths).
You can use [`extends`](#extends) but the paths must be relative to the remote repository root.

If you provide [`scripts`](#scripts) in a remote file, the [scripts](#source_dir) folder must be in the **root of the repository**.
If you provide [`scripts`](#scripts) in a remote config file, the [scripts](#source_dir) folder must also be in the **root of the repository**.

**Note**

Configuration in `remotes` will be merged to configuration in `lefthook.yml`, so the priority will be the following:
The configuration from `remotes` will be merged to the local config using the following priority:

- `lefthook.yml`
- `remotes`
- `lefthook-local.yml`
1. Local main config (`lefthook.yml`)
1. Remote configs (`remotes`)
1. Local overrides (`lefthook-local.yml`)

This can be changed in the future. For convenience, please use `remotes` configuration without any hooks configuration in `lefthook.yml`.
This priority may be changed in the future. For convenience, if you use `remotes`, please don't configure any hooks.

### `git_url`

Expand Down Expand Up @@ -428,13 +428,13 @@ remotes:
ref: v1.0.0
```

> **Note**
> :warning: **Note**
>
> :warning: If you initially had `ref` option, ran `lefthook install`, and then removed it, lefthook won't decide which branch/tag to use as a ref. So, if you added it once, please, use it always to avoid issues in local setups.
> If you initially had `ref` option, ran `lefthook install`, and then removed it, lefthook won't decide which branch/tag to use as a ref. So, if you added it once, please, use it always to avoid issues in local setups.

### `configs`

**Default:** `- lefthook.yml`
**Default:** `[lefthook.yml]`

An optional array of config paths from remote's root.

Expand All @@ -444,32 +444,32 @@ An optional array of config paths from remote's root.
# lefthook.yml
remotes:
- git_url: git@github.com:evilmartians/remote
- git_url: git@github.com:evilmartians/lefthook
ref: v1.0.0
configs:
- examples/ruby-linter.yml
- examples/test.yml
```

More complicated example.
Example with multiple remotes merging multiple configurations.

```yml
# lefthook.yml
remotes:
- git_url: git@github.com:evilmartians/remote
- git_url: git@github.com:org/lefthook-configs
ref: v1.0.0
configs:
- examples/ruby-linter.yml
- examples/test.yml
- git_url : https://github.com:example/repository
- git_url: https://github.com/org2/lefthook-configs
configs:
- lefthooks/pre_commit.yml
- lefthooks/post_merge.yml
- git_url : https://github.com:example2/repository2
ref: specific_branch
- git_url: https://github.com/org3/lefthook-configs
ref: feature/new
configs:
- example/pre-push.yml
- configs/pre-push.yml
```

Expand Down
6 changes: 4 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ type Config struct {
NoTTY bool `mapstructure:"no_tty,omitempty"`
AssertLefthookInstalled bool `mapstructure:"assert_lefthook_installed,omitempty"`
Colors interface{} `mapstructure:"colors,omitempty"`
Remote *Remote `mapstructure:"remote,omitempty"` // Deprecated in favor of Remotes
Remotes []*Remote `mapstructure:"remotes,omitempty"`

// Deprecated: use Remotes
Remote *Remote `mapstructure:"remote,omitempty"`
Remotes []*Remote `mapstructure:"remotes,omitempty"`

Hooks map[string]*Hook `mapstructure:"-"`
}
Expand Down
47 changes: 33 additions & 14 deletions internal/config/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ func readOne(fs afero.Fs, path string, names []string) (*viper.Viper, error) {
return nil, NotFoundError{fmt.Sprintf("No config files with names %q could not be found in \"%s\"", names, path)}
}

// mergeAll merges (.lefthook or lefthook) and (extended config) and (remotes)
// and (.lefthook-local or .lefthook-local) configs.
// mergeAll merges configs using the following order.
// - lefthook/.lefthook
// - files from `extends`
// - files from `remotes`
// - lefthook-local/.lefthook-local.
func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) {
extends, err := readOne(fs, repo.RootPath, []string{"lefthook", ".lefthook"})
if err != nil {
Expand Down Expand Up @@ -124,27 +127,24 @@ func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) {
return extends, nil
}

// mergeRemotes merges remotes config to the current one.
// mergeRemotes merges remote configs to the current one.
func mergeRemotes(fs afero.Fs, repo *git.Repository, v *viper.Viper) error {
var remote *Remote // Deprecated
var remotes []*Remote
var remote *Remote // Use for backward compatibility

err := v.UnmarshalKey("remotes", &remotes)
if err != nil {
return err
}

// Use for backward compatibility
// Deprecated
err = v.UnmarshalKey("remote", &remote)
if err != nil {
return err
}

// Use for backward compatibility
// If "remote" key exists, append it to "remotes"
// Backward compatibility
if remote != nil {
// Not logged because it's breaking tests
// log.Warn("DEPRECATED: \"remote\" key is deprecated, use \"remotes\" instead")
remotes = append(remotes, remote)
}

Expand All @@ -153,10 +153,8 @@ func mergeRemotes(fs afero.Fs, repo *git.Repository, v *viper.Viper) error {
continue
}

// Use for backward compatibility with "remote(s).config" instead of "remote(s).configs"
// Use for backward compatibility with "remote(s).config"
if remote.Config != "" {
// Not logged because it's breaking tests
// log.Warn("DEPRECATED: \"config\" key is deprecated, use \"configs\" instead for remotes")
remote.Configs = append(remote.Configs, remote.Config)
}

Expand All @@ -169,7 +167,7 @@ func mergeRemotes(fs afero.Fs, repo *git.Repository, v *viper.Viper) error {
configFile := config
configPath := filepath.Join(remotePath, configFile)

log.Debugf("Merging remote config: %s", configPath)
log.Debugf("Merging remote config: %s: %s", remote.GitURL, configPath)

_, err = fs.Stat(configPath)
if err != nil {
Expand Down Expand Up @@ -267,7 +265,28 @@ func unmarshalConfigs(base, extra *viper.Viper, c *Config) error {
return err
}

return base.Unmarshal(c)
if err := base.Unmarshal(c); err != nil {
return err
}

// Deprecation handling

if c.Remote != nil {
log.Warn("DEPRECATED: \"remote\" option is deprecated and will be omitted in the next major release, use \"remotes\" option instead")
c.Remotes = append(c.Remotes, c.Remote)
}
c.Remote = nil

for _, remote := range c.Remotes {
if remote.Config != "" {
log.Warn("DEPRECATED: \"remotes\".\"config\" option is deprecated and will be omitted in the next major release, use \"configs\" option instead")
remote.Configs = append(remote.Configs, remote.Config)
}

remote.Config = ""
}

return nil
}

func addHook(hookName string, base, extra *viper.Viper, c *Config) error {
Expand Down
30 changes: 18 additions & 12 deletions internal/config/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,10 @@ pre-commit:
SourceDir: DefaultSourceDir,
SourceDirLocal: DefaultSourceDirLocal,
Colors: nil,
Remote: &Remote{
GitURL: "git@github.com:evilmartians/lefthook",
Remotes: []*Remote{
{
GitURL: "git@github.com:evilmartians/lefthook",
},
},
Hooks: map[string]*Hook{
"pre-commit": {
Expand Down Expand Up @@ -488,10 +490,12 @@ pre-commit:
SourceDir: DefaultSourceDir,
SourceDirLocal: DefaultSourceDirLocal,
Colors: nil,
Remote: &Remote{
GitURL: "git@github.com:evilmartians/lefthook",
Ref: "v1.0.0",
Config: "examples/custom.yml",
Remotes: []*Remote{
{
GitURL: "git@github.com:evilmartians/lefthook",
Ref: "v1.0.0",
Configs: []string{"examples/custom.yml"},
},
},
Hooks: map[string]*Hook{
"pre-commit": {
Expand Down Expand Up @@ -573,9 +577,11 @@ pre-push:
SourceDir: DefaultSourceDir,
SourceDirLocal: DefaultSourceDirLocal,
Colors: nil,
Remote: &Remote{
GitURL: "https://github.com/evilmartians/lefthook",
Config: "examples/config.yml",
Remotes: []*Remote{
{
GitURL: "https://github.com/evilmartians/lefthook",
Configs: []string{"examples/config.yml"},
},
},
Extends: []string{"local-extend.yml"},
Hooks: map[string]*Hook{
Expand Down Expand Up @@ -830,9 +836,9 @@ pre-commit:
Colors: nil,
Remotes: []*Remote{
{
GitURL: "https://github.com/evilmartians/lefthook",
Ref: "v1.0.0",
Config: "examples/custom.yml",
GitURL: "https://github.com/evilmartians/lefthook",
Ref: "v1.0.0",
Configs: []string{"examples/custom.yml"},
},
{
GitURL: "https://github.com/evilmartians/lefthook",
Expand Down
7 changes: 4 additions & 3 deletions internal/config/remote.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package config

type Remote struct {
GitURL string `mapstructure:"git_url" yaml:"git_url" json:"git_url,omitempty" toml:"git_url"`
Ref string `mapstructure:"ref,omitempty" yaml:",omitempty" json:"ref,omitempty" toml:"ref,omitempty"`
Config string `mapstructure:"config,omitempty" yaml:",omitempty" json:"config,omitempty" toml:"config,omitempty"` // Deprecated in favor of Configs
GitURL string `mapstructure:"git_url" yaml:"git_url" json:"git_url,omitempty" toml:"git_url"`
Ref string `mapstructure:"ref,omitempty" yaml:",omitempty" json:"ref,omitempty" toml:"ref,omitempty"`
// Deprecated
Config string `mapstructure:"config,omitempty" yaml:",omitempty" json:"config,omitempty" toml:"config,omitempty"`
Configs []string `mapstructure:"configs,omitempty" yaml:",omitempty" json:"configs,omitempty" toml:"configs,omitempty"`
}

Expand Down
44 changes: 19 additions & 25 deletions internal/git/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,9 @@ const (
// RemoteFolder returns the path to the folder where the remote
// repository is located.
func (r *Repository) RemoteFolder(url string, ref string) string {
directoryName := filepath.Base(
strings.TrimSuffix(url, filepath.Ext(url)),
)

if ref != "" {
directoryName = directoryName + "-" + ref
}

return filepath.Join(
r.RemotesFolder(),
directoryName,
remoteDirectoryName(url, ref),
)
}

Expand All @@ -40,25 +32,15 @@ func (r *Repository) RemotesFolder() string {
// specified as a remote config repository. If successful, the path to the root
// of the repository will be returned.
func (r *Repository) SyncRemote(url, ref string) error {
remotesPath := filepath.Join(r.InfoPath, remotesFolder)
remotesPath := r.RemotesFolder()

err := r.Fs.MkdirAll(remotesPath, remotesFolderMode)
if err != nil && !errors.Is(err, os.ErrExist) {
return err
}

directoryName := filepath.Base(
strings.TrimSuffix(url, filepath.Ext(url)),
)

if ref != "" {
directoryName = directoryName + "-" + ref
}

remotePath := filepath.Join(
remotesPath,
directoryName,
)
directoryName := remoteDirectoryName(url, ref)
remotePath := filepath.Join(remotesPath, directoryName)

_, err = r.Fs.Stat(remotePath)
if err == nil {
Expand Down Expand Up @@ -96,10 +78,10 @@ func (r *Repository) updateRemote(path, ref string) error {
return nil
}

func (r *Repository) cloneRemote(cwd, directoryName, url, ref string) error {
log.Debugf("Cloning remote config repository: %v/%v", cwd, directoryName)
func (r *Repository) cloneRemote(dest, directoryName, url, ref string) error {
log.Debugf("Cloning remote config repository: %v/%v", dest, directoryName)

cmdClone := []string{"git", "-C", cwd, "clone", "--quiet", "--depth", "1"}
cmdClone := []string{"git", "-C", dest, "clone", "--quiet", "--depth", "1"}
if len(ref) > 0 {
cmdClone = append(cmdClone, "--branch", ref)
}
Expand All @@ -112,3 +94,15 @@ func (r *Repository) cloneRemote(cwd, directoryName, url, ref string) error {

return nil
}

func remoteDirectoryName(url, ref string) string {
name := filepath.Base(
strings.TrimSuffix(url, filepath.Ext(url)),
)

if ref != "" {
name = name + "-" + ref
}

return name
}
5 changes: 0 additions & 5 deletions internal/lefthook/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ func (l *Lefthook) Install(force bool) error {
return err
}

// For backward compatibility with single remote config
if cfg.Remote != nil {
cfg.Remotes = append(cfg.Remotes, cfg.Remote)
}

for _, remote := range cfg.Remotes {
if remote.Configured() {
if err := l.repo.SyncRemote(remote.GitURL, remote.Ref); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions internal/lefthook/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,6 @@ Run 'lefthook install' manually.`,
filepath.Join(l.repo.RootPath, cfg.SourceDirLocal),
}

// For backward compatibility with single remote config
if cfg.Remote != nil {
cfg.Remotes = append(cfg.Remotes, cfg.Remote)
}

for _, remote := range cfg.Remotes {
if remote.Configured() {
// Append only source_dir, because source_dir_local doesn't make sense
Expand Down
11 changes: 7 additions & 4 deletions testdata/remote.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ remote:
ref: v1.4.0

-- lefthook-dump.yml --
DEPRECATED: "remote" option is deprecated and will be omitted in the next major release, use "remotes" option instead
DEPRECATED: "remotes"."config" option is deprecated and will be omitted in the next major release, use "configs" option instead
pre-commit:
scripts:
good_job.js:
runner: node
remote:
config: examples/with_scripts/lefthook.yml
git_url: https://github.com/evilmartians/lefthook
ref: v1.4.0
remotes:
- git_url: https://github.com/evilmartians/lefthook
ref: v1.4.0
configs:
- examples/with_scripts/lefthook.yml
Loading

1 comment on commit 1bf3f79

@NikitaCOEUR
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great !
Thanks again for this thorough review.

Please sign in to comment.