All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/ledmon: new package
@ 2023-03-27 16:54 Maksim Kiselev
  2023-07-29 22:31 ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 2+ messages in thread
From: Maksim Kiselev @ 2023-03-27 16:54 UTC (permalink / raw)
  To: buildroot; +Cc: Maxim Kochetkov, Thomas Petazzoni, Maksim Kiselev

Enclosure LED Utilities

ledmon and ledctl are userspace tools designed to control storage
enclosure LEDs. The user must have root privileges to use these tools.

These tools use the SGPIO and SES-2 protocols to monitor and control LEDs.
They been verified to work with Intel(R) storage controllers (i.e. the
Intel(R) AHCI controller) and have not been tested with storage controllers of
other vendors (especially SAS/SCSI controllers).

For backplane enclosures attached to ISCI controllers, support is limited to
Intel(R) Intelligent Backplanes.

Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
 package/Config.in                             |  1 +
 .../ledmon/0001-Add-disable-doc-option.patch  | 57 +++++++++++++++++++
 package/ledmon/Config.in                      | 11 ++++
 package/ledmon/ledmon.hash                    |  3 +
 package/ledmon/ledmon.mk                      | 14 +++++
 5 files changed, 86 insertions(+)
 create mode 100644 package/ledmon/0001-Add-disable-doc-option.patch
 create mode 100644 package/ledmon/Config.in
 create mode 100644 package/ledmon/ledmon.hash
 create mode 100644 package/ledmon/ledmon.mk

diff --git a/package/Config.in b/package/Config.in
index 0f8dab3e71..3c2f98bdda 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -525,6 +525,7 @@ endmenu
 	source "package/iucode-tool/Config.in"
 	source "package/kbd/Config.in"
 	source "package/lcdproc/Config.in"
+	source "package/ledmon/Config.in"
 	source "package/libiec61850/Config.in"
 	source "package/libmanette/Config.in"
 	source "package/libubootenv/Config.in"
diff --git a/package/ledmon/0001-Add-disable-doc-option.patch b/package/ledmon/0001-Add-disable-doc-option.patch
new file mode 100644
index 0000000000..4803c37698
--- /dev/null
+++ b/package/ledmon/0001-Add-disable-doc-option.patch
@@ -0,0 +1,57 @@
+From 4c356662faaa5aa2dc0b0eb713dc5134a70eb3b0 Mon Sep 17 00:00:00 2001
+From: Maksim Kiselev <bigunclemax@gmail.com>
+Date: Mon, 27 Mar 2023 17:45:15 +0300
+Subject: [PATCH] Add '--disable-doc' option
+
+Introduce a configure option to disable documentation installation in case if it is not required.
+
+Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
+---
+ Makefile.am  | 8 ++++++--
+ configure.ac | 8 +++++++-
+ 2 files changed, 13 insertions(+), 3 deletions(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index d2b6f3a..f021768 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -20,6 +20,10 @@ if SYSTEMD_CONDITION
+   OPTIONAL_SUBDIR = systemd
+ endif
+ 
+-SUBDIRS = doc src $(OPTIONAL_SUBDIR)
++if DOC_CONDITION
++  DOC_SUBDIR = doc
++  dist_doc_DATA = README.md
++endif
++
++SUBDIRS = src $(DOC_SUBDIR) $(OPTIONAL_SUBDIR)
+ EXTRA_DIST = config/config.h systemd/ledmon.service.in
+-dist_doc_DATA = README.md
+diff --git a/configure.ac b/configure.ac
+index 001a049..510bb85 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -71,6 +71,12 @@ AS_IF([test "x$enable_systemd" = xyes], [SYSTEMD_STR=yes], [SYSTEMD_STR=no])
+ 
+ AM_CONDITIONAL([SYSTEMD_CONDITION], [test "$SYSTEMD_STR" = yes])
+ 
++AC_ARG_ENABLE(doc, AS_HELP_STRING([--disable-doc], [do not install ledmon documentaion]),,[enable_doc=yes])
++
++AS_IF([test "x$enable_doc" = xyes], [DOC_STR=yes], [DOC_STR=no])
++
++AM_CONDITIONAL([DOC_CONDITION], [test "$DOC_STR" = yes])
++
+ # target directory for ledmon service file
+ AC_SUBST([SYSTEMD_PATH], "$(pkg-config systemd --variable=systemdsystemunitdir)")
+ 
+@@ -86,5 +92,5 @@ $PACKAGE_NAME $VERSION configuration:
+   Preprocessor flags:      ${AM_CPPFLAGS} ${CPPFLAGS}
+   C compiler flags:        ${AM_CFLAGS} ${CFLAGS}
+   Common install location: ${prefix}
+-  configure parameters:    --enable-systemd=${SYSTEMD_STR}
++  configure parameters:    --enable-systemd=${SYSTEMD_STR} --enable-doc=${DOC_STR}
+ ])
+-- 
+2.39.2
+
diff --git a/package/ledmon/Config.in b/package/ledmon/Config.in
new file mode 100644
index 0000000000..467a75fa56
--- /dev/null
+++ b/package/ledmon/Config.in
@@ -0,0 +1,11 @@
+config BR2_PACKAGE_LEDMON
+	bool "ledmon"
+	depends on BR2_PACKAGE_PCIUTILS
+	depends on BR2_PACKAGE_SG3_UTILS
+	depends on BR2_PACKAGE_HAS_UDEV
+	help
+	  Enclosure LED Utilities. The ledmon application is
+	  a daemon process used to monitor a state of software
+	  RAID devices (md only) or a state of block devices.
+
+	  https://github.com/intel/ledmon
diff --git a/package/ledmon/ledmon.hash b/package/ledmon/ledmon.hash
new file mode 100644
index 0000000000..5c0c183ddc
--- /dev/null
+++ b/package/ledmon/ledmon.hash
@@ -0,0 +1,3 @@
+# Locally calculated
+sha256  97534302a60f03b90e69228a6a56096cf3fdfc8eb31fea52a974445268fdf5d9  ledmon-v0.96.tar.gz
+sha256  dcc100d4161cc0b7177545ab6e47216f84857cda3843847c792a25289852dcaa  COPYING
diff --git a/package/ledmon/ledmon.mk b/package/ledmon/ledmon.mk
new file mode 100644
index 0000000000..dba8a99b8c
--- /dev/null
+++ b/package/ledmon/ledmon.mk
@@ -0,0 +1,14 @@
+################################################################################
+#
+# ledmon
+#
+################################################################################
+
+LEDMON_VERSION = v0.96
+LEDMON_SITE = $(call github,intel,ledmon,$(LEDMON_VERSION))
+LEDMON_DEPENDENCIES = pciutils sg3_utils udev
+LEDMON_LICENSE = GPL-2.0
+LEDMON_LICENSE_FILES = COPYING
+LEDMON_AUTORECONF = YES
+
+$(eval $(autotools-package))
-- 
2.39.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Buildroot] [PATCH] package/ledmon: new package
  2023-03-27 16:54 [Buildroot] [PATCH] package/ledmon: new package Maksim Kiselev
@ 2023-07-29 22:31 ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-07-29 22:31 UTC (permalink / raw)
  To: Maksim Kiselev; +Cc: Maxim Kochetkov, buildroot

Hello Maksim,

First of all, thanks a lot for your patch, and sorry for the very long
delay to provide feedback. We are currently doing an effort of handling
our backlog of patches, and therefore reviewing/merging old patches.

Your patch is almost ready to merge with some tweaks, but one of them
needs your support. See below.

On Mon, 27 Mar 2023 19:54:23 +0300
Maksim Kiselev <bigunclemax@gmail.com> wrote:

> Enclosure LED Utilities
> 
> ledmon and ledctl are userspace tools designed to control storage
> enclosure LEDs. The user must have root privileges to use these tools.
> 
> These tools use the SGPIO and SES-2 protocols to monitor and control LEDs.
> They been verified to work with Intel(R) storage controllers (i.e. the
> Intel(R) AHCI controller) and have not been tested with storage controllers of
> other vendors (especially SAS/SCSI controllers).
> 
> For backplane enclosures attached to ISCI controllers, support is limited to
> Intel(R) Intelligent Backplanes.
> 
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> ---
>  package/Config.in                             |  1 +
>  .../ledmon/0001-Add-disable-doc-option.patch  | 57 +++++++++++++++++++
>  package/ledmon/Config.in                      | 11 ++++
>  package/ledmon/ledmon.hash                    |  3 +
>  package/ledmon/ledmon.mk                      | 14 +++++
>  5 files changed, 86 insertions(+)
>  create mode 100644 package/ledmon/0001-Add-disable-doc-option.patch
>  create mode 100644 package/ledmon/Config.in
>  create mode 100644 package/ledmon/ledmon.hash
>  create mode 100644 package/ledmon/ledmon.mk

We need you to add an entry in the DEVELOPERS file for this package, so
that you're notified by e-mail when:

- There are build failures affecting your package
- A new version becomes available upstream and the package needs to be updated
- A CVE is reported on your package and needs to be fixed

> diff --git a/package/ledmon/0001-Add-disable-doc-option.patch b/package/ledmon/0001-Add-disable-doc-option.patch
> new file mode 100644
> index 0000000000..4803c37698
> --- /dev/null
> +++ b/package/ledmon/0001-Add-disable-doc-option.patch
> @@ -0,0 +1,57 @@
> +From 4c356662faaa5aa2dc0b0eb713dc5134a70eb3b0 Mon Sep 17 00:00:00 2001
> +From: Maksim Kiselev <bigunclemax@gmail.com>
> +Date: Mon, 27 Mar 2023 17:45:15 +0300
> +Subject: [PATCH] Add '--disable-doc' option
> +
> +Introduce a configure option to disable documentation installation in case if it is not required.
> +
> +Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>

We need all patches to have an Upstream: tag that indicates where the
patch has been submitted upstream. This project has a Github repo, so
it should be trivial to submit this patch through a pull request, and
indicate the URL of that pull request as the value of the Upstream: tag.

> diff --git a/package/ledmon/Config.in b/package/ledmon/Config.in
> new file mode 100644
> index 0000000000..467a75fa56
> --- /dev/null
> +++ b/package/ledmon/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_LEDMON
> +	bool "ledmon"
> +	depends on BR2_PACKAGE_PCIUTILS
> +	depends on BR2_PACKAGE_SG3_UTILS
> +	depends on BR2_PACKAGE_HAS_UDEV

For pciutils and sg3_utils, we definitely want to use "select" instead
of "depends on".

So, we need:

	depends on BR2_PACKAGE_HAS_UDEV
	depends on BR2_TOOLCHAIN_HAS_THREADS # sg3_utils
	select BR2_PACKAGE_PCIUTILS
	select BR2_PACKAGE_SG3_UTILS

> +	help
> +	  Enclosure LED Utilities. The ledmon application is
> +	  a daemon process used to monitor a state of software
> +	  RAID devices (md only) or a state of block devices.
> +
> +	  https://github.com/intel/ledmon

And then:

comment "ledmon needs udev and a toolchain w/ threads"
	depends on !BR2_PACKAGE_UDEV || !BR2_TOOLCHAIN_HAS_THREADS

Another aspect: did you try to build ledmon with different toolchains
in Buildroot? In particular I see a pull request at
https://github.com/intel/ledmon/pull/139 that fixes build issues with
the Musl C library, so I'm wondering if it builds with musl. I can
suggest you to do a run of ./utils/test-pkg for ledmon to verify that
it builds with a set of toolchains.

> diff --git a/package/ledmon/ledmon.mk b/package/ledmon/ledmon.mk
> new file mode 100644
> index 0000000000..dba8a99b8c
> --- /dev/null
> +++ b/package/ledmon/ledmon.mk
> @@ -0,0 +1,14 @@
> +################################################################################
> +#
> +# ledmon
> +#
> +################################################################################
> +
> +LEDMON_VERSION = v0.96
> +LEDMON_SITE = $(call github,intel,ledmon,$(LEDMON_VERSION))

Please use:

LEDMON_VERSION = 0.96
LEDMON_SITE = $(call github,intel,ledmon,v$(LEDMON_VERSION))

So that the LEDMON_VERSION variable only includes the version, without
the "v" prefix. This will require a small change in the .hash file, as
the tarball name will change.

> +LEDMON_DEPENDENCIES = pciutils sg3_utils udev

You need to add host-pkgconf to this list, as the configure script uses
pkg-config.

> +LEDMON_LICENSE = GPL-2.0
> +LEDMON_LICENSE_FILES = COPYING

Perhaps we can add a comment here that says:

# The code base also include a COPYING.LIB file with the LGPL-2.1 text,
# and some source files are published under LGPL-2.1, but all of them are
# at some point linked with GPL-2.0 code, making the resulting binaries
# GPL-2.0 licensed

(hoping that this statement is correct of course)

Could you rework those different details and send an updated version of
your patch? It would be very useful!

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-07-29 22:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 16:54 [Buildroot] [PATCH] package/ledmon: new package Maksim Kiselev
2023-07-29 22:31 ` Thomas Petazzoni via buildroot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.