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

Delete vendored code and update CI to Go 1.13 #158

Merged
merged 5 commits into from
Oct 26, 2019
Merged

Delete vendored code and update CI to Go 1.13 #158

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1bfcc1
Delete vendored code and update CI to Go 1.13
josephlr Oct 24, 2019
478808f
Building on Go 1.12 requires setting GO111MODULE
josephlr Oct 24, 2019
7ec5f3a
Add building for older Go Versions
josephlr Oct 25, 2019
5879439
Update documentation:
josephlr Oct 25, 2019
880eaf7
Use Makefile to set GO111MODULE=on
josephlr Oct 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
14 changes: 9 additions & 5 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: go
sudo: false
dist: bionic
go: 1.12.x
go: 1.13.x

notifications:
email: false
Expand All @@ -21,7 +21,7 @@ jobs:
script: make

- stage: presubmits
env: Generate, Format, and Lint
name: Generate, Format, and Lint
install:
- make tools
script:
Expand All @@ -32,16 +32,20 @@ jobs:
- make lint

- &build
env: Build and Unit Tests
name: Build and Unit Tests
install: skip
env: GO111MODULE=on
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also wondering whether everyone using Go 1.11 or 1.12 will have to set GO111MODULE=on? Perhaps the Makefile should do it automatically?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Something like this?

diff --git a/Makefile b/Makefile
index 79e096e..21aace6 100644
--- a/Makefile
+++ b/Makefile
@@ -86,10 +86,10 @@ default: $(BIN)/$(NAME) $(PAM_MODULE)
 all: tools gen default format lint test
 
 $(BIN)/$(NAME): $(GO_FILES) $(C_FILES)
-	go build $(GO_FLAGS) -o $@ ./cmd/$(NAME)
+	GO111MODULE=on go build $(GO_FLAGS) -o $@ ./cmd/$(NAME)
 
 $(PAM_MODULE): $(GO_FILES) $(C_FILES)
-	go build -buildmode=c-shared $(GO_FLAGS) -o $@ ./$(PAM_NAME)
+	GO111MODULE=on go build -buildmode=c-shared $(GO_FLAGS) -o $@ ./$(PAM_NAME)
 	rm -f $(BIN)/$(PAM_NAME).h
 
 gen: $(BIN)/protoc $(BIN)/protoc-gen-go $(PROTO_FILES)
@@ -100,9 +100,9 @@ format: $(BIN)/goimports
 	clang-format -i -style=Google $(C_FILES)
 
 lint: $(BIN)/golint $(BIN)/staticcheck $(BIN)/misspell
-	go vet ./...
+	GO111MODULE=on go vet ./...
 	go list ./... | xargs -L1 golint -set_exit_status
-	staticcheck ./...
+	GO111MODULE=on staticcheck ./...
 	misspell -source=text $(FILES)
 
 clean:
@@ -119,7 +119,7 @@ export TEST_FILESYSTEM_ROOT = $(MOUNT)
 endif
 
 test:
-	go test -p 1 ./...
+	GO111MODULE=on go test -p 1 ./...
 
 test-setup:
 	dd if=/dev/zero of=$(IMAGE) bs=1M count=20
