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

golang: update to 1.14.4 #850

Merged
merged 3 commits into from Jun 8, 2020
Merged

golang: update to 1.14.4 #850

merged 3 commits into from Jun 8, 2020

Conversation

benma
Copy link
Contributor

@benma benma commented Jun 7, 2020

shiftcrypto/bitbox-wallet-app:5 created and based on the change in the
Docker image.

@benma benma mentioned this pull request Jun 7, 2020
Merged
@x1ddos
Copy link
Contributor

x1ddos commented Jun 7, 2020

With go1.14 now, how about removing GO111MODULE=off and lifting the requirement of

Clone this repository to $GOPATH/src/github.com/digitalbitbox/bitbox-wallet-app ...

in https://github.com/digitalbitbox/bitbox-wallet-app#requirements?

I've just tried building frontend/android/goserver with gomobile on 1.14 without GO111MODULE=off and it seems to work:

$ gomobile bind -x -a -ldflags="-s -w" -target android .
WORK=/tmp/gomobile-work-303315762
GOOS=android CGO_ENABLED=1 $GOPATH/bin/gobind -lang=go,java -outdir=$WORK github.com/digitalbitbox/bitbox-wallet-app/frontends/android/goserver
mkdir -p $WORK/src
...
$ ls -lh
total 37M
415 Nov  9  2019 Makefile
135 Nov  9  2019 README.md
8.0K Jun  7 10:02 goserver-sources.jar
37M Jun  7 10:02 goserver.aar
5.2K May 14 19:10 goserver.go

This repo is the only one living in my old GOPATH.

@x1ddos
Copy link
Contributor

x1ddos commented Jun 7, 2020

With go1.14 now, how about removing GO111MODULE=off and lifting the requirement of

We'll have to do it sooner or later. Why not now.

@benma
Copy link
Contributor Author

benma commented Jun 7, 2020

With go1.14 now, how about removing GO111MODULE=off and lifting the requirement of

Clone this repository to $GOPATH/src/github.com/digitalbitbox/bitbox-wallet-app ...

in https://github.com/digitalbitbox/bitbox-wallet-app#requirements?

I've just tried building frontend/android/goserver with gomobile on 1.14 without GO111MODULE=off and it seems to work:

$ gomobile bind -x -a -ldflags="-s -w" -target android .
WORK=/tmp/gomobile-work-303315762
GOOS=android CGO_ENABLED=1 $GOPATH/bin/gobind -lang=go,java -outdir=$WORK github.com/digitalbitbox/bitbox-wallet-app/frontends/android/goserver
mkdir -p $WORK/src
...
$ ls -lh
total 37M
415 Nov  9  2019 Makefile
135 Nov  9  2019 README.md
8.0K Jun  7 10:02 goserver-sources.jar
37M Jun  7 10:02 goserver.aar
5.2K May 14 19:10 goserver.go

This repo is the only one living in my old GOPATH.

With go1.14 now, how about removing GO111MODULE=off and lifting the requirement of

Clone this repository to $GOPATH/src/github.com/digitalbitbox/bitbox-wallet-app ...

in https://github.com/digitalbitbox/bitbox-wallet-app#requirements?

I've just tried building frontend/android/goserver with gomobile on 1.14 without GO111MODULE=off and it seems to work:

$ gomobile bind -x -a -ldflags="-s -w" -target android .
WORK=/tmp/gomobile-work-303315762
GOOS=android CGO_ENABLED=1 $GOPATH/bin/gobind -lang=go,java -outdir=$WORK github.com/digitalbitbox/bitbox-wallet-app/frontends/android/goserver
mkdir -p $WORK/src
...
$ ls -lh
total 37M
415 Nov  9  2019 Makefile
135 Nov  9  2019 README.md
8.0K Jun  7 10:02 goserver-sources.jar
37M Jun  7 10:02 goserver.aar
5.2K May 14 19:10 goserver.go

This repo is the only one living in my old GOPATH.

Originally posted by @x1ddos in #850 (comment)

README updated.

Without GO111MODULE=off, I believe you are building the mobile bindings without the vendored deps. There is no -mod=vendor flag in gomobile bind (yet or ever?).

@x1ddos
Copy link
Contributor

x1ddos commented Jun 7, 2020

Without GO111MODULE=off, I believe you are building the mobile bindings without the vendored deps. There is no -mod=vendor flag in gomobile bind (yet or ever?).

Oh, you're right! Didn't realize in my previous build in #850 (comment) it was actually downloading packages, revealed by adding -v verbose flag.

Commented on golang/go#34181.

@x1ddos
Copy link
Contributor

x1ddos commented Jun 7, 2020

A couple other places to update to 1.14:

@x1ddos x1ddos self-requested a review June 7, 2020 19:45
@benma
Copy link
Contributor Author

benma commented Jun 8, 2020

A couple other places to update to 1.14:

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/go.mod#L3

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/Makefile#L35

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/Makefile#L35

done (the last two are the same)

@x1ddos
Copy link
Contributor

x1ddos commented Jun 8, 2020

A couple other places to update to 1.14:

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/go.mod#L3

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/Makefile#L35

* https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/Makefile#L35

done (the last two are the same)

sorry, last one was copy&paste error. it was supposed to be https://github.com/digitalbitbox/bitbox-wallet-app/blob/04925927/Makefile#L39, in osx-init make target. looks like it's still at 1.13.

benma and others added 2 commits June 8, 2020 11:36
@benma
golang: update to 1.14.4
ef630dc 
shiftcrypto/bitbox-wallet-app:5 created and based on the change in the
Docker image.
@x1ddos @benma
vendor: sync deps
51d8729 
Go 1.14, which is what's used on CI runners, says to
"run 'go mod vendor' to sync". Otherwise, it complains with:

go: inconsistent vendoring in
/Users/runner/go/src/github.com/digitalbitbox/bitbox-wallet-app:
golang.org/x/net@v0.0.0-20190620200207-3b0461eec859: is
explicitly required in go.mod, but vendor/modules.txt indicates
golang.org/x/net@v0.0.0-20190404232315-eb5bcb51f2a3

This commit is the result of "make go-vendor".

Actually, had to update go.mod with v0.0.0-20190620200207-3b0461eec859
for golang.org/x/net. Simply running "make go-vendor" without go.mod
changes resulted in the same error.

This is vendoring madness. Worst and most confusing design in the whole
history of Go.
@benma
Copy link
Contributor Author

benma commented Jun 8, 2020

make target. looks like it's still at 1.13.

ah maybe that's why it didn't work.

anyway, lots of places to update the Go version in :O

@benma
make envinit: GO111MODULE=off with gomobile init
f2fc360 
Build started failing on Travis on macOS with:

```
gomobile init

gomobile: go install golang.org/x/mobile/cmd/gobind failed: exit status 1

can't load package: package golang.org/x/mobile/cmd/gobind: cannot find package "." in:

	/Users/travis/go/src/github.com/digitalbitbox/bitbox-wallet-app/vendor/golang.org/x/mobile/cmd/gobind
```
x1ddos
x1ddos approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@x1ddos x1ddos left a comment

Choose a reason for hiding this comment

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

LGTM

Somehow vendor/modules.txt updates got into the update to 1.14 commit but ok.

@benma benma merged commit 0a62ce8 into digitalbitbox:master Jun 8, 2020
@benma
Copy link
Contributor Author

benma commented Jun 8, 2020

Somehow vendor/modules.txt updates got into the update to 1.14 commit but ok.

hmm i did make go-vendor in an interactive rebase in every commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@x1ddos x1ddos x1ddos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@benma @x1ddos