Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nephros opened this issue Apr 15, 2023 · 9 comments · Fixed by #428
Closed

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

nephros opened this issue Apr 15, 2023 · 9 comments · Fixed by #428
Labels
bug something that needs fixing

Comments

@nephros
Copy link
Contributor

nephros commented Apr 15, 2023

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 nephros added the bug something that needs fixing label Apr 15, 2023
@nephros
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

Full debug log:
pm_debug_mine.log

@nephros
Copy link
Contributor Author

nephros commented Apr 15, 2023

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
Copy link
Contributor Author

nephros commented Apr 15, 2023

e2c9525 nephros@de9cc74 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
Copy link
Contributor Author

nephros commented Apr 15, 2023

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.

nephros pushed a commit to nephros/patchmanager that referenced this issue Apr 15, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Apr 15, 2023
@nephros nephros linked a pull request Apr 15, 2023 that will close this issue
nephros added a commit that referenced this issue Apr 19, 2023
Fixes an issue where some patches, after having been converted from or to 64-bit fail to apply/activate even though their format is correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant