Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Fix problems in Makefile #705

Closed
wants to merge 2 commits into from
Closed

Fix problems in Makefile #705

wants to merge 2 commits into from

Conversation

yangdongsheng
Copy link
Contributor

No description provided.

@yangdongsheng yangdongsheng changed the title Fix problems in Fix problems in Makefile Dec 16, 2015
@yangdongsheng
Copy link
Contributor Author

@yangdongsheng
Copy link
Contributor Author

@asdfsx @MironWong

@tombee
Copy link
Contributor

tombee commented Dec 16, 2015

LGTM

@@ -10,7 +10,7 @@ clean:
@rm -rf controller/controller

build:
@cd controller && godep go build -a -tags "netgo static_build" -installsuffix netgo -ldflags "-w -X github.com/shipyard/shipyard/version.GitCommit=$(COMMIT)" .
@cd controller && godep go build -a -tags "netgo static_build" -installsuffix netgo -ldflags "-w -X github.com/shipyard/shipyard/version.GitCommit $(COMMIT)" .
Copy link
Owner

Choose a reason for hiding this comment

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

We need the = as this may not work in the future (this is a Go warning upon build).

Copy link
Owner

Choose a reason for hiding this comment

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

To be clearer, it should not be changed from the previous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But, we got an error in "make image" with =:

go version

go version go1.4.2 linux/amd64

Do you mean I need to upgrade my golang?

Copy link
Owner

Choose a reason for hiding this comment

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

You shouldn't get an error and yes you should be using Go 1.5. Thanks!
On Dec 17, 2015 12:38 AM, "yangdongsheng" notifications@github.com wrote:

In Makefile
#705 (comment):

@@ -10,7 +10,7 @@ clean:
@rm -rf controller/controller

build:

  • @cd controller && godep go build -a -tags "netgo static_build" -installsuffix netgo -ldflags "-w -X github.com/shipyard/shipyard/version.GitCommit=$(COMMIT)" .
  • @cd controller && godep go build -a -tags "netgo static_build" -installsuffix netgo -ldflags "-w -X github.com/shipyard/shipyard/version.GitCommit $(COMMIT)" .

But, we got an error in "make image" with =:
go version

go version go1.4.2 linux/amd64

Do you mean I need to upgrade my golang?


Reply to this email directly or view it on GitHub
https://github.com/shipyard/shipyard/pull/705/files#r47871188.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, can I say our Makefile is not compatible to Go version before 1.5? What about making it compatible?

Dongsheng Yang added 2 commits December 18, 2015 10:44
Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
@yangdongsheng
Copy link
Contributor Author

@ehazlett and @tombee I updated it to make it compatible to different versions of golang, please take a look

@tombee
Copy link
Contributor

tombee commented Dec 18, 2015

@yangdongsheng is there a particular reason you want it to be compatible with 1.4?

IMO we should just leave it, and ensure we build with 1.5. Dockerfile.build uses golang:1.5.

@yangdongsheng
Copy link
Contributor Author

@tombee I am using RHEL7, and I met this error in my building :(

@tombee
Copy link
Contributor

tombee commented Dec 18, 2015

Perhaps we can look at creating a script that automates the creations of the build image from Dockerfile.build, and then runs make inside a container.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants