Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[build.webkit.org] Add WebKitGTK and WPE bots for Ubuntu 22.04 #1323

Closed
wants to merge 3 commits into from
Closed

[build.webkit.org] Add WebKitGTK and WPE bots for Ubuntu 22.04 #1323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91022fc
[RISC-V] Build fixes after 251455@main
zdobersek Jun 14, 2022
fe40e70
run-benchmark script fails to find the lsof command on Linux
clopez Jun 14, 2022
c28c6a1
[build.webkit.org] Add WebKitGTK and WPE bots for Ubuntu 22.04
dpino Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/llint/WebAssembly.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const NumberOfWasmArguments = NumberOfWasmArgumentJSRs + NumberOfWasmArgumentFPR
# All callee saves must match the definition in WasmCallee.cpp

# These must match the definition in WasmMemoryInformation.cpp
if X86_64 or ARM64 or ARM64E
if X86_64 or ARM64 or ARM64E or RISCV64
const wasmInstance = csr0
const memoryBase = csr3
const boundsCheckingSize = csr4
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/wasm/WasmMemoryInformation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const PinnedRegisterInfo& PinnedRegisterInfo::get()
unsigned numberOfPinnedRegisters = 2;
if (!Context::useFastTLS())
++numberOfPinnedRegisters;
#if CPU(X86_64) || CPU(ARM64)
#if CPU(X86_64) || CPU(ARM64) || CPU(RISCV64)
GPRReg baseMemoryPointer = GPRInfo::regCS3;
GPRReg boundsCheckingSizeRegister = GPRInfo::regCS4;
#elif CPU(ARM)
Expand Down
22 changes: 19 additions & 3 deletions Tools/CISupport/build-webkit-org/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
{ "name": "gtk-linux-bot-18", "platform": "gtk" },
{ "name": "gtk-linux-bot-19", "platform": "gtk" },
{ "name": "gtk-linux-bot-20", "platform": "gtk" },
{ "name": "gtk-linux-bot-21", "platform": "gtk" },

{ "name": "jsconly-linux-igalia-bot-1", "platform": "jsc-only" },
{ "name": "jsconly-linux-igalia-bot-2", "platform": "jsc-only" },
Expand All @@ -135,7 +136,8 @@
{ "name": "wpe-linux-bot-7", "platform": "wpe" },
{ "name": "wpe-linux-bot-8", "platform": "wpe" },
{ "name": "wpe-linux-bot-9", "platform": "wpe" },
{ "name": "wpe-linux-bot-10", "platform": "wpe" }
{ "name": "wpe-linux-bot-10", "platform": "wpe" },
{ "name": "wpe-linux-bot-11", "platform": "wpe" }
],

"builders": [
Expand Down Expand Up @@ -467,11 +469,17 @@
"workernames": ["gtk-linux-bot-10"]
},
{
"name": "GTK-Linux-64-bit-Release-Ubuntu-LTS-Build", "factory": "BuildFactory",
"name": "GTK-Linux-64-bit-Release-Ubuntu-2004-Build", "factory": "BuildFactory",
"platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
"additionalArguments": ["--no-experimental-features"],
"workernames": ["gtk-linux-bot-11"]
},
{
"name": "GTK-Linux-64-bit-Release-Ubuntu-LTS-Build", "factory": "BuildFactory",
"platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
"additionalArguments": ["--no-experimental-features"],
"workernames": ["gtk-linux-bot-21"]
},
{
"name": "GTK-Linux-64bit-Release-Packaging-Nightly-Ubuntu1804", "factory": "BuildAndGenerateMiniBrowserBundleFactory",
"platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
Expand Down Expand Up @@ -613,18 +621,26 @@
"workernames": ["wpe-linux-bot-9"]
},
{
"name": "WPE-Linux-64-bit-Release-Ubuntu-LTS-Build", "factory": "BuildFactory",
"name": "WPE-Linux-64-bit-Release-Ubuntu-2004-Build", "factory": "BuildFactory",
"platform": "wpe", "configuration": "release", "architectures": ["x86_64"],
"additionalArguments": ["--no-experimental-features"],
"workernames": ["wpe-linux-bot-10"]
},
{
"name": "WPE-Linux-64-bit-Release-Ubuntu-LTS-Build", "factory": "BuildFactory",
"platform": "wpe", "configuration": "release", "architectures": ["x86_64"],
"additionalArguments": ["--no-experimental-features"],
"workernames": ["wpe-linux-bot-11"]
}
],

