Re: [Buildroot] [PATCH 2/3] package/roc: new package

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Théo, All,

On 2022-03-10 16:20 +0100, Théo Lebrun via buildroot spake thusly:
> Roc is a toolkit for real-time audio streaming over the network.
> https://roc-streaming.org/
> 
> Goal: optional dependency to PipeWire.
> Requires host-ragel to build.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
[--SNIP--]
> diff --git a/package/roc/0001-fix-build-that-used-removed-functionality.patch b/package/roc/0001-fix-build-that-used-removed-functionality.patch
> new file mode 100644
> index 0000000000..50147905c8
> --- /dev/null
> +++ b/package/roc/0001-fix-build-that-used-removed-functionality.patch
> @@ -0,0 +1,28 @@
> +build: fix build that used removed functionality
> +
> +The SConstruct file used the "SourceCode()" method that was removed in
> +SCons 4.0.0, after being deprecated in 2.0.0.
> +
> +This was fixed upstream but is not in the latest release yet.
> +https://github.com/roc-streaming/roc-toolkit/commit/abdfbb94df98fe88be4dd92ca587500126558411

Then please just backport that upstream commit (git format-patch, then
add your SoB line).

[--SNIP--]
> diff --git a/package/roc/Config.in b/package/roc/Config.in
> new file mode 100644
> index 0000000000..3fd74cab3f
> --- /dev/null
> +++ b/package/roc/Config.in
> @@ -0,0 +1,28 @@
> +comment "ROC needs a toolchain w/ shared libraries, C++, NPTL"
> +	depends on BR2_STATIC_LIBS || !BR2_INSTALL_LIBSTDCPP || \
> +		!BR2_TOOLCHAIN_HAS_THREADS_NPTL
> +	depends on BR2_USE_MMU
> +	depends on BR2_TOOLCHAIN_HAS_SYNC_4
> +
> +config BR2_PACKAGE_ROC
> +	bool "roc"
> +	# depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_1

Drop commented-out dependency.

> +	depends on !BR2_STATIC_LIBS
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # libuv
> +	depends on BR2_USE_MMU # libuv
> +	depends on BR2_TOOLCHAIN_HAS_SYNC_4 # libuv

Depenencies order are:
  - arch dependencies
  - toolchain dependencies
  - package dependencies

https://buildroot.org/downloads/manual/manual.html#_config_files

> +	select BR2_PACKAGE_HOST_PYTHON3
> +	select BR2_PACKAGE_HOST_PYTHON3_SSL
> +	select BR2_PACKAGE_LIBUV
> +	help
> +	  Roc is a toolkit for real-time audio streaming over the
> +	  network.
> +
> +	  https://roc-streaming.org/
> +
> +if BR2_PACKAGE_ROC

Empty line after an if (not a written rule, but it's more readable).

> +config BR2_PACKAGE_ROC_TOOLS
> +	bool "build roc-{conv,recv,send} tools"
> +	default y

We sually shy away from 'default y', unless there is a good reason,
which thus needs to be explained in the commit log.

Empty line before an endif (ditto).

[--SNIP--]
> diff --git a/package/roc/roc.mk b/package/roc/roc.mk
> new file mode 100644
> index 0000000000..90d226ffbd
> --- /dev/null
> +++ b/package/roc/roc.mk
> @@ -0,0 +1,75 @@
> +################################################################################
> +#
> +# roc
> +#
> +################################################################################
> +
> +ROC_VERSION = v0.1.5
> +ROC_SITE = $(call github,roc-streaming,roc-toolkit,$(ROC_VERSION))
> +ROC_LICENSE = MPL-2.0
> +ROC_LICENSE_FILES = LICENSE
> +ROC_INSTALL_STAGING = YES
> +ROC_DEPENDENCIES = \
> +	host-gengetopt \
> +	host-pkgconf \
> +	host-ragel \
> +	host-scons \
> +	libuv
> +
> +ROC_SCONS_OPTS = \
> +	--disable-tests \
> +	--disable-examples \
> +	--disable-doc \
> +	--disable-libunwind \
> +	--build-3rdparty=openfec

> +# We are not building OpenFEC ourselves as they are using a forked version. See
> +# the following note in the documentation:
> +# https://github.com/roc-streaming/roc-toolkit/blob/c89687330bfce6f4dce79826f7a235b581f2b49d/docs/sphinx/building/dependencies.rst

This must be explained in the commit log.

But why can't we use openfec as a separate package, using the roc fork?
The original OpenFEC has not been updated sine 2014: http://openfec.org/
The fork is not in a much better shape, not having been updated since
2019: https://github.com/roc-streaming/openfec

So, I would think it would be OK to package the roc fork of OpenFEC,
rather than use the bundled version.

Also, using --build-3rdparty means the download is done at configure
time (or even at build time), but we try to be able to build without
network; the following is expected to work:

    $ make source
    [unplug network]
    $ make

but --build-3rdparty will make that faile, AFAICS.

> +# The shared library is enabled by default. --enable-lib does not exist.
> +ifneq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y)

Why not use positive logic;

    ifeq ($(BR2_STATIC_LIBS),y)

> +ROC_SCONS_OPTS += --disable-lib
> +endif
> +
> +ifeq ($(BR2_PACKAGE_ROC_TOOLS),y)
> +
> +# Tools are enabled by default. --enable-tools does not exist.
> +
> +# SoX and Pulseaudio support is only used for tools or examples, which is why
> +# their support is inside the TOOLS condition.

Are examples built by default? If yes, can we disable them? If yes,
should we do that unconditionally, or add an option (I'd vote against
the option)?

> +ifeq ($(BR2_PACKAGE_SOX),y)
> +ROC_DEPENDENCIES += sox
> +else
> +ROC_SCONS_OPTS += --disable-sox
> +endif
> +
> +ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
> +ROC_DEPENDENCIES += pulseaudio
> +else
> +ROC_SCONS_OPTS += --disable-pulseaudio
> +endif
> +
> +else
> +ROC_SCONS_OPTS += --disable-tools
> +endif
> +
> +define ROC_BUILD_CMDS
> +	(cd $(@D); \

Enclosing the build commands in parentheses is useless; it just spawns
an extra shell for nothing. Ditto for all _CMDS below, of course.

> +	$(TARGET_CONFIGURE_OPTS) CROSS=$(TARGET_CROSS) \
> +		$(SCONS) $(ROC_SCONS_OPTS))
> +endef
> +
> +define ROC_INSTALL_STAGING_CMDS
> +	(cd $(@D); \
> +	$(TARGET_CONFIGURE_OPTS) CROSS=$(TARGET_CROSS) \
> +		$(SCONS) $(ROC_SCONS_OPTS) --prefix="$(STAGING_DIR)/usr" install)
> +endef
> +
> +define ROC_INSTALL_TARGET_CMDS
> +	(cd $(@D); \
> +	$(TARGET_CONFIGURE_OPTS) CROSS=$(TARGET_CROSS) \
> +		$(SCONS) $(ROC_SCONS_OPTS) --prefix="$(TARGET_DIR)/usr" install)
> +endef
> +
> +$(eval $(generic-package))
> -- 
> 2.35.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot