search

sailfishos-patches / patchmanager

Patchmanager for SailfishOS
https://openrepos.net/content/patchmanager/patchmanager
Other
21 stars 22 forks source link

[Bug] 64-bit-mangled patches sometimes fail to apply #426

Closed nephros closed 1 year ago

nephros commented 1 year ago

SailfishOS VERSION: 4.4.0.x and 4.5.0.x
HARDWARE: Sony Xperia 10 III
PATCHMANAGER VERSION: 3.2.8

BUG DESCRIPTION

Some patches, after having been 32-to-64-bit mangled, fail to apply/activate even though they appear to be correct.

So far this has been reproduced on 64-bit systems only.

STEPS TO REPRODUCE

Example patches which show this:

ADDITIONAL INFORMATION

See also discussion in this thread: https://forum.sailfishos.org/t/patches-by-ichthyosaurus/15387

nephros commented 1 year ago

Using old_remorse_timer as an example:

  1. enable 64/32 bit mangling feature
  2. install the patch
  3. using Patchmanager GUI, try to activate the patch once
  4. this results in a mangled patch at /tmp/patchmanager3/patches/old_remorse_timer/unified_diff_64bit.patch
  5. copy /tmp/patchmanager, /tmp/patchmanager3/patches/old_remorse_timer/unified_diff_64bit.patch and /usr/share/patchmanager/patches/old_remorse_timer/unified_diff.patch to a testing directory
  6. cd <testdir> 
nemo@PGXperiiia10:~/tmp/patchmanager $ patch --dry-run -p1 -d ./ < 64-patch/unified_diff_64bit.patch
checking file usr/lib64/qt5/qml/Sailfish/Silica/private/RemorseBase.qml
Hunk #1 FAILED at 88.
1 out of 1 hunk FAILED
can't find file to patch at input line 61
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -ur a/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml b/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml
|--- a/usr/lib64/qt5/qml/Sailfish/Silica/RemorsePopup.qml       2021-09-22 16:04:38.000000000 +0200
|+++ b/usr/lib64/qt5/qml/Sailfish/Silica/RemorsePopup.qml       2021-09-22 17:17:32.000000000 +0200
--------------------------
File to patch:

nemo@PGXperiiia10:~/tmp/patchmanager $ ls usr/lib64/qt5/qml/Sailfish/Silica/
Background/          private/             Button.qml           Page.qml             SecondaryButton.qml

... so the file to be patched does indeed not exist in what PM calls CacheDir.

nephros commented 1 year ago

When trying to apply/activate form GUI, the journal logs this error:

Apr 15 11:21:48 PGXperiiia10 patchmanager[21270]: bool PatchManagerObject::tryToUnlinkFakeParent(const QString&) Failed when trying to change (cd) from directory "/usr/lib" to "qt5"
Apr 15 11:21:48 PGXperiiia10 patchmanager[21270]: void PatchManagerObject::doPatch(const QVariantMap&, const QDBusMessage&, bool) Sending reply.

... which is from here: https://github.com/sailfishos-patches/patchmanager/blob/master/src/bin/patchmanager-daemon/patchmanagerobject.cpp#L2540

Suspicion: something in doPrepareCache still reads the unmangled paths instead of the mangled ones, and fails to prepare the environment for the patching process/script.

nephros commented 1 year ago

Debug log:


estart[12193]: [D] unknown:0 - void PatchObject::apply(QJSValue)
estart[12193]: [D] unknown:0 - void PatchObject::setBusy(bool) true
estart[12193]: [D] expression for onBusyChanged:410 - onBusyChanged: old_remorse_timer true
estart[12193]: [D] unknown:0 - QDBusPendingCallWatcher* PatchManager::applyPatch(const QString&)
patchmanager[28749]: QVariantMap PatchManagerObject::applyPatch(const QString&) "old_remorse_timer"
patchmanager[28749]: void PatchManagerObject::doPatch(const QVariantMap&, const QDBusMessage&, bool) QMap(("name", QVariant(QString, "old_remorse_timer"))("user_request", QVariant(bool, true))) true
patchmanager[28749]: bool PatchManagerObject::doPatch(const QString&, bool, QString*) "old_remorse_timer" true
patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) "old_remorse_timer" true
patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) Processing patch file "/usr/lib/qt5/qml/Sailfish/Silica/private/RemorseBase.qml"
patchmanager[28749]: bool PatchManagerObject::tryToLinkFakeParent(const QString&) "/usr/lib/qt5/qml/Sailfish/Silica/private"
patchmanager[28749]: bool PatchManagerObject::tryToLinkFakeParent(const QString&) Symlinking "/usr/lib/qt5" to "/tmp/patchmanager/usr/lib/qt5" true
patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) Processing patch file "/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml"
patchmanager[28749]: bool PatchManagerObject::checkIsFakeLinked(const QString&) "/usr/lib/qt5/qml/Sailfish/Silica"
patchmanager[28749]: bool PatchManagerObject::checkIsFakeLinked(const QString&) "/usr/lib/qt5/qml/Sailfish/Silica" already has a faking symlink "/usr/lib/qt5"
patchmanager[28749]: QVariant PatchManagerObject::getSettings(const QString&, const QVariant&) const "bitnessMangle" QVariant(bool, false) QVariant(QString, "true")
patchmanager[28749]: bool PatchManagerObject::doPatch(const QString&, bool, QString*) Starting "/usr/libexec/pm_apply" ("old_remorse_timer")
patchmanager[28749]: bool PatchManagerObject::doPatch(const QString&, bool, QString*) Successfully started process (bool): false
patchmanager[28749]: bool PatchManagerObject::doPatch(const QString&, bool, QString*)

[[ skip patch log output ]]

patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) "old_remorse_timer" false
patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) Processing patch file "/usr/lib/qt5/qml/Sailfish/Silica/private/RemorseBase.qml"
patchmanager[28749]: bool PatchManagerObject::tryToUnlinkFakeParent(const QString&) "/usr/lib/qt5/qml/Sailfish/Silica/private"
patchmanager[28749]: bool PatchManagerObject::tryToUnlinkFakeParent(const QString&) Removing "/usr/lib/qt5" true
patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) Processing patch file "/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml"
patchmanager[28749]: bool PatchManagerObject::tryToUnlinkFakeParent(const QString&) "/usr/lib/qt5/qml/Sailfish/Silica"
patchmanager[28749]: bool PatchManagerObject::tryToUnlinkFakeParent(const QString&) Failed when trying to change (cd) from directory "/usr/lib" to "qt5"
patchmanager[28749]: void PatchManagerObject::doPatch(const QVariantMap&, const QDBusMessage&, bool) OK (bool): false
patchmanager[28749]: void PatchManagerObject::notify(const QString&, PatchManagerObject::NotifyAction) "Sailfish 2.0 style Remorse Timer" PatchManagerObject::NotifyAction(NotifyActionFailedApply)
patchmanager[28749]: void PatchManagerObject::notify(const QString&, PatchManagerObject::NotifyAction) "Failed to activate Patch" "Activating Patch Sailfish 2.0 style Remorse Timer failed!"
lipstick[20033]: [D] displayNotification:559 - Warning: Notification has both preview summary and preview body but no actions. Remove the preview body or add an action: Patchmanager  Failed to activate Patch Activating Patch Sailfish 2.0 style Remorse Timer failed!
patchmanager[28749]: void PatchManagerObject::notify(const QString&, PatchManagerObject::NotifyAction) 2045
patchmanager[28749]: void PatchManagerObject::doPatch(const QVariantMap&, const QDBusMessage&, bool) Sending reply.
estart[12193]: [D] unknown:0 - void PatchObject::setBusy(bool) false
estart[12193]: [D] expression for onBusyChanged:410 - onBusyChanged: old_remorse_timer false
nephros commented 1 year ago

