Enforcing pull request workflow and green CI status of PRs

Hi everyone,

Starting from 2022-10-04, we’re going to tighten the branch protection settings by disabling direct pushes to protected branches (i.e. master, beta, and the ones starting with branch/) and requiring status checks to pass. This means that all changes will need to go through a regular pull request workflow and require the build tests to pass (i.e. that they be green) on Buildbot before being merged.

As part of this change, we’re introducing two new checks as well. Manifests will be linted with flatpak-builder-lint to ensure compliance with best practices we suggest doing the initial review phase. If your app should be exempted from specific linter rules, please open an issue with an explanation why.

Additionally, if a manifest contains a stanza for flatpak-external-data-checker, it will be validated to ensure update detection works correctly.

7 Likes

Hi @barthalion :wave:

Very happy that I’ll be able to integrate flathub checks in my automated test suites / CI.
Where I can see how those are used by Flathub and with which parameters?

Thank you.

Hey @sonny!

The command used by Flathub CI is flatpak run --command=flatpak-builder-lint org.flatpak.Builder --exceptions manifest.json. You may want to omit the --exceptions as we use it to allow specific errors to pass if the usage is legit.

It is also possible to use the linter without flatpak, by installing it first with pip install git+https://github.com/flathub/flatpak-builder-lint. It requires flatpak-builder to work, but versions available across distributions should work fine.

1 Like

I believe I ran into some problems because of this new gating.

This build:

threw a warning:

INFO    src.manifest: Checking 1 external data items
INFO    src.manifest: Started check [1/1] archive sweethome3d/SweetHome3D-7.0.2-linux-x64.tgz (from com.sweethome3d.Sweethome3d.json)
ERROR   src.manifest: Failed to check archive sweethome3d/SweetHome3D-7.0.2-linux-x64.tgz with HTMLChecker: Error querying for new versions: Connection timeout to host http://www.sweethome3d.com/history.jsp
WARNING src.main: Check finished with 1 error(s)

and was never published.

There are no notifications of any kind from the build system that the build failed, so for a week, I thought SweetHome3D had had its runtime updated within a couple of hours of me merging the changes in:

A week later somebody filed a new issue about it:

I changed the check URL to an https URL, and this time the PR test build failed:

INFO    src.manifest: Checking 1 external data items
INFO    src.manifest: Started check [1/1] archive sweethome3d/SweetHome3D-7.0.2-linux-x64.tgz (from com.sweethome3d.Sweethome3d.json)
ERROR   src.manifest: Failed to check archive sweethome3d/SweetHome3D-7.0.2-linux-x64.tgz with HTMLChecker: Error querying for new versions: Connection timeout to host https://www.sweethome3d.com/history.jsp
WARNING src.main: Check finished with 1 error(s)

To be fair, it just means that it’s likely it has never worked correctly, or broke after it has been introduced. It looks like the project blocks requests depending on user-agent; it works fine with curl, but doesn’t via urllib3 or python-requests. Removing x-data-checker from the manifest will unblock the build, while the issue should be reported to flatpak-external-data-checker repo.

It certainly used to work, sometimes a bit too eagerly, as shown by those 15 PRs flathubbot opened:
https://github.com/flathub/com.sweethome3d.Sweethome3d/pulls?q=is%3Apr+is%3Aclosed+author%3Aflathubbot

This test program works correctly on my system, what makes you think the user-agent is the problem?

$ cat foo.py 
import urllib3
http = urllib3.PoolManager()
url = 'https://www.sweethome3d.com/history.jsp'
resp = http.request('GET', url)
print(resp.status)
$ python3 foo.py 
200

Because it hangs on my laptop and builders:

>>> resp = http.request('GET', url)
2022-10-05 14:01:46,728 DEBUG Starting new HTTPS connection (2): www.sweethome3d.com:443

While finishes instantly with curl. Already experimented with user-agent and unfortunately that is not the reason for the hang.

IPv4 vs. IPv6? This seems to be the problem with most of our network-related build problems :wink:

Where should I file a bug about this check’s failure? It looks like a regression to me.

Even if the sweethome3d.com server is to blame, it would be useful to have a bit more debugging information, such as which IP address was contacted so as to give us a bit more information and maybe a way to reproduce the problem.

Looks from the discussion on the Flathub Matrix that IPv6 was the problem. I filed an upstream bug, thanks!
https://sourceforge.net/p/sweethome3d/bugs/1119/

it would be useful to have a bit more debugging information, such as which IP address was contacted so as to give us a bit more information and maybe a way to reproduce the problem.

Please file a bug to f-e-d-c.

Filed as:

1 Like

Hi,

I think this is a bad idea because it gets in the way.

When making the initial or heavy changes to the package it is maybe good to do it like that (that depends on the person I think); but when the package is super stable and the only thing you have to do is raise the version number and hash then your change really gets in the way and is counter productive especially for those having jenkins task pushing the update on github, you force all of us to rewrite all our scripts for very little benefit (in my case there are only downside).

Please reconsider this change.

Cheers,
Alex

Consider flathub is full of people who merge completely untested and broken things on daily basis. It’s great that you keep your app in good quality but flathub become too big to rely on good faith alone.

It doesn’t justify to impose an inefficient workflow to all.

Maybe this could be opt-out using flathub.json?

I can’t count the number of YOLO merge request I have seen. Those merged before the build even started, and too many failed.

And that’s for the repos I was still subscribed to (i.e. I hadn’t unsubscribed yet)

It is opt-out. You may ask for exemption and get one. This is part of design.