@@ -174,17 +174,17 @@ TOOLS := $(addprefix $(BIN)/,protoc golint protoc-gen-go goimports staticcheck g
 tools: $(TOOLS)
 
 $(BIN)/golint:
-	go build -o $@ golang.org/x/lint/golint
+	GO111MODULE=on go build -o $@ golang.org/x/lint/golint
 $(BIN)/protoc-gen-go:
-	go build -o $@ github.com/golang/protobuf/protoc-gen-go
+	GO111MODULE=on go build -o $@ github.com/golang/protobuf/protoc-gen-go
 $(BIN)/goimports:
-	go build -o $@ golang.org/x/tools/cmd/goimports
+	GO111MODULE=on go build -o $@ golang.org/x/tools/cmd/goimports
 $(BIN)/staticcheck:
-	go build -o $@ honnef.co/go/tools/cmd/staticcheck
+	GO111MODULE=on go build -o $@ honnef.co/go/tools/cmd/staticcheck
 $(BIN)/gocovmerge:
-	go build -o $@ github.com/wadey/gocovmerge
+	GO111MODULE=on go build -o $@ github.com/wadey/gocovmerge
 $(BIN)/misspell:
-	go build -o $@ github.com/client9/misspell/cmd/misspell
+	GO111MODULE=on go build -o $@ github.com/client9/misspell/cmd/misspell
 
 # Non-go tools downloaded from appropriate repository
 PROTOC_VERSION := 3.6.1

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree this is the right idea, but Makefile supports a mechanism to set environment variables for all invocations, so I'll just use that.

I'll also add an explict check in the CI that go get github.com/google/fscrypt/cmd/fscrypt works, as that line is mentioned in the README.

Copy link
Member Author

Choose a reason for hiding this comment

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

As for the tool related code, i'll fix that in #161

script:
- go build github.com/google/fscrypt/cmd/fscrypt
- make

- <<: *build
go: 1.12.x

- <<: *build
go: 1.11.x

- env: Integration Tests
- name: Integration Tests
sudo: required
addons:
apt:
Expand All @@ -57,7 +61,7 @@ jobs:
- goveralls -coverprofile=coverage.out -service=travis-ci

- stage: deploy
env: Release Binaries
name: Release Binaries
install: skip
script: skip
before_deploy: make
Expand Down
19 changes: 7 additions & 12 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ checks you should make sure that in your submission:
- If you made any changes to files ending in `.proto`, the corresponding
`.pb.go` files should be regenerated with `make gen`.
- Any issues found by `make lint` have been addressed.
- If any dependencies have changed, run `go mod tidy` and `go mod vendor`.
- If any dependencies have changed, run `go mod tidy`.
- `make coverage.out` can be used to generate a coverage report for all of the
tests, but isn't required for submission
(ideally most code would be tested, we are far from that ideal).
Expand All @@ -75,7 +75,6 @@ make test-setup
make all
make test-teardown
go mod tidy
go mod vendor
```
and everything succeeds, and no files are changed, you're good to submit.

Expand Down Expand Up @@ -114,11 +113,9 @@ test filesystem.

### Changing dependencies

fscrypt's dependencies are managed using the [Go 1.11 module system](https://github.com/golang/go/wiki/Modules).
If you add or remove a dependency, be sure to update `go.mod`, `go.sum`, and the
`vendor/` directory by running `go mod tidy` and `go mod vendor`. fscrypt still
vendor's it's dependencies for compatibility with older users, but this will
probobly be removed once the module system becomes widespread.
fscrypt's dependencies are managed using the
[Go 1.11 module system](https://github.com/golang/go/wiki/Modules).
If you add or remove a dependency, be sure to update `go.mod` and `go.sum`.

Also, when adding a dependency, the license of the package must be compatible
with [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0). See the
Expand All @@ -139,17 +136,15 @@ your code.
- Downloads [`protoc`](https://github.com/google/protobuf) to compile the
`.proto` files.
- Turns each `.proto` file into a matching `.pb.go` file using
[`protoc-gen-go`](https://github.com/golang/protobuf/tree/master/protoc-gen-go)
(built from source in `vendor/`).
[`protoc-gen-go`](https://github.com/golang/protobuf/tree/master/protoc-gen-go).

`make format` runs:
- [`goimports`](https://godoc.org/golang.org/x/tools/cmd/goimports)
(built from source in `vendor/`) on the `.go` files.
on the `.go` files.
- [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html)
on the `.c` and `.h` files.

`make lint` runs:
- [`go vet`](https://golang.org/cmd/vet/)
- [`golint`](https://github.com/golang/lint) (built from source in `vendor/`)
- [`golint`](https://github.com/golang/lint)
- [`staticcheck`](https://github.com/dominikh/go-tools/tree/master/cmd/staticcheck)
(built from source in `vendor/`)
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ override GO_LINK_FLAGS += $(VERSION_FLAG) $(DATE_FLAG) -extldflags "$(LDFLAGS)"
override GO_FLAGS += --ldflags '$(GO_LINK_FLAGS)'

###### Find All Files and Directories ######
FILES := $(shell find . \( -path ./vendor -o -path "./.*" \) -prune -o -type f -printf "%P\n")
FILES := $(shell find . -path '*/.git*' -prune -o -type f -printf "%P\n")
GO_FILES := $(filter %.go,$(FILES))
GO_NONGEN_FILES := $(filter-out %.pb.go,$(GO_FILES))
GO_DIRS := $(sort $(dir $(GO_FILES)))
Expand Down
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ information about each of the commands.
## Building and Installing

fscrypt has a minimal set of build dependencies:
* [Go](https://golang.org/doc/install) 1.10 or higher
* [Go](https://golang.org/doc/install) 1.11 or higher. Older versions may work
but they are not tested or supported.
* A C compiler (`gcc` or `clang`)
* `make`
* Headers for [`libpam`](http://www.linux-pam.org/).
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/google/fscrypt

go 1.12
go 1.11

require (
github.com/golang/protobuf v1.2.0
Expand Down
3 changes: 0 additions & 3 deletions vendor/github.com/golang/protobuf/AUTHORS
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions vendor/github.com/golang/protobuf/CONTRIBUTORS
View file
Open in desktop

This file was deleted.

28 changes: 0 additions & 28 deletions vendor/github.com/golang/protobuf/LICENSE
View file
Open in desktop

This file was deleted.

Loading