[sailfishos/udisks2] Suggestion: Merge Patch-0013 and Patch-0005 into Patch-0004

This issue was filed in August 2020, plus some details added in the following months: [Suggestion] Merge Patch-0013 and Patch-0005 into Patch-0004 (#3) · Issues · mer-core / udisks2 · GitLab

In June 2021, I took the chance (when Jolla started to address the open Merge Requests at https://git.sailfishos.org/ in order to close their Gitlab instance) to bring this to @tomin’s attention (who handled the MRs left open there) and received as reply:

I wrote that down to our task about upgrading udisks2. Hopefully it will be solved once that is worked on.

and

Yes, I had already added a mention of patch 0013 to the internal comment. I also agree that we should not make these accumulated patches unless there is a good reason to do so. I don’t know why that’s been done here but I know in some other places we have just adjusted the existing patches.

Because mer-core · GitLab has been put into archived state a few days after this little, initial conversation about this issue, and the issue tracker at GitHub - sailfishos/udisks2 is switched off, this issue has to be re-filed here to be able to track and discuss it any longer.

I still would appreciate any feedback on the content of this issue (in contrast to formal feedback, as already provided).

P.S.: Cut & Paste of the core content of this issue (for details, see the original issue report).

[Suggestion] Merge Patch-0013 and Patch-0005 into Patch-0004

Currently rpmbuild is commanded to patch a Patch by Jolla with another Patch by Jolla (edit: “twice”, in the meantime), which is a hard to capture and trace when looking at it spontaneously, hindering easy understanding and hence occasional maintenance.

In this specific case Patch-0005 is rather rectifying, enhancing and superseding Patch-0004 than falling into a different category of functionality, thus IMO they are better merged in this source code repositories than when building the RPM.

If I missed a relevant aspect (e.g., that one of these patches does not originate from Jolla), please denote that and close this issue.

Update in June 2021

Oh, since I filed this issue, Patch-0013 was added, which solely patches a file, which was created per Patch-0004.
Now I understand, that this was the reason, why you had to rebase MR !29 (merged)!

Thus the note for the Jolla internal task IMO should read now:

Merge Patch-0013 and Patch-0005 into Patch-0004, because Patches 13 and 5 are simply additional changes to Patch 4.
Specifically Patch 13 is patching a file, which is actually created by Patch 4: mount-sd@.service (i.e., this file is not part of the original udisks2).

  • IMO this is really hard to notice and fully comprehend, hence error-prone!
  • Plus it complicates trivial merges (as we have seen with MR !29 (merged)), because they have to goto multiple files simultaneously, and may require rebasing, if another such Patch patching a Patch or a MR to a Patch already patching a Patch is employed.
  • Also note that these Patches are even (partially) patching the same line, which makes diffs and merges really ugly.
2 Likes