ungoogled-software / ungoogled-chromium-debian

Debian, Ubuntu, and others packaging for ungoogled-chromium
386 stars 49 forks source link

Update unified branch to Ungoogled Chromium 111.0.5563.64 #317

Closed SugaryHull closed 1 year ago

SugaryHull commented 1 year ago

This fork successfully builds in a clean Debian 11 chroot environment with no hacky changes or patches, just the refreshed patches I added to debian/patches. The switch to Clang/LLVM 13 was necessary because of a required build flag that can't be patched out from the build script without code changes. It is also hardcoded to use lld-13 by the lld-13 patch out of necessity, as Debian's (and I would assume Ubuntu's) updated Clang and LLD packages don't overwrite the symlinks provided by the base versions (Clang/LLVM 10 in focal and 11 in Bullesye). The old shim patch in ungoogled-chromium-debian was replaced with a new Debian upstream shim patch that's much simpler. The Clang version check bypass was sourced from Gentoo's patch pool for 110.

SugaryHull commented 1 year ago

Looks like the CI failed to apply chromium-110-compiler.patch. Do I just need to update something related to the CI? It actually failed at the Clang 13 commit. Does the CI not have that version?

PF4Public commented 1 year ago

CI failed at all of them:

  1. debian/patches/clang13.patch should probably be --- a/build/ […] +++ b/build/ This makes no sence, there should be another reason. Maybe that's insignificant.
  2. debian/patches/lld-13.patch should probably be --- a/build/ […] +++ b/build/ This makes no sence, there should be another reason. Maybe that's insignificant.
  3. debian/patches/chromium-110-compiler.patch probably needs offsets fixed.

It is a pity that Debian lags behind with clang.

SugaryHull commented 1 year ago

How would I go about fixing the offsets?

SugaryHull commented 1 year ago

I'll try using Debian's patch for the compiler bypass and see if that fixes the issue.

SugaryHull commented 1 year ago

I'll try using Debian's patch for the compiler bypass and see if that fixes the issue.

Got a different error. This time it didn't like the optimizations patch. Is it possible that the order of the patches is wrong in my series file? https://api.cirrus-ci.com/v1/task/5759136797818880/logs/validate_patches.log

SugaryHull commented 1 year ago

I'll try using Debian's patch for the compiler bypass and see if that fixes the issue.

Their patch doesn't even work anyway. Guess I'll have to revert to the Gentoo patch and try to fix the offsets.

SugaryHull commented 1 year ago

I ran the validate_patches script locally with the verbose parameter and got the following errors. Looks like the use-package-optimizations patch is entirely outdated and has the wrong path for build.gn and is missing the files it's supposed to patch. Debian's patcher just skips it rather than throw an error. https://0bin.net/paste/exfFIAXz#Cgpltdp-/pnx4PcVPzMX7MrVXShzktE/eq0EFEoIeL/

SugaryHull commented 1 year ago

I ran the validate_patches script locally with the verbose parameter and got the following errors. Looks like the use-package-optimizations patch is entirely outdated and has the wrong path for build.gn ~and is missing the files it's supposed to patch~. Debian's patcher just skips it rather than throw an error. https://0bin.net/paste/exfFIAXz#Cgpltdp-/pnx4PcVPzMX7MrVXShzktE/eq0EFEoIeL/

This turned out to most likely be user error on my part when running the verification script. Forgot to change the -l parameter to the correct location.

SugaryHull commented 1 year ago

I fixed the patches and the CI is happy. Are we good to merge or are my changes too specific to Debian 11?

SugaryHull commented 1 year ago

One minor nit: .65 is the Windows release, Linux packaging should target .64 instead

Ah. I'll fix that and verify that the patches still work before committing.

SugaryHull commented 1 year ago

Should be fixed now

ghost commented 1 year ago

This fork successfully builds in a clean Debian 11 chroot environment with no hacky changes or patches, just the refreshed patches I added to debian/patches. The switch to Clang/LLVM 13 was necessary because of a required build flag that can't be patched out from the build script without code changes. It is also hardcoded to use lld-13 by the lld-13 patch out of necessity, as Debian's (and I would assume Ubuntu's) updated Clang and LLD packages don't overwrite the symlinks provided by the base versions (Clang/LLVM 10 in focal and 11 in Bullesye). The old shim patch in ungoogled-chromium-debian was replaced with a new Debian upstream shim patch that's much simpler. The Clang version check bypass was sourced from Gentoo's patch pool for 110.

This may present a problem with OBS, using a newer Clang. Clang 13 is available via Debian backports on Bullseye but may be a no go as I recall it being hard to get backports working. For Ubuntu, we can just shift the window to Jammy and drop Focal. It's past the specified support window anyway and Clang 13 is available in the standard repositories. But I do not see this as a reason to reject this PR.

Speaking of, have you checked whether the JS performance issues are still present? We need to make sure that is addressed before we put out any new builds.

ghost commented 1 year ago

See #247 for the issue I was trying to get to the bottom of.

SugaryHull commented 1 year ago

This may present a problem with OBS, using a newer Clang. Clang 13 is available via Debian backports on Bullseye but may be a no go as I recall it being hard to get backports working.

Clang/LLVM 13 is in the main Debian repos for Bullseye and has been since June 2022 according to the changelog file. IIRC, it was included in mainline Debian 11 to build either Chromium or Firefox ESR 102 https://packages.debian.org/bullseye/clang-13 https://metadata.ftp-master.debian.org/changelogs//main/l/llvm-toolchain-13/llvm-toolchain-13_13.0.1-6~deb11u1_changelog

ghost commented 1 year ago

It says "Backport to bullseye". Backports are an entirely separate repository and are not usually enabled by default. That's what I was getting at, that getting them to be accessible to the builder may prove difficult. That said it appears OBS has backports as an option so I'll just have to see if I can get it to actually work this time.

It may be that I was having issues with unofficial repositories because when I first started working on this packaging we were using LLVM's own repositories which proved difficult or impossible to get working on OBS or Launchpad.

ghost commented 1 year ago

In any case it is a problem for me to work out. We may want to rebase the packaging against Bullseye once I officially remove Focal support. What this means is seeing if we can re-enable support for system libraries that were previously disabled because the previous baseline didn't have a version that was compatible with the latest chromium but it's just a bit newer than Focal and may not have anything worth considering.

SugaryHull commented 1 year ago

It says "Backport to bullseye". Backports are an entirely separate repository and are not usually enabled by default.

That doesn't mean it's in bullseye-backports, just that it was backported to the main repo specifically for use in another program's build system. I also linked the Debian packages page for Clang 13 on Debian 11, which says it's in the main Bullseye repo, but I'll also link my sources.list file and the output of apt list | grep clang-13 https://0bin.net/paste/VT-RTC+o#eQyc1F20xRmBi8Dtn8CmJ4tgi95JDXNG6ilaPO39s8Y

SugaryHull commented 1 year ago

but it's just a bit newer than Focal and may not have anything worth considering.

Bullseye is going to become Debian OldStable fairly soon anyway, likely by the end of Q2 2023/start of Q3 2023.

ghost commented 1 year ago

Ok, the backports website doesn't even list Clang for bullesye. It seems my information is outdated. There was a time anything that had to be "backported" on Debian, like a newer compiler, would never be placed in the main repositories. I guess Debian is making exceptions to that now.

SugaryHull commented 1 year ago

I guess Debian is making exceptions to that now.

Two exceptions were made in this case. This was uploaded to the main repo and it was a non-maintainer upload. I think building browsers is the only exception Debian has made to the backporting rules in recent memory

SugaryHull commented 1 year ago

Speaking of, have you checked whether the JS performance issues are still present? We need to make sure that is addressed before we put out any new builds.

I'll let .64 build while I'm at work and I'll test it after work

ghost commented 1 year ago

Speaking of, have you checked whether the JS performance issues are still present? We need to make sure that is addressed before we put out any new builds.

I'll let .64 build while I'm at work and I'll test it after work

It's fine if it's not but we need to resolve it if it's still a problem. I just couldn't figure out why it was happening. I had looked into runtime flags, build flags, patch differences, and the like and came up empty. But if it's still a problem it can be investigated as a separate problem. This PR is just for upgrading to the latest release.

SugaryHull commented 1 year ago

It's fine if it's not but we need to resolve it if it's still a problem. I just couldn't figure out why it was happening. I had looked into runtime flags, build flags, patch differences, and the like and came up empty. But if it's still a problem it can be investigated as a separate problem. This PR is just for upgrading to the latest release.

I got a Jetstream2 score of 222.289. The only other Chromium 111 score I could find was this one with a score of 259.280. The difference is most likely caused by that particular score being achieved on a different CPU than mine. Mine's a Ryzen 7 5700G and their's is a Ryzen 9 5900HX. The Speedometer score is pretty different, though; 181 +/- 7.4 vs 262 +/- 0.33

SugaryHull commented 1 year ago

I also had no issues with uBlock Origin, which is also an unpacked extension

ghost commented 1 year ago

I'm going to build it and test it in a VM to see if I can still reproduce the original reported bug as the reporter described.

ghost commented 1 year ago

I just got a build failure from missing QT5 dependencies. I added them to GIT already but is this now expected? I mean, did you intend for this to be enabled by default now?

ghost commented 1 year ago

I'll leave it for the moment but I'm not sure if we should leave it enabled or not. But I just found the flag for disabling it.

SugaryHull commented 1 year ago

I just got a build failure from missing QT5 dependencies. I added them to GIT already but is this now expected? I mean, did you intend for this to be enabled by default now?

It was enabled when I forked. I had forgotten about that and didn't document installing Qt5 in my notes. Apologies

ghost commented 1 year ago

Ok. Well I'll see if it gets to remain the default or not. Seems Debian doesn't yet enable it but Arch does.