*: rework validation scripts

.travis.yml

@@ -5,15 +5,13 @@ sudo: required
 services:
   - docker
 
-# We can't test Go 1.5, because golint requires >1.6 (which is annoying).
-# Also note that this probably doesn't work with `make ci` right now.
 go:
-  - 1.7
-  #- 1.6
-  #- master
+  - 1.x
 
 before_install:
   - go get -u github.com/cpuguy83/go-md2man
+  - go get -u github.com/vbatts/git-validation
+  - go get -u github.com/golang/lint/golint
 
 env:
   - DOCKER_IMAGE="opensuse/amd64:42.2"
@@ -26,7 +24,4 @@ notifications:
 
 script:
   - chmod a+rwx . # Necessary to make Travis co-operate with Docker.
-  - make umoci
-  - make umoci.static
-  - make local-validate-reproducible
   - make DOCKER_IMAGE=$DOCKER_IMAGE ci

CHANGELOG.md

@@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 - Fix several minor bugs in `hack/release.sh` that caused the release artefacts
   to not match the intended style. openSUSE/umoci#155
+- A recent configuration issue caused `go vet` and `go lint` to not run as part
+  of our CI jobs. This means that some of the information submitted as part of
+  [CII best practices badging][cii] was not accurate. This has been corrected,
+  and after review we concluded that only stylistic issues were discovered by
+  static analysis. openSUSE/umoci#158
+
+[cii]: https://bestpractices.coreinfrastructure.org/projects/1084
 
 ## [0.3.0] - 2017-07-20
 ### Added

Makefile

@@ -45,7 +45,6 @@ STATIC_BUILD_FLAGS := $(BUILD_FLAGS) -ldflags "-s -w -extldflags '-static' -X ma
 .DEFAULT: umoci
 
 GO_SRC = $(shell find . -name \*.go)
-.PHONY: umoci umoci.static umoci.cover install install.static
 
 umoci: $(GO_SRC)
 	$(GO) build ${DYN_BUILD_FLAGS} -o $(BUILD_DIR)/$@ ${CMD}
@@ -60,9 +59,11 @@ umoci.cover: $(GO_SRC)
 release:
 	hack/release.sh -S "$(GPG_KEYID)" -r release/$(VERSION) -v $(VERSION)
 
+.PHONY: install
 install: $(GO_SRC)
 	$(GO) install -v ${DYN_BUILD_FLAGS} ${CMD}
 
+.PHONY: install.static
 install.static: $(GO_SRC)
 	$(GO) install -v ${STATIC_BUILD_FLAGS} ${CMD}
 
@@ -75,37 +76,45 @@ clean:
 	rm -f umoci umoci.static
 	rm -f $(MANPAGES)
 
+.PHONY: validate
 validate: umociimage
 	docker run --rm -it -v $(PWD):/go/src/$(PROJECT) $(UMOCI_IMAGE) make local-validate
 
-EPOCH_COMMIT ?= 97ecdbd53dcb72b7a0d62196df281f131dc9eb2f
 .PHONY: local-validate
-local-validate: local-validate-reproducible
-	test -z "$$(gofmt -s -l . | grep -v '^vendor/' | grep -v '^third_party/' | tee /dev/stderr)"
-	out="$$(golint $(PROJECT)/... | grep -v '/vendor/' | grep -v '/third_party/' | grep -vE 'system/utils_linux.*ALL_CAPS|system/mknod_linux.*underscores')"; \
-	if [ -n "$$out" ]; then \
-		echo "$$out"; \
-		exit 1; \
-	fi
-	go vet $(shell go list $(PROJECT)/... | grep -v /vendor/ | grep -v /third_party/)
-	#@echo "git-validation"
-	#@git-validation -v -run DCO,short-subject,dangling-whitespace $(EPOCH_COMMIT)..HEAD
+local-validate: local-validate-git local-validate-go local-validate-reproducible
+
+# TODO: Remove the special-case ignored system/* warnings.
+.PHONY: local-validate-go
+local-validate-go:
+	@which gofmt    >/dev/null 2>/dev/null || (echo "ERROR: gofmt not found." && false)
+	test -z "$$(gofmt -s -l . | grep -vE '^vendor/|^third_party/' | tee /dev/stderr)"
+	@which golint   >/dev/null 2>/dev/null || (echo "ERROR: golint not found." && false)
+	test -z "$$(golint $(PROJECT)/... | grep -vE '/vendor/|/third_party/' | grep -vE 'system/utils_linux.*ALL_CAPS|system/mknod_linux.*underscores' | tee /dev/stderr)"
+	@go doc cmd/vet >/dev/null 2>/dev/null || (echo "ERROR: go vet not found." && false)
+	test -z "$$($(GO) vet $$($(GO) list $(PROJECT)/... | grep -vE '/vendor/|/third_party/') 2>&1 | tee /dev/stderr)"
+
+EPOCH_COMMIT ?= 97ecdbd53dcb72b7a0d62196df281f131dc9eb2f
+.PHONY: local-validate-git
+local-validate-git:
+	@which git-validation > /dev/null 2>/dev/null || (echo "ERROR: git-validation not found." && false)
+ifdef TRAVIS_COMMIT_RANGE
+	git-validation -q -run DCO,short-subject
+else
+	git-validation -q -run DCO,short-subject -range $(EPOCH_COMMIT)..HEAD
+endif
 
 # Make sure that our builds are reproducible even if you wait between them and
 # the modified time of the files is different.
 .PHONY: local-validate-reproducible
 local-validate-reproducible:
-	@mkdir -p .tmp-validate
-	@echo " [MAKE-A] umoci"
-	@make -B umoci && cp umoci .tmp-validate/umoci.a
-	@echo " [WAIT]"
+	mkdir -p .tmp-validate
+	make -B umoci && cp umoci .tmp-validate/umoci.a
+	@echo sleep 10s
 	@sleep 10s && touch $(GO_SRC)
-	@echo " [MAKE-B] umoci"
-	@make -B umoci && cp umoci .tmp-validate/umoci.b
-	@echo "  [CMP] umoci"
-	@diff -s .tmp-validate/umoci.{a,b}
-	@sha256sum .tmp-validate/umoci.{a,b}
-	@rm -r .tmp-validate/umoci.{a,b}
+	make -B umoci && cp umoci .tmp-validate/umoci.b
+	diff -s .tmp-validate/umoci.{a,b}
+	sha256sum .tmp-validate/umoci.{a,b}
+	rm -r .tmp-validate/umoci.{a,b}
 
 MANPAGES_MD := $(wildcard doc/man/*.md)
 MANPAGES    := $(MANPAGES_MD:%.md=%)
@@ -151,5 +160,5 @@ shell: umociimage
 	docker run --rm -it -v $(PWD):/go/src/$(PROJECT) $(UMOCI_IMAGE) bash
 
 .PHONY: ci
-ci: umoci umoci.cover validate doc test-unit test-integration
+ci: umoci umoci.cover doc local-validate test-unit test-integration
 	$(GO) tool cover -func <(egrep -v 'vendor|third_party' $(COVERAGE))

cmd/umoci/config.go

@@ -135,7 +135,7 @@ func config(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	fromDescriptorPaths, err := engineExt.ResolveReference(context.Background(), fromName)

cmd/umoci/gc.go

@@ -57,7 +57,7 @@ func gc(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Run the GC.

cmd/umoci/new.go

@@ -60,7 +60,7 @@ func newImage(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Create a new manifest.

cmd/umoci/raw-runtime-config.go

@@ -128,7 +128,7 @@ func rawConfig(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	fromDescriptorPaths, err := engineExt.ResolveReference(context.Background(), fromName)

cmd/umoci/repack.go

@@ -117,7 +117,7 @@ func repack(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Create the mutator.

cmd/umoci/stat.go

@@ -64,7 +64,7 @@ func stat(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	manifestDescriptorPaths, err := engineExt.ResolveReference(context.Background(), tagName)

cmd/umoci/tag.go

@@ -66,7 +66,7 @@ func tagAdd(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Get original descriptor.
@@ -114,7 +114,7 @@ func tagRemove(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Remove it.
@@ -151,7 +151,7 @@ func tagList(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	names, err := engineExt.ListReferences(context.Background())

cmd/umoci/unpack.go

@@ -126,7 +126,7 @@ func unpack(ctx *cli.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "open CAS")
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	fromDescriptorPaths, err := engineExt.ResolveReference(context.Background(), fromName)

mutate/mutate.go

@@ -137,7 +137,7 @@ func New(engine cas.Engine, src casext.DescriptorPath) (*Mutator, error) {
 	}
 
 	return &Mutator{
-		engine: casext.Engine{engine},
+		engine: casext.NewEngine(engine),
 		source: src,
 	}, nil
 }

mutate/mutate_test.go

@@ -57,7 +57,7 @@ func setup(t *testing.T, dir string) (cas.Engine, ispec.Descriptor) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 
 	// Write a tar layer.
 	var buffer bytes.Buffer
@@ -458,7 +458,7 @@ func TestMutatePath(t *testing.T) {
 	defer os.RemoveAll(dir)
 
 	engine, manifestDescriptor := setup(t, dir)
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 	defer engine.Close()
 
 	// Create some additional structure.

oci/casext/casext.go

@@ -22,8 +22,17 @@ package casext
 
 import "github.com/openSUSE/umoci/oci/cas"
 
+// TODO: Convert this to an interface and make Engine private.
+
 // Engine is a wrapper around cas.Engine that provides additional, generic
 // extensions to the transport-dependent cas.Engine implementation.
 type Engine struct {
 	cas.Engine
 }
+
+// NewEngine returns a new Engine which acts as a wrapper around the given
+// cas.Engine and provides additional, generic extensions to the
+// transport-dependent cas.Engine implementation.
+func NewEngine(engine cas.Engine) Engine {
+	return Engine{Engine: engine}
+}

oci/casext/json_dir_test.go

@@ -49,7 +49,7 @@ func TestEngineBlobJSON(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error opening image: %+v", err)
 	}
-	engineExt := Engine{engine}
+	engineExt := NewEngine(engine)
 	defer engine.Close()
 
 	type object struct {
@@ -145,7 +145,7 @@ func TestEngineBlobJSONReadonly(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error opening image: %+v", err)
 		}
-		engineExt := Engine{engine}
+		engineExt := NewEngine(engine)
 
 		digest, _, err := engineExt.PutBlobJSON(ctx, test.object)
 		if err != nil {
@@ -163,7 +163,7 @@ func TestEngineBlobJSONReadonly(t *testing.T) {
 		if err != nil {
 			t.Errorf("unexpected error opening ro image: %+v", err)
 		}
-		newEngineExt := Engine{newEngine}
+		newEngineExt := NewEngine(newEngine)
 
 		blobReader, err := newEngineExt.GetBlob(ctx, digest)
 		if err != nil {

oci/casext/refname_dir_test.go

@@ -254,7 +254,7 @@ func TestEngineReference(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error opening image: %+v", err)
 	}
-	engineExt := Engine{engine}
+	engineExt := NewEngine(engine)
 	defer engine.Close()
 
 	descMap, err := fakeSetupEngine(t, engineExt)
@@ -318,7 +318,7 @@ func TestEngineReferenceReadonly(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error opening image: %+v", err)
 	}
-	engineExt := Engine{engine}
+	engineExt := NewEngine(engine)
 
 	descMap, err := fakeSetupEngine(t, engineExt)
 	if err != nil {
@@ -336,7 +336,7 @@ func TestEngineReferenceReadonly(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error opening image: %+v", err)
 		}
-		engineExt := Engine{engine}
+		engineExt := NewEngine(engine)
 
 		if err := engineExt.UpdateReference(ctx, name, test.index); err != nil {
 			t.Errorf("UpdateReference: unexpected error: %+v", err)
@@ -353,7 +353,7 @@ func TestEngineReferenceReadonly(t *testing.T) {
 		if err != nil {
 			t.Errorf("unexpected error opening ro image: %+v", err)
 		}
-		newEngineExt := Engine{newEngine}
+		newEngineExt := NewEngine(newEngine)
 
 		gotDescriptorPaths, err := newEngineExt.ResolveReference(ctx, name)
 		if err != nil {

oci/layer/unpack.go

@@ -87,7 +87,7 @@ func isLayerType(mediaType string) bool {
 //
 // FIXME: This interface is ugly.
 func UnpackManifest(ctx context.Context, engine cas.Engine, bundle string, manifest ispec.Manifest, opt *MapOptions) error {
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 
 	// Create the bundle directory. We only error out if config.json or rootfs/
 	// already exists, because we cannot be sure that the user intended us to
@@ -227,7 +227,7 @@ func UnpackManifest(ctx context.Context, engine cas.Engine, bundle string, manif
 //
 // XXX: I don't like this API. It has way too many arguments.
 func UnpackRuntimeJSON(ctx context.Context, engine cas.Engine, configFile io.Writer, rootfs string, manifest ispec.Manifest, opt *MapOptions) error {
-	engineExt := casext.Engine{engine}
+	engineExt := casext.NewEngine(engine)
 
 	var mapOptions MapOptions
 	if opt != nil {

pkg/mtreefilter/mask_test.go

@@ -62,7 +62,7 @@ func TestMaskDeltas(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
-	var mtreeKeywords []mtree.Keyword = append(mtree.DefaultKeywords, "sha256digest")
+	mtreeKeywords := append(mtree.DefaultKeywords, "sha256digest")
 
 	// Create some files.
 	if err != ioutil.WriteFile(filepath.Join(dir, "file1"), []byte("contents"), 0644) {