"schedulers": [ { "type": "AnyBranchScheduler", "name": "main", "change_filter": "main_filter", "treeStableTimer": 45.0,
"builderNames": ["GTK-Linux-64-bit-Release-Build", "GTK-Linux-64-bit-Debug-Build",
"GTK-Linux-64-bit-Release-Clang",
"GTK-Linux-64-bit-Release-Debian-Stable-Build",
"GTK-Linux-64-bit-Release-Ubuntu-2004-Build",
"GTK-Linux-64-bit-Release-Ubuntu-LTS-Build",
"WPE-Linux-64-bit-Release-Ubuntu-2004-Build",
"WPE-Linux-64-bit-Release-Ubuntu-LTS-Build",
"JSCOnly-Linux-AArch64-Release",
"JSCOnly-Linux-ARMv7-Thumb2-Release", "JSCOnly-Linux-ARMv7-Thumb2-SoftFP-Release",
Expand Down
25 changes: 24 additions & 1 deletion Tools/CISupport/build-webkit-org/factories_unittest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,17 @@ class TestExpectedBuildSteps(unittest.TestCase):
'delete-stale-build-files',
'compile-webkit'
],
'GTK-Linux-64-bit-Release-Ubuntu-2004-Build': [
'configure-build',
'configuration',
'clean-and-update-working-directory',
'checkout-specific-revision',
'show-identifier',
'kill-old-processes',
'delete-WebKitBuild-directory',
'delete-stale-build-files',
'compile-webkit'
],
'GTK-Linux-64bit-Release-Packaging-Nightly-Ubuntu1804': [
'configure-build',
'configuration',
Expand Down Expand Up @@ -1633,7 +1644,19 @@ class TestExpectedBuildSteps(unittest.TestCase):
'delete-stale-build-files',
'jhbuild',
'compile-webkit'
]
],
'WPE-Linux-64-bit-Release-Ubuntu-2004-Build': [
'configure-build',
'configuration',
'clean-and-update-working-directory',
'checkout-specific-revision',
'show-identifier',
'kill-old-processes',
'delete-WebKitBuild-directory',
'delete-stale-build-files',
'jhbuild',
'compile-webkit'
],
}

def setUp(self):
Expand Down
5 changes: 4 additions & 1 deletion Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
import re
import subprocess
import shutil
import sys
import time

Expand Down Expand Up @@ -66,7 +67,9 @@ def _find_http_server_port(self):
self._server_port = connections[0].laddr[1]
except ImportError:
try:
output = subprocess.check_output(['/usr/sbin/lsof', '-a', '-P', '-iTCP', '-sTCP:LISTEN', '-p', str(self._server_process.pid)])
# lsof on Linux is shipped on /usr/bin typically, but on Mac on /usr/sbin
lsof_path = shutil.which('lsof') or '/usr/sbin/lsof'
output = subprocess.check_output([lsof_path, '-a', '-P', '-iTCP', '-sTCP:LISTEN', '-p', str(self._server_process.pid)])
self._server_port = int(re.search(r'TCP .*:(\d+) \(LISTEN\)', str(output)).group(1))
except Exception as error:
_log.info('Error: %s' % error)
Expand Down
15 changes: 13 additions & 2 deletions Tools/glib/dependencies/apt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,25 @@

# If the package $1 is available, prints it. Otherwise prints $2.
# Useful for handling when a package is renamed on new versions of Debian/Ubuntu.
function aptIfElse {
aptIfElse() {
if apt-cache show $1 &>/dev/null; then
echo $1
else
echo $2
fi
}

aptIfExists() {
local ret=$(apt show "$1" 2>/dev/null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dpino I'm not going to pretend to understand quite why this is the case, but per this comment this formulation needs to look instead like local ret; ret=$(apt show "$1" 2>/dev/null) (but maybe with a newline? see below), otherwise $? always ends up 0 and running Tools/gtk/install-dependencies will try to install a bunch of nonexistent packages like "libenchant-dev" and this breaks the install request such that important packages like "ruby" won't be installed.

I ran into this when trying to update the searchfox wubkat tree to use the new explicit support for Ubuntu 22.04 by using the install-dependencies script and the configure step failed because there was no ruby because the call to "install-dependencies" only installed a few python packages. I added set -x to the script so I could see what the commands were and it was clear from the output that the evaluation of $? in the next line was incorrectly 0 so I searched and found the above stackoverflow comment and when I changed the script to instead have:

    local ret;
    ret=$(apt show "$1" 2>/dev/null)

the script then installed everything as expected. I only tried this formulation with the newline, I didn't try on the same line.

Note that I think it's also the case that apt is explicitly not supposed to be used for scripting so it might be better to use apt-cache show here like is used above? Like when used in a sub-shell it complains "WARNING: apt does not have a stable CLI interface. Use with caution in scripts."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. This was indeed a source of errors that was preventing running install-dependencies in different platforms.

The reason why the code isn't working as it's writen is because in Bash an assigment always returns exit code 0. So, as a rule of thumb, is generally wrong in Bash to initialize a variable and inmediately after check the result of its assigned expression. That's why it's necessary to break down the statement into declaration of var, assigned expression and evaluation of result. I remember @aperezdc actually warned me about this once in another review of this code.

Unrelated to this but perhaps you already noticed you also needed to manually install the package unidef. It's pending to be added in apt dependencies file.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shell-wise: Aha, that does make sense, thanks! That will probably make a good addition to https://github.com/mozsearch/mozsearch/blob/master/docs/bash-scripting-cheatsheet.md

Build-wise: Indeed, the build broke because of (I'm assuming there's a missing "f" typo in your current comment) unifdef not being installed and then I (very gratefully!) realized that support for Ubuntu 22.04 had been added so I thought I'd try using it. I've restored the "wubkat" indexer behavior to fail if the build step fails, so once the build starts working again wubkat should go back to always having semantic data available at the trade-off of the webkit tree seeming to have fatal build errors surprisingly frequently so data will be stale when that happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the name of the package is unifdef. I noticed it has been already added in Tools/gtk/dependencies/atk in the patch that introduced this dependency 253528@main. So nothing pending to be done here.

We're currently in the middle of an upgrade of webkit-search.igalia.com to Ubuntu 22.04 (plus upgrade of the mozsearch checkout). I've been reviewing your wubkat scripts. They're very useful, I learned many things. There are other things, on the hand, that could be improved or you'd like to reconsinder. I think I will open a PR to comment on the code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A PR would be fantastic, thank you! I very much am not familiar with building webkit and mainly was just trying to get things working at all, and clearly leaned very heavily on your existing (public) config and the config in emilio's branch.

The canonical searchfox discussion channel is https://chat.mozilla.org/#/room/#searchfox:mozilla.org (and maybe it's federated?) if you want to chat more about searchfox outside of specific pull requests, etc. I'll actually be working this week on finishing implementing a TOML configuration file that will allow for custom mappings for what's a test file and allow for more normalized ingestion of JSON files with per-file info, plus support for custom templating to augment the rendered HTML as an evolution of the current test info / bugzilla mapping / coverage data ingestion mechanism we use for mozilla-central.

Copy link
Contributor Author

@dpino dpino Aug 22, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created bug and opened PR to fix the aptIfExists error and update system library dependencies #3525

if [[ $? -ne 0 ]]; then
return
fi
if [[ "$ret" =~ "(virtual)" ]]; then
return
fi
echo "$1"
}

PACKAGES=(
# These are dependencies necessary for building WebKitGTK/WPE.
autoconf
Expand Down Expand Up @@ -58,9 +69,9 @@ PACKAGES=(
libcgi-pm-perl
psmisc
pulseaudio-utils
python-gi
ruby-highline
ruby-json
$(aptIfExists python-gi)

# These are dependencies necessary for building with the Flatpak SDK.
flatpak
Expand Down
18 changes: 9 additions & 9 deletions Tools/gtk/dependencies/apt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ PACKAGES+=(
geoclue-2.0
gnome-common
libedit-dev
libenchant-dev
libfaad-dev
libffi-dev
libgirepository1.0-dev
Expand Down Expand Up @@ -35,24 +34,24 @@ PACKAGES+=(
libxtst-dev
nasm
xfonts-utils
$(aptIfExists libenchant-dev)

# These are dependencies necessary for running tests.
cups-daemon
dbus-x11
hunspell
hunspell-en-gb
hunspell-en-us
python-psutil
python-yaml
weston
xvfb
$(aptIfExists python-psutil)
$(aptIfExists python-yaml)

# These are dependencies necessary for building the jhbuild.
bison
flex
gobject-introspection
icon-naming-utils
libcroco3-dev
libcups2-dev
libdrm-dev
libevdev-dev
Expand All @@ -75,18 +74,19 @@ PACKAGES+=(
libxfont-dev
libxkbcommon-x11-dev
libxkbfile-dev
python-dev
ragel
x11proto-bigreqs-dev
x11proto-composite-dev
x11proto-gl-dev
x11proto-input-dev
x11proto-randr-dev
x11proto-resource-dev
x11proto-scrnsaver-dev
x11proto-video-dev
x11proto-xcmisc-dev
x11proto-xf86dri-dev
xtrans-dev
xutils-dev
$(aptIfExists libcroco3-dev)
$(aptIfExists python-dev)
$(aptIfExists x11proto-bigreqs-dev)
$(aptIfExists x11proto-composite-dev)
$(aptIfExists x11proto-resource-dev)
$(aptIfExists x11proto-xcmisc-dev)
)