Skip to content

Mesa should depend_on('glproto')#11360

Merged
chuckatkins merged 3 commits intospack:developfrom
hartzell:mesa-bug
May 6, 2019
Merged

Mesa should depend_on('glproto')#11360
chuckatkins merged 3 commits intospack:developfrom
hartzell:mesa-bug

Conversation

@hartzell
Copy link
Copy Markdown
Contributor

@hartzell hartzell commented May 2, 2019

This should be ready to go, see conversation for details.


ToDo:

  • I suspect that this should be conditional on the opengl variant, but would appreciate guidance from someone more familiar with that world.

The mesa package refers to GL/glproto.h. On systems that don't have the OS packages installed, this leads to failures during the build e.g. this comment in 01482.

This fixes it. Tested on a minimally provisioned CentOS 7.

@opadron, can you check it on your systems (I'm using the same base system that @odoublewen is, but I'm sure he'll test it too...).

The mesa package refers to `GL/glproto.h`.  On systems that don't have
the OS packages installed, this leads to failures during the build
[e.g. this comment in
01482](spack#10482 (comment)).

This fixes it.  Tested on a minimally provisioned CentOS 7.
@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented May 3, 2019

FWIW, using the current develop branch and trying to build mesa w/out OpenGL support, e.g.:

spack install 'mesa @19.0.0 ~opengl %gcc@8.2.0 ^ninja@1.9.0 ^pcre+jit ^perl@5.26.2 ^python@3.6.5'

fails with (from the spack-build.out):

meson.build:342:4: ERROR:  Problem encountered: Cannot build GLX support without X11 platform support and at least one OpenGL API

So I'm not sure how to go about testing the need to parameterize the dependency. That said, this version of the dependency that's parameterized on OpenGL support also fixes @odoublewen's issue.

diff --git a/var/spack/repos/builtin/packages/mesa/package.py b/var/spack/repos/builtin/packages/mesa/package.py
index 3ddf1bead..717f2aa53 100644
--- a/var/spack/repos/builtin/packages/mesa/package.py
+++ b/var/spack/repos/builtin/packages/mesa/package.py
@@ -33,7 +33,7 @@ class Mesa(MesonPackage):
     depends_on('libxml2')
     depends_on('zlib')
     depends_on('expat')
-    depends_on('glproto')
+    depends_on('glproto', when='+opengl')

     # Internal options
     variant('llvm', default=True, description="Enable LLVM.")

I'd be happy to do it this way too....

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented May 3, 2019

ps. I think that Spack members(committers?) can push to this branch. I'd be happy if they did in the name of simple timely merges.

@chuckatkins
Copy link
Copy Markdown

Good catch! I believe this can actually be gated on +glx and made a build-only dep

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented May 3, 2019

It doesn't seem to work as a build-only dependency, which is weird since it only installs headers. type=('build','link') works, as does no type constraint (since build/link is the default.

Making it when='+glx' works.

@chuckatkins
Copy link
Copy Markdown

I believe the build-only error is a mesa bug. I'm shepherding a fix upstream right now and I'll try it as a patch:

https://gitlab.freedesktop.org/mesa/mesa/merge_requests/806

@chuckatkins
Copy link
Copy Markdown

@hartzell The build-only dep should work now with the associated patch. I'll merge the patch into upstream mesa as soon as someone takes a look at it there.

Copy link
Copy Markdown

@chuckatkins chuckatkins left a comment

Choose a reason for hiding this comment

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

@ax3l I believe this is good to go once @hartzell can verify the build-only dep works for their environment with the included patch.

@chuckatkins
Copy link
Copy Markdown

@opadron could you also test building this in a minimal bare bones container

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented May 3, 2019

This works for me with @chuckatkins patch. Lets get this etc, etc...

@chuckatkins
Copy link
Copy Markdown

I just finished the merge in upstream mesa so the problem should be limited to the 19.0.x releases and be included in upcoming 19.1 release.

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented May 5, 2019

ping

@opadron
Copy link
Copy Markdown
Member

opadron commented May 6, 2019

@hartzell, @chuckatkins and I agree that this is ready for merge. The only hangup is the failing tests on Travis. We think those are just transient issues, and hope to confirm as much by restarting their tests. We'll be ready to merge once we can get Travis to come back green.

@chuckatkins
Copy link
Copy Markdown

It looks like one of the travis builds timed out so I just did a dummy force push witt no changes to re-trigger the ci.

@chuckatkins chuckatkins merged commit 96a95bb into spack:develop May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants