Add aarch64 support

[b100dian]

This adds the correct lib64 location for the preloaded so.
Also, because a previous version of this might have been installed from b100dian OBS or chum, which used the 64-bit binary in /lib/, that needs to be cleared from ld.so.preload.

Old description

Without bash installed, the busybox shell was erroring out with "Substitution failure". This changes the script to no longer need `${PIPESTATUS[0]}` which is the cause of the above failure.

Also, when building for aarch64, there is some mismatch between where a lib is packaged and where is looked up. The 'fix' is to currently just hardcode "lib" - that's why I say its' the naive fix, only to unblock building/running.

[Olf0]

@b100dian, IMO the changes you labelled with "busybox support" are bug fixes, but are not related to supporting busybox!

1. false | true; echo $? results in 0 (i.e., "O.K") on both, ash and bash.
2. set -o pipefail; false | true; echo $? results in 1 (i.e., "fail") on both, ash and bash.

• Consequently, your comments "# for busybox" (twice: in each of the two files patched) is not correct really necessary.
• Also the addition of ; test ${PIPESTATUS[0]} -eq 0 is superfluous, AFAICS.

But you detected at least two places in which $? (after a pipeline applying or testing a patch) is never evaluated, and more places in which only one of the cases (failure or success) is checked for!

IMO these shall be rectified, along setting set -o pipefail (what you already do).

[b100dian]

Thanks @Olf0 for looking into this - the reference to busybox is actually because ${PIPESTATUS[0]} is erroring out on busybox with "Substitution failed". Does this make sense? Let me adda a description to the PR:)

[Olf0]

@b100dian, what you labelled as "naive aarch64 support" is truly "naive", but might be an appropriate "quick & dirty hack" to make Patchmanager working on aarch64.
But because

1. hardcoding library paths, for which RPM macros exist is evil (but I understand that there is no other "quick & dirty" way to let your patched Patchmanager still work on ARMv7-A),
2. this "naive aarch64 support" also comes with a set of fixes, which are completely unrelated (and mislabelled "busybox support"),

IMO you shall separate these!

I do not think that this "naive aarch64 support" is appropriate for the "master" branch, in contrast to the fixes labelled "busybox support", when they are enhanced and complete.
Thus these must be separated structurally, in order to be separately applicable.

[b100dian]

Agreed - this was just a branch based on the sfos4-fix branch, let me cherry pick the non-aarch64 related commits in a PR against master

[Olf0]

Thanks @Olf0 for looking into this - the reference to busybox is actually because ${PIPESTATUS[0]} is erroring out on busybox with "Substitution failed". Does this make sense? Let me adda a description to the PR:)

EDIT: Sorry it is late and I have a slight headache: You are correctly deleting ; test ${PIPESTATUS[0]} -eq 0 / substituting it by set -o pipefail.
And that would have been the better way to handle it in the first place (i.e., years ago).

Oh, I did not try it, because it is superfluous on both, ash ("busybox") and bash, and has not been there before (AFAICS).

But thinking about it, that does make sense, because ${foo[0]} is a bash-only syntax for accessing the first element (i.e., at index 0) of a bash-array. Most other shells do not implement arrays and no other has this syntax: Hence the error.

P.S.: Mind that all this code shall still run under bash (because that is what SailfishOS releases before 3.2.0 use), the task is only to make it busybox's ash compatible.

P.P.S.: By reading the error reports WRT Patchmanager 3 on recent SailfishOS releases, I suspect there are more "Bashisms" hidden somewhere in Patchmanager's shell code.

But because I am fighting a nasty breakage on recent SailfishOS releases of a tool, which I wrote and maintain, the most I can offer is to review every now and then. Plus I am only good at shell code!

[Olf0]

Sorry, I misunderstood (that you are correctly deleting ; test ${PIPESTATUS[0]} -eq 0 / substituting it by set -o pipefail) and hence corrected myself, above.

Mind, that you might also want to integrate @CODeRUS existing fixes for SailfishOS 4.x (i.e., Merge Request #48, although I partially do not like them, because they include a change, which breaks Patchmanager for SailfishOS < 4).

[CODeRUS]

because they include a change, which breaks Patchmanager for SailfishOS < 4).

which one please?

[Olf0]

Agreed - this was just a branch based on the sfos4-fix branch, let me cherry pick the non-aarch64 related commits in a PR against master

Well, thinking about it, this was nitpicking by me: IIRC @CODeRUS clearly stated, that he will not do anything for Patchmanager, any more, hence this is rather about maintaining your own fork in a suitable manner; consequently what is "suitable" is up to you.

[Olf0]

