A few buildsystem fixes #3437
A few buildsystem fixes #3437
Conversation
There was a problem hiding this comment.
Hello @cgwalters, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request titled "A few buildsystem fixes". This PR bundles several distinct improvements and fixes related to the project's build system, CI setup, documentation generation, and test infrastructure. The main goals are to quiet documentation build warnings, add a .dockerignore file for cleaner Docker builds, update the CentOS CI configuration to use a more modern approach for enabling necessary repositories (CRB and EPEL), and enhance the test suite's web server handling to optionally use a simple Python server if the dedicated ostree-trivial-httpd is not available.
Highlights
- Documentation Warnings: Includes a change to the API documentation configuration (
apidoc/ostree-docs.xml) which is intended to reduce the number of warnings generated during the documentation build process. - Docker Ignore File: Adds a new
.dockerignorefile to prevent unnecessary files and directories (like build artifacts and lock files) from being included in the Docker build context. - CentOS CI Updates: Modifies the CI scripts (
ci/installdeps.shandci/libbuild.sh) to update how dependencies and build environments are set up on CentOS, specifically enabling the CodeReady Linux Builder (CRB) and EPEL repositories using moderndnfcommands. - Test Webserver Flexibility: Refactors the test helper script (
tests/libtest.sh) to introduce a common function for starting a web server (run_webserver). This new function can fall back to using a simple Python HTTP server if theOSTREE_HTTPDvariable (presumably pointing toostree-trivial-httpd) is not set, making tests more robust in environments without the dedicated server.
Changelog
Click here to see the changelog
- .dockerignore
- Added a new file to specify paths to ignore during Docker builds, including
Dockerfile,target/, andCargo.lock.
- Added a new file to specify paths to ignore during Docker builds, including
- apidoc/ostree-docs.xml
- Added an include for
xml/annotation-glossary.xml(line 44), likely to address documentation build warnings.
- Added an include for
- ci/installdeps.sh
- Added installation of
ccache(line 19). - Removed old CentOS-specific logic for installing EPEL and
python34(lines 30-33).
- Added installation of
- ci/libbuild.sh
- Updated the CentOS buildroot setup logic to use
dnf install dnf-utils,dnf config-manager --enable crb, anddnf install epel{,-next}-release(lines 43-45).
- Updated the CentOS buildroot setup logic to use
- tests/libtest.sh
- Added
set +xtorun_exit_cmds(line 46) andlibtest_cleanup_gpg(line 766) to quiet verbose output during test cleanup. - Introduced a new function
run_webserver(lines 288-306) to standardize starting a web server for tests. - Modified
run_webserverto use a Python webserver (webserver.py) if theOSTREE_HTTPDenvironment variable is not set (lines 293-299). - Refactored
setup_fake_remote_repo1(line 339),setup_fake_remote_repo2(line 382), andsetup_fake_remote_repo_httpd_only(line 546) to use the newrun_webserverfunction instead of directly invoking${OSTREE_HTTPD}.
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Builds fail, tests break,
CI scripts need a shake.
Fixes bundled here,
Making things more clear,
Less noise for goodness sake.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces several valuable fixes and updates to the build system and test infrastructure. The changes align well with the description, addressing API documentation warnings, adding a .dockerignore file, updating CentOS build dependencies, and improving the test webserver logic. The code is generally clear and follows established patterns within the project. Good work!
Summary of Findings
- CentOS Dependency Update: The removal of CentOS 7 specific dependency installation (
python34) inci/installdeps.shis a functional change in the CI environment, dropping explicit support for an older OS version in favor of newer methods inci/libbuild.sh. This aligns with the PR's goal but represents a change in the supported test matrix.
Merge Readiness
The pull request addresses build system and test infrastructure improvements. The changes are well-explained and appear correct for their intended purpose. There are no critical or high severity issues identified. The changes related to CentOS dependencies seem appropriate for updating the CI environment to newer versions. The pull request appears ready to be merged. I am unable to approve the pull request directly; please have other reviewers review and approve this code before merging.
f5d807c to
f52fd90
Compare
44b804d to
6889dac
Compare
Should have done this long ago, this greatly reduces the spam in the terminal logs from builds. Signed-off-by: Colin Walters <walters@verbum.org>
It's way too noisy. Signed-off-by: Colin Walters <walters@verbum.org>
This ensures we don't pick up things we shouldn't from the source. Signed-off-by: Colin Walters <walters@verbum.org>
- Do the modern way to enable the buildroot with crb and epel Signed-off-by: Colin Walters <walters@verbum.org>
This blends better with docker builds where we have a blanket ignore of `target` in .dockerignore. Right now we use git.mk to generate .gitignore and autotools drop stuff all over the place. Signed-off-by: Colin Walters <walters@verbum.org>
Otherwise there's a lot of test spam. Signed-off-by: Colin Walters <walters@verbum.org>
Noticed in drive by. Signed-off-by: Colin Walters <walters@verbum.org>
6889dac to
2b7dba7
Compare
|
/override ci/prow/fcos-e2e |
|
@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/fcos-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@cgwalters: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/test fcos-e2e |
apidoc: Quiet many warnings
Should have done this long ago, this greatly reduces the spam
in the terminal logs from builds.
dockerignore: Add
This ensures we don't pick up things we shouldn't from the source.
ci: Updates for centos builds
tests/libtest: Just use python as a webserver if no libsoup
We only have a very few tests that actually need what
we have in ostree-trivial-httpd that supports things like serving
random 500 errors etc.
If we don't have libsoup, then just use a python webserver.