ungoogled-software / ungoogled-chromium-archlinux

Arch Linux packaging for ungoogled-chromium
BSD 3-Clause "New" or "Revised" License
340 stars 36 forks source link

Build failures #123

Closed ghost closed 3 years ago

ghost commented 3 years ago

I've tested ours and Arch's official packaging and they all fail with the same error. The only common factor I can see is that these builds are all in freshly created containers or other environments. I'm assuming Arch only succeeds due to using an older installation to build.

@networkException You have any thoughts on this issue? I'm surprised no one has opened a bug on the Arch bug tracker given that it also impacts their official package too.

networkException commented 3 years ago

Interesting, where did you observe the issues (meaning just obs or locally and / or in docker)?

I've been reworking the CI pipeline on a local branch, basically building a docker image out of all built files required for a given versions (PKGBUILD, patches, chromium tar) and using that as a base for actually building, for the last day and although I haven't let ninja run further than ~3k objects it didn't complain really.

I originally planned to discuss the docker branch when its more mature but it might be relevant to talk about it already, so I'd like to ask about an opinion on it. My main motivation is reducing repeated code that has to run in every build_x job at the moment, as well as focusing on making ungoogled-software/ungoogled-chromium-archlinux more of a "single source of truth", maintaining its own tooling as much as possible to mitigate trust concerns and having better control over build behavior.

With the rework I would have also planned to offer "build using docker" as an option for anyone interested in building a release themselves, just needing to pull our latest stable image to get started.

Until now my take on the build issues in general were obs having some different version of either python or pipewire installed (I might be wrong) so maybe a next step after an initial migration of the github ci to use our own container would be to use the same container (without the 6h build timeout) in obs (only if thats possible of course) as it would effectively give us full control over the build context.

ghost commented 3 years ago

AFAIK OBS creates a fresh container image for each build. This has never been an issue for the other distributions we build on OBS and until Chromium 90 was not an issue on Arch either. It's why I suspect it's some kind of upstream issue because of how volatile Arch is even with all their precautions.

ghost commented 3 years ago

The error I see is this: flatc failed with exit code -5

And the AUR users also have the same issue. So it seems to be an issue for more than just us. See here in the comments: https://aur.archlinux.org/packages/ungoogled-chromium

nikolowry commented 3 years ago

The error I see is this: flatc failed with exit code -5

And the AUR users also have the same issue. So it seems to be an issue for more than just us. See here in the comments: https://aur.archlinux.org/packages/ungoogled-chromium

Additional context to hopefully helps with investigating:

nikolowry commented 3 years ago