because they include a change, which breaks Patchmanager for SailfishOS < 4).

which one please?

Dear @CODeRUS,
a. it is really nice to hear from you. Also because this is a small indication that you are well.
b. But I am not, as written before, tonight: Sorry it is late and I have a slight headache.
c. I will try to look it up again, check and write a comment at MR #48, tomorrow evening.
d. Why the hell do such conversations always "explode", when one is expecting it the least?
e. I must go to bed, now.

[Olf0]

Agreed - this was just a branch based on the sfos4-fix branch, let me cherry pick the non-aarch64 related commits in a PR against master

Oh, I see! Another nonsense comment by me, tonight: You already use the sfos4-fix branch as a base for your patches.
Really not my day, today. :-(

[Olf0]

because they include a change, which breaks Patchmanager for SailfishOS < 4).

which one please?

Answered here.

[Olf0]

This is looking fine.

Side note: BTW,
grep libpreloadpatchmanager /etc/ld.so.preload > /dev/null is better written as
grep -qF libpreloadpatchmanager /etc/ld.so.preload, but that is unrelated to this specific change.

P.S: I wonder why I am able to "approve" and to "request changes" for this MR. Have I become more than a simple Patchmanager contributor here? Will check, but for now I will leave this as a "comment".

[b100dian]

Yeah, but it doesnt work yet https://build.sailfishos.org/package/live_build_log/home:b100dian:patchmanager/patchmanager/sailfish_latest_aarch64/aarch64

[Olf0]

Well (just stating the obvious), they really are not there, because
[ 232s] install -m 755 -p libpreloadpatchmanager.so /home/abuild/rpmbuild/BUILDROOT/patchmanager-3.0.65+sfos4.fix.busybox+sfos4.fix.20210913211513.1.g68347dc-1.6.1.jolla.aarch64/usr/lib/libpreloadpatchmanager.so
[ 232s] : --strip-unneeded /home/abuild/rpmbuild/BUILDROOT/patchmanager-3.0.65+sfos4.fix.busybox+sfos4.fix.20210913211513.1.g68347dc-1.6.1.jolla.aarch64/usr/lib/libpreloadpatchmanager.so
and
[ 233s] extracting debug info from /home/abuild/rpmbuild/BUILDROOT/patchmanager-3.0.65+sfos4.fix.busybox+sfos4.fix.20210913211513.1.g68347dc-1.6.1.jolla.aarch64/usr/lib/libpreloadpatchmanager.so
=> The [install] section uses /usr/lib/as library path.
(I am too tired to ponder why; will try to think about it tomorrow.)

Consequently
[ 234s] error: File not found: /home/abuild/rpmbuild/BUILDROOT/patchmanager-3.0.65+sfos4.fix.busybox+sfos4.fix.20210913211513.1.g68347dc-1.6.1.jolla.aarch64/usr/lib64/libpreloadpatchmanager.so
during files check (triggered by the %{_libdir}/libpreload%{name}.so line in the %files section) and again in the subsequent RPMbuild phase.

[Olf0]

On first sight, the %qmake5_install seems to be the culprit.
I will have to look up, what it really expands to, but this provides a hint: https://github.com/sailfishos/sdk-setup/blob/master/sdk-setup/src/mb2#L4984
Well, you can do that too, by rpm --showrc | fgrep qmake5_install
Usually such macros call a shell script.

[Olf0]

But the build phase (defined in section %build) also puts libpreloadpatchmanager.so into /usr/lib/, i.e.:

%qtc_qmake5 "PROJECT_PACKAGE_VERSION=%{version}"
%qtc_make %{?_smp_mflags}

So maybe you might have to adapt the "makefiles" (but there are not any: this is where I lack the understanding for such a "modern" build process).
Or maybe one of the above macros accepts parameters with which one can control the output directory for libpreloadpatchmanager.so (which is compiled by these statements, I assume).

[nephros]

But the build phase (defined in section %build) also puts libpreloadpatchmanager.so into /usr/lib/, i.e.:

That's because src/preload/preload.pro tells it to install it there:

TEMPLATE = lib
QT =
CONFIG += plugin
CONFIG += link_pkgconfig
PKGCONFIG += libshadowutils
INCLUDEPATH += /usr/include/libshadowutils
QMAKE_CFLAGS += -std=c11

LIBS = -ldl

SOURCES += \
    src/preloadpatchmanager.c

TARGET = preloadpatchmanager
target.path = /usr/lib

INSTALLS = target

Q: Is the library actually supposed to end up in lib64? Some things should go to /usr/lib even in 64bit systems.

(Note the ld.preload configuration in the .spec also hardcodes /usr/lib not /usr/lib64)

[nephros]

Q: Is the library actually supposed to end up in lib64? Some things should go to /usr/lib even in 64bit systems.

(Note the ld.preload configuration in the .spec also hardcodes /usr/lib not /usr/lib64)

Disregarding whether this is correct, this fixes the compile/install/package error:

nephros@4fa3e0f
b100dian#1

Builder here: https://build.sailfishos.org/package/show/home:nephros:branches:home:b100dian:patchmanager/patchmanager

[b100dian]

That was the missing step, thanks @nephros - and now I had to patch the upgrade from lib/ to lib64/ :)

[b100dian]

I just tested the upgrade path and it seems to work now. If no-one has anything against, tomorrow I will merge @nephros PRs then branch out patchmanager3.1 and merge --squash the sfos4-fix by coderus, then re-target this PR on top of it.

@Olf0 would you want to start a thread on FSO? We talked earlier and agree that "A forum (thread) is IMO the best tool to communicate and organise a Patchmanager developers team in an open and transparent manner" but I'd like to do it on forum.sailfishos.org nonetheless. Would be useful for logging bugs like this one https://forum.sailfishos.org/t/patchmanager-patches-in-koli-4-0-1/4925/143 and the future ones:)

[Olf0]

I just tested the upgrade path and it seems to work now.

Now it really makes sense, per sed -i "/libpreloadpatchmanager/ d" /etc/ld.so.preload

But I think this can be simplified and made consistent with the rest of the spec file by using RPM variables:

ARCH=$(getconf LONG_BIT)
SUFFIX=$([[ "$ARCH" == "64" ]] && echo "64" || echo "")
sed -i "/libpreload%{name}/ d" /etc/ld.so.preload
echo "%{_libdir}/libpreload%{name}.so" >> /etc/ld.so.preload

[Olf0]

Done that (among other beautifications and clarifications, without altering functionality) per MR b100dian#2

[Olf0]

@Olf0 would you want to start a thread on FSO? We talked earlier and agree that "A forum (thread) is IMO the best tool to communicate and organise a Patchmanager developers team in an open and transparent manner" but I'd like to do it on forum.sailfishos.org nonetheless. Would be useful for logging bugs like this one https://forum.sailfishos.org/t/patchmanager-patches-in-koli-4-0-1/4925/143 and the future ones:)

I think this needs a bit more consideration:

1. I think we need a place for users of Patchmanager to report and discuss their experiences, as for the example you provided.
   • FSO is well suited for this, because most SailfishOS users have an account there.
   • But before starting such a thread, I would like to be able to state where PM's future development location is (along with its issue tracker etc.).
     Maybe @SfietKonstantin / Basil is ignoring GitHub notifications, I will try to contact him via email this weekend, to check if we can stay here or have to move.
   • IMO it makes sense to open such a user facing thread, when the announcement for a new release (and release location, with a link to it) at Openrepos can be made.
   • Hence started with a new Openrepos repository (https://openrepos.net/content/patchmanager) and will draft a front-page for it.
   • I will try to check with Andrey per mail if he is O.K., if we choose a license (spec file states "ToDo"!) Are you O.K. with LGPL-2.1-only, because that would be my primary suggestion (I have become a bit of a FLOSS license expert and have a pile of reasons)?
   • For the icon at Openrepos, I consider to reuse the application icon. But that is also carries Andrey's copyright (and no license, yet), because he created it. We do need his "Go" for the icon and all the code, before we can proceed in a legitimate way.
2. "A forum (thread) to communicate and organise a Patchmanager developers in an open and transparent manner"
   • Thinking about it, it seems to be helpful, if this is not "next door" to the PM users thread.
   • As stated, the infrastructure provided by Jolla regularly undergoes sudden changes (main git repository in its 3rd iteration, bug tracker in its 3rd iteration (but now it is not a bug tracker, but a forum to report bugs), build infrastructure in its 2nd iteration with the next iteration announced (but unclear what it may be)), which were always handled awkwardly by Jolla.
   • You and @nephros do not seem to like TMO:
     • Can you explain why?
     • Maybe using the wiki of PMs source repo at GH be a better approach:
       • It provides structure.
       • It is where the source code is.

[nephros]

I don't have anyrhing against TMO per se, but:

• I don't have an account there at the moment and am reluctant to add yet another
• FSO is where most of modern-day SFOS discussion happens, and even Jolla guys sometimes drop by
• for both users and devs it's annoying to have a million different places to find information.
• TMO is fullof ancient and confusing old stuff. Great if you're trying to resurrect a N9, but for new stuff not so much

I agree though that the future should be decided in before making an announcement. OTOH PM is broken for many right now, perhaps we should focus on shipping something useful first and then get communication in order.

I also agree that Jolla relying on Jolla infrastructure is a bit of a risk, however I don't think it's hugely important, as build servers can relativly easily changed (I myself have used four different ways to build packages) and code repo is external to Jolla anyway. Having discussions where the users are is worth the risk.

License: obviously it's up to the principle authors to decide, but can you give a reason or two why you'd prefer LGPL?

Code repo: if we can continue here that would be best , if not however, I 'd like to suggest moving away from Microsoft Github - just out of principle. (owner, monopoly, proprietary git extensions etc).

[b100dian]

it makes sense to open such a user facing thread, when the announcement for a new release (and release location, with a link to it) at Openrepos can be made.

PM is broken for many right now, perhaps we should focus on shipping something useful first and then get communication in order.

Folks, fully agree. I was excited about the progress:)

