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.

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

add recipe for EGL #3192

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

simeonschaub
Copy link
Member

No description provided.

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

It's unfortunate that Libglvnd provides a library with the same name. This will likely cause a warning if a user loads both packages

cd build
apk add py3-mako meson wayland-dev

# pkgconfig returns the wrong path here. It's an ugly hack, but it works
Copy link
Member

Choose a reason for hiding this comment

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

I'll have a look about what is going on there, but in any case you must delete this link at the end, otherwise it'll end up inside the tarball

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, right! I forgot.


# Bash recipe for building across all platforms
script = raw"""
mkdir build
Copy link
Member

Choose a reason for hiding this comment

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

The automatic installation of the license looks for files with reasonable names in the only directory inside /workspace/srcdir. If there are more than one directory here, this will fail. So it's better to enter the mesa directory and create the build folder inside it

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, will change that

@simeonschaub
Copy link
Member Author

It's unfortunate that Libglvnd provides a library with the same name. This will likely cause a warning if a user loads both packages

Will it just cause a warning or could it also lead to the wrong library being used?

Any idea what this means?

[08:56:32] /opt/aarch64-linux-gnu/bin/../lib/gcc/aarch64-linux-gnu/8.1.0/../../../../aarch64-linux-gnu/bin/ld: skipping incompatible /opt/x86_64-linux-musl/lib/libLLVM-11.so when searching for -lLLVM-11
[08:56:32] /opt/aarch64-linux-gnu/bin/../lib/gcc/aarch64-linux-gnu/8.1.0/../../../../aarch64-linux-gnu/bin/ld: cannot find -lLLVM-11
``

@giordano
Copy link
Member

Will it just cause a warning or could it also lead to the wrong library being used?

Whatever happens when two packages export the same variable. Warning + you must qualify the variable with its module?

@simeonschaub
Copy link
Member Author

Ah, I thought it was something specific to BB. I don't think it should matter too much then, since I think this warning is only shown in interactive usage, not when it's done in a package. Packages only need to remember to qualify the module.

@giordano
Copy link
Member

Now that I think of it, you can't dlopen two libraries with the same soname, the first one will win

# `julia build_tarballs.jl --help` to see a usage message.
using BinaryBuilder, Pkg

name = "EGL"
Copy link
Member

Choose a reason for hiding this comment

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

Why is this called EGL and not Mesa, which we have already, but only for Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was a bit confused about this as well TBH. I think the current MESA jll should definitely have a different name. I named this EGL, because that is what my goal of building was initially, but I will need GBM and GLESv2 as well, so I wasn't unhappy that they were built alongside as well. MESA does have additional components like OpenCL and some stuff relating to Xorg as well, which I don't build here, but naming this jll MESA probably makes a lot more sense than the package we currently call MESA.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we can't fold this into the existing Mesa_jll? You should be able to enable multiple APIs in one build, and make the Unix-only options conditional on the OS.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that sounds like a good plan. I plan to pick this up eventually, but I am just a bit busy ATM. Feel free to take this over if you are interested in this though.

@simeonschaub
Copy link
Member Author

Would you be okay with merging a fixed-up version of this with a more obscure name for now? I think we can probably separate out some of the components into separate jlls, but I looked into it a bit and the interdependencies and dealing with the right driver components didn't seem very straightforward.

@giordano
Copy link
Member

For the name, yes, I don't think calling this Mesa is an option at all at this point, so we have to stick to something like this. I'm a bit worried about the clash with libglvnd, that's a real concern at runtime. Have you seen how Linux distributions go about this? Maybe the mark the two packages as conflicting?

E/EGL/build_tarballs.jl Outdated Show resolved Hide resolved
@simeonschaub
Copy link
Member Author

Have you seen how Linux distributions go about this? Maybe the mark the two packages as conflicting?

It looks like Debian declares libegl as a dependency for libglvnd, so I assume that libglvnd can probably be made to use the libegl provided here. Of course that would also mean that libglvnd always depends on wayland. Arch actually seems to have a circular dependency between the two, so not sure what's going on there. I can try to make this recipe more minimal and split the components up a bit more, so we can maybe have them as separate jlls.

@simeonschaub
Copy link
Member Author

Oh, no wait! I think libegl actually comes from libglvnd in Debian? https://tracker.debian.org/pkg/libglvnd

@giordano
Copy link
Member

Yes, that's what I was looking at now: they just split the libglvnd source into multiple binary packages. But as far as I can see the package you're building here isn't involved

@giordano
Copy link
Member

It appears that the package they build out of this source has libEGL_mesa.so, not libEGL.so: https://packages.debian.org/sid/amd64/libegl-mesa0/filelist

@giordano
Copy link
Member

egl-lib-suffix sounds relevant

@giordano
Copy link
Member

Also, in Debian they build with glvnd=true (option in meson_options.txt), I don't know if that's useful for us

@jpsamaroo
Copy link
Contributor

So the way that things work between Mesa and Libglvnd is the following:

  • If building Mesa without -Dglvnd, libEGL.so is produced and will be directly opened by the client
  • If building Mesa with -Dglvnd, libEGL_mesa.so is produced. The client will open Libglvnd's libEGL.so, which will forward calls to libEGL_mesa.so

Basically, if you're going to link Libglvnd into a process, then Mesa better have been built with -Dglvnd. Otherwise you'll end up with two libEGL.so libraries, which will just not work correctly.

@giordano
Copy link
Member

Pinging @Wimmerer who's also interested in Mesa

@jpsamaroo
Copy link
Contributor

I think we should probably name this Mesa_EGL_jll, and (if possible) patch GLFW_jll to use an environment variable (or similar) to select between loading Libglvnd_jll and Mesa_EGL_jll. This is one of those cases where, without us building all of the graphics libs, we need to support differences in OS configurations manually.

@simeonschaub
Copy link
Member Author

Yeah, I double-checked locally that this doesn't make a difference

jpsamaroo added a commit to jpsamaroo/Yggdrasil that referenced this pull request Feb 23, 2022
giordano pushed a commit that referenced this pull request Feb 23, 2022
@jpsamaroo
Copy link
Contributor

I get different errors locally, relating to using an old version of Wayland_protocols_jll. I'll work on this locally and see how far I can get.

@jpsamaroo
Copy link
Contributor

Just a quick update: I've got a local build that works great with GLFW! I've been able to run a CImGui demo as well, proving that it actually works. I'm in the process of working out the last bug before pushing changes (which are somewhat extensive, but we also now enable a lot more mesa functionality).

Patch meson's finding of wayland-scanner
Patch mesa's loader to attempt relpath dlopen first
Enable X11 support
Build libGL.so
@jpsamaroo
Copy link
Contributor

Not sure what's up with the static_assert failure.

Aside: Do we need/want this built on glibc?

@jpsamaroo
Copy link
Contributor

Alternatively: this is now building the full set of userspace graphics drivers. Do we instead want to move this to Mesa_jll? That would imply that we would be shipping the entire userspace graphics stack (minus the compositor), and would be able to do away with Libglvnd.

@simeonschaub
Copy link
Member Author

Aside: Do we need/want this built on glibc?

Would be nice at least. Not everyone's using Alpine you know... 😅

Do we instead want to move this to Mesa_jll?

I don't really care either way. Perhaps @giordano has some thoughts?

@jpsamaroo
Copy link
Contributor

Not everyone's using Alpine you know...

Oops, sorry, I had assumed this was to fix issues on musl-based distros!

@simeonschaub
Copy link
Member Author

My initial motivation for this was actually because I had the silly idea to build a window manager entirely using BinaryBuilder...

@jpsamaroo
Copy link
Contributor

Awesome!

@giordano I'm 👍 to merging this. I can send another PR for the augmented platform support for supporting multiple LLVM versions.

M/Mesa_EGL/build_tarballs.jl Outdated Show resolved Hide resolved

# Dependencies that must be installed before this package can be built
dependencies = [
BuildDependency("Wayland_jll"), # FIXME: HostBuildDependency
Copy link
Member

@giordano giordano Feb 26, 2022

Choose a reason for hiding this comment

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

What does the FIXME refer to?

M/Mesa_EGL/build_tarballs.jl Outdated Show resolved Hide resolved
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
# TODO: symbol mangling

if with_platform_wayland
- dep_wl_scanner = dependency('wayland-scanner', native: true)
Copy link
Member

Choose a reason for hiding this comment

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

Uhm, why is this patch necessary? If they need a native wayland-scanner isn't the solution to add HostBuildDependency("Wayland_jll")? We do that already in

# Need a host Wayland for wayland-scanner
HostBuildDependency("Wayland_jll"; platforms=linux),
# Need a host Wayland for wayland-scanner
HostBuildDependency("Wayland_jll"; platforms=x11_platforms),
HostBuildDependency("Wayland_jll"),

Copy link
Contributor

Choose a reason for hiding this comment

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

This and the FIXME were there because meson had problems with choosing to use the provided wayland-scanner from HostBuildDependency("Wayland_jll"); I don't remember the error, but it was cryptic and unhelpful 😄 I've reverted that change, and am trying to figure out the build failure.

@giordano
Copy link
Member

giordano commented Mar 1, 2022

@jpsamaroo bump 🙂 There are a couple of questions above

@jpsamaroo
Copy link
Contributor

sandbox:${WORKSPACE}/srcdir/mesa/build # cat ${host_prefix}/lib/pkgconfig/wayland-scanner.pc
prefix=/workspace/destdir
exec_prefix=${prefix}
datarootdir=${prefix}/share
pkgdatadir=${datarootdir}/wayland
wayland_scanner=${exec_prefix}/bin/wayland-scanner

Name: Wayland Scanner
Description: Wayland scanner
Version: 1.19.0

How do we deal with pkg-config files for HostBuildDependency?

@giordano
Copy link
Member

giordano commented Mar 2, 2022

How do we deal with pkg-config files for HostBuildDependency?

If the problem is that prefix is inaccurate you can perhaps fix it in-place? Something like

sed -i "s?prefix=.*?prefix=${host_prefix}?" "${host_prefix}/lib/pkgconfig/wayland-scanner.pc"

@jpsamaroo
Copy link
Contributor

@giordano would appreciate any advice on the current failure.

@giordano
Copy link
Member

giordano commented Mar 3, 2022

You have Wayland_jll only as a host dependency, not a target one

# Dependencies that must be installed before this package can be built
dependencies = [
HostBuildDependency("Wayland_jll"),
Dependency("libLLVM_jll"; compat="11.0.0"),
Copy link
Member

Choose a reason for hiding this comment

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

So this will work only in julia v1.6?

Copy link
Contributor

Choose a reason for hiding this comment

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

It requires LLVM >= 11 (and I tested successfully on LLVM 12). What's the compat notation for that?

Copy link
Member

Choose a reason for hiding this comment

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

@vchuravy what's the kosher way of dealing with this? Use libLLVM_jll as dependency but not specify a compat? Use libLLVM_jll as a BuildDependency (libllvm will be anyway available at runtime)? Or perhaps follow LLVMExtra now that we have that mechanism in place?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, seeing that this links directly to libLLVM-11jl.so I think the the LLVMExtra's approach is the way to go. I'm actually surprised it worked for you with other Julia versions with different LLVM 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, no, it didn't 😄 I had to switch which libLLVM I imported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants