[Rpm-maint] [rpm-software-management/rpm] Sort files before passing to file attribute dependency generators (PR #3786)

[Michal Domonkos]

dmnks left a comment (rpm-software-management/rpm#3786)

Sorry for the delay here, I wanted to take some time to dig deeper into this part of the codebase (wasn't familiar with it previously at all), and specifically look at whether there wasn't some hidden assumption (along the call stack) about the order of indexes in the `fnx` array that the explicit sorting added in this patch could potentially affect/break.

Turns out it's indeed safe. The only assumption is that the indices in `fnx` correspond to those in `paths` that we pass to the generator executable in `genDeps()`. But that's assured since `paths` is constructed *from* `fnx` (i.e. in the same order) and that happens *after* the sort.

The input file list (that we pass to `rpmfcClassify()` as `argv`) is already sorted explicitly (like you mentioned in the commit message), in both `rpmbuild` and `rpmdeps` (the two places where RPM calls that function). Of course, an API user would have to sort the `argv` themselves in order to ensure reproducibility, and we could mention that in the docstring perhaps, but I'm not sure it's *that* important (for those users).

> Looks to me like the ordered section only applies lower down and does not affect `rpmfcAttributes()`? Regardless, it was immediately reverted due to effectively removing parallelisation.

Oh. Indeed, the ordered section is below the classification routine. I didn't look close enough at that patch, apparently :sweat_smile: 

> Either way, sorting after parallel construction seems like the right approach to me.

Yep, I agree, and will merge now. I've also written a test but I'll submit it as a separate PR afterwards.

Thanks for the patch!

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3786#issuecomment-2991415815
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/3786/c2991415815 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250620/fa8f3466/attachment.htm>