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 build constraint to allow compiling without cgo #604

Closed
wants to merge 2 commits into from

Conversation

alfrunes
Copy link
Contributor

The PKCS11 feature (not used by the backend) prevents compiling mender-artifact without enabling cgo.

Signed-off-by: Alf-Rune Siqveland <alf.rune@northern.tech>
Because of the openssl PKCS11 dependency, it's not possible to compile
mender-artifact without `cgo` enabled.

Signed-off-by: Alf-Rune Siqveland <alf.rune@northern.tech>
@alfrunes alfrunes requested review from lluiscampos and a user April 25, 2024 13:06
@mender-test-bot
Copy link

@alfrunes, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".


my commands and options

You can trigger a pipeline on multiple prs with:

  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can trigger GitHub->GitLab branch sync with:

  • mentioning me and sync

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I think this should be made into a feature flag (such as nopkcs11 or something like that). I think silently disabling features if cgo is disabled is surprising and unintuitive.

However, independent from that, I don't think it will work as intended, because I think lzma requires cgo as well, and we can't disable that. Not 100% sure though.

@alfrunes
Copy link
Contributor Author

I think this should be made into a feature flag (such as nopkcs11 or something like that). I think silently disabling features if cgo is disabled is surprising and unintuitive.

This is exactly how we do it for lzma (I found out after filing this PR), and I agree that disabling features should be explicit. However, this feature abuse the built-in linux constraint which makes impossible to opt out (in the backend we do not use this dependency). I will change the build tag to pkcs11 and have it set by default in the Makefile.

However, independent from that, I don't think it will work as intended, because I think lzma requires cgo as well, and we can't disable that. Not 100% sure though.

For lzma I will similarly add a different (Go) dependency if compiled without cgo since this one is crucial.

@alfrunes
Copy link
Contributor Author

Thoughts on this one? #605

@@ -1,5 +1,5 @@
# Keep golang version aligned with latest yocto release
FROM golang:1.17.13-bullseye as builder
FROM golang:1.22.2-bullseye as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep the version aligned to yocto, see comment above.

Coincidentally, we are discussing this in the upcoming client team meeting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please keep in mind that go1.17 has been out of support for almost 2 years. Is it not possible to use the most recent version of Go when compiling Yocto?

Copy link
Contributor

@lluiscampos lluiscampos Apr 26, 2024

Choose a reason for hiding this comment

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

Yocto maintains the toolchain, porting security fixes (same as for example Debian or other OSs do for the same time period that they support the OS).

Fixing the version here does have any impact in the final yocto build nor in the deb packages build.

Fixing the version here is only a way for us (me) to early catch when updates will break in today latest yocto LTS. Then we can decide to accept the breakage and patch the code there or reject the update for the time being. It is a difficult balance - to give you a small picture: we still support dunfell LTS which uses golang 1.14 and it is really hard and time consuming delivering our software there.

@alfrunes
Copy link
Contributor Author

Superseded by #605

@alfrunes alfrunes closed this Apr 29, 2024
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.

5 participants