sailfishos-chum / sailfishos-chum-gui

GUI application for utilising the SailfishOS:Chum community repository
https://openrepos.net/content/olf/sailfishoschum-gui-installer
MIT License
13 stars 16 forks source link

[Bug] Command line warnings suggests things going wrong (v0.6.7) #289

Open Olf0 opened 1 month ago

Olf0 commented 1 month ago

When the SailfishOS:Chum GUI app is started at the command line, it emits a lot of warnings related to repository authentication. This was observed with sailfishos-chum-gui-0.6.7-2 on SailfishOS 4.5.0 and 3.2.1. These warnings address three different repository hosters:

  1. A single warning right after the start of the SailfishOS:Chum GUI app addressing the SailfishOS-OBS: [W] unknown:0 - Failed to refresh Chum repository "Failed to obtain authentication." But this does not seem to be true (see section "Next try, …"), at the GUI this refresh appears to be working fine.
  2. Many pairs of lines addressing GitLab.com via the GitLab connector: [W] unknown:0 - GitLab: Failed to fetch repository data for GitLab "<name>/<repo>" [W] unknown:0 - GitLab: Error: "Host requires authentication"
    • I wonder why the "Error" in the second line is labelled as a warning.
    • I slightly wonder why the warning "Failed to fetch repository data" comes first and the "authentication error" second.
    • I was unable to determine comprehensively if something is not working; on first sight all seems to be O.K.
  3. Many pairs of lines addressing codeberg.org via the Forgejo connector: [W] unknown:0 - Forgejo: Failed to fetch repository data for Forgejo "<repo>" [W] unknown:0 - Forgejo: Error: "Error transferring https://codeberg.org/api/v1/repos/<repo> - server replied: Not Found"
    • I wonder why the "Error" in the second line is labelled as a warning.
    • I slightly wonder why the warning "Failed to fetch repository data" comes first and the "authentication error" second.
    • I was unable to determine comprehensively if something is not working; on first sight all seems to be O.K.

Ultimately, either some or all of these warnings are wrong (because on first sight all seems to be working fine, but this assessment lacks thorough scrutinising) or something does go wrong with the repository authentication. I quickly checked the access tokens and they seem to be correct: at GitHub with quoting (i.e. framed in quotation marks / "double quotes"), at the Sailfish-OBS bare, i.e. without quoting, in both cases occupying a single line without a concluding newline character.

rinigus commented 1 month ago

It would have to be checked whether the corresponding tokens expired. As for GitLab.

Warning 1 looks to be coming from the way GUI is started. Maybe wasn't done via phone and has no auth to pkcon refresh

Re Forgejo: I wonder if someone just put in their SPEC

Olf0 commented 1 month ago

It would have to be checked whether the corresponding tokens expired. As for GitLab.

AFAIU this can only be done by the one who created the token ("token owner"), correct?

Warning 1 looks to be coming from the way GUI is started. Maybe wasn't done via phone and has no auth to pkcon refresh

I see, that I should have been more concise and have to add additional information:

@jaimeMF, do you have any additional observations to add? Did you also see the initial warning "Failed to refresh Chum repository …" on SailfishOS 4.5.0?

Re Forgejo: I wonder if someone just put in their SPEC

Hmm, I am not sure if I parse this statement correctly. If you mean to ask if the new code was not tested thoroughly: Well, initially this is basically what the author of the code wrote ("tested via curl" means to me that the principle was tested, not the actual implementation), but later there was a reply which seems to be a confirmation of running the actual code ("I've been running a build of this for a bit"). Now I ask myself if this was a build from the Sailfish-OBS with the CodeBerg-token I generated or the one @nephros initially created for himself? @nephros, can you please answer that rsp. retest the current release, i.e. SailfishOS:Chum GUI app v0.6.7-2; actually I would appreciate, if you try both, the build at GitHub and the one at Sailfish-OBS' Chum, because the access tokens are supplied via an indirection at GitHub.

OTOH there was an explicit call for reviewing the code. As I was the only one looking at this (I would not call my few checks&balances questions "a review") and there currently is no other user than the code author @nephros himself, I decided after 2½ weeks to ask nephros to self-review his code and merged PR #271 after positive feedback from him.

rinigus commented 1 month ago

AFAIU this can only be done by the one who created the token ("token owner"), correct?

I checked and it seems to be expired. Made a new one and set it at OBS (testing and chum). Unfortunately, they let it do for a year in a time. Rebuilds started at OBS. BTW, it can probably be tested by trying to use it for accessing GraphGL API

Failed to refresh

Hmm, terminal from the phone should be fine. I would have tested by starting from Lipstick itself and traced journal for the messages. But your way should be OK. No idea then

Forgejo

No, I mean that maybe someone managed to put "" in the SPEC of one of the packages at OBS Chum or Testing.

Re calls for review

I am sorry for taking so long time or skipping the replies. Its due to allocating too little time to work on SFOS during a last year or more. But its great that you actively maintain Chum GUI and there are new contributors coming in. Thank you for it!

Olf0 commented 1 month ago

AFAIU this can only be done by the one who created the token ("token owner"), correct?

I checked and it seems to be expired. Made a new one and set it at OBS (testing and chum). Unfortunately, they let it do for a year in a time. Rebuilds started at OBS. BTW, it can probably be tested by trying to use it for accessing GraphGL API

Thank you, now all the GitLab authentication warnings are gone when using the new build; i.e. only categories 1 and 3 remain from the initial post in this discussion thread.

Please mind to also add this token enclosed by double-quotes as a GH-secret, or the CI-builds at GH will not work properly. Edit: Done that.

Forgejo

No, I mean that maybe someone managed to put "" in the SPEC of one of the packages at OBS Chum or Testing.

Because that was my initial suspicion, I triple checked all tokens to be without quotes at the SFOS-OBS and enclosed in double-quotes as GH-secrets (this is why all secrets were dated "7 days ago"):

I assume the fact that there are no GitHub-related authentication warnings means that I did this correctly.

Re calls for review

I am sorry for taking so long time or skipping the replies. Its due to allocating too little time to work on SFOS during a last year or more. But its great that you actively maintain Chum GUI and there are new contributors coming in. Thank you for it!

Its my pleasure freeing you and @piggz from tedious tasks as handling issues and PRs, release management, maintaining the infrastructure (CI workflows etc.), documentation etc., so you can take care of maintaining the code you originally wrote (and writing code at other places). Both of you know that writing and properly reviewing C++ and QML code is about the only thing I cannot help you out with, hence we have to implement some mechanism by which I can "summon" ( :wink: ) at least one of you in a timely manner (i.e. a few weeks) to review and reply to questions arising during the review, after I performed the initial handling of a PR (checking that it makes sense, trying to understand what the code exactly does, asking basic Checks&Balances questions). Would it be helpful, if I use the GitHub-mechanism to request a review from both of you both instead of addressing you by "@rinigus, @piggz", so these review requests are easily to distinguish from being addressed in comments?

nephros commented 1 month ago

I tested the forgejo support using my own token, built with the GH action of my forked repo. What I tested using curl only is Gitea, as I don't have an account at any Gitea deployment.

Please note that when initializing the GH, GL and FJ/gitea support, we iterate URLs given in .spec or Chum metadata. (See the isProject() method of either of those).

https://github.com/nephros/sailfishos-chum-gui/blob/ae984a97ff9821bf223f8fccc7b6b07c4479a5eb/src/chumpackage.cpp#L232

Some of the 'Not found' errors will come from that, when any of the metadata URLs is actually wrong, or the guess is wrong.


But. The URL given in the Forgejo error message does seem wrong, it looks like I have a bug in constructing the URL, will check that:

[W] unknown:0 - Forgejo: Error: "Error transferring https://codeberg.org/api/v1/repos/sailfish-moreutils - server replied: Not Found"

I guess that should be https://codeberg.org/api/v1/repos/nephros/sailfish-moreutils instead.

nephros commented 1 month ago

I tested the forgejo support using my own token, built with the GH action of my forked repo.

Please note that when initializing the GH, GL and FJ/gitea support, we iterate URLs given in .spec or Chum metadata. (See the isProject() method of either of those).

https://github.com/nephros/sailfishos-chum-gui/blob/ae984a97ff9821bf223f8fccc7b6b07c4479a5eb/src/chumpackage.cpp#L232

Some of the 'not found' errors will come from that, when any of the metadata URLs is actually wrong, or the guess is wrong.

But. The URL given in the Forgejo error message does seem wrong, it looks like I have a bug in constructing the URL, will check that:

[W] unknown:0 - Forgejo: Error: "Error transferring https://codeberg.org/api/v1/repos/sailfish-moreutils - server replied: Not Found"

I guess that should be https://codeberg.org/api/v1/repos/nephros/sailfish-moreutils instead.

Actually no.
What happens is that the metadata in the package is indeed wrong in the revision that's on Chum currently (it has https://codeberg.org:nephros/sailfish-moreutils instead of https://codeberg.org/nephros/sailfish-moreutils).

This has been corrected in the Chum:Testing version via: https://codeberg.org/nephros/sailfish-moreutils/commit/183f6421bf1aeddbdc6da3cde37f1fa791e53cba

In such a case the parseUrl method, which splits on "/" characters, will determine the wrong repository 'slug', i.e. username/reponame combination, leading to a try to get metadata for a nonexisting repo, and is correctly reporting the result: Not found.

While this is an Error on the API side, it's not an error for the Chum GUI, because we can happily continue without the repo metadata having been fetched (so we report a Warning about the error).

Olf0 commented 1 month ago

Some of the 'not found' errors will come from that, either when any of the metadata URLs is actually wrong, or the guess is wrong.

The command line output now is:

[nemo@sailfish ~]$ sailfishos-chum-gui 
[D] unknown:0 - Using Wayland-EGL
[W] unknown:0 - Failed to refresh Chum repository "Failed to obtain authentication."
[D] unknown:0 - GitLab support added for "gitlab.com"
[D] unknown:0 - Forgejo support added for "codeberg.org"
[W] unknown:0 - Forgejo: Failed to fetch repository data for Forgejo "sailfish-moreutils"
[W] unknown:0 - Forgejo: Error:  "Error transferring https://codeberg.org/api/v1/repos/sailfish-moreutils - server replied: Not Found"
[W] unknown:0 - Forgejo: Failed to fetch repository data for Forgejo "sailfish-moreutils"
[W] unknown:0 - Forgejo: Error:  "Error transferring https://codeberg.org/api/v1/repos/sailfish-moreutils - server replied: Not Found"

So we are down to twice the same error (?!?) from a single package\@codeberg. I was just looking at moreutils' spec file, when I saw that you already fixed the metadata.

Hence I just slightly wonder, why it was reported twice, but that was also the case for all GitLab repos (when they still failed) in two rounds (iterating over all repos).

Olf0 commented 1 month ago

Nice, now we are down to only the single category 1, which remains from the initial post in this discussion thread. I.e. when @nephros will have updated sailfishos:chum:testing/moreutils from 0.68+git1 and submitted it to sailfishos:chum proper.

To which rinigus replied:

Hmm, terminal from the phone should be fine. I would have tested by starting from Lipstick itself and traced journal for the messages. But your way should be OK. No idea then.


And my request to @rinigus:

Please mind to also add this [new Gitlab-] token enclosed by double-quotes as a GH-secret, or the CI-builds at GH will not work properly.

Edit: BS, I can do that, so I did it.


Edit 2: But I missed another remaining question to @rinigus and @piggz:

Would it be helpful, if I use the GitHub-mechanism to request a review from both of you both instead of addressing you by "@rinigus, @piggz", so these review requests are easily to distinguish from being addressed in comments?

nephros commented 1 month ago

Hence I just slightly wonder, why it was reported twice, but that was also the case for all GitLab repos (when they still failed) in two rounds (iterating over all repos).

One thing that may lead to this is when URLs are given multiple times in the metadata.

We try three URLs in order:

m_packaging_repo_url, m_repo_url, m_url, which correspond to the Custom/PackagingRepo, Custom/Repo, and either Links/Homepage or Url (from .spec) fields in the metadata .

If two are e.g. Gitlab URLs but lead to an error, and the third is not a Gitlab URL, you'll see two Warnings about Not found.

(This does not seem to be the case for the example with Forejo/moreutils here though.)

nephros commented 1 month ago

Nice, now we are down to only the single category 1, which remains from the initial post in this discussion thread. I.e. when @nephros will have updated sailfishos:chum:testing/moreutils from 0.68+git1 and submitted it to sailfishos:chum proper.

To which rinigus replied:

Hmm, terminal from the phone should be fine. I would have tested by starting from Lipstick itself and traced journal for the messages. But your way should be OK. No idea then.

When I start it on CLI, I get a polkit popup asking for a security code or fingerprint confirmation ("Authentication is required to change software repository parameters"). This sounds like it is coming from ssu, probably when Chum GUI is checking which repo is enabled on the device. (See /usr/share/polkit-1/actions/org.freedesktop.packagekit.policy)

