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

src: Port Go build wrapper from Shell to Python & support externaly defined build options #1031

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

HarryMichal
Copy link
Member

No description provided.

@HarryMichal HarryMichal added 6. Minor Change Should not cause breakage 3. New Feature New feature 2. Build System Issue is related to the build system 3. Refinement Reduces technical debt of the codebase labels Mar 22, 2022
@HarryMichal HarryMichal changed the title Port Go build wrapper from Shell to Python & support externaly defined build options src: Port Go build wrapper from Shell to Python & support externaly defined build options Mar 22, 2022
@HarryMichal HarryMichal added this to the Release 0.1.0 milestone Mar 22, 2022
Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Yes, this will be good to have. I once thought of it, but the shell script started giving me nightmares.

Maybe this is be a good time to add some tests to sanity check the built binary?

The go-build-wrapper script encapsulates a lot of subtle, but crucial, details and we don't want to regress anything. eg., the path to the dynamic linker, the rpath, etc.. Ideally we would add the test first to test the old script, and then introduce the new script -- that way we can trivially verify (through git bisect, etc.) that everything is good.

output_dir = sys.argv[2]
version = sys.argv[3]
cc = sys.argv[4]
dyn_linker = sys.argv[5]
Copy link
Member

Choose a reason for hiding this comment

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

Would be better to use terms like dynamic_linker, dynamic-linker everywhere. They are easier to grep for (eg., searching for dynamic will get you better results), and can defend against typos by making them stand out.

@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal HarryMichal force-pushed the build-script-update branch from 531cf4b to a29ac02 Compare March 25, 2022 20:37
@softwarefactory-project-zuul
Copy link

Build failed.

Toolbx requires some binary-level adjustments to accomplish its job.
Potential regressions in the wrapper build script need to be caught to
prevent breakage.
@HarryMichal HarryMichal force-pushed the build-script-update branch from a29ac02 to 74cb8e7 Compare March 25, 2022 21:11
@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal HarryMichal force-pushed the build-script-update branch from 74cb8e7 to 704e1d0 Compare March 25, 2022 21:45
Some downstreams (e.g., Fedora) set additional flags while building
software in their repositories. This also applies to software written in
Go. This work removes the need for manually patching the 'go build'
wrapper script, needed for integrating 'go build' into Meson, to add the
desired build options.

This introduces a new build option 'go_build_flags'.
@HarryMichal HarryMichal force-pushed the build-script-update branch from 704e1d0 to d0b7a84 Compare March 25, 2022 21:56
@softwarefactory-project-zuul
Copy link

Build succeeded.

package binary

import (
"debug/elf"
Copy link
Member

Choose a reason for hiding this comment

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

How does Go's debug/elf compare with other implementations like GNU Binutils, in terms of architecture support and the ability to handle a diverse variety of ELFs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Overall I'd say the support is lower than of readelf from GNU Binutils. From what I understood though, debug/elf supports both 32-bit and 64-bit architectures and recognizes many machine types and many OS ABIs. Though, am not sure how the last two parts translate into "architecture support". Most likely didn't answer your question in full.

Probably is noteworthy that the tool is for checking the Toolbx binary and not an arbitrary binary. My presumption is that whether Go is capable of producing a binary on a certain architecture, the created binary also has to be processable by the debug/elf package.

@HarryMichal HarryMichal requested a review from debarshiray May 12, 2022 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. Build System Issue is related to the build system 3. New Feature New feature 3. Refinement Reduces technical debt of the codebase 6. Minor Change Should not cause breakage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants