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.

Sign up for GitHub

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

Jump to bottom

keep only master branch for latest #160

Merged
merged 12 commits into from
Nov 9, 2017
Merged

keep only master branch for latest #160

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d8ba908
keep only master branch for latest
appleboy Nov 2, 2017
0fb22db
add DRONE_ environment variable
appleboy Nov 3, 2017
197b737
remove additional yaml attributes
appleboy Nov 3, 2017
3c1cfa6
update
appleboy Nov 6, 2017
0927e34
replace commit branch with refs
appleboy Nov 9, 2017
ff05dfd
reverse the orderin
appleboy Nov 9, 2017
c591da7
remove commit branch frone struct.
appleboy Nov 9, 2017
5fa26eb
UseDefaultTag for skip build if not default branch.
appleboy Nov 9, 2017
13ce200
remove DefaultBranch
appleboy Nov 9, 2017
ce53e21
add unit test.
appleboy Nov 9, 2017
a5011aa
remove default branch
appleboy Nov 9, 2017
bb68110
update comment
appleboy Nov 9, 2017
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
39 changes: 27 additions & 12 deletions cmd/drone-docker/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,17 @@ func main() {
Usage: "docker email",
EnvVar: "PLUGIN_EMAIL,DOCKER_EMAIL",
},
cli.StringFlag{
Name: "commit.branch",
Value: "master",
Usage: "git commit branch",
EnvVar: "DRONE_COMMIT_BRANCH",
},
cli.StringFlag{
Name: "default.branch",
Copy link
Member

Choose a reason for hiding this comment

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

let's call this repo.branch which is consistent with some of the other plugins and the plugin starter

Copy link
Author

Choose a reason for hiding this comment

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

Done. 0927e34

Usage: "repository default branch",
EnvVar: "DRONE_REPO_BRANCH",
},
}

