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

Git VCS driver: Support non-default target refs #345

Merged
merged 1 commit into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
/node_modules
.DS_Store
*.exe
/db
/data
16 changes: 15 additions & 1 deletion config-example.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
"dbpath" : "data",
"title" : "Hound",
"health-check-uri" : "/healthz",
"vcs-config" : {
"git": {
"detect-ref" : true
}
},
"repos" : {
"SomeGitRepo" : {
"url" : "https://www.github.com/YourOrganization/RepoOne.git"
Expand All @@ -12,13 +17,19 @@
"ms-between-poll": 10000,
"exclude-dot-files": true
},
"GitRepoWithDetectRefDisabled" : {
"url" : "https://www.github.com/YourOrganization/RepoOne.git",
"vcs-config" : {
"detect-ref" : false
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

config.json could probably use some more complete documentation generally, but just for the sake of completeness, could you add an example of using a custom ref somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do - thanks!

"SomeMercurialRepo" : {
"url" : "https://www.example.com/foo/hg",
"vcs" : "hg"
},
"Subversion" : {
"url" : "http://my-svn.com/repo",
"url-pattern" : {
"url-pattern" : {
"base-url" : "{url}/{path}{anchor}"
},
"vcs" : "svn"
Expand Down Expand Up @@ -49,6 +60,9 @@
"url-pattern" : {
"base-url" : "https://{url}/src/master/{path}{anchor}",
"anchor" : "#{filename}-{line}"
},
"vcs-config" : {
"ref" : "main"
}
},
"RepoWithPollingDisabled" : {
Expand Down
73 changes: 62 additions & 11 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const (
defaultPollEnabled = true
defaultTitle = "Hound"
defaultVcs = "git"
defaultBaseUrl = "{url}/blob/master/{path}{anchor}"
defaultBaseUrl = "{url}/blob/{rev}/{path}{anchor}"
defaultAnchor = "#L{line}"
defaultHealthCheckURI = "/healthz"
)
Expand Down Expand Up @@ -55,11 +55,12 @@ func (r *Repo) PushUpdatesEnabled() bool {
}

type Config struct {
DbPath string `json:"dbpath"`
Title string `json:"title"`
Repos map[string]*Repo `json:"repos"`
MaxConcurrentIndexers int `json:"max-concurrent-indexers"`
HealthCheckURI string `json:"health-check-uri"`
DbPath string `json:"dbpath"`
Title string `json:"title"`
Repos map[string]*Repo `json:"repos"`
MaxConcurrentIndexers int `json:"max-concurrent-indexers"`
HealthCheckURI string `json:"health-check-uri"`
VCSConfigMessages map[string]*SecretMessage `json:"vcs-config"`
}

// SecretMessage is just like json.RawMessage but it will not
Expand Down Expand Up @@ -116,15 +117,67 @@ func initRepo(r *Repo) {
}
}

// Populate missing config values with default values.
func initConfig(c *Config) {
// Populate missing config values with default values and
// merge global VCS configs into repo level configs.
func initConfig(c *Config) error {
if c.MaxConcurrentIndexers == 0 {
c.MaxConcurrentIndexers = defaultMaxConcurrentIndexers
}

if c.HealthCheckURI == "" {
c.HealthCheckURI = defaultHealthCheckURI
}

return mergeVCSConfigs(c)
}

func mergeVCSConfigs(cfg *Config) error {
globalConfigLen := len(cfg.VCSConfigMessages)
if globalConfigLen == 0 {
return nil
}

globalConfigVals := make(map[string]map[string]interface{}, globalConfigLen)
for vcs, configBytes := range cfg.VCSConfigMessages {
var configVals map[string]interface{}
if err := json.Unmarshal(*configBytes, &configVals); err != nil {
return err
}

globalConfigVals[vcs] = configVals
}

for _, repo := range cfg.Repos {
var globalVals map[string]interface{}
globalVals, valsExist := globalConfigVals[repo.Vcs]
if !valsExist {
continue
}

repoBytes := repo.VcsConfig()
var repoVals map[string]interface{}
if len(repoBytes) == 0 {
repoVals = make(map[string]interface{}, len(globalVals))
} else if err := json.Unmarshal(repoBytes, &repoVals); err != nil {
return err
}

for name, val := range globalVals {
if _, ok := repoVals[name]; !ok {
repoVals[name] = val
}
}

repoBytes, err := json.Marshal(&repoVals)
if err != nil {
return err
}

repoMessage := SecretMessage(repoBytes)
repo.VcsConfigMessage = &repoMessage
}

return nil
}

func (c *Config) LoadFromFile(filename string) error {
Expand Down Expand Up @@ -155,9 +208,7 @@ func (c *Config) LoadFromFile(filename string) error {
initRepo(repo)
}

initConfig(c)

return nil
return initConfig(c)
}

func (c *Config) ToJsonString() (string, error) {
Expand Down
18 changes: 18 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"encoding/json"
"path/filepath"
"runtime"
"testing"
Expand Down Expand Up @@ -34,4 +35,21 @@ func TestExampleConfigsAreValid(t *testing.T) {
t.Fatal(err)
}
}

// Ensure that global VCS config vals are merged
repo := cfg.Repos["SomeGitRepo"]
vcsConfigBytes := repo.VcsConfig()
var vcsConfigVals map[string]interface{}
json.Unmarshal(vcsConfigBytes, &vcsConfigVals)
if detectRef, ok := vcsConfigVals["detect-ref"]; !ok || !detectRef.(bool) {
t.Error("global detectRef vcs config setting not set for repo")
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be a t.Fatal since vcsConfigVals is used in the test below this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm the vcsConfigVals in the next block are pulled from another repo config and are used in a separate assertion. I think we'd want to proceed to capture all the errors in one go.

}

repo = cfg.Repos["GitRepoWithDetectRefDisabled"]
vcsConfigBytes = repo.VcsConfig()
json.Unmarshal(vcsConfigBytes, &vcsConfigVals)
if detectRef, ok := vcsConfigVals["detect-ref"]; !ok || detectRef.(bool) {
t.Error("global detectRef vcs config setting not overriden by repo-level setting")
}

}
96 changes: 85 additions & 11 deletions vcs/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,49 @@ package vcs

import (
"bytes"
"encoding/json"
"fmt"
"io"
"log"
"os/exec"
"path/filepath"
"regexp"
"strings"
)

const defaultRef = "master"

var headBranchRegexp = regexp.MustCompile(`HEAD branch: (?P<branch>.+)`)

func init() {
Register(newGit, "git")
}

type GitDriver struct{}
type GitDriver struct {
DetectRef bool `json:"detect-ref"`
Ref string `json:"ref"`
refDetetector refDetetector
}

type refDetetector interface {
detectRef(dir string) string
}

type headBranchDetector struct {
}

func newGit(b []byte) (Driver, error) {
return &GitDriver{}, nil
var d GitDriver

if b != nil {
if err := json.Unmarshal(b, &d); err != nil {
return nil, err
}
}

d.refDetetector = &headBranchDetector{}

return &d, nil
}

func (g *GitDriver) HeadRev(dir string) (string, error) {
Expand Down Expand Up @@ -47,43 +72,61 @@ func (g *GitDriver) HeadRev(dir string) (string, error) {
return strings.TrimSpace(buf.String()), cmd.Wait()
}

func run(desc, dir, cmd string, args ...string) error {
func run(desc, dir, cmd string, args ...string) (string, error) {
c := exec.Command(cmd, args...)
c.Dir = dir
if out, err := c.CombinedOutput(); err != nil {
out, err := c.CombinedOutput()
if err != nil {
log.Printf(
"Failed to %s %s, see output below\n%sContinuing...",
desc,
dir,
out)
return err
}
return nil

return string(out), nil
}

func (g *GitDriver) Pull(dir string) (string, error) {
if err := run("git fetch", dir,
targetRef := g.targetRef(dir)

if _, err := run("git fetch", dir,
"git",
"fetch",
"--prune",
"--no-tags",
"--depth", "1",
"origin",
fmt.Sprintf("+%s:remotes/origin/%s", defaultRef, defaultRef)); err != nil {
fmt.Sprintf("+%s:remotes/origin/%s", targetRef, targetRef)); err != nil {
return "", err
}

if err := run("git reset", dir,
if _, err := run("git reset", dir,
"git",
"reset",
"--hard",
fmt.Sprintf("origin/%s", defaultRef)); err != nil {
fmt.Sprintf("origin/%s", targetRef)); err != nil {
return "", err
}

return g.HeadRev(dir)
}

func (g *GitDriver) targetRef(dir string) string {
var targetRef string
if g.Ref != "" {
targetRef = g.Ref
} else if g.DetectRef {
targetRef = g.refDetetector.detectRef(dir)
}

if targetRef == "" {
targetRef = defaultRef
}

return targetRef
}

func (g *GitDriver) Clone(dir, url string) (string, error) {
par, rep := filepath.Split(dir)
cmd := exec.Command(
Expand All @@ -99,11 +142,42 @@ func (g *GitDriver) Clone(dir, url string) (string, error) {
return "", err
}

return g.HeadRev(dir)
return g.Pull(dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give me more details on what this additional Pull does here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clone initially checks out the default HEAD branch. The Pull ensures that we check out out the target ref.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, much appreciated.

}

func (g *GitDriver) SpecialFiles() []string {
return []string{
".git",
}
}

func (d *headBranchDetector) detectRef(dir string) string {
output, err := run("git show remote info", dir,
"git",
"remote",
"show",
"origin",
)

if err != nil {
log.Printf(
"error occured when fetching info to determine target ref in %s: %s. Will fall back to default ref %s",
dir,
err,
defaultRef,
)
return ""
}

matches := headBranchRegexp.FindStringSubmatch(output)
if len(matches) != 2 {
log.Printf(
"could not determine target ref in %s. Will fall back to default ref %s",
dir,
defaultRef,
)
return ""
}

return matches[1]
}
Loading