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

feat(arm): Makefile add armv6 and arm64 to releases #35

Merged
merged 1 commit into from
Jan 31, 2019
Merged

feat(arm): Makefile add armv6 and arm64 to releases #35

merged 1 commit into from
Jan 31, 2019

Conversation

karlskewes
Copy link
Contributor

Description

Update release target of Makefile to include building armv6 and arm64 binaries.

Note: it seems possible to build all binaries on amd64 or armv6 (with dep changes).

Motivation and Context

On path to close #16 .

Docker images to come later.

How Has This Been Tested?

On armv7 RPi 3B tested version/etc. Not doing full proxy workflow:

karl@pi1:~ $ ./oauth2_proxy-linux-armv6 
2019/02/01 08:33:17 main.go:107: Invalid configuration:
  missing setting: cookie-secret
  missing setting: client-id
  missing setting: client-secret
  missing setting for email validation: email-domain or authenticated-emails-file required.
      use email-domain=* to authorize all email addresses

karl@pi1:~ $ ./oauth2_proxy-linux-armv6 --version
oauth2_proxy v3.0.0-17-g090ff11-dirty (built with go1.11.5)

Same again on arm64 Rock64 SBC:

rock64@k8s-w-01:~$ ./oauth2_proxy-linux-arm64 
2019/02/01 08:34:19 main.go:107: Invalid configuration:
  missing setting: cookie-secret
  missing setting: client-id
  missing setting: client-secret
  missing setting for email validation: email-domain or authenticated-emails-file required.
      use email-domain=* to authorize all email addresses

rock64@k8s-w-01:~$ ./oauth2_proxy-linux-arm64 --version
oauth2_proxy v3.0.0-17-g090ff11-dirty (built with go1.11.5)

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

@karlskewes karlskewes requested a review from a team January 31, 2019 19:37
JoelSpeed
JoelSpeed reviewed Jan 31, 2019
View reviewed changes
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Looks good, just one quick question to help my understanding (not an arm user)

Makefile
@@ -54,10 +54,16 @@ release: lint test
mkdir release
GOOS=darwin GOARCH=amd64 go build -ldflags="-X main.VERSION=${VERSION}" -o release/$(BINARY)-darwin-amd64 github.com/pusher/oauth2_proxy
GOOS=linux GOARCH=amd64 go build -ldflags="-X main.VERSION=${VERSION}" -o release/$(BINARY)-linux-amd64 github.com/pusher/oauth2_proxy
GOOS=linux GOARCH=arm64 go build -ldflags="-X main.VERSION=${VERSION}" -o release/$(BINARY)-linux-arm64 github.com/pusher/oauth2_proxy
GOOS=linux GOARCH=arm GOARM=6 go build -ldflags="-X main.VERSION=${VERSION}" -o release/$(BINARY)-linux-armv6 github.com/pusher/oauth2_proxy
Copy link
Member

Choose a reason for hiding this comment

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

Am I right in thinking that if we changed this to GOARM=7 then it wouldn't run on original Pi's and Pi zeros whereas currently it runs on all Pi's?

Copy link

Choose a reason for hiding this comment

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

I believe that is correct. It allows new v7 only asm instructions to be used for the build that the zero doesn't have. v7 is a superset of v6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed.
With go and dep dependencies I decided to just leave this at armv6 for now.
As it builds and runs on my armv7 I figure it's safe to start with.

@JoelSpeed JoelSpeed self-assigned this Jan 31, 2019
JoelSpeed
JoelSpeed approved these changes Jan 31, 2019
View reviewed changes
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@JoelSpeed JoelSpeed merged commit c6d2126 into oauth2-proxy:master Jan 31, 2019
michael-freidgeim-webjet pushed a commit to MNF/oauth2-proxy that referenced this pull request Sep 4, 2021
@Kevin-Li-Webjet
Merge pull request oauth2-proxy#35 from Webjet/tableau-teamcity
f97a641 
updated args
Jing-ze pushed a commit to Jing-ze/oauth2-proxy that referenced this pull request Nov 19, 2024
@slievrly
optimize: add contributing docs (oauth2-proxy#35)
294c267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfox1111 kfox1111 kfox1111 left review comments

@JoelSpeed JoelSpeed JoelSpeed approved these changes

Assignees

@JoelSpeed JoelSpeed

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Request] Include Release & Docker build for Raspberry Pi (ARMv6/7 Architecture)
3 participants
@karlskewes @kfox1111 @JoelSpeed