Open klzgrad opened 2 years ago
https://github.com/klzgrad/naiveproxy/releases/tag/cronet-test4
Looks like win build is missing
All targets except darwin have gcc's cross kit, I think the script is only needed for darwin amd64 and darwin arm64.
Or just run the compilation natively in the darwin github runner.
What is the openwrt os in naiveproxy? They seem to be basically linux
OpenWrt has different libc and many different arch specific flags.
Windows builds missing in cronet test4 because of flaky test case. Don't matter.
The script will be using clang and replicate chromium toolchain, so no gcc here.
Before doing anything is first to make plans, besides the targets already support in naiveproxy CI, what else should be included in the scope. E.g. freebsd, riscv, big endian archs. If unable to support, how to deal with it.
I try locally:
GOARCH=arm64 CC="clang -target arm64-pc-linux-gnu --sysroot /usr/aarch64-linux-gnu -fuse-ld=lld -Wno-unused-command-line-argument" CGO_LDFLAGS_ALLOW="-fuse- ld=lld" CGO_ENABLED=1 go build -v -o naive-go ./naive
but get:
runtime/cgo
#runtime/cgo
ld.lld: error: cannot open crtbeginS.o: No such file or directory
ld.lld: error: unable to find library -lgcc
ld.lld: error: unable to find library -lgcc
ld.lld: error: cannot open crtendS.o: No such file or directory
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
I'll be clear: Don't spend effort on toolchains. It's my problem to solve.
The first problem, define the list of supported targets. Ok, let me list unsupported or unknown targets that need discussion/investigation to decide:
I have no experience with uncommon targets, so maybe it's up to you.
It doesn't make sense to do executable static linking on Android because executables are usually not directly run, only mediated through Java.
FreeBSD is officially not supported by Chromium, see https://github.com/freebsd/chromium. And there is at least one actual case of incompatibility (root trust store being different on BSD). So it's OK to not support BSDs at this time.
Is dynamic linking with cross compile simple? I assume it's no more than using vanilla cross gcc to link libcronet.so without special flags, and there are known recipes for doing so?
The question with dynamic linking is, apart from Android, who else is going to use it? It's slightly easier to build but produces an extra file in deployment. This question determines which platforms are included in the dynamic linking showcase.
Static linking questions:
Is it considered bad practice in Go with using CGO_CFLAGSALLOW=.* or very long CGO{C,LD} FLAGS?
If link_shared.go is different for every platform, is it okay to generate this file at build time, or prepare a different variant of this file for each platform, or use #cgo pkg-config and generate cornet.pc? (build constraints can't represent openwrt variants)
is it okay to generate this file at build time, or prepare a different variant of this file for each platform, or use #cgo pkg-config and generate cornet.pc?
Generate a file at build time might be hard, it is not possible to run arbitrary code during go build
, nor there are built-in code-generator support suitable for our use case (go generate
is disabled for dependencies).
I'd suggest preparing different variants for each platform. No idea on OpenWRT variants though, maybe (ab)use go build -tags
?[1] Given that we already have to tell users a lot of env vars to set (CC, CGO_LDFLAGS_ALLOW, etc). This doesn't sound too bad.
[1] Example: go-sqlite3 uses tags to conditionally enable optional features.
prepare a different variant of this file for each platform
There are three ways.
// #cgo CFLAGS: -DPNG_DEBUG=1
// #cgo amd64 386 CFLAGS: -DX86=1
// #cgo LDFLAGS: -lpng
https://pkg.go.dev/cmd/go#hdr-Build_constraints
//go:build (linux && 386) || (darwin && !cgo)
If a file's name, after stripping the extension and a possible _test suffix, matches any of the following patterns:
*_GOOS
*_GOARCH
*_GOOS_GOARCH
(example: source_windows_amd64.go) where GOOS and GOARCH represent any known operating system and architecture values respectively, then the file is considered to have an implicit build constraint requiring those terms (in addition to any explicit constraints in the file).
Edit: Use https://github.com/klzgrad/naiveproxy/releases/tag/cronet-test8 now.
See if this works well here, also if there is need to change the file organization.
x64 is the naming convention of Chromium. x86_64 is the naming convention of OpenWrt. The two naming conventions have different levels of granularity and cannot replace each other in one-to-one mapping. Example: linux-arm means armv7-neon by default (though I haven't proved the exact config used by Chrome's official release), even though the Debian sysroot used to build it is "armhf", i.e. Armv7 CPU with Thumb-2 and VFP3D16. So, there are two architecture naming conventions, and they have to coexist.
https://github.com/SagerNet/cronet-go/commit/61c4e931991e709db871ebd03920cb3e5f7e8d13
Not quite what I imagined. The main issue I see is you're manually hardcoding build flags in Go files, causing very high maintaince cost in the future.
The reason I put the flag plumbing code in shell scripts (make-cronet-cgo-sdk.sh, go-build.sh, go_env.sh) is it can react to changes better. The build flags are generated by the Chromium build system, which is subject to changes in (1) Chromium upstream updates, (2) GN options at the level of naiveproxy for the purpose of creating proxy apps, (3) Ninja build flags generated from GN files, (4) changes in the special handling to work around CGO oddities. There are many places that can change, and the script can barely keep up with all the subtleties even with the already detailed comments. Now you abstract and break up the build flags into small pieces with different organization and their original contexts are lost, and if there are any issues down the line, it's pure churn to trace back to the original state. The shell scripts tried very hard to minimize special cases and keep to the original as close as possible, so to minimize possible future maintanance cost.
Example of the kind of changes that can happen:
Each of the above changes can cause the final build flags to change.
According to my tests, the generated link_*.go files must always be included in the same go package that uses cronet, otherwise go will isolate them.
I recommend using json instead of shell scripts to save build parameters, otherwise it's not easy for go scripts to use it.
will isolate them
How to understand this?
using json
ok, np.
link_shared.go only exists because https://github.com/golang/go/issues/25930. Some official Chromium flags are needed for LTO build and they cannot appear twice. CGO duplicates flags in CGO_LDFLAGS but not from #cgo LDFLAGS
. So those flags have to be put in link_shared.go.
Edit:
instead of shell scripts
go-build.sh does these things which I don't know Go can do:
go_env.sh can be expressed in json, no problem.
go-build.sh does these things which I don't know Go can do
I think all of these can be done with go, e.g. to download llvm, just specify the version:
https://github.com/SagerNet/cronet-go/tree/main/cmd/prebuild/main.go
all of these can be done with go, e.g. to download llvm, just specify the version
I'm sure it can, but why? It's one line of code in shell script.
How to understand this?
For example: libcronet ignored by linker?
I'm sure it can, but why? It's one line of code in shell script.
It acts as an environment initialization tool and a wrapper for go build that can be invoked by go run from third-party projects, while shell scripts are more complex.
invoked by go run from third-party projects,
Ok, Go land assumes no alien element. That's a reason.
libcronet ignored by linker?
It is because ./libcronet.so is a relative path? https://stackoverflow.com/questions/28037827/how-to-use-a-relative-path-for-ldflags-in-golang
Does it work if it is ${SRCDIR}/libcronet.so
?
./libcronet.so currently works, but not ${SRCDIR}. It points to the absolute path of the cronet-go package.
Assuming a standalone project a depends on cronet-go, running the predefined prebuild and build programs seems to be the easiest way. LDFLAGS added in a cannot be used by cronet-go, just like runtime/cgo does not use LDFLAGS added by cronet-go.
Again, I'm against hand coding a series of link_*.go files for every platform because it's miserable to maintain.
But I'm not against automatically creating a series of link*.go files. Best is if it can be created at build time from json files, so it creates no churn in git history. Worse is if there is a tool that automatically update the link*.go files from latest json files, messing up git history but requires no great manual intervention. It's terrible if you have to update link_*.go each time naiveproxy syncs with upstream.
Update link_*.go each time naiveproxy syncs with upstream
It seems to be the only way.
And I think each cronet-go version needs a fixed upstream version, always latest will cause more problems.
The go-build.json would look like this:
{
"host_arch": "One of Linux, Windows, Darwin",
"target_cpu": "In Chromium's naming convention: x64, x86, arm, arm64, mipsel, mips64el. Can be empty.",
"clang_host_arch": "One of Linux_x64, Win, Mac",
"clang_revision": "Download Clang from https://commondatastorage.googleapis.com/chromium-browser-clang/<clang_host_arch>/clang-<clang_revision>.tgz",
"android_img": "Android build can run with rootfs from https://dl.google.com/android/repository/sys-img/android/<android_img>.zip",
"pie": "Whether pie buildmode is supported, boolean",
"mips_float_abi": "One of soft, hard; or empty"
"CGO_CFLAGS": "value in environment variable CGO_CFLAGS",
"CGO_LDFLAGS": "value in environment variable CGO_LDFLAGS",
"shared_ldflags": "value in #cgo LDFLAGS in link_shared_*.go",
"static_ldflags": "value in #cgo LDFLAGS in link_static_*.go",
}
Terrible to update link*.go each time naiveproxy syncs with upstream manually. It's survivable if there is a script to automatically update all the link*.go from go-build.json.
Always latest is probably a requirement, because not being the latest will quickly become out of date than the latest Chrome. It's all about future maintainance cost, how much time does it take to update. If it's automated and checked by CI, it's just updating a version number.
each cronet-go version needs a fixed upstream version
Sure, I assume cronet-go would release in sync with Chromium upstream, with version numer like 101.0.4951.41.
It would be better to create a separate go-build.json with all the targets as a json array, otherwise each update will download all the targets' unrelated files.
Defining a specification for the json file is also a maintanance burden.
I assume (each version of) cronet-go would submodule naiveproxy to a fixed version. Vendoring is not viable because naiveproxy has no linear commit history; it rebases from new roots too often.
But, I propose the build artifacts of libcronet.so and libcronet_static.a used by cronet-go should be generated by CI pipelines in this repo, not downloaded from naiveproxy releases, because releases are not reliable (Edit: I delete old releases to avoid accumulating download counts) and a single version can create multiple releases.
And use a Go tool to directly replicate the parsing logic in make-cronet-cgo-sdk.sh, with the input from ./out/Release/obj/components/cronet/cronet_example_external.ninja
generated during CI pipelines. With this there is no need to define, create and collect go-build.jsons. There is a small cost in keeping the parsing login between make-cronet-cgo-sdk.sh and make_cronet_cgo_sdk.go in sync, but I can do it.
Also, I'm not against breaking up the build flags per se, but the breaking-up needs to be automated so changes can flow to the broken up files automatically.