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

PRODENG-2527 Added make install target to the Makefile #419

Closed
wants to merge 1 commit into from

Conversation

cranzy
Copy link
Contributor

@cranzy cranzy commented Feb 8, 2024

@cranzy cranzy requested a review from james-nesbitt February 8, 2024 10:42
@@ -53,6 +53,9 @@ build-all: builder
GOOS=darwin GOARCH=amd64 $(GO) go build $(BUILD_FLAGS) -o bin/launchpad-darwin-x64 main.go
GOOS=darwin GOARCH=arm64 $(GO) go build $(BUILD_FLAGS) -o bin/launchpad-darwin-arm64 main.go

install: build
sudo install -m 755 bin/launchpad /usr/local/bin/launchpad
Copy link
Collaborator

@kke kke Feb 8, 2024

Choose a reason for hiding this comment

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

It is some kind of convention to have this as:

	install -d $(DESTDIR)$(PREFIX)/bin/
	install -m 755 launchpad $(DESTDIR)$(PREFIX)/bin/

and have this somewhere in the top:

PREFIX = /usr/local

This way `make install PREFIX=/some/package/manager/library" will work correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also it shouldn't sudo on its own, user should use sudo make install.

@@ -53,6 +53,9 @@ build-all: builder
GOOS=darwin GOARCH=amd64 $(GO) go build $(BUILD_FLAGS) -o bin/launchpad-darwin-x64 main.go
GOOS=darwin GOARCH=arm64 $(GO) go build $(BUILD_FLAGS) -o bin/launchpad-darwin-arm64 main.go

install: build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
install: build
install: launchpad

@james-nesbitt
Copy link
Collaborator

This is likely abandonded. Closing.

@cranzy cranzy deleted the 2527-makefile-install-target branch February 13, 2024 08:22
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.

3 participants