Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion repos/spack_repo/builtin/packages/alsa_lib/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,27 @@ class AlsaLib(AutotoolsPackage):
space library that developers compile ALSA applications against."""

homepage = "https://www.alsa-project.org"
url = "ftp://ftp.alsa-project.org/pub/lib/alsa-lib-1.2.3.2.tar.bz2"
url = "ftp://ftp.alsa-project.org/pub/lib/alsa-lib-1.2.15.2.tar.bz2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as an FYI, it is not necessary to update the url each time a new version is added. Spack determines this from older urls automatically.

git = "https://github.com/alsa-project/alsa-lib.git"

license("LGPL-2.1-or-later")

version("1.2.15.2", tag="v1.2.15.2", commit="63a981865a1c7d9501ae556e28ae3bb53d015b61")
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 a particular reason to use a tag here? That prevents caching the source.

version("1.2.9", sha256="dc9c643fdc4ccfd0572cc685858dd41e08afb583f30460b317e4188275f615b2")
version("1.2.8", sha256="1ab01b74e33425ca99c2e36c0844fd6888273193bd898240fe8f93accbcbf347")
version("1.2.3.2", sha256="e81fc5b7afcaee8c9fd7f64a1e3043e88d62e9ad2c4cff55f578df6b0a9abe15")
version("1.2.2", sha256="d8e853d8805574777bbe40937812ad1419c9ea7210e176f0def3e6ed255ab3ec")
version("1.1.4.1", sha256="91bb870c14d1c7c269213285eeed874fa3d28112077db061a3af8010d0885b76")

variant("python", default=False, description="enable python")
variant("plugins", default=False, description="enable plugins")

patch("python.patch", when="@1.1.4:1.1.5 +python")

depends_on("c", type="build") # generated
depends_on("autoconf", type="build", when="@1.2.15.2 build_system=autotools")
depends_on("automake", type="build", when="@1.2.15.2 build_system=autotools")
depends_on("libtool", type="build", when="@1.2.15.2 build_system=autotools")
Comment on lines +34 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems surprising. Why would only this particular version depend on these packages? And what other option for build_system is there?


depends_on("python", type=("link", "run"), when="+python")

Expand All @@ -34,6 +42,8 @@ class AlsaLib(AutotoolsPackage):
def configure_args(self):
spec = self.spec
args = []
if spec.satisfies("+plugins"):
args.append("--with-pcm-plugins=all --with-ctl-plugins=all --enable-pcm")
if spec.satisfies("+python"):
args.append(f"--with-pythonlibs={spec['python'].libs.ld_flags}")
args.append(f"--with-pythonincludes={spec['python'].headers.include_flags}")
Expand Down
57 changes: 57 additions & 0 deletions repos/spack_repo/builtin/packages/alsa_plugins/package.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Copyright Spack Project Developers. See COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)

import os

from spack_repo.builtin.build_systems.autotools import AutotoolsPackage

from spack.package import *


class AlsaPlugins(AutotoolsPackage):
"""The ALSA Plugins package contains plugins for various audio libraries and sound servers."""

homepage = "https://github.com/alsa-project/alsa-plugins"
url = "https://github.com/alsa-project/alsa-plugins"
git = "https://github.com/alsa-project/alsa-plugins"

# notify when the package is updated.
maintainers("biddisco")

license("LGPL-2.1-or-later", checked_by="biddisco")

version("1.2.12", tag="v1.2.12", commit="52574cb5ccbb8b546df2759e4b341a20332269b6")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here about using a git tag instead of a download.


depends_on("c", type="build")
depends_on("autoconf", type="build")
depends_on("automake", type="build")
depends_on("libtool", type="build")

depends_on("alsa-lib")
depends_on("pulseaudio +alsa", when="+pulseaudio")

variant("pulseaudio", default=True, description="Enable pulseaudio support")
Copy link
Contributor

Choose a reason for hiding this comment

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

Often it is easier to set the default as False, except for really indispensable functionality.


conflicts("platform=darwin", msg="ALSA (+plugins) only works for Linux")

def configure_args(self):
args = []
if "+pulseaudio" in self.spec:
args.append("--enable-pulseaudio")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion to use self.enable_or_disable to avoid automatically finding a system pulseaudio.

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g.

args += self.enable_or_disable("libs")

return args

def setup_build_environment(self, env):
env.prepend_path("PKG_CONFIG_PATH", self.spec["alsa-lib"].prefix.lib64.pkgconfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised that this is necessary. Normally this should be found automatically. Any reason you could identify for why it isn't?


def setup_run_environment(self, env):
# 1. ALSA plugin directory
alsa_plugin_dir = os.path.join(self.prefix.lib, "alsa-lib")
if os.path.exists(alsa_plugin_dir):
env.prepend_path("ALSA_PLUGIN_DIR", alsa_plugin_dir)

# 2. PulseAudio libraries (needed for dlopen dependencies)
if "pulseaudio" in self.spec:
pulseaudio_libdir = os.path.join(self.spec["pulseaudio"].prefix.lib64, "pulseaudio")
if os.path.exists(pulseaudio_libdir):
env.prepend_path("LD_LIBRARY_PATH", pulseaudio_libdir)
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 no other variable that can be used and that alsa respects? LD_LIBRARY_PATH is a big hammer and this can lead to significant side effects (even though they are mitigated here by adding pulseaudio to the path components).

57 changes: 54 additions & 3 deletions repos/spack_repo/builtin/packages/pulseaudio/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)

import os

from spack_repo.builtin.build_systems.autotools import AutotoolsPackage
from spack_repo.builtin.build_systems.meson import MesonPackage

from spack.package import *


class Pulseaudio(AutotoolsPackage):
class Pulseaudio(AutotoolsPackage, MesonPackage):
"""PulseAudio is a sound system for POSIX OSes, meaning that it is a proxy
for your sound applications.