[b100dian]

Re: TMO
I don't use it, that's why I mostly prefer FSO. Also, forums, as chat services, cannot be quicly evaluated for freedom of expresiveness and even then, things can change quickly. I think code (which we have each a copy of) is more important a.t.m.

[Olf0]

Continuing this discussion at #57

[Olf0]

I didn't have full access and merging (or squashing) to a non-default branch was not my idea of inviting people here to create issues and maybe make PRs

O.K., I thought this does not matter, because it is currently just us three looking at this.
Actually I became a bit confused by what went into sailfishos-patches:master, sailfishos-patches:sfos4-fix and b100dian:sfos4-fix, plus why you changed the default branch multiple times.
BTW, there also is the branch sailfishos-patches:sfos4?!?

I will look at the result, when you merged this PR and everything (branches, code) has settled a bit. I do trust you to do the right things, I but just cannot fully follow: Don't mind (explaining etc.), I will see the outcome.

This 'fixes' the point 'Do we have owner' for me. I would like to be a bit conservative at first and don't make any other disruptive changes immediately.

Absolutely agreed: The baseline was, "only move when we have to".
Because you seem to have full administrative access for this repo now, the idea of moving might have become obsolete (unless other hurdles appear).

Tell me what do you think should be changed immediately (like - I required reviews for merging as a first team thing).

As you already switched on discussions, created new build actions, required reviews for merging: These were the essential things.
You could upgrade me to a contributor (or administrator, if you like); currently I cannot perform any administrative work (not even handling issues, except for commenting them).

[Olf0]

BTW, there also is the branch sailfishos-patches:sfos4?!?

Comparing it with sailfishos-patches:master or sailfishos-patches:sfos4-fix (basically the same result) shows some changes we already have in b100dian:sfos4-fix (by nephros), some which make sense to me, and some I do not understand: master...sfos4

[b100dian]

sfos4 is reminiscent of closed #47 - I deleted it, can be restored the PR interface

[b100dian]

The default branch matters precisely because if you have the .github/workflows in it it will use GH actions

sfos4-fix can be also be deleted, as it can be restored from #48
I'll leave it for now because it helps with the explanation:

Changes I've done:

• copied master to legacy: git push origin master:legacy
• copied patchamanager3 to master
• Squashed (not merged) the sfos4 fix. You can see there is only one commit in master for that: f9d0ea5
• Then changed the target of this PR to be master. Realized it is not merging because it was based on sfos4-fix, but sfos4-fix was just squashed into master (different history:).
• locally git rebase origin/sfos4-fix --onto master got my commits (and yours) replayed on the master (the command is approximate, the rebase did not include the 'busybox support' which was already in master..)
• manually inspected git diff HEAD..master to match this PR (this is when I switched it back to point to sfos4-fix)
• was satisfied, pushed the changes (--force), then switched the PR back to target master.

Then I added aarch64 to the GH workflow.

This PR still needs review you know:)

I've upgrade you contributor, I was also under the impression that you are part of some team for reviews worked

[Olf0]

sfos4 is reminiscent of closed #47 - I deleted it, can be restored the PR interface

O.K. (actually I hate to see stuff deleted, without knowing for sure, that it is obsolete or superfluous).
As said, it already contained the fixes @nephros rediscovered.

I just kindly ask you and / or @nephros to check, if its two commits contain other relevant or useful changes (primarily the first one):
aaa59d9
75217c3

P.S.: Will start reviewing now.

[Olf0]

Looking good.

[Olf0]

Approve a second time? O.K.

[b100dian]

Did you accept the invitation for contributors?;)

[Olf0]

Did you accept the invitation for contributors?;)

