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

build: Write a GCC specs file into workspace #1737

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

xnox
Copy link
Contributor

@xnox xnox commented Jan 9, 2025

Write out a GCC specs file into the workspace, that toolchain can
import to adjust build settings on per-package basis. For now, this
only adds Package Metadata ELF note settings.

In the future, this can be extended with other settings, for example
controlling generation of debug symbols, march/mtune/optimisations,
etc.

Potentially workspace is not a good location for this file. Maybe
worth considering to move this from /home/build/.melange.gcc.spec to
/etc/melange/gcc.spec.

By itself this PR is innert, until our default spec files start including the newly generated snippets when present. See:

For the proposed usage of this change, which initially makes it opt-in for 3 packages to shake out any e2e integration.

Write out a GCC specs file into the workspace, that toolchain can
import to adjust build settings on per-package basis. For now, this
only adds Package Metadata ELF note settings.

In the future, this can be extended with other settings, for example
controlling generation of debug symbols, march/mtune/optimisations,
etc.

Potentially workspace is not a good location for this file. Maybe
worth considering to move this from /home/build/.melange.gcc.spec to
/etc/melange/gcc.spec.
@stevebeattie
Copy link
Member

Potentially workspace is not a good location for this file. Maybe worth considering to move this from /home/build/.melange.gcc.spec to /etc/melange/gcc.spec.

Perhaps I'm overthinking things, but I do think in general a sensible direction for melange to go is to keep build configuration settings separate and ideally not tamperable by the build process of the packaged software itself, especially if we're providing a structured mechanism for said configuration (i.e. environment changes that are made outside of a run: invocation).

In this case, of course, the build can override or unset gcc/linker flags as part of how it invokes the compiler, so if it wanted to eliminate or modify the emitted ELF package note, it could still do so. Presumably we'd want a CI check to ensure that the note is generated as expected.

@xnox
Copy link
Contributor Author

xnox commented Jan 9, 2025

Potentially workspace is not a good location for this file. Maybe worth considering to move this from /home/build/.melange.gcc.spec to /etc/melange/gcc.spec.

Perhaps I'm overthinking things, but I do think in general a sensible direction for melange to go is to keep build configuration settings separate and ideally not tamperable by the build process of the packaged software itself, especially if we're providing a structured mechanism for said configuration (i.e. environment changes that are made outside of a run: invocation).

In this case, of course, the build can override or unset gcc/linker flags as part of how it invokes the compiler, so if it wanted to eliminate or modify the emitted ELF package note, it could still do so. Presumably we'd want a CI check to ensure that the note is generated as expected.

Correct, all of this is bypassable if so desired.

W.r.t. checks I do need to file a feature request for melange lint checks, to verify at build time, that binaries are auditable (i.e. go binaries have buildinfo, dependency metatada, and sybmols; rust binaries have cargo-auditable info; elf binaries have package-metadata) such that we can start to be alerted about binaries getting produced in the .apk which lack these things. And possibly a mirrored ci-* check that can be rerun on existing .apks to verify these things. Note sure if this will end up in mono, melange, or wolfictl.

smoser
smoser previously approved these changes Jan 14, 2025
Copy link
Contributor

@smoser smoser left a comment

Choose a reason for hiding this comment

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

I'm fine with this as a step towards getting the file written into some immutable path (like /melange/ or something).

If that is doable soon, then lets do it, if not then lets land this and open an issue to do it right.

pkg/build/build.go Outdated Show resolved Hide resolved
pkg/build/build.go Outdated Show resolved Hide resolved
Co-authored-by: Jon Johnson <jonjohnsonjr@gmail.com>
Signed-off-by: Dimitri John Ledkov <19779+xnox@users.noreply.github.com>
Co-authored-by: Jon Johnson <jonjohnsonjr@gmail.com>
Signed-off-by: Dimitri John Ledkov <19779+xnox@users.noreply.github.com>
@xnox xnox requested review from jonjohnsonjr and smoser January 15, 2025 00:08
@xnox
Copy link
Contributor Author

xnox commented Jan 15, 2025

I'm fine with this as a step towards getting the file written into some immutable path (like /melange/ or something).

If that is doable soon, then lets do it, if not then lets land this and open an issue to do it right.

this has to do right now as a bussiness need.

Location of this spec file doesn't really matter.... because builder in most/all runners today have the full filesystem as writable, as all files are owned by the same uid and everything is mounted read-write. Meaning even if this file was part of the apko image at /etc/melange/gcc.spec build pipelines would still be able to modify it like all the other files.

Read-only bind mounts do exist, but likely only in docker and not really in bubblewrap or qemu runners.

We need to work on isolating the builds better (root owned file system, with non-root builder uid for the pipeline executor) but until then, this would have to be enough.

documented all this at #1750 (comment)

@xnox xnox merged commit 7c33aed into chainguard-dev:main Jan 15, 2025
36 checks passed
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