[Rpm-maint] (no subject)

[Panu Matilainen]

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 -