if err := app.Run(os.Args); err != nil {
Expand All @@ -209,18 +220,20 @@ func run(c *cli.Context) error {
Email: c.String("docker.email"),
},
Build: docker.Build{
Remote: c.String("remote.url"),
Name: c.String("commit.sha"),
Dockerfile: c.String("dockerfile"),
Context: c.String("context"),
Tags: c.StringSlice("tags"),
Args: c.StringSlice("args"),
ArgsEnv: c.StringSlice("args-from-env"),
Squash: c.Bool("squash"),
Pull: c.BoolT("pull-image"),
Compress: c.Bool("compress"),
Repo: c.String("repo"),
LabelSchema: c.StringSlice("label-schema"),
Remote: c.String("remote.url"),
Name: c.String("commit.sha"),
Branch: c.String("commit.branch"),
DefaultBranch: c.String("default.branch"),
Dockerfile: c.String("dockerfile"),
Context: c.String("context"),
Tags: c.StringSlice("tags"),
Args: c.StringSlice("args"),
ArgsEnv: c.StringSlice("args-from-env"),
Squash: c.Bool("squash"),
Pull: c.BoolT("pull-image"),
Compress: c.Bool("compress"),
Repo: c.String("repo"),
LabelSchema: c.StringSlice("label-schema"),
},
Daemon: docker.Daemon{
Registry: c.String("docker.registry"),
Expand All @@ -243,6 +256,8 @@ func run(c *cli.Context) error {
plugin.Build.Tags = docker.DefaultTagSuffix(
c.String("commit.ref"),
c.String("tags.suffix"),
c.String("commit.branch"),
c.String("default.branch"),
)
}

Expand Down
26 changes: 14 additions & 12 deletions docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,20 @@ type (

// Build defines Docker build parameters.
Build struct {
Remote string // Git remote URL
Name string // Docker build using default named tag
Dockerfile string // Docker build Dockerfile
Context string // Docker build context
Tags []string // Docker build tags
Args []string // Docker build args
ArgsEnv []string // Docker build args from env
Squash bool // Docker build squash
Pull bool // Docker build pull
Compress bool // Docker build compress
Repo string // Docker build repository
LabelSchema []string // Label schema map
Remote string // Git remote URL
Name string // Docker build using default named tag
Dockerfile string // Docker build Dockerfile
Context string // Docker build context
Tags []string // Docker build tags
Args []string // Docker build args
ArgsEnv []string // Docker build args from env
Squash bool // Docker build squash
Pull bool // Docker build pull
Compress bool // Docker build compress
Repo string // Docker build repository
LabelSchema []string // Label schema map
Branch string // Docker build branch
DefaultBranch string // Docker latest branch
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be removed since it isn't being used

Copy link
Author

Choose a reason for hiding this comment

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

Done. c591da7

}

// Plugin defines the Docker plugin parameters.
Expand Down
12 changes: 9 additions & 3 deletions tags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (

// DefaultTagSuffix returns a set of default suggested tags
// based on the commit ref with an attached suffix.
func DefaultTagSuffix(ref, suffix string) []string {
tags := DefaultTags(ref)
func DefaultTagSuffix(ref, suffix, commitBranch, defaultBranch string) []string {
tags := DefaultTags(ref, commitBranch, defaultBranch)
if len(suffix) == 0 {
return tags
}
Expand All @@ -26,10 +26,16 @@ func DefaultTagSuffix(ref, suffix string) []string {

// DefaultTags returns a set of default suggested tags based on
// the commit ref.
func DefaultTags(ref string) []string {
func DefaultTags(ref, commitBranch, defaultBranch string) []string {
Copy link
Member

@bradrydzewski bradrydzewski Nov 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think we can remove commitBranch since this information is already passed in the ref field. For example if commitBranch == master we know that ref == refs/heads/master

Copy link
Author

Choose a reason for hiding this comment

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

Done. 0927e34


if defaultBranch != "" && commitBranch != defaultBranch && !strings.HasPrefix(ref, "refs/tags/") {
return []string{}
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure we ever want to return an empty string here. We should always return a default tag.

Copy link
Author

Choose a reason for hiding this comment

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

maybe we can add a new flag to confirm that if the user wants to build tags from another branch which is not equal the default branch.

  1. default branch equal to commit branch. return latest tag
  2. new flag BuildFromBranch
    2.1: default branch not equal to commit branch. return commit branch tag if BuildFromBranch is true
    2.2: default branch not equal to commit branch. return empty tag if BuildFromBranch is false

Copy link
Member

@bradrydzewski bradrydzewski Nov 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

default branch not equal to commit branch. return empty tag

An empty tag is not valid and will fail to publish. What is the goal with returning an empty branch?

Copy link
Author

Choose a reason for hiding this comment

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

@bradrydzewski I meant empty value like []string{}

Copy link
Author

Choose a reason for hiding this comment

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

and we can check the tag array in Exec like

	if len(p.Build.Tags) == 0 {
		fmt.Println("build tags not found")
		return nil
	}

Copy link
Author

Choose a reason for hiding this comment

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

@bradrydzewski Any other suggestion?

}

if !strings.HasPrefix(ref, "refs/tags/") {
return []string{"latest"}
}

v := stripTagPrefix(ref)
version, err := semver.NewVersion(v)
if err != nil {
Expand Down
71 changes: 46 additions & 25 deletions tags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,30 @@ func Test_stripTagPrefix(t *testing.T) {

func TestDefaultTags(t *testing.T) {
var tests = []struct {
Before string
After []string
Before string
CommitBranch string
DefaultBranch string
After []string
}{
{"", []string{"latest"}},
{"refs/heads/master", []string{"latest"}},
{"refs/tags/0.9.0", []string{"0.9", "0.9.0"}},
{"refs/tags/1.0.0", []string{"1", "1.0", "1.0.0"}},
{"refs/tags/v1.0.0", []string{"1", "1.0", "1.0.0"}},
{"refs/tags/v1.0.0-alpha.1", []string{"1.0.0-alpha.1"}},
{"", "master", "", []string{"latest"}},
{"refs/heads/master", "master", "", []string{"latest"}},
{"refs/tags/0.9.0", "master", "", []string{"0.9", "0.9.0"}},
{"refs/tags/1.0.0", "master", "", []string{"1", "1.0", "1.0.0"}},
{"refs/tags/v1.0.0", "master", "", []string{"1", "1.0", "1.0.0"}},
{"refs/tags/v1.0.0-alpha.1", "master", "", []string{"1.0.0-alpha.1"}},

// malformed or errors
{"refs/tags/x1.0.0", []string{"latest"}},
{"v1.0.0", []string{"latest"}},
{"refs/tags/x1.0.0", "master", "", []string{"latest"}},
{"v1.0.0", "master", "", []string{"latest"}},

// defualt branch
{"refs/heads/master", "master", "master", []string{"latest"}},
{"refs/heads/test", "test", "master", []string{}},
{"refs/tags/v1.0.0", "v1.0.0", "master", []string{"1", "1.0", "1.0.0"}},
}

for _, test := range tests {
got, want := DefaultTags(test.Before), test.After
got, want := DefaultTags(test.Before, test.CommitBranch, test.DefaultBranch), test.After
if !reflect.DeepEqual(got, want) {
t.Errorf("Got tag %v, want %v", got, want)
}
Expand All @@ -50,16 +57,22 @@ func TestDefaultTags(t *testing.T) {

func TestDefaultTagSuffix(t *testing.T) {
var tests = []struct {
Before string
Suffix string
After []string
Before string
CommitBranch string
DefaultBranch string
Suffix string
After []string
}{
// without suffix
{
After: []string{"latest"},
After: []string{"latest"},
CommitBranch: "master",
DefaultBranch: "",
},
{
Before: "refs/tags/v1.0.0",
Before: "refs/tags/v1.0.0",
CommitBranch: "master",
DefaultBranch: "",
After: []string{
"1",
"1.0",
Expand All @@ -68,25 +81,33 @@ func TestDefaultTagSuffix(t *testing.T) {
},
// with suffix
{
Suffix: "linux-amd64",
After: []string{"linux-amd64"},
CommitBranch: "master",
DefaultBranch: "",
Suffix: "linux-amd64",
After: []string{"linux-amd64"},
},
{
Before: "refs/tags/v1.0.0",
Suffix: "linux-amd64",
CommitBranch: "master",
DefaultBranch: "",
Before: "refs/tags/v1.0.0",
Suffix: "linux-amd64",
After: []string{
"1-linux-amd64",
"1.0-linux-amd64",
"1.0.0-linux-amd64",
},
},
{
Suffix: "nanoserver",
After: []string{"nanoserver"},
CommitBranch: "master",
DefaultBranch: "",
Suffix: "nanoserver",
After: []string{"nanoserver"},
},
{
Before: "refs/tags/v1.9.2",
Suffix: "nanoserver",
CommitBranch: "master",
DefaultBranch: "",
Before: "refs/tags/v1.9.2",
Suffix: "nanoserver",
After: []string{
"1-nanoserver",
"1.9-nanoserver",
Expand All @@ -96,7 +117,7 @@ func TestDefaultTagSuffix(t *testing.T) {
}

for _, test := range tests {
got, want := DefaultTagSuffix(test.Before, test.Suffix), test.After
got, want := DefaultTagSuffix(test.Before, test.Suffix, test.CommitBranch, test.DefaultBranch), test.After
if !reflect.DeepEqual(got, want) {
t.Errorf("Got tag %v, want %v", got, want)
}
Expand Down