-
Notifications
You must be signed in to change notification settings - Fork 156
Updates to build sys and CONTRIBUTING.md #1638
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,8 +8,12 @@ | |
| # operate as part of "a build" that results in a bootc binary | ||
| # plus data files. The two key operations are `make` | ||
| # and `make install`. | ||
| # We expect code run from here is inside a container with low | ||
| # We expect code run from here is (or can be) inside a container with low | ||
| # privileges - running as a nonzero UID even. | ||
| # | ||
| # Understanding Makefile vs xtask.rs: Basically use xtask.rs if what | ||
| # you're doing would turn into a mess of bash code, whether inline here | ||
| # or externally in e.g. ./ci/somebashmess.sh etc. | ||
|
|
||
| prefix ?= /usr | ||
|
|
||
|
|
@@ -89,6 +93,16 @@ install-all: install install-ostree-hooks | |
| bin-archive: all | ||
| $(MAKE) install DESTDIR=tmp-install && $(TAR_REPRODUCIBLE) --zstd -C tmp-install -cf target/bootc.tar.zst . && rm tmp-install -rf | ||
|
|
||
| build-unit-tests: | ||
| cargo t --no-run | ||
|
|
||
| # We separate the build of the unit tests from actually running them in some cases | ||
| install-unit-tests: build-unit-tests | ||
| cargo t --no-run --frozen | ||
| install -D -m 0755 -t $(DESTDIR)/usr/lib/bootc/units/ $$(cargo t --no-run --message-format=json | jq -r 'select(.profile.test == true and .executable != null) | .executable') | ||
cgwalters marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| install -d -m 0755 /usr/bin/ | ||
| echo -e '#!/bin/bash\nset -xeuo pipefail\nfor f in /usr/lib/bootc/units/*; do echo $$f && $$f; done' > $(DESTDIR)/usr/bin/bootc-units && chmod a+x $(DESTDIR)/usr/bin/bootc-units | ||
|
|
||
| test-bin-archive: all | ||
| $(MAKE) install-all DESTDIR=tmp-install && $(TAR_REPRODUCIBLE) --zstd -C tmp-install -cf target/bootc.tar.zst . && rm tmp-install -rf | ||
|
|
||
|
|
@@ -98,23 +112,19 @@ test-bin-archive: all | |
| # We intentionally don't gate on this for local builds in cargo.toml | ||
| # because it impedes iteration speed. | ||
| CLIPPY_CONFIG = -A clippy::all -D clippy::correctness -D clippy::suspicious -D clippy::disallowed-methods -Dunused_imports -Ddead_code | ||
| validate-rust: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am going to type
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah fair point, though it's probably good since I think ideally we all switch to Though, a big tension for me now is using a toolbox and not devcontainer, one ends up with needing e.g. rust-analyzer on the host system, which means you're building all the Rust code there anyways... |
||
| validate: | ||
| cargo fmt -- --check -l | ||
| cargo test --no-run | ||
| (cd crates/ostree-ext && cargo check --no-default-features) | ||
| (cd crates/lib && cargo check --no-default-features) | ||
| cargo check --features=composefs-backend | ||
| cargo clippy -- $(CLIPPY_CONFIG) | ||
| env RUSTDOCFLAGS='-D warnings' cargo doc --lib | ||
| .PHONY: validate-rust | ||
| .PHONY: validate | ||
| fix-rust: | ||
| cargo clippy --fix --allow-dirty -- $(CLIPPY_CONFIG) | ||
| .PHONY: fix-rust | ||
|
|
||
| validate: validate-rust | ||
| ruff check | ||
| .PHONY: validate | ||
|
|
||
| update-generated: | ||
| cargo xtask update-generated | ||
| .PHONY: update-generated | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it may be nicer to invoke
sudowithin the justfile?I know it goes against best practice, though if someone must remember which commands require
sudoand which ones don't, I know I'd probably just default to sudoing more than needed.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so this
test-containerentrypoint is expected to be ran both rootful and rootless depending on the executing workflow.Yeah, there's not really anything that can be done here.
I no longer seem to be able to resolve my own threads, so feel free to resolve/ignore this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking at this btw!
With https://github.com/bootc-dev/bcvk and
bcvk to-diskin particular, everything fully supports rootless (and rootful).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(But I still need to wire up bcvk into the CI on this repo and drop all the
sudo)