-
Notifications
You must be signed in to change notification settings - Fork 145
specfile: exclude doc directories from package #1352
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
Conversation
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 aims to fix lint failures caused by empty documentation directories in the RPM package. The approach involves explicitly listing only files (not directories) for packaging from the documentation directory, and ensuring a specific problematic directory contains a .keepdir file. The changes look good and effectively address the described issue.
Summary of Findings
- RPM Spec: Path formatting in
findcommand: Incontrib/packaging/bootc.specat line 126, thefindcommand uses-printf '/%{_docdir}/%%P\n'. Since%{_docdir}(e.g.,/usr/share/doc/bootc) already starts with a/, this results in paths inbootcdoclist.txtstarting with//(e.g.,//usr/share/doc/bootc/somefile.txt). While most systems normalize this, using-printf '%{_docdir}/%%P\n'would produce cleaner paths (e.g.,/usr/share/doc/bootc/somefile.txt). This is a low-severity stylistic improvement and a review comment was not added due to the project's review settings for comment severity.
Merge Readiness
The changes are well-implemented and address the problem described in the pull request. The code appears ready for merging. As a reviewer, I am not authorized to approve the pull request, so please ensure it undergoes any further required review and approval processes.
|
Before the change After the change |
f9851cd to
bc78a1c
Compare
When we don't install the documentation, rpm still install empty directories, leading to lint failures.
|
For the record I think anyone affected by this with earlier bootc versions can work around this by just doing |
|
I confirm (that's what we do) |
Worth noting though for anyone using the The hack I went with is coreos/fedora-coreos-config#3589. |
When we don't install the documentation, rpm-ostree (rpm works fine) still install empty directories, leading to lint failures.
Fixes #1351