Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Angelo Compagnucci <angelo.compagnucci@gmail.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 2/4 v2] package/libcupsfilters: new package
Date: Fri, 22 Aug 2025 23:06:30 +0200	[thread overview]
Message-ID: <20250822230630.76b6f36a@windsurf> (raw)
In-Reply-To: <20250820163921.1968030-3-angelo.compagnucci@gmail.com>

Hello Angelo,

Thanks a lot for this great work! I have a number of comments/questions
below.

On Wed, 20 Aug 2025 18:39:19 +0200
Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote:


> diff --git a/package/libcupsfilters/Config.in b/package/libcupsfilters/Config.in
> new file mode 100644
> index 0000000000..e664869a41
> --- /dev/null
> +++ b/package/libcupsfilters/Config.in
> @@ -0,0 +1,34 @@
> +config BR2_PACKAGE_LIBCUPSFILTERS
> +	bool "libcupsfilters"
> +	# needs fork()
> +	depends on BR2_USE_MMU
> +	depends on BR2_INSTALL_LIBSTDCPP # qpdf
> +	depends on !BR2_STATIC_LIBS
> +	depends on BR2_USE_WCHAR # libglib2
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
> +	depends on BR2_PACKAGE_CUPS
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5 # qpdf
> +	depends on BR2_TOOLCHAIN_HAS_ATOMIC # qpdf

For the ones that don't have a comment, are they directly dependencies
due to libcupsfilter itself?

> +	select BR2_PACKAGE_JPEG

This dependency doesn't seem to be mandatory according to
https://github.com/OpenPrinting/libcupsfilters/blob/master/INSTALL.md#prerequisites-for-compilation

> +	select BR2_PACKAGE_FONTCONFIG

Same.

> +	select BR2_PACKAGE_FREETYPE

There's no mention of freetype in INSTALL.md nor in configure.ac.

> +	select BR2_PACKAGE_LCMS2
> +	select BR2_PACKAGE_LIBGLIB2

There's no mention of glib2 in INSTALL.md nor in configure.ac.

> diff --git a/package/libcupsfilters/libcupsfilters.mk b/package/libcupsfilters/libcupsfilters.mk
> new file mode 100644
> index 0000000000..9cf449ac8d
> --- /dev/null
> +++ b/package/libcupsfilters/libcupsfilters.mk
> @@ -0,0 +1,93 @@
> +################################################################################
> +#
> +# libcupsfilters
> +#
> +################################################################################
> +
> +LIBCUPSFILTERS_VERSION = 2.1.1
> +LIBCUPSFILTERS_SITE = https://github.com/OpenPrinting/libcupsfilters/releases/download/$(LIBCUPSFILTERS_VERSION)
> +LIBCUPSFILTERS_LICENSE = GPL-2.0, GPL-2.0+, GPL-3.0, GPL-3.0+, LGPL-2, LGPL-2.1+, MIT, BSD-4-Clause
> +LIBCUPSFILTERS_LICENSE_FILES = COPYING
> +LIBCUPSFILTERS_CPE_ID_VENDOR = linuxfoundation

Where did you find this? I don't see any CPE entry about libcupsfilter,
so we can't know which CPE it will get for the moment.

> +LIBCUPSFILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg

As requested above: this needs to be revisited.

> +LIBCUPSFILTERS_INSTALL_STAGING = YES
> +
> +LIBCUPSFILTERS_CONF_OPTS = \
> +	--disable-mutool \
> +	--disable-foomatic \
> +	--disable-braille \
> +	--enable-imagefilters \

What is the impact of this? Do we need it unconditionally?

> +	--with-cups-config=$(STAGING_DIR)/usr/bin/cups-config \
> +	--with-sysroot=$(STAGING_DIR) \
> +	--with-pdftops=pdftops \
> +	--with-jpeg \

jpeg seems optional.

> +	--with-test-font-path=/dev/null \
> +	--without-rcdir

You can probably check in ./configure --help for other things to
disable, like:

	--without-jpegxl

(I honestly didn't find any other).


> +define LIBCUPSFILTERS_INSTALL_INIT_SYSV
> +	$(INSTALL) -D -m 0755 package/cups-filters/S82cups-browsed \
> +		$(TARGET_DIR)/etc/init.d/S82cups-browsed

We really don't want libcupsfilter to install an init script that comes
from package/cups-filters/. If libcupsfilter is now responsible for
this, the init script should be moved there (or possibly copied
initially, and then removed in the commit that bumps cups-filters, or
something like that).

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2025-08-22 21:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-20 16:39 [Buildroot] [PATCH 0/4 v2] Bump cups-filter to the latest version Angelo Compagnucci
2025-08-20 16:39 ` [Buildroot] [PATCH 1/4 v2] package/qpdf: bump to version 12.2.0 Angelo Compagnucci
2025-08-22 21:08   ` Thomas Petazzoni via buildroot
2025-08-23 15:48     ` Angelo Compagnucci
2025-08-20 16:39 ` [Buildroot] [PATCH 2/4 v2] package/libcupsfilters: new package Angelo Compagnucci
2025-08-22 21:06   ` Thomas Petazzoni via buildroot [this message]
2025-08-20 16:39 ` [Buildroot] [PATCH 3/4 v2] package/libppd: " Angelo Compagnucci
2025-08-22 21:22   ` Thomas Petazzoni via buildroot
2025-08-20 16:39 ` [Buildroot] [PATCH 4/4 v2] package/cups-filters: bump to version 2.0.1 Angelo Compagnucci
2025-08-22 21:25   ` Thomas Petazzoni via buildroot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250822230630.76b6f36a@windsurf \
    --to=buildroot@buildroot.org \
    --cc=angelo.compagnucci@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox