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.

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

Add Go 1.16 to prerequisites (contributing doc) #1065

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

stefanprodan
Copy link
Member

Changes to contributing doc:

  • add image automation repos to GOTK list
  • add prerequisites for running the tests locally

@stefanprodan stefanprodan added the area/docs Documentation related issues and pull requests label Mar 8, 2021
Makefile Outdated
Comment on lines 29 to 30
install-dev: $(EMBEDDED_MANIFESTS_TARGET)
CGO_ENABLED=0 go build -o /usr/local/bin ./cmd/flux
Copy link
Member

@hiddeco hiddeco Mar 8, 2021

Choose a reason for hiding this comment

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

This has never worked on my system, as /usr/local/bin is protected, and I do not run my make commands with sudo:

$ make install-dev
CGO_ENABLED=0 go build -o /usr/local/bin ./cmd/flux
go build github.com/fluxcd/flux2/cmd/flux: copying /tmp/go-build3338692359/b001/exe/a.out: open /usr/local/bin/flux: permission denied
make: *** [Makefile:35: install-dev] Error 1

If you change it to something like this, it will prompt for a password, while (hopefully) not creating any problems on MacOS:

Suggested change
install-dev: $(EMBEDDED_MANIFESTS_TARGET)
CGO_ENABLED=0 go build -o /usr/local/bin ./cmd/flux
install-dev: $(EMBEDDED_MANIFESTS_TARGET)
CGO_ENABLED=0 go build -o /tmp/flux-dev ./cmd/flux
sudo install -Dm755 /tmp/flux-dev /usr/local/bin/flux

This results in the following output, which should make clear why it asks for a password:

$ make install-dev
CGO_ENABLED=0 go build -o /tmp/flux-dev ./cmd/flux
sudo install -Dm755 /tmp/flux-dev /usr/local/bin/flux
[sudo] password for hidde:

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that we are in a deadlock here, you don't want sudo make and I don't want make sudo.

Copy link
Member

@hiddeco hiddeco Mar 9, 2021

Choose a reason for hiding this comment

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

sudo make would make Go run as root, which is not something you want as all Go Modules are cached for the normal user.

My suggestion makes this target work on any system, and the issue you have with it for MacOS (or more specifically, Homebrew) can be overcome by running brew with the --overwrite flag. Given all this, I see much stronger arguments in favor of my suggestion, than against.

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've undone the changes to Makefile

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
@stefanprodan stefanprodan merged commit ed47182 into main Mar 12, 2021
@stefanprodan stefanprodan deleted the build-prerequisites branch March 12, 2021 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Documentation related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants