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 -buildvcs=false to Makefile #104

Merged
merged 1 commit into from
Jan 19, 2023
Merged

Add -buildvcs=false to Makefile #104

merged 1 commit into from
Jan 19, 2023

Conversation

SreeeS
Copy link
Contributor

@SreeeS SreeeS commented Jan 19, 2023

Summary

Make release is failing on amazon-ecs-agent when we update the submodules.

The Error message is:

GOOS=linux CGO_ENABLED=0 go build -installsuffix cgo -a -ldflags "\
     -X github.com/aws/amazon-ecs-cni-plugins/pkg/version.GitShortHash=a04bd6ec \
     -X github.com/aws/amazon-ecs-cni-plugins/pkg/version.GitPorcelain=0 \
     -X github.com/aws/amazon-ecs-cni-plugins/pkg/version.Version=2020.09.0 -s" \
     -o /go/src/github.com/aws/amazon-ecs-cni-plugins/bin/plugins/ecs-eni github.com/aws/amazon-ecs-cni-plugins/plugins/eni
error obtaining VCS status: exit status 128
	Use -buildvcs=false to disable VCS stamping.
make: *** [Makefile:40: bin/plugins/ecs-eni] Error 1
make: *** [build-ecs-cni-plugins] Error 2

This change is to include -buildvcs=false flag to the go build in Make file.

This error occurred after we updated the amazon-ecs-cni-plugins sub module in amazon-ecs-agent to pull the latest commit due to this change in golang: golang/go#37475

Implementation details

Added -buildvcs=false flag in Make file.
-buildvcs=false excludes version control information when golang version is >= 1.18.

Testing

make unit-test
make release on ecs-agent repo

New tests cover the changes: no

Description for the changelog

bug-Fixed failed make release on ecs-agent repo

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@SreeeS SreeeS marked this pull request as ready for review January 19, 2023 20:20
@mythri-garaga
Copy link

mythri-garaga commented Jan 19, 2023

Any implications of this setting flag to false? Can we mention that in the summary too?

@sparrc
Copy link
Contributor

sparrc commented Jan 19, 2023

Would be good to mention in the summary why and when this started failing, I believe this is only applicable to when the plugins are used as git submodules, and due to this change in golang correct? golang/go#37475

@SreeeS
Copy link
Contributor Author

SreeeS commented Jan 19, 2023

Any implications of this setting to flag to false? Can we mention that in the summary too?

Generally, setting the buildvcs to false will disable VCS stamping. Disabling the VCS stamping can make it harder to understand how it was built, hard to track and reproduce the issue in case of errors. In our case we are using this as a sub module plugin as this would not be in the root directory there seems to be a discrepancy in status of VCS, which is making the make release to fail on agent repo when we update the cni-plugin sub modules.

@sparrc
Copy link
Contributor

sparrc commented Jan 19, 2023

Disabling the VCS stamping can make it harder to understand how it was built, hard to track and reproduce the issue in case of errors.

what does this mean? what specifically does this flag disable? Are there any features we currently use that this disables?

@prateekchaudhry
Copy link

https://tip.golang.org/doc/go1.18

This flag seems to be introduced in golang 1.18. Will this change be backwards compatible with go version of the system? Do we want to make this flag conditional on go version?

@SreeeS
Copy link
Contributor Author

SreeeS commented Jan 19, 2023

Would be good to mention in the summary why and when this started failing, I believe this is only applicable to when the plugins are used as git submodules, and due to this change in golang correct? golang/go#37475

I agree, updated the summary.

@SreeeS
Copy link
Contributor Author

SreeeS commented Jan 19, 2023

Disabling the VCS stamping can make it harder to understand how it was built, hard to track and reproduce the issue in case of errors.

what does this mean? what specifically does this flag disable? Are there any features we currently use that this disables?

I was mentioning general implications. We don't depend on it.

@SreeeS
Copy link
Contributor Author

SreeeS commented Jan 19, 2023

https://tip.golang.org/doc/go1.18

This flag seems to be introduced in golang 1.18. Will this change be backwards compatible with go version of the system? Do we want to make this flag conditional on go version?

Added logic to use the flag based on golang version

@SreeeS SreeeS merged commit 66bc7c7 into aws:dev Jan 19, 2023
SreeeS added a commit that referenced this pull request Jan 20, 2023
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.

4 participants