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

"Co-authored-by" trailer #57

Merged
merged 9 commits into from Mar 21, 2018
@@ -153,6 +153,30 @@ script uses `git filter-branch` to update the names of all of the
authors/committers to the currently set pair. It acts on your active branch
(using the passed ref as the start point).

### "Co-authored-by" trailer support

Set the environment variable `GIT_DUET_CO_AUTHORED_BY` to `1` if you want
to have a "Co-authored-by" trailer for each co-author in your commit message
rather than having a "Signed-off-by" trailer for the committer.

This gives every contributor attribution in their Github's contribution graph
as described [here](https://github.com/blog/2496-commit-together-with-co-authors).

If `GIT_DUET_CO_AUTHORED_BY` is set, `git duet` will install a prepare-commit-msg
hook file into the local repository by default. If, in addition, `GIT_DUET_GLOBAL` is set,
`git-duet` will instead install the prepare-commit-msg hook file into `git config --global init.templatedir`.
If the value `git config --global init.templatedir` is not set, `git-duet` will set it
to `$HOME/.git-template`.

The common workflow is as follows:
- set `GIT_DUET_SET_GIT_USER_CONFIG` so that `git duet` sets the author for normal git commands
- run `git duet` (which will install the prepare-commit-msg hook)
- if you have `GIT_DUET_GLOBAL` set, run `git init` once in existing repos
so that the hook file gets copied from the `init.templatedir`
- thereafter, use the normal git commands (and not the git-duet-subcommands
like `git duet-commit`, `git duet-merge` etc.)
- The prepare-commit-msg hook will append a `Co-authored-by` trailer for each co-author.

### Global Config Support

If you're jumping between projects and don't want to think about
@@ -365,11 +389,11 @@ If you want to use the default hook (as shown above), install it while
in your repo like so:

``` bash
git duet-install-hook
git duet-install-hook pre-commit
```

Don't worry if you forgot you already had a `pre-commit` hook installed.
The `git duet-install-hook` command will refuse to overwrite it.
The `git duet-install-hook pre-commit` command will refuse to overwrite it.

### RubyMine integration

@@ -14,6 +14,7 @@ type Configuration struct {
Namespace string
PairsFile string
EmailLookup string
CoAuthoredBy bool
Global bool
RotateAuthor bool
SetGitUserConfig bool
@@ -50,6 +51,10 @@ func NewConfiguration() (config *Configuration, err error) {
return nil, err
}

if config.CoAuthoredBy, err = strconv.ParseBool(getenvDefault("GIT_DUET_CO_AUTHORED_BY", "0")); err != nil {
return nil, err
}

config.StaleCutoff = time.Duration(cutoff) * time.Second

return config, nil
@@ -3,18 +3,25 @@ package main
import (
"bytes"
"fmt"
"io/ioutil"
"os"
"os/exec"
"os/user"
"path"
"strings"

duet "github.com/git-duet/git-duet"
"github.com/pborman/getopt"
)

const hook = `
#!/usr/bin/env bash
const preCommit = "pre-commit"
const prepareCommitMsg = "prepare-commit-msg"
const preCommitHook = `#!/usr/bin/env bash
exec git duet-pre-commit "$@"
`
const prepareCommitMsgHook = `#!/usr/bin/env bash
exec git duet-prepare-commit-msg "$1"
`

func main() {
var (
@@ -23,29 +30,91 @@ func main() {
)

getopt.Parse()
getopt.SetParameters(fmt.Sprintf("{ %s | %s }", preCommit, prepareCommitMsg))

if *help {
getopt.Usage()
os.Exit(0)
}

output := new(bytes.Buffer)
cmd := exec.Command("git", "rev-parse", "--show-toplevel")
cmd.Stdout = output
if err := cmd.Run(); err != nil {
args := getopt.Args()
if len(args) != 1 {
getopt.Usage()
os.Exit(1)
}
hookFileName := args[0]

var hook string
if hookFileName == preCommit {
hook = preCommitHook
} else if hookFileName == prepareCommitMsg {
hook = prepareCommitMsgHook
} else {
getopt.Usage()
os.Exit(1)
}

config, err := duet.NewConfiguration()
if err != nil {
fmt.Println(err)
os.Exit(1)
}

hookPath := path.Join(strings.TrimSpace(output.String()), ".git", "hooks", "pre-commit")
var hooksDir string
if config.Global {
gitConfig := &duet.GitConfig{Namespace: config.Namespace, SetUserConfig: config.SetGitUserConfig}
gitConfig.Scope = duet.Global
templateDir, err := gitConfig.GetInitTemplateDir()
if err != nil {
fmt.Println(err)
os.Exit(1)
}
if templateDir == "" {
usr, err := user.Current()
if err != nil {
fmt.Println(err)
os.Exit(1)
}
templateDir = path.Join(usr.HomeDir, ".git-template")
if err := gitConfig.SetInitTemplateDir(templateDir); err != nil {
fmt.Println(err)
os.Exit(1)
}
}
if err := os.MkdirAll(path.Join(templateDir, "hooks"), os.ModePerm); err != nil {
fmt.Println(err)
os.Exit(1)
}
hooksDir = path.Join(templateDir, "hooks")
} else {
hooksDir = getLocalHooksDir()
}

hookPath := path.Join(hooksDir, hookFileName)

hookFile, err := os.OpenFile(hookPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, os.ModePerm)
hookFile, err := os.OpenFile(hookPath, os.O_CREATE|os.O_RDWR, os.ModePerm)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
defer hookFile.Close()

b, err := ioutil.ReadAll(hookFile)
if err != nil {
fmt.Println(err)
os.Exit(1)
}

contents := strings.TrimSpace(string(b))
if contents != "" {
if hook == preCommitHook && contents != strings.TrimSpace(preCommitHook) ||

This comment has been minimized.

Copy link
@jszwedko

jszwedko Feb 25, 2018

Member

I'm concerned that this doesn't allow for future versions of git-duet to overwrite with updated hook contents.

This comment has been minimized.

Copy link
@jszwedko

jszwedko Feb 25, 2018

Member

However, this should be rare and I think we can address it if it comes up by including an index of previous hook contents for comparison (or hashes) so I'm OK with this as-is.

hook == prepareCommitMsgHook && contents != strings.TrimSpace(prepareCommitMsgHook) {
fmt.Printf("can't install hook: file %s already exists\n", hookPath)
os.Exit(1)
}
os.Exit(0) // hook file with the desired content already exists
}

if _, err = hookFile.WriteString(hook); err != nil {
fmt.Println(err)
os.Exit(1)
@@ -54,4 +123,16 @@ func main() {
if !*quiet {
fmt.Printf("git-duet-install-hook: Installed hook to %s\n", hookPath)
}

}

func getLocalHooksDir() string {
output := new(bytes.Buffer)
cmd := exec.Command("git", "rev-parse", "--show-toplevel")
cmd.Stdout = output
if err := cmd.Run(); err != nil {
fmt.Println(err)
os.Exit(1)
}
return path.Join(strings.TrimSpace(output.String()), ".git", "hooks")
}
@@ -0,0 +1,65 @@
package main

import (
"fmt"
"io/ioutil"
"os"
"strings"

"github.com/git-duet/git-duet"
"github.com/pborman/getopt"
)

func main() {

getopt.Parse()
commitMsgFile := getopt.Args()[0]

configuration, err := duet.NewConfiguration()
if err != nil {
fmt.Println(err)
os.Exit(1)
}

var gitConfig *duet.GitConfig
if configuration.Global {
gitConfig = &duet.GitConfig{
Namespace: configuration.Namespace,
Scope: duet.Global,
SetUserConfig: configuration.SetGitUserConfig,
}
} else {
gitConfig, err = duet.GetAuthorConfig(configuration.Namespace, configuration.SetGitUserConfig)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
}

committers, err := gitConfig.GetCommitters()
if err != nil {
fmt.Println(err)
os.Exit(1)
}
if committers == nil || len(committers) == 0 {
os.Exit(0)

This comment has been minimized.

Copy link
@jszwedko

jszwedko Feb 25, 2018

Member

This appears to be assuming GIT_DUET_CO_AUTHORED_BY implies GIT_DUET_SET_GIT_USER_CONFIG. I agree with this decision, but I think we'll need update to duet.Configuration to codify this (and maybe include this in the README).

This comment has been minimized.

Copy link
@jszwedko

jszwedko Feb 25, 2018

Member

This also makes me realize we'll need to consider the various paths users might take opting in to this feature, e.g.:

git solo js # without GIT_DUET_SET_GIT_USER_CONFIG, so only sets `git-duet` specific configuration
export GIT_DUET_CO_AUTHORED_BY=1
git commit -m 'test' # will not use the author set by `git solo js`

I'll think about this a bit more.

This comment has been minimized.

Copy link
@ansd

ansd Mar 1, 2018

Author Contributor

I think there is just a single path how users can opt in, namely by running git duet with GIT_DUET_CO_AUTHORED_BY set.

I think in your above example, it's okay that the commit doesn't set js as author since the user didn't run git solo with either GIT_DUET_SET_GIT_USER_CONFIG or GIT_DUET_CO_AUTHORED_BY (the latter implicitly setting GIT_DUET_SET_GIT_USER_CONFIG).

This comment has been minimized.

Copy link
@jszwedko

jszwedko Mar 10, 2018

Member

Right, I'm just thinking that people might have an expectation, in my example, that GIT_DUET_CO_AUTHORED_BY would pick up the previous git solo even without GIT_DUET_SET_USER_CONFIG. I'm willing to concede that it feels likely that users will set that variable and then call git duet though.

}

var coAuthorsTrailer string
for _, c := range committers {
coAuthorsTrailer += "Co-authored-by: " + c.Name + " <" + c.Email + ">\n"
}
coAuthorsTrailer = strings.TrimSuffix(coAuthorsTrailer, "\n")

commitMsg, err := ioutil.ReadFile(commitMsgFile)
if err != nil {
fmt.Print(err)
os.Exit(1)
}

err = ioutil.WriteFile(commitMsgFile, []byte(fmt.Sprintf("%s\n%s", string(commitMsg), coAuthorsTrailer)), 0644)
if err != nil {
fmt.Print(err)
os.Exit(1)
}
}
@@ -3,6 +3,7 @@ package main
import (
"fmt"
"os"
"os/exec"

"github.com/git-duet/git-duet"
"github.com/pborman/getopt"
@@ -64,6 +65,9 @@ func main() {

printAuthor(author)
printNextComitter(committers)
if configuration.CoAuthoredBy {
installPrepareCommitMsgHook()
}
os.Exit(0)
}

@@ -109,6 +113,10 @@ func main() {
printAuthor(author)
printNextComitter(committers)
}

if configuration.CoAuthoredBy {
installPrepareCommitMsgHook()
}
}

func printAuthor(author *duet.Pair) {
@@ -128,3 +136,15 @@ func printNextComitter(committers []*duet.Pair) {
fmt.Printf("GIT_COMMITTER_NAME='%s'\n", committers[0].Name)
fmt.Printf("GIT_COMMITTER_EMAIL='%s'\n", committers[0].Email)
}

func installPrepareCommitMsgHook() {
cmd := exec.Command("git-duet-install-hook", "prepare-commit-msg")
cmd.Stderr = os.Stderr
cmd.Stdout = os.Stdout

err := cmd.Run()
if err != nil {
fmt.Println(err)
os.Exit(1)
}
}
@@ -266,6 +266,21 @@ func (gc *GitConfig) GetMtime() (mtime time.Time, err error) {
return time.Unix(mtimeUnix, 0), nil
}

func (gc *GitConfig) GetInitTemplateDir() (templateDir string, err error) {
templateDir, err = gc.getUnnamespacedKey("init.templatedir")
if err != nil {
return "", err
}
return templateDir, err
}

func (gc *GitConfig) SetInitTemplateDir(path string) (err error) {
if err = gc.setUnnamespacedKey("init.templatedir", path); err != nil {
return err
}
return nil
}

func (gc *GitConfig) getKey(key string) (value string, err error) {
output := new(bytes.Buffer)
cmd := gc.configCommand(fmt.Sprintf("%s.%s", gc.Namespace, key))
@@ -278,6 +293,18 @@ func (gc *GitConfig) getKey(key string) (value string, err error) {
return strings.TrimSpace(output.String()), nil
}

func (gc *GitConfig) getUnnamespacedKey(key string) (value string, err error) {
output := new(bytes.Buffer)
cmd := gc.configCommand(key)
cmd.Stdout = output

err = newIgnorableCommand(cmd, 1).Run()
if err != nil {
return "", err
}
return strings.TrimSpace(output.String()), nil
}

func (gc *GitConfig) unsetKey(key string) (err error) {
if err = newIgnorableCommand(
gc.configCommand("--unset-all", fmt.Sprintf("%s.%s", gc.Namespace, key)),
@@ -184,7 +184,7 @@ load test_helper
fi

git solo -q jd
git duet-install-hook -q
git duet-install-hook -q pre-commit
git config --unset-all "$GIT_DUET_CONFIG_NAMESPACE.mtime"
add_file
run git duet-commit -q -m 'Testing stale hook fire'
@@ -201,7 +201,7 @@ load test_helper
fi

git duet -q jd fb
git duet-install-hook -q
git duet-install-hook -q pre-commit
git config --unset-all "$GIT_DUET_CONFIG_NAMESPACE.mtime"
add_file
run git duet-commit -q -m 'Testing stale hook fire'
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.