For sake of completeness, this is the relevant pm_apply output:

                                                  ----------------------------------
                                                  pm_apply 2023-04-15T11:37:54+02:00
                                                  ----------------------------------

                                                  old_remorse_timer

                                                  Using patch file: /usr/share/patchmanager/patches/old_remorse_timer/unified_diff.patch

                                                  ----------------------------------
                                                  Checking paths for 64-bit --> 32-bit conversion
                                                  ----------------------------------

                                                  Mangle candidates: /usr/lib/qt5/qml /usr/lib/jolla-mediaplayer /usr/lib/maliit/plugins

                                                  Converting library path reference /usr/lib/qt5/qml 2 times

                                                  OK, converted 2 library path references and created: /tmp/patchmanager3/patches/old_remorse_timer/unified_diff_64bit.patch

                                                  ----------------------------------
                                                  Test if already applied patch
                                                  ----------------------------------

                                                  checking file usr/lib64/qt5/qml/Sailfish/Silica/private/RemorseBase.qml
                                                  Hunk #1 FAILED at 88.
                                                  1 out of 1 hunk FAILED
                                                  can't find file to patch at input line 61
                                                  Perhaps you used the wrong -p or --strip option?
                                                  The text leading up to this was:
                                                  --------------------------
                                                  |diff -ur a/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml b/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml
                                                  |--- a/usr/lib64/qt5/qml/Sailfish/Silica/RemorsePopup.qml     2021-09-22 16:04:38.000000000 +0200
                                                  |+++ b/usr/lib64/qt5/qml/Sailfish/Silica/RemorsePopup.qml     2021-09-22 17:17:32.000000000 +0200
                                                  --------------------------
                                                  File to patch:
                                                  Skip this patch? [y]
                                                  Skipping patch.
                                                  1 out of 1 hunk ignored
nephros commented 1 year ago

patchmanager[28749]: void PatchManagerObject::doPrepareCache(const QString&, bool) Processing patch file "/usr/lib/qt5/qml/Sailfish/Silica/RemorsePopup.qml"

So it's obvious that doPrepareCache operates on unmangled file names.

The list is iterated here:

https://github.com/sailfishos-patches/patchmanager/blob/master/src/bin/patchmanager-daemon/patchmanagerobject.cpp#L553:

for (const QString &fileName : m_patchFiles.value(patchName)) {

and m_patchFiles is populated rather early in the daemons life cycle:

https://github.com/sailfishos-patches/patchmanager/blob/master/src/bin/patchmanager-daemon/patchmanagerobject.cpp#L1672-L1676

( General observation: Looking at all of these I'm very confused by the usage of m_patchFiles vs m_fileToPatch - they seem to perform similar functions? )

nephros commented 1 year ago

Full debug log: pm_debug_mine.log

nephros commented 1 year ago

Added some debugging info in https://github.com/nephros/patchmanager/tree/fix-426.

First examination points to "git-style" diffs.

These get mangled correctly:

--- /usr/lib/foo
+++ /usr/lib/foo

... these will not:

--- a/usr/lib/foo
+++ b/usr/lib/foo

... that is to say, the script pm_apply does these correctly, and so will produce a correct patch.
However, the path mangle functions in the daemon do NOT deal with the a/ b/ prefixes, and therefore doPrepareCache fails to prepare the correct files in the cache dir.

nephros commented 1 year ago

https://github.com/sailfishos-patches/patchmanager/commit/e2c95252e6faed62e5c82e5c9a063975869ce651 https://github.com/nephros/patchmanager/commit/de9cc74c15bc64d21c8d356b61a9c486e9b38d9d seems to fix that.

@b100dian if you have time, could you give the section https://github.com/sailfishos-patches/patchmanager/blob/master/src/bin/patchmanager-daemon/patchmanagerobject.cpp#L1651-L1682 a look over, and see how to properly fix this? And perhaps put some comments in there what/why things are happening.

I'm not sure why the path walking at https://github.com/sailfishos-patches/patchmanager/blob/master/src/bin/patchmanager-daemon/patchmanagerobject.cpp#L1658-L1667 is there at all.

Looking at my personal collection of installed patches, we have these styles of patch beginnings we should be able to deal with:

--- /usr/...
+++ /usr/...
--- a/usr/...
+++ b/usr/...
--- orig/usr/...
+++ patch/usr/...
--- jolla/usr/...
+++ developername/usr/...

And special case variants of the above for new files:

--- /dev/null
+++ {user,b,patch}/usr/...
nephros commented 1 year ago

As a side note, I have also noticed that conflict detection sometimes works, sometimes it doesn't and I'm quite sure it's for the same reason - and it appears to be fixed by the same fix.