Expand All @@ -19,10 +22,15 @@ class Pulseaudio(AutotoolsPackage):
achieved using a sound server."""

homepage = "https://www.freedesktop.org/wiki/Software/PulseAudio/"
url = "https://freedesktop.org/software/pulseaudio/releases/pulseaudio-13.0.tar.xz"
url = "https://freedesktop.org/software/pulseaudio/releases/pulseaudio-17.0.tar.xz"

license("LGPL-2.1-or-later")

build_system(
conditional("autotools", when="@:16"), conditional("meson", when="@17:"), default="meson"
)

version("17.0", sha256="053794d6671a3e397d849e478a80b82a63cb9d8ca296bd35b73317bb5ceb87b5")
version("13.0", sha256="961b23ca1acfd28f2bc87414c27bb40e12436efcf2158d29721b1e89f3f28057")

variant("alsa", default=False, description="alsa support")
Expand All @@ -32,6 +40,13 @@ class Pulseaudio(AutotoolsPackage):

depends_on("c", type="build") # generated
depends_on("cxx", type="build") # generated
with when("build_system=autotools"):
depends_on("autoconf", type="build")
depends_on("automake", type="build")
depends_on("libtool", type="build")
with when("build_system=meson"):
depends_on("meson", type="build")
depends_on("gettext", type="build")

depends_on("[email protected]:", when="+alsa")
depends_on("[email protected]:")
Expand All @@ -58,6 +73,9 @@ class Pulseaudio(AutotoolsPackage):
depends_on("[email protected]:")
depends_on("m4", type="build")

# if spec.satisfies("%gcc@8:"):
os.environ["LDFLAGS"] = "-Wl,--copy-dt-needed-entries"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not canonical in spack. Modifications to the environment are not intended to leak outside the spack invocation (this does), and we have setup_build_environment or flag_handler for this.


def configure_args(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

It is safer to add the configure_args and meson_args to separate builders, e.g.

class AutotoolsBuilder(autotools.AutotoolsBuilder):

args = [
"--disable-systemd-daemon",
Expand All @@ -71,8 +89,8 @@ def configure_args(self):
"--with-systemduserunitdir=no",
"CPPFLAGS={0}".format(self.spec["libtool"].headers.cpp_flags),
"LDFLAGS={0}".format(self.spec["libtool"].libs.search_flags),
"--libdir={0}".format(self.prefix.lib64),
]

# toggle based on variants
args += self.enable_or_disable("alsa")
args += self.enable_or_disable("openssl")
Expand All @@ -97,3 +115,36 @@ def configure_args(self):
)

return args

def meson_args(self):
return [
"-Ddatabase=gdbm",
"-Ddoxygen=false",
"-Dbluez5=disabled",
"-Dx11=disabled",
"-Dtests=false",
"-Ddefault_library=shared",
"-Dprefix={0}".format(self.prefix),
"-Dlibdir={0}/lib64".format(self.prefix),
"-Dbashcompletiondir={0}/share/bash-completion/completions".format(self.prefix),
"-Dsystemduserunitdir={0}/lib/systemd/user".format(self.prefix),
]
Comment on lines +120 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

We typically try to make sure that two build systems of the same package at least in spirit behave the same way. In the case of configure, you have enable dbus and glib2, but that's not explicitly added for meson. Can you verify that?


def setup_build_environment(self, env):
if self.spec.satisfies("build_system=meson"):
env.append_flags("CPPFLAGS", "-I{0}".format(self.spec["libiconv"].prefix.include))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be CXXFLAGS? CPP is for the preprocessor.

I am also surprised these are necessary if you depend on libiconv. Why does the build not find the right include directory?

env.append_flags("LDFLAGS", "-L{0} -liconv".format(self.spec["libiconv"].prefix.lib))

def setup_run_environment(self, env):
# Always add ALSA plugin directory
env.prepend_path("ALSA_PLUGIN_DIR", join_path(self.prefix.lib, "alsa-lib"))

# If pulseaudio is in the spec, make sure its libraries are on the runtime linker path
if self.spec.satisfies("^pulseaudio"):
# 1️⃣ Standard lib64 dir
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to stick to regular ascii charsets.

env.prepend_path("LD_LIBRARY_PATH", self.spec["pulseaudio"].prefix.lib64)

# 2️⃣ Also check for a pulseaudio subdirectory (some builds install here)
pulseaudio_libdir = join_path(self.spec["pulseaudio"].prefix.lib64, "pulseaudio")
if os.path.exists(pulseaudio_libdir):
env.prepend_path("LD_LIBRARY_PATH", pulseaudio_libdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as earlier about LD_LIBRARY_PATH.

Loading