Patches with targets in /usr/lib[,64] do not apply on 32/64bit systems

[nephros]

Systems with 64bit userland use /usr/lib64 instead of /usr/libas a library path. Patches which target files in either of those locations will therefore not apply on the other platforms even though the patched files themselves might be the same.

This affects e.g. all "silica" patches, as the Sailfish QML files live under $LIBDIR/qt5/qml/Sailfish/*, and basically anything QML that isn't applications in /usr/share.

This means patch developers must currently ship and deploy two versions of each patch, and create two projects on web catalog, or use RPM packaging which installs the appropriate patch version.

Question:

Shall patchmanager implement something that helps with this problem?

Solutions that come to mind:

• use something like patch -p3 --dry-run -d /usr/lib{,64}/ when a /usr/lib string is detected in the patch file
  • probably the simplest and fastest way, but is it reliable?
  • breaks when files outside of /usr/lib are patched from the same diff file
• accept a second diff file in the patch hierarchy (e.g. unified_diff_64.patch) so devs can ship either or both
  • this would require modifcation of both patchmanager and the Web Catalog validator for submitting tarfiles
• modifying Web Catalog to mangle any uploaded diffs to create the missing variant, and supply the correct tarball when PM downloads, so PM always receives a valid patch depending on bittiness
  • wonky, as the catalog can't verify that a file will exist on the system
• modify /usr/lib* paths in existing unified_diff files when installing a patch from Web Catalog
• modify /usr/lib* paths in existing unified_diff files when applying a patch at runtime
• faking the libpath (force /usr/lib also on 64bit) somehow when applying a patch
  • the last three would allow old 32bit patches to be used without the authors needing to update them
• Other ideas?

[nephros]

Whatever solution, it would probably be useful to add a section in the patch.json schema to allow authors to specify whether their patch is for 32 or 64 bit systems, and whether it is expected to work on either.

something like

"bittiness": "32|64|96"

meaning 32-bit only, 64-bit only, or both, respectively. A missing entry should mean "32-bit only". Users can opt to ignore it, just like they can with compatibility.

[CODeRUS]

patch ... < sed "s#b/usr/lib#b/usr/lib64#g" instead

[b100dian]

There's also the reverse fact - patches written for 64 bits only would fail on 32 bits.

[CODeRUS]

add vice versa conversion for 32 bit systems :)

[b100dian]

Definitely possible to add the feature to web catalog, I've been playing with this https://github.com/CODeRUS/django-test/compare/master...b100dian:bitness (work in progress, needs to link the bitness choice to the unified_diff64.patch presence)

[CODeRUS]

do you have any examples of difference in files of lib and lib64?

[b100dian]

I think it boils down to patches going to /usr/lib/qt5/qml/ - example https://coderus.openrepos.net/pm2/project/sailfishos-default-allowed-orientations-patch So the sed fix would work with that, but if you also want to patch other /usr/lib/paths, say, systemd then the sed command won't work (there's not /usr/lib64/systemd.

[nephros]

do you have any examples of difference in files of lib and lib64?

You mean examples of e.g. Sailfish Silica files that are different between 32 and 64 bit versions? I don't have an example of any, but I think it's possible there will be differences. It's probably not safe to assume they are always identical, although they will be in most cases.

But trying to apply and failing sometimes (i.e. files are different) is better than always failing (files are not different, but path is).

[nephros]

I think it boils down to patches going to /usr/lib/qt5/qml/ - example https://coderus.openrepos.net/pm2/project/sailfishos-default-allowed-orientations-patch So the sed fix would work with that, but if you also want to patch other /usr/lib/paths, say, systemd then the sed command won't work (there's not /usr/lib64/systemd.

Fix paths case-by-case - we now know /usr/lib64/qt5/qml is one candidate, and probably the big one, so s@/usr/lib/qt5/qml@/usr/lib64/qt5/qml@g, and add other paths as we (or patch authors) discover them.
We might even try to detect if the new path exists before doing the replacement.

[b100dian]

Fix paths case-by-case - we now know /usr/lib64/qt5/qml is one candidate,

I think this is a better fix - not an /usr/lib/ overall replacement, a specific one, and not requiring web catalog changes (though I am more confident we can do that now than before). Also, this is not a folder that will be present in both lib and lib64. (remember we should patch in the opposite direction too:)

[nephros]

Agreed.

The question is where and when to do the mangling. pm_apply.sh would be the obvious one, and easy to hack. The other possibility is while/after installing a WC patch, but that doesn't work for RPM patches.

So if it is pm_apply.sh, do we do it transient, so every time mangle, apply, and delete the result after, or install a new fixed patch into the original patch dir, or perhaps a 'bitness_override' dir we then search first when applying?

(...sorry for the run-on sentence, being a German speaker does that to your mind sometimes...)

[b100dian]

We can use a name like unified_diff-corrected.patch that will be generated every time (so transient-ish) before calling pm_apply, and $2 to signal the change from PATCH_NAME="unified_diff.patch" to the corrected one.

Every time -> because there might be other reasons to 'correct' patches down the road, and we wouldn't want to cache a previous version.

"corrected"? Maybe there's a better term. unified-diff-patched.patch :P or "edited"

run-on sentence

I have that in my 'source' language too, can't complain

[Olf0]

"corrected"? Maybe there's a better term. unified-diff-patched.patch :P or "edited"

"edited" sounds better and more neutral IMO: unified_diff-edited.patch

[Olf0]

Just for reference: This has been enhanced as discussed and documented in issue #220 per PRs #221 and #225, plus subsequent PRs #237, (#238) / #239, #241 and #242.