I saw chromium (https://archlinux.org/packages/extra/x86_64/chromium/) had 4 updates over the past 16 days, but trying to build locally with makepkg results in the same error:

[5816/48013] ACTION //extensions/browser/api/declarative_net_request/flat:extension_ruleset_gen(//build/toolchain/linux/unbundle:default)
FAILED: gen/extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h 
python ../../build/gn_run_binary.py flatc -c --keep-prefix -o gen/extensions/browser/api/declarative_net_request/flat -I ../../ ../../extensions/browser/api/declarative_net_request/flat/extension_ruleset.fbs
flatc failed with exit code -5
[5833/48013] ACTION //third_party/blink/renderer/bindings:generate_bindings_all(//build/toolchain/linux/unbundle:default)
ninja: build stopped: subcommand failed.

@foutrelis and @felixonmars any insights you could provide?

ghost commented 3 years ago

I noticed that Arch also provides flatc in their flatbuffers or so package. Maybe we can switch to using the system flatc to get past this issue.

networkException commented 3 years ago

Part of the problem seems to actually be the flatc binary provided in the chromium sources. I added a quick hack to build/gn_run_binary.py

if args[0] == './flatc':
    args[0] = '/usr/bin/flatc';

that makes it build further. Both binaries output flatc version 1.12.0

ghost commented 3 years ago

That's built on the fly for Debian too but it doesn't error out there despite using the same sources. So either there's enough differences in the patches to break flatc or there's a toolchain issue here. We patch Arch less than we patch Debian so I would suspect the latter is more likely.

Ahrotahn commented 3 years ago

I believe I figured it out:
Remove -fcf-protection from your makepkg.conf CFLAGS and start a clean build.


I actually didn't have any issues building and didn't notice this until @braewoods commented about it on the main issue tracker.
Clean builds of both ungoogled and regular chromium built fine for me, so I set up a container this morning and finally encountered the issue.

flatc is being built and isn't shipped as a binary, so it had to be compiling differently somehow.
I ran strace on flatc and found that it was encountering a breakpoint and exiting with a SIGTRAP which is really odd.
I checked gdb to find where this was happening but could not find anything in the code that would cause this.

After thinking further on this I realized that something had to have changed with arch as the code was identical in both the container and outside. Since I was running the same kernel and software versions the only thing that made sense was a config difference. I ran pacdiff and sure enough there was a pacnew for makepkg.conf that I hadn't yet applied.

There was an update to pacman recently that introduced some new flags, including -fcf-protection. This explains why new installs encountered the issue while some with existing installs were not.

networkException commented 3 years ago

That would explain why the ci pipeline doesn't break either, it uses a custom config file

jstkdng commented 3 years ago

yeah, it uses an old makepkg.conf now how to ship this change to everyone without telling them to modify their makepkg.conf file?

Ahrotahn commented 3 years ago

The PKGBUILD could be updated to include CFLAGS=${CFLAGS/-fcf-protection} and CXXFLAGS=${CXXFLAGS/-fcf-protection}.

jstkdng commented 3 years ago

I'm trying something like that on my side but it keeps failing on the OBS

Edit: using your flags worked, thanks @Ahrotahn

ghost commented 3 years ago

Applied the workaround and launched a test build.

jstkdng commented 3 years ago
[ 1590s] [14019/47493] CXX obj/base/base/sha1_boringssl.o
[ 1590s] [14020/47493] CXX obj/base/base/platform_thread_linux.o
[ 1590s] [14021/47493] CXX obj/base/base/process_iterator_linux.o
[ 1590s] [14022/47493] CXX obj/base/base/persistent_histogram_storage.o
[ 1590s] [14023/47493] CXX obj/base/base/allocator_shim.o
[ 1590s] FAILED: obj/base/base/allocator_shim.o 
[ 1590s] clang++ -MMD -MF obj/base/base/allocator_shim.o.d -DUSE_SYMBOLIZE -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -DUSE_X11=1 -DOFFICIAL_BUILD -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNO_UNWIND_TABLES -D_GNU_SOURCE -DCR_CLANG_REVISION=\"llvmorg-13-init-1559-g01b87444-3\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBASE_IMPLEMENTATION -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_40 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_40 -DUSING_SYSTEM_ICU=1 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DUCHAR_TYPE=uint16_t -DU_IMPORT=U_EXPORT -I../.. -Igen -I../../third_party/perfetto/include -Igen/third_party/perfetto/build_config -Igen/third_party/perfetto -Igen/shim_headers/zlib_shim -Igen/shim_headers/icui18n_shim -I../../third_party/abseil-cpp -I../../third_party/boringssl/src/include -I../../third_party/protobuf/src -Igen/protoc_out -Igen/third_party/perfetto -fno-delete-null-pointer-checks -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -fno-unwind-tables -fno-asynchronous-unwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -flto=thin -fsplit-lto-unit -fwhole-program-vtables -m64 -march=x86-64 -msse3 -Xclang -fdebug-compilation-dir -Xclang . -no-canonical-prefixes -Wall -Wextra -Wimplicit-fallthrough -Wunreachable-code -Wthread-safety -Wextra-semi -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-psabi -Wno-ignored-pragma-optimize -Wno-implicit-int-float-conversion -Wno-final-dtor-non-final-class -Wno-builtin-assume-aligned-alignment -Wno-deprecated-copy -Wno-non-c-typedef-for-linkage -Wno-max-tokens -fno-omit-frame-pointer -g0 -ftrivial-auto-var-init=pattern -fsanitize=cfi-vcall -fsanitize-blacklist=../../tools/cfi/ignores.txt -fsanitize=cfi-icall -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wexit-time-destructors -Wshadow -Wglobal-constructors -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -Wexit-time-destructors -O2 -fno-ident -fdata-sections -ffunction-sections -DPROTOBUF_ALLOW_DEPRECATED=1 -std=c++14 -fno-trigraphs -Wno-trigraphs -fno-exceptions -fno-rtti -fvisibility-inlines-hidden  -D__DATE__=  -D__TIME__=  -D__TIMESTAMP__= -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2,-D_GLIBCXX_ASSERTIONS         -Wformat -Werror=format-security         -fstack-clash-protection  -Wno-builtin-macro-redefined -Wno-unknown-warning-option -c ../../base/allocator/allocator_shim.cc -o obj/base/base/allocator_shim.o
[ 1590s] ../../base/allocator/allocator_shim.cc:397:2: error: This code cannot be used when exceptions are turned on.
[ 1590s] #error This code cannot be used when exceptions are turned on.
[ 1590s]  ^
[ 1590s] 1 error generated.
[ 1591s] [14024/47493] CXX obj/base/base/address_pool_manager_bitmap.o
[ 1591s] [14025/47493] CXX obj/base/base/process_metrics_linux.o
[ 1591s] [14026/47493] CXX obj/base/base/address_pool_manager.o
[ 1591s] [14027/47493] CXX obj/base/base/allocator_shim_default_dispatch_to_partition_alloc.o
[ 1591s] [14028/47493] CXX obj/base/base/partition_alloc_hooks.o
[ 1591s] [14029/47493] CXX obj/base/base/partition_lock.o
[ 1591s] [14030/47493] CXX obj/base/base/partition_oom.o
[ 1592s] [14031/47493] CXX obj/base/base/partition_alloc_features.o
[ 1592s] [14031/47493] CXX obj/base/base/partition_alloc_features.o

It failed for me again, maybe more flags have to be removed

bsdice commented 3 years ago

Man, this bug was driving me nuts. Thanks guys or gals for the helpful tips. I reverted the flag changes and trying a build now. Hat-tip to @Ahrotahn for finding the flag change. I wonder what else might be breaking silently with this change. After flatc I saw protoc from protobuf package also failing during package build. Maintainers are in for a surprise on next bulk rebuild or version bump... Arch is on wrong track here imho. What every Linux desktop user needs in absolutely good working order is three things: Kernel, systemd, web browser. Just can't break any of that.

For reference here is the RFC that recommended the build flag change: https://gitlab.archlinux.org/archlinux/rfcs/-/blob/master/rfcs/0003-buildflags.rst#specification

Relevant commit for makepkg.conf: https://github.com/archlinux/svntogit-packages/commit/a790c389cb0fd2ddd35e1f581ee337f6891801fc#diff-3e341d2d9c67be01819b25b25d5e53ea3cdf3a38d28846cda85a195eb9b7203a

jstkdng commented 3 years ago

at this point I'm inclined to hard set our CFLAGS and ignore system ones.

ghost commented 3 years ago

Doesn't really matter to me what we do but I do want a solution. But given that this started from an upstream change I suspect we're going to have a lot of trouble ahead for us.

jstkdng commented 3 years ago

that array is used to keep libraries that shouldn't be deleted, not to use the system counterparts

jstkdng commented 3 years ago

Just managed to get it to build but the resulting package is unstable af.

jstkdng commented 3 years ago

@Area51Kacz it segfaults while loading any site, had to downgrade.

and... could we try to add this flatbuffers at this (declare -gA _system_libs) array?

nope, that is for only a few libraries, not third party binaries

jstkdng commented 3 years ago

@Area51Kacz

  1. not really, I'm building release builds on the OBS, can't get debug info from that, just sigsegv when trying to load a page
  2. already have those flags in my PKGBUILD, they don't fix this problem.

Edit: I get this 2021-05-02_18-19

jstkdng commented 3 years ago

I have it on opt depends, check it out: https://github.com/jstkdng/aur/blob/master/ungoogled-chromium/PKGBUILD

optdepends=('pipewire: WebRTC desktop sharing under Wayland'
            'kdialog: needed for file dialogs in KDE'
            'org.freedesktop.secrets: password storage backend on GNOME / Xfce'
            'kwallet: for storing passwords in KWallet on KDE desktops')
jstkdng commented 3 years ago

@Area51Kacz no

networkException commented 3 years ago

@Area51Kacz This has nothing to do with bromite and --no-sandbox is not an option to use in a privacy and security focused project like ungoogled-chromium. Please refrain from commenting unrelated things

networkException commented 3 years ago

I will not argue with you why it's an extremely bad idea to disable sandboxing in a modern browser. Again, please stop sending unrelated and useless comments

ghost commented 3 years ago

@Area51Kacz @networkException is correct. You're wasting our time. If you persist I will be forced to take action to correct it.

jstkdng commented 3 years ago

did you ban them @braewoods ?

ghost commented 3 years ago

No, I don't have that ability. I only deleted the comments.

jstkdng commented 3 years ago

Alright, finally managed to get a working build of uc

image

The fix was setting CFLAGS to previous values known to work

  # use fixed flags as system flags break build
  CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt"
  CXXFLAGS="$CFLAGS"

  # Facilitate deterministic builds (taken from build/config/compiler/BUILD.gn)
  CFLAGS+='   -Wno-builtin-macro-redefined'
  CXXFLAGS+=' -Wno-builtin-macro-redefined'
  CPPFLAGS+=' -D__DATE__=  -D__TIME__=  -D__TIMESTAMP__='
jstkdng commented 3 years ago

Maybe the arch devs will find a better solution in future releases, if they do we can update our own.

bsdice commented 3 years ago

I also succeeded with a build of 90.0.4430.93, after over 6+ hours compilation, using

CPPFLAGS="-D_FORTIFY_SOURCE=2" CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt" CXXFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt" LDFLAGS="-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now" MAKEFLAGS="-j$(($(nproc)+1))" DEBUG_CFLAGS="-g -fvar-tracking-assignments" DEBUG_CXXFLAGS="-g -fvar-tracking-assignments"

Quick check using VcXsrv X11 over network showed no crashes while browsing various sites.

So miscompile bug is upstream in clang, judging from PKGBUILD makedepends?

ghost commented 3 years ago

It's either that or they foolishly enabled a risky optimization globally. Not all optimizations are intended to be used in all cases.

bsdice commented 3 years ago

Or, they only tested the new flags thoroughly with gcc and not clang. For example https://github.com/archlinux/svntogit-community/blob/packages/flatbuffers/trunk/PKGBUILD depends on gcc-libs, so gcc.

In either case why is https://github.com/archlinux/svntogit-packages/commits/packages/chromium/trunk not showing the build defect? Pacman not yet updated on pkgbuild.com, or using older makepkg.conf overriding new one, or something else, would be interesting to know.

nikolowry commented 3 years ago

@Ahrotahn great catch! Makes total sense why the maintainers of chromium could push new builds while mine would fail locally.

@jstkdng I was able to build successfully with the latest AUR push!

Prior to that, I was also able to build successfully with the makepkg.conf changes by removing -fcf-protection and -fexceptions:

CFLAGS=${CFLAGS/-fexceptions}
CFLAGS=${CFLAGS/-fcf-protection}
CXXFLAGS="$CFLAGS"

# Facilitate deterministic builds (taken from build/config/compiler/BUILD.gn)
CFLAGS+='   -Wno-builtin-macro-redefined'
CXXFLAGS+=' -Wno-builtin-macro-redefined'
CPPFLAGS+=' -D__DATE__=  -D__TIME__=  -D__TIMESTAMP__='

But that resulted in a crash during page load like @jstkdng previously described. So it does seem the best route to take is hard setting the CFLAGS. I'm not a C/C++ developer, so I won't speculate on what the most optimal CFLAGS might be.

Regardless, great job everyone for working through this!

Area51Kacz commented 3 years ago

@bsdice its working (tried some news sites + youtube + radiostreaming ... id try if discord works too)

https://build.opensuse.org/package/binaries/home:Area51Kacz:branches:home:ungoogled_chromium:testing/ungoogled-chromium-arch222/Arch

_as a funny surprise... with your flags it builds 80% faster than with the (old) native flags and seems functional (13306 vs 23960) https://build.opensuse.org/package/show/home:Area51Kacz:branches:home:ungoogled_chromium:testing/ungoogled-chromium-arch_

skrat commented 3 years ago

So is this going to be available at some point? Right now the repo is empty.

https://download.opensuse.org/repositories/home:/ungoogled_chromium/Arch/x86_64

jstkdng commented 3 years ago

@skrat

It's available on my repository and in the aur

ghost commented 3 years ago

Well it's basically this way because of the dumpster fire that came about due to changes in Arch. I have yet to fix the build.

networkException commented 3 years ago

Alright, finally managed to get a working build of uc

image

The fix was setting CFLAGS to previous values known to work

  # use fixed flags as system flags break build
  CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt"
  CXXFLAGS="$CFLAGS"

  # Facilitate deterministic builds (taken from build/config/compiler/BUILD.gn)
  CFLAGS+='   -Wno-builtin-macro-redefined'
  CXXFLAGS+=' -Wno-builtin-macro-redefined'
  CPPFLAGS+=' -D__DATE__=  -D__TIME__=  -D__TIMESTAMP__='

I mean those flags work fine, why not use them until there's an upstream fix?

Area51Kacz commented 3 years ago

@networkException and these flags works on bare obs (clean instalation)

networkException commented 3 years ago

@braewoods I'm not really a C++ dev myself or anything, is there anything stopping us from using said build flags? There don't seem to be any runtime issues at all

ghost commented 3 years ago

@networkException Go ahead then. I'm not an Arch user myself so it really doesn't matter to me what you do as long as it builds.

Area51Kacz commented 3 years ago

and @networkException these flags leads to faster builds (its good regardless if we build on pc - laptop or the cloud

networkException commented 3 years ago

Can you stop commenting on closed issues about things that do not contribute to this project in any helpful way?

I'm extremely sorry for everyone that still gets notifications about this.

networkException commented 3 years ago

(someone wrote and deleted later)

@networkException well allow me to break the rules on necrobumping, but there is still no package in the OBS repo that you advertise in the README

Huh are you sure? I have the repository in my pacman conf (cat output of that above) and pacman can definitely find the package... I will quickly test installing in a docker container as I already installed the package manually from a workflow artifact image

DAC324 commented 2 years ago

The error I see is this: flatc failed with exit code -5 And the AUR users also have the same issue. So it seems to be an issue for more than just us. See here in the comments: https://aur.archlinux.org/packages/ungoogled-chromium

Additional context to hopefully helps with investigating:

The same applies to protoc and protozero_plugin. If compiled with the system clang 11, both fail with an exception (signal 5). You must use the clang version downloadable via the tools/update.py script. See below the amendments necessary to PKGBUILD for enabling the Google toolchain:

# uncomment if bundled clang is to be used
  echo "Download prebuilt clang from Google"
  cd "$srcdir/chromium-$pkgver"
  ./tools/clang/scripts/update.py
# If you don't want to trust prebuilt binaries (fair!),
# you can use tools/clang/scripts/package.py to build that same compiler from source.
# ./tools/clang/scripts/package.py

# For use of the bundled clang
# if not defined or commented out, system clang will be used
_clang_path="${srcdir}/chromium-${pkgver}/third_party/llvm-build/Release+Asserts/bin/"
  export CC="${_clang_path}clang"
  export CXX="${_clang_path}clang++"
  export AR="${_clang_path}llvm-ar"