There was none! Just checked again in my GH-notifications.
But I am labelled as "Contributor" (just like nephros, and you as "Contributor" and "Author").

The documentation is not really helpful, either: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews and liked pages there.
It details the roles at "organisations" (where I have seen the more granular rights), but I should have "Write access" now. Will try to create a new branch for testing.

[Olf0]

According to the documentation "collaborators" do have "write access" at "personal repositories": https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-github-user-account/managing-user-account-settings/permission-levels-for-a-user-account-repository#collaborator-access-for-a-repository-owned-by-a-user-account

But as a matter of fact, my approval turned the state of this PR to "red"!

[b100dian]

Hmm

[Olf0]

BTW, it seems that PRs are "automatically merged" after a successful review, i.e. that a reviewer's O.K. triggers merging of a PR.
Such automatisms give me a bad feeling, I would rather have a human to click on "merge now". E.g., meanwhile something may have changed in an (technically) non-conflicting way in the branch to merge to, which renders parts of the merge senseless.

[Olf0]

I will log out and then back in: Maybe the notification then appears.

[b100dian]

Sounds familiar but doesn't seem enabled, where did you see this?

[Olf0]

1. Still no notification for the contributor status.
   But notifications for your new messages do appear.
2. Good that it is unchecked.
   I have seen this here, in the third box under the comments:

Merging is blocked
Merging can be performed automatically with 1 approving review.

[b100dian]

I will approve the remose timer PR to see if it has this outcome:)
(no it did not)

[Olf0]

1. Well, I have seen that I can revert commits, so I seem to have elevated rights.
   Can you stop the "pending invite"? You still can re-do it at any later time.
2. BTW, you have a "pending invite" as an admin for the PM-Developers:patchmanager repository: Do you see that? (Because you have not reacted, although that is just experimenting with the features of "organisations".)
3. Why don't you just unset the "merge approval necessary"-setting and merge, for now. I am too tired and tired of testing out a GH-bug, which we may have triggered by inviting me as a collaborator, although I may have been one (partially; I don't know: some things looked like it, other did not) before.

[b100dian]

no rush, will wait for @nephros. And maybe the email in your spam folder. (I can merge but who's waiting on this, us - tomorrow)

[Olf0]

And maybe the email in your spam folder.

From my experiments with roles and orgs, I know that all notification show up in the web-frontend, hence I looked only there: This one did not.
=> Accepted & Retrying

[Olf0]

O.K., I could merge now.
I will leave it to you, if you want a single squashed commit or a regular merge of all commits.
Or do you want me to try merging?

[b100dian]

Yeah, squash it

…

On Tuesday, September 21, 2021, olf wrote: O.K., I could merge now. I will leave it to you, if you want a single squashed commit or a regular merge of all commits. Or do you want me to try merging? -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #49 (comment)

-- Sent from my Sailfish device

[Olf0]

Yeah, squash it

Done.

P.S.: In the personal GH-settings for notifications, in the section "Actions" there is the paragraph "Notifications for workflow runs on repositories set up with GitHub Actions." with three choices: "Email" was set, but "Web" was not.
Maybe enabling "Web" (as done now) would have made the difference.

[Olf0]

... and my fears of auto-merging were just based on bad wording in GH's web-frontend. And you already have schown that this setting is off.

[Olf0]

I just kindly ask you and / or @nephros to check, if its two commits contain other relevant or useful changes (primarily the first one):
aaa59d9
75217c3

O.K., looking at it more closely, I believe that all relevant changes in the first commit were carried over by Coderus from the sfos4 to the sfos4-fix branch (with small enhancements).
Hence I also conclude that sfos4 was just a stale branch as part of PR #47, as Vlad already pointed out.

And the sfos4-fix branch should be deleted now, because what I merged into master tonight was based on it (and it sure did not receive any further commits).
But as I want to keep the "4 eyes" principle, this is my suggestion and somebody else can do this or I will do it, if someone else states "O.K.".

[b100dian]

sfos4-fix can be also be deleted, as it can be restored from #48

@Olf0 sleep, tomorrow we patch from /var/run 😜

[Olf0]

Oh, took me a while to have an idea what this may address. 🤔

It is nice to see you and @nephros so motivated and active, but I am usually quite tired, when taking a look at this after work. 😪
P.S.: Currently, this is stealing much of my spare time.

[b100dian]

We are all working.. - @nephros seems to have a head start now, but we may jump in and fill in the gaps when he's underwater:)

[nephros]

We can absolutely reduce speed, if we're in it for the long term after all (hopefully). Not necesary at all to comment of 5 threads every couple of hours.

Don't let software steal spare time. Unless you like wasting it on it.