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] Command line warnings suggests things going wrong (v0.6.7) #289

Open
Olf0 opened this issue Jun 1, 2024 · 15 comments
Open

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

Olf0 opened this issue Jun 1, 2024 · 15 comments
Labels
bug Something isn't working

Comments

@Olf0
Copy link
Collaborator

Olf0 commented Jun 1, 2024

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.

@Olf0 Olf0 added the bug Something isn't working label Jun 1, 2024
@rinigus
Copy link
Contributor

rinigus commented Jun 2, 2024

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

Olf0 commented Jun 2, 2024

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:

  • I started sailfishos-chum-gui at the command line logged into the phone via ssh.
  • Your question made me retest by starting it directly in a terminal on the phone: The warning 1 "Failed to refresh Chum repository …" is still displayed.
  • "… at the GUI this refresh appears to be working fine." does address exactly this initial, automatic repo refresh (i.e. not one triggered via the top pulley menu): It runs through all usual steps and seems to take as long as usual.

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

rinigus commented Jun 2, 2024

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

Olf0 commented Jun 2, 2024

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 created a new CodeBerg-token (my second one) and inserted it at both places.
  • I took the GH-token from the SFOS-OBS and inserted it enclosed in double-quotes as a GH-secret again.
  • [Obsolete now] I did the same with the GitLab-token.

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" ( 😉 ) 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
Copy link
Contributor

nephros commented Jun 2, 2024

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

nephros commented Jun 2, 2024

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

Olf0 commented Jun 2, 2024

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

Olf0 commented Jun 2, 2024

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

nephros commented Jun 2, 2024

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

nephros commented Jun 2, 2024

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

Olf0 commented Jun 2, 2024

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

Olf0 commented Jun 4, 2024

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

rinigus commented Jun 4, 2024

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

nephros commented Jun 4, 2024

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

Olf0 commented Jun 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants