cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
46 stars 26 forks source link

Use fsinfo from Cockpit, drop our copy #589

Closed allisonkarlitskaya closed 3 months ago

allisonkarlitskaya commented 3 months ago

Sync up our cockpitlib, bringing fsinfo with it. Drop our copy.

This went upstream with very few changes, so we don't need to do anything to adjust to it.

Blockers:

allisonkarlitskaya commented 3 months ago

wait_js_cond(ph_is_present("[data-item='dotdot'].symlink.folder")): Uncaught (in promise) Error: condition did not become true

Okay, that's clear enough: everything except arch and rawhide have an older version of the bridge with the bug around handling of symlinks to ... We had a workaround in our fsinfo code for that, but I removed it when porting it upstream, since the bug is fixed there.

Probably we should add the workaround back again upstream, since without it, anyone who bundles fsinfo from Cockpit repo has an effective depend on bridge version 318.

martinpitt commented 3 months ago

In F40, 318 moved to stable a week ago. But indeed the image refresh was stuck for a bit and only landed this morning. Updating F39 is stuck because of https://bugzilla.redhat.com/show_bug.cgi?id=2284431 , we could at most disable the unit test there. In Debian, I've worked quite a lot on the update (lots of issues), but it's finally on its way in.

So let's either bump the dependency to ≥ 318 or reapply the workaround, yes.

allisonkarlitskaya commented 3 months ago

I took a look at it and we can't really do it properly because the workaround is very specific to Cockpit Files (which is why I dropped it when upstreaming).

I'd prefer to bump the depend.

martinpitt commented 3 months ago

I retried both f40 and they pass now. We can stall this for a few days until d-testing catches up, then we just need to deal with F39 somehow. I can crowbar it into F39 on Monday (ignoring the PCP crash in the unit tests on i386). I agree that it's better to bump the dependency.

martinpitt commented 3 months ago

My d-testing refresh landed, also green. I finally gave in and disabled pcp on i386 on Fedora 39 as well, and updated the package: https://bodhi.fedoraproject.org/updates/FEDORA-2024-7f96e33d91 Once this goes in (in 14 days, or if someone gives an extra karma, hint hint), we can refresh our bots image and finally land this.

allisonkarlitskaya commented 3 months ago

this just needs a dependency bump to cockpit ≥ 318

This is already included in the latest push, I think? Please let me know if I missed a spot...

martinpitt commented 3 months ago

@allisonkarlitskaya Right, sorry -- I didn't actually check the diff again. Then let's just do a clean rebase after landing f39, so that we don't count the failures against our flake statistics.

martinpitt commented 3 months ago
podman run -it --rm fedora:39 dnf info cockpit

Shows 318. I set the gears in motion: https://github.com/cockpit-project/bots/issues/6546