Skip to content

build: stage packages outside temp dirs#144

Closed
MikkoParkkola wants to merge 1 commit intotrailofbits:mainfrom
MikkoParkkola:copilot/release-build-policy-20260410
Closed

build: stage packages outside temp dirs#144
MikkoParkkola wants to merge 1 commit intotrailofbits:mainfrom
MikkoParkkola:copilot/release-build-policy-20260410

Conversation

@MikkoParkkola
Copy link
Copy Markdown

Summary

  • copy packaged release artifacts into dist/ before cleanup
  • remove temporary package-build/ output after packaging
  • clear musl target intermediates after the package recipe completes

Notes

  • release artifacts that may be used in production are preserved
  • cleanup only targets debug and intermediate build artifacts by default

Validation

  • just --list
  • git diff --check

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 10, 2026 16:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Stages Linux packaging outputs into a persistent dist/ directory before cleaning up temporary build directories, so release artifacts survive cleanup while intermediates are removed.

Changes:

  • Add a dist/ staging directory and copy packaged artifacts from package-build/ into it.
  • Remove package-build/ after packaging completes.
  • Clean the musl target build artifacts after packaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread justfile
Comment on lines +51 to +52
mkdir -p dist
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f {} dist/ \;
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

just package now creates a top-level dist/ directory and leaves packaged artifacts there, but dist/ is not currently ignored (see .gitignore only ignores /target and /package-build). This will consistently dirty the working tree and increases the risk of accidentally committing release artifacts. Consider adding /dist (or /dist/*) to .gitignore, or writing artifacts under an already-ignored build output directory.

Suggested change
mkdir -p dist
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f {} dist/ \;
mkdir -p target/dist
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f {} target/dist/ \;

Copilot uses AI. Check for mistakes.
Comment thread justfile
cmake -S package -B package-build -DMQUIRE_REPOSITORY_PATH="$(pwd)"
cmake --build package-build --target package
mkdir -p dist
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f {} dist/ \;
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The find ... -exec cp ... \; form spawns a separate cp process for each matched artifact. If multiple packages are produced (e.g., TGZ + RPM + DEB), consider batching with -exec ... + (or an equivalent approach) to reduce process overhead.

Suggested change
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f {} dist/ \;
find package-build -maxdepth 1 -type f -name 'mquire-*' -exec cp -f -t dist/ {} +

Copilot uses AI. Check for mistakes.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@alessandrogario
Copy link
Copy Markdown
Member

Hello @MikkoParkkola,

I'm not sure I understand the reason the packaging code path is being changed this way. I'm going to close this PR for now, but feel free to open an issue with a blueprint for the change you'd like to propose so it can be further discussed

Thanks!

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