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

Cleanup Makefile, build out-of-source into bin/ dir #145

Merged
merged 6 commits into from
May 13, 2020
Merged

Conversation

matzf
Copy link
Contributor

@matzf matzf commented May 12, 2020

  • Build out-of-source into bin/ directory
  • Use binaries in bin/ for integration tests instead of installing
  • Binary names now match names also used by the packages
  • make install installs into the "correct" directory also used by go install
  • update golangci-lint version (and run gomft to make lint pass)

Fixes #144
Fixes #118


This change is Reviewable

matzf added 2 commits May 12, 2020 16:05
Use binaries in bin/ for integration tests instead of installing.
Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, 5 of 5 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @matzf)


Makefile, line 5 at r1 (raw file):

DESTDIR = $(shell go env GOBIN GOPATH | eval; echo $${GOBIN:-$${GOPATH:-$${HOME}/go}/bin})

I had some trouble parsing this. Would :
DESTDIR = $(shell echo $${GOBIN:-$${GOPATH}/bin:-$${HOME}/go}/bin}) not be sufficient?
The output of go env is ignored here?


Makefile, line 33 at r1 (raw file):

install: all
  # Note: install everything but the examples

Nice solution

Copy link
Contributor Author

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @FR4NK-W)


Makefile, line 5 at r1 (raw file):

Previously, FR4NK-W wrote…
DESTDIR = $(shell go env GOBIN GOPATH | eval; echo $${GOBIN:-$${GOPATH:-$${HOME}/go}/bin})

I had some trouble parsing this. Would :
DESTDIR = $(shell echo $${GOBIN:-$${GOPATH}/bin:-$${HOME}/go}/bin}) not be sufficient?
The output of go env is ignored here?

Done.
The go env also takes into account some go config files (you can eg set GOBIN in .config/go/env, for whatever reason).
I did the loading of the variables wrong though, so perhaps that's why you were confused. Now it should work and I documented it a little bit.


Makefile, line 33 at r1 (raw file):

Previously, FR4NK-W wrote…

Nice solution

Done.

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)


Makefile, line 43 at r3 (raw file):

./_examples/helloworld/

Why does the helloworld one need to be explicited?

Copy link
Contributor Author

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on @FR4NK-W)


Makefile, line 43 at r3 (raw file):

Previously, FR4NK-W wrote…
./_examples/helloworld/

Why does the helloworld one need to be explicited?

It simply does not seem to be run otherwise. IIRC, directories with _ are ignored by default, so that seems to make sense.

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 5 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


Makefile, line 43 at r3 (raw file):

Previously, matzf (Matthias Frei) wrote…

It simply does not seem to be run otherwise. IIRC, directories with _ are ignored by default, so that seems to make sense.

👍

For later reference (https://golang.org/cmd/go/):

Directory and file names that begin with "." or "_" are ignored by the go tool, as are directories named "testdata".

@matzf matzf merged commit 4ed2931 into master May 13, 2020
@matzf matzf deleted the matzf/makefile branch May 13, 2020 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong build targets Make install may corrupt $GOPATH/bin
2 participants