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

fix build information when to build in docker #782

Merged
merged 3 commits into from
Aug 4, 2020

Conversation

cwen0
Copy link
Member

@cwen0 cwen0 commented Aug 4, 2020

Signed-off-by: cwen0 [email protected]

What problem does this PR solve?

Fix:

/usr/local/bin # ./chaos-controller-manager -version
Controller manager Version: version.Info{GitVersion:"v0.0.0-master+$Format:%h$", GitCommit:"$Format:%H$", BuildDate:"2020-08-04T05:59:29Z", GoVersion:"go1.14.4", Compiler:"gc", Platform:"linux/amd64"}

Fixed:

Controller manager Version: version.Info{GitVersion:"v1.0.0-alpha.1.156+4463f1d8c63391", GitCommit:"4463f1d8c633918962b96a73553ae848677558b8", BuildDate:"2020-08-04T07:37:13Z", GoVersion:"go1.14.4", Compiler:"gc", Platform:"linux/amd64"}

What is changed and how does it work?

Add a build-arg

@cwen0 cwen0 added the type/bug-fix Fix for a previously reported bug. label Aug 4, 2020
@cwen0 cwen0 requested review from yeya24, YangKeao and fewdan August 4, 2020 08:15

COPY . /src
WORKDIR /src
RUN --mount=type=cache,target=/root/.cache/go-build \
make binary
IMG_LDFLAGS=$LDFLAGS make binary
Copy link
Member

Choose a reason for hiding this comment

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

Why not name "IMG_LDFLAGS" as "LDFLAGS" directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to reference IMG_LDFLAGS to LDFLAGS in Makefile, If we use LDFLAGS directly, an error will be report when compiling:

 Recursive variable `LDFLAGS' references itself (eventually).  Stop.

Copy link
Member

@fewdan fewdan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 42dcde4 into chaos-mesh:master Aug 4, 2020
@cwen0 cwen0 deleted the fix_ldflags branch August 19, 2020 06:20
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
* fix build information when to build in docker

Signed-off-by: cwen0 <[email protected]>

* make good

Signed-off-by: cwen0 <[email protected]>

Co-authored-by: ti-srebot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants