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 -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to