Now, whether you get that prompt, or get the "Use code of fingerprint" prompt on the terminal IME very much depends on how the user session was started. I haven't figured out what exactly makes this difference, but it's definitely different whether the session was started from the phone, or via ssh.
(As I usually use screen, I never remember which one was the initial shell session so sometimes get one, sometimes the other.)

Olf0 commented 1 month ago

Hence I just slightly wonder, why it was reported twice, but that was also the case for all GitLab repos (when they still failed) in two rounds (iterating over all repos).

One thing that may lead to this is when URLs are given multiple times in the metadata.

We try three URLs in order:

m_packaging_repo_url, m_repo_url, m_url, which correspond to the Custom/PackagingRepo, Custom/Repo, and either Links/Homepage or Url (from .spec) fields in the metadata .

If two are e.g. Gitlab URLs but lead to an error, and the third is not a Gitlab URL, you'll see two Warnings about Not found.

(This does not seem to be the case for the example with Forejo/moreutils here though.)

AFAIU the for loop you are referring to in ChumPackage::setDetails iterates up to three consecutive times over the same package, which IMO contradicts the observation "… it was reported twice […] for all GitLab repos (when they still failed) in two rounds ([each time] iterating over all repos).", which can be nicely seen in the original output.


WRT the only remaining warning ("…Failed to refresh Chum repository "Failed to obtain authentication."") from the initial post in this discussion thread and /usr/share/polkit-1/actions/org.freedesktop.packagekit.policy):

I haven't figured out what exactly makes this difference, but it's definitely different whether the session was started from the phone, or via ssh.

I guess this is why rinigus made me try both, which made no difference.

But I do not see a polkit-popup when I start sailfishos-chum-gui on CLI, though git-blame shows that the file org.freedesktop.packagekit.policy.in was not altered in the past 8 years (except for porting to the meson build system 5 years ago), so I should see the same behaviour on SFOS 3.2.1 (but I may have altered the policy).

Olf0 commented 1 month ago

Another observation

I accidentally hit "Remove" in the pulley menu while having salfishos-chum-gui started at the command line. The remorse timer for deletion started, but also it emitted at the CLI: [W] unknown:0 - Failed Chum::PackageOperation(PackageRemove) "harbour-audiocut;1.5.0-1;noarch;installed" "Failed to obtain authentication." I did not want to try, if it would have really removed the package, so I stopped the remorse timer. But I sure can test that, if this is deemed helpful.

rinigus commented 1 month ago

Would it be helpful, if I use the GitHub-mechanism to request a review from both of you both

I would have to figure out how to start sorting emails from Github. Last night resulted in 32 new emails and it is not trivial to find such ping in them. Sure, try to either ping or request review. I'll try to respond.

nephros commented 1 month ago

Would it be helpful, if I use the GitHub-mechanism to request a review from both of you both

I would have to figure out how to start sorting emails from Github. Last night resulted in 32 new emails and it is not trivial to find such ping in them. Sure, try to either ping or request review. I'll try to respond.

Yes, GH (email) notifications are annoyingly plenty. Jolla Mind2 will sort this out ;)

Myself, I use sailfish github integration (shameless plug) while we're waiting.

Olf0 commented 3 weeks ago

With v0.6.7-3 there are only a two warnings of this issue report left, which are:

From the initial posting

  1. A single warning right after the start of the SailfishOS:Chum GUI app addressing the SailfishOS-OBS: [W] unknown:0 - Failed to refresh Chum repository "Failed to obtain authentication." But this does not seem to be true (see section "Next try, …"), at the GUI this refresh appears to be working fine.

and

Another observation

I accidentally hit "Remove" in the pulley menu while having salfishos-chum-gui started at the command line. The remorse timer for deletion started, but also it emitted at the CLI: [W] unknown:0 - Failed Chum::PackageOperation(PackageRemove) "harbour-audiocut;1.5.0-1;noarch;installed" "Failed to obtain authentication." I did not want to try, if it would have really removed the package, so I stopped the remorse timer. But I sure can test that, if this is deemed helpful.


I.e. all warnings, which constituted real errors, appear to be eliminated now.

The two aforementioned, remaining warnings may be cosmetic issues, though the second one ("another observation") still has to be tested more thoroughly.