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

mesa18: Added a legacy mesa package using the older autotools build #11553

Closed
wants to merge 2 commits into from

Conversation

chuckatkins
Copy link

Create a backwards compatible mesa package relying on the older autotools build to bypass the python 2/3 compatibility problems.

@chuckatkins
Copy link
Author

Replaces #11543

@citibeth
Copy link
Member

I really don't understand this. We have mesa, osmesa, mesa18, plus changes to Spack defaults.

It should (is) possible to create a single package that uses CMake for newer versions and Autotools for older versions. Can we do it that way?

@chuckatkins
Copy link
Author

I agree this is confusing. The packages themselves are mesa for >= 19.x using the Meson build system and mesa18 for the previous release using AutoTools. osmesa as a virtual goes with the high level concept with the OpenGL packages of separating front ends and backends. i.e. the front ends being EGL, GLX, and OSMesa, and the backends being OpenGL, OpenGL ES and OpenGL ES2. The mesa software can provide all of these or some of these, and similarly a vendor supplied driver like NVIdia can provide all of these or some of these (except OSMesa, only mesa can provide that). The transition to the new meson build has created extensive problems with forcing a python 3 dependency. Even though it's only a build time dependency, spack is not able to separate that out in concretization so a package that depends on mesa (using the new meson build) and python 2 has an unsatisfied version constraint on python. So this allows the mesa18 package to be used for packages that require python 2 and mesa for packages that require python 3. And with the osmesa virtual, a downstream package can specify depends_on('osmesa') and then base don their python dependencies, spack can appropriate resolve whether mesa or mesa18 should be used.

All of this is currently being done to workaround improper dependency resolution at concretization.

@citibeth
Copy link
Member

I agree this is confusing. The packages themselves are mesa for >= 19.x using the Meson build system and mesa18 for the previous release using AutoTools. osmesa as a virtual goes with the high level concept with the OpenGL packages of separating front ends and backends. i.e. the front ends being EGL, GLX, and OSMesa, and the backends being OpenGL, OpenGL ES and OpenGL ES2. The mesa software can provide all of these or some of these, and similarly a vendor supplied driver like NVIdia can provide all of these or some of these (except OSMesa, only mesa can provide that).

More explanation of the above is required.

All of this is currently being done to workaround improper dependency resolution at concretization.

See #11468 for discussion on how you can create a python2 variant to hack around this problem. Also note that Python2 will no longer be supported as of January 1 2020. Are you able to simplify your life and move forward with a Python3-only system?

#11468

@chuckatkins
Copy link
Author

See #11468 for discussion on how you can create a python2 variant to hack around this problem.

Both mesa and mesa18 have some python build time requirement because of a configure-time code generation step, which is really version agnostic, i.e. works with 2 or 3. But the current mesa package builds with meson which has a hard python 3 requirement, so there's no way to make a python 2 variant.

Also note that Python2 will no longer be supported as of January 1 2020. Are you able to simplify your life and move forward with a Python3-only system?

I'd love to. But that's not feasible. We're getting packages like vtk and paraview to drop python 2 and be python 3 only moving forward in all current and future versions but python 2 will still be there for older versions.

I can try to combine the two into a single package and just inherit from the base Package instead of MesonPackage or AutotoolsPackage if you'd rather.

@chuckatkins
Copy link
Author

Given how different the base MesonPackage and AutotoolsPackage classes are, I think it will be much cleaner to keep these as separate packages rather than try to mash them into one.

Also the motivation for adding the osmesa virtual package was to expose it in a similar way as gl and glx. However, given that there's not really any other "provider" for it beside mesa then I can drop the extra virtual and just have provides('mesa+osmesa', when='+osmesa')

@chuckatkins
Copy link
Author

Scratch that, provides seems to not work with variants so osmesa needs to be virtual.

@citibeth
Copy link
Member

@adamjstewart @alalazo

I can try to combine the two into a single package and just inherit from the base Package instead of MesonPackage or AutotoolsPackage if you'd rather.

Yes I think this is the best. And we need to figure out how to do it cleanly (see below), without creating multiple packages just because the build system changes. This issue will come up repeatedly as projects move away from Autotools, but AFAIK you're the first. So let's figure out a clean solution now.

Given how different the base MesonPackage and AutotoolsPackage classes are, I think it will be much cleaner to keep these as separate packages rather than try to mash them into one.

Rather than mashing, the idea is to use (I believe) the Strategy Design Pattern:
https://en.wikipedia.org/wiki/Strategy_pattern

Your mesa/package.py should look something like this:

class AutotoolsMesa(AutotoolsPackage):
    def configure_args(...):
        ...

def MesonMesa(MesonPackage):
    def .build_whatever..():
        ....

class Mesa(Package):
    homepage=...
    url=...
    version(...)
    depends_on(...)

    def build(...):
        if spec.version > '19':
            new MesonMesa(self).build()
        else:
             new AutotoolsMesa(self).build()

This is just a sketch. But the idea is to have a "clean" AutotoolsMesa package and a "clean" MesonMesa package; and to switch between the two of them. I'm pinging @alalazo because he implemented the way package builds are structure; and he'd have the best knowledge of how to delegate in this way.

@adamjstewart
Copy link
Member

I personally like @alalazo's suggestion in #11543 (comment). I think that's the easiest way to keep the old versions available but use the base classes Spack provides.

@citibeth
Copy link
Member

citibeth commented May 31, 2019 via email

@alalazo
Copy link
Member

alalazo commented May 31, 2019

@citibeth @adamjstewart A few thoughts I noted down on packages that might support multiple build-systems are at #10411 (comment) In my opinion it will be at another scale of complexity with respect to maintaining a legacy repository + it needs strong support from the concretizer.

@chuckatkins
Copy link
Author

I'm not sure if you've understood what the python2 variant is for. It's
to get around concretizer bugs, where you'd otherwise write something like:

depends_on('xyz', when='^python@2')

Instead, you require that if the user is doing a Python2 build that they
set the all: python2 variant in the YAML files. Then you write instead:

depends_on('xyz', when='+python2')

I get it. My point though was that it's not possible to make a mesa+python2 variant because the dependency is in the build system, not the package itself.

Can you just use an older version of Spack to build older versions of
Mesa? Most major Python pacakges will be Python3-only soon; there will be
no point in trying to build with newer versions of these packages, since
NONE of them will work with Python2.

So spack as a repository will be no longer allowing packages to support python 2? Will all previous versions of a package that depend on python 2 need to be removed? I just don't see that as feasible. That's essentially the situation here. foo+python@v5 depends on python 2 and mesa, foo+python@v6 depends on python 3 an mesa. Since mesa was updated and now has a hard python 3 requirement then an older version of mesa is required to support foo+python@v5. Just abandoning python 2 support for existing packages isn't an option.

@chuckatkins
Copy link
Author

@alalazo

A few thoughts I noted down on packages that might support multiple build-systems are at #10411 (comment) In my opinion it will be at another scale of complexity with respect to maintaining a legacy repository + it needs strong support from the concretizer.

I'm in agreement here on this. Melding these two package files into one is beginning to look like it will result in something that, sure works, but is going to be extremely complex and quickly become unmaintainable by anyone not deeply familiar with the internal spack class structures.

@chuckatkins
Copy link
Author

In the interest of pragmatism and getting numerous issues unstuck and resolved here, I'm going to abandon this PR and back out the updated meson-based mesa package and replace it with this autotools one. That will "make all right in the world again" with all the problems this has caused. When a better solution for how to handle build system migrations is figured out then I'll be happy to re-introduce the updated mesa package. Until then though I think the best solution for all the downstream packages depending on mesa is to do a build system roll back.

@citibeth
Copy link
Member

citibeth commented May 31, 2019 via email

@citibeth
Copy link
Member

@citibeth @adamjstewart A few thoughts I noted down on packages that might support multiple build-systems are at #10411 (comment) In my opinion it will be at another scale of complexity with respect to maintaining a legacy repository + it needs strong support from the concretizer.

So... are you supporting the idea in #10411, or are you suggesting we settle on a legacy repository approach? I don't yet understand how legacy repos would work.

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.

None yet

4 participants