[Rpm-maint] (no subject)
pmatilai at redhat.com
Fri Jun 30 13:23:18 UTC 2017
On 06/29/2017 02:26 PM, Mark Wielaard wrote:
> Hi Panu,
> On Mon, 2017-06-26 at 14:01 +0300, Panu Matilainen wrote:
>> Please do the refactor to helper function(s) in a separate patch from
>> the rest of the changes, it'll be easier to review and bisect too if it
>> ever comes to that.
> OK, the following patchset first adds the helper functions and then uses
> them for the build-id file list generation.
>> mkattr() with non-NULL fn argument ceases to be meaningful here, and
>> since it's not even used for anything, whether the mode should be 644 or
>> 755 nobody knows, certainly not that function. Better just drop
>> non-defattr case from it entirely.
> Right. But in that case we can just remove the fn argument completely.
> I added it as a separate patch.
> [PATCH 1/3] Extract package file list processing in separate functions.
> [PATCH 2/3] Use a file list to add build-id files to pkgList.
> [PATCH 3/3] Change mkattr to always create a %defattr with explicitly
Right, this is more or less what I had in mind. Didn't have a chance to
do a detailed review, I'll leave that up to Florian (I'll be away all of
July) but a couple of comments, just FWIW:
This series makes the actual change in 2/3 much more obvious, thanks for
taking the trouble of splitting it up.
It's a bit unfortunate that so much of 1/3 is s/a.foo/a->foo/ changes
which make it look more complicated than it is. I sometimes do yet
another preliminary step to isolate those into a commit of their own.
I'm not sure it's necessary to specifically check against RPMFILE_DOC
etc in generated content - we're not generating those now, but who knows
in the future? In fact, since -debuginfo packages do not depend on the
main packages, they probably should include copies of the licenses of
that package (that is of course outside the scope of this patch series,
just something I realized)
- Panu -
More information about the Rpm-maint