Disallow branches altogether in sources

[bbhtt]

Only branch: already blocked

branch + commit: If HEAD of branch does not point to the commit, build fails.

[hfiguiere]

This happened today.

Linter would have prevented it.

I remember clearly making this check fatal.

[bbhtt]

Changed in https://github.com/flathub-infra/flatpak-builder-lint/commit/883cc94d42b616703373eec5c9409b2771d37a4d

I think we can safely disallow branch + commit combination after notifying apps using it

[bbhtt]

Ok using a commit hash in branch actually works even when HEAD of branch changes, it behaves just like using only a commit.

There's no possible combination in which both branches and a commit is required. I think I can remove the if not commit part.

[hfiguiere]

a plain commit has always worked.

I think I remember one issue where someone wanted to always update to the tip of the branch and f-e-d-c would use that. It's probably the only (albeit still broken IMHO) case. Instead f-e-d-c should just accept "branch" as a parameter.

[bbhtt]

Yes, we can treat them on a case by case basis. For plain apps, combined with fedc it kind of makes them "nightly" which isn't allowed

It only make sense for infra apps like the flatpak for x-checker itself.

[hfiguiere]

What I see is not making it an error if there is a f-e-d-c stanza.

[bbhtt]

One issue it doesn't require a fedc stanza to update them, fedc will auto update the commit to the latest one if branch: branch_name is present.

This is how the x-checker flatpak works, see for example https://github.com/flathub/org.flathub.flatpak-external-data-checker/pull/245/

[hfiguiere]

oh. TIL

[bbhtt]

This is the list of affected apps. Excluded baseapps, extensions for now.

I'll add exceptions for these temporarily and land the change. We can deal with them one by one later.

Need to check the direct upload apps too.

com.github.bajoja.indicator-kdeconnect.json
com.github.KRTirtho.Spotube
com.github.Matoking.protontricks
com.github.sixpounder.GameOfLife
com.hack_computer.Clubhouse
com.her01n.BatteryInfo
community.pathofbuilding.PathOfBuilding
com.sourcepole.kadas
com.wiz.Note
de.rwth_aachen.ient.RDPlot
dev.atoft.Reactions
de.willuhn.Jameica
fr.romainvigier.MetadataCleaner
io.atom.Atom
io.botfather.Botfather
io.github.am2r_community_developers.AM2RLauncher
io.github.arunsivaramanneo.GPUViewer
io.github.fabrialberio.pinapp
io.github.igaldino.CoyoteTM
io.github.JaGoLi.ytdl_gui
io.github.lainsce.Quilter
io.github.seadve.Kooha
io.mpv.Mpv
io.qt.QtCreator
net.rpcs3.RPCS3
net.rpdev.OpenTodoList
org.aliflang.lang
org.dhewm3.Dhewm3
org.flathub.flatpak-external-data-checker
org.flycast.Flycast
org.goldendict.GoldenDict
org.guerinoni.qTsConverter
org.musicbrainz.Picard
org.pegasus_frontend.Pegasus
org.purei.Play
org.radare.iaito
org.sdrangel.SDRangel
org.shotcut.Shotcut
org.sonic3air.Sonic3AIR
org.taisei_project.Taisei
org.upnproutercontrol.UPnPRouterControl
org.viking.Viking
org.yamagi.YamagiQ2
rocks.koreader.KOReader
sa.sy.bluerecorder

[hfiguiere]

For shotcut: https://github.com/flathub/org.shotcut.Shotcut/pull/102

[hfiguiere]

I don't see it in com.sourcepole.kadas The branches have already been commented out, albeit the manifest has a top-level branch, but that's unrelated.

[bbhtt]

Thought I removed the branch: stable from the list, one or two might have slipped in because often there is a source with branch: stable too eg. Kooha.

[bbhtt]

After excluding EOLs and flase-positives the list wasn't that long, so I just opened issues https://github.com/search?q=is%3Aissue+author%3Abbhtt+%22Please+remove+usages+of+branch+property+from+manifests%22&type=issues

None of the direct uploads use this. Let's land this now