-
Notifications
You must be signed in to change notification settings - Fork 80
frontend, backend, rpmbuild: extract Exclu*Arch/BuildArch for all tar… #4135
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
base: main
Are you sure you want to change the base?
frontend, backend, rpmbuild: extract Exclu*Arch/BuildArch for all tar… #4135
Conversation
| from norpm.overrides import override_macro_registry | ||
| from norpm.exceptions import NorpmError | ||
|
|
||
| DEFAULT_OVERRIDE_URL = "https://raw.githubusercontent.com/praiskup/norpm-macro-overrides/refs/heads/main/distro-arch-specific.json" |
Check warning
Code scanning / vcs-diff-lint
Line too long (131/120) Warning
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.
Code Review
This pull request introduces significant improvements by enabling the extraction of architecture-specific tags (like ExclusiveArch, ExcludeArch, and BuildArch) from spec files for all target distributions. This is achieved through new functionality in rpmbuild/copr_rpmbuild/extract_specfile_tags.py and integration into rpmbuild/copr_rpmbuild/automation/srpm_results.py. The API versions between frontend and backend have been updated to support these changes, and the frontend now provides more detailed distribution information in the SRPM build record. Overall, this enhances the build process's ability to handle architecture-specific requirements more accurately.
| @property | ||
| def target_distros(self): | ||
| """ | ||
| Get the list of distributions we build this package against. | ||
| """ | ||
| # Handle distributions_in_build first to optimize a bit; the | ||
| # distributions_in_build is a subset of distributions_in_project | ||
| # and if available - we can avoid some macro expansion cycles. | ||
| for field_name in ["distributions_in_build", | ||
| "distributions_in_project"]: | ||
| if self.task[field_name]: | ||
| self.log.info("Using %s for this build.", field_name) | ||
| return self.task[field_name] | ||
| raise RuntimeError("Running against too old copr-frontend") |
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.
The target_distros property introduces a hard dependency on the frontend providing distributions_in_build or distributions_in_project fields. If an older frontend is used that does not supply this data, it will result in a RuntimeError and cause the build to fail. Consider implementing a more graceful fallback mechanism or a clearer version check at an earlier stage to prevent abrupt build failures for incompatible frontend versions.
| print("WARNING: Building for all architectures since " | ||
| f"the spec file parser: failed: {err}") |
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.
Using print for warnings or errors is not ideal for a production application, as it bypasses the standard logging infrastructure. Please replace this with a call to the logging module for consistent log handling and better integration with monitoring systems.
| print("WARNING: Building for all architectures since " | |
| f"the spec file parser: failed: {err}") | |
| logging.warning("WARNING: Building for all architectures since " | |
| f"the spec file parser: failed: {err}") |
Pull Request validationFailed🔴 Failed or pending checks:
🔴 Review - Missing review from a member (2 required) |
55faf27 to
e5cc54e
Compare
e5cc54e to
0fe4205
Compare
Move the tested image to Fedora 43. Fixes: fedora-copr#4103
6697cfb to
c8deb74
Compare
|
/packit test |
c8deb74 to
0fc414c
Compare
|
/packit test |
The `main` variant (Rawhide) uses a locally created `make dist` tarball, to work-around the current FTBFS against the newest GCC: https://src.fedoraproject.org/rpms/hello/c/af27df4609302190668b8db4cb85380aab4f7795?branch=rawhide Waiting for a new GNU Hello release: https://savannah.gnu.org/bugs/?67961
0fc414c to
c68003d
Compare
|
/packit test |
…gets
Relates: #1315
Relates: #4088