Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt
@ 2025-11-21 16:34 Yann E. MORIN via buildroot
  2025-11-22 12:31 ` Julien Olivain via buildroot
  2025-11-22 12:48 ` Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 3+ messages in thread
From: Yann E. MORIN via buildroot @ 2025-11-21 16:34 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN, Thomas Petazzoni

Commit 1187c34d88e1 (support/scripts: move merged-usr errors message
into check-merged-usr.sh) introduced the use of getopt to parse its
options; doing so allowed to use long option (with two leading dashes),
which is more descriptive than the usual one-character options.

However, getopt is part of util-linux; it is not a shell built-in.
util-linux is not a prerequisite of Buildroot, so we may end up running
on a system where it is missing.

We could add host-util-linux as a dependency when the system does not
provide getopt, but that's not very nice; even though host-skeleton does
not need to check for merged-bin for now, it does not need getopt, and
thus we could add host-util-linux (which depends on host-skeleton) as a
dependency of skeleton-custom. But that will not be tenable over the
long run, especially if/when we do a merged-bin in host dir.

Requiring that util-linux be installed system-wide is not nice either;
it's an additional requirement on the host.

We can do like we do in the oter scripts, though: use the shell built-in
getopts. Its usage is slightly different, and does not support long
options. As it's just for use in an intenral script, we can live with
the less descriptive options, though.

Switch to using getopts, it removes the need for a new host dependency.

Fixes: 1187c34d88e16191753bfd0c3b4cd2f914813f31
Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v1 -> v2;
  - fix type's parameter: it's ${OPTARG}, not ${2}
  - reword commit title
---
 Makefile                                   |  6 ++--
 package/skeleton-custom/skeleton-custom.mk |  6 ++--
 support/scripts/check-merged               | 35 +++++++---------------
 3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/Makefile b/Makefile
index ff25027c13..d1d1c88943 100644
--- a/Makefile
+++ b/Makefile
@@ -783,9 +783,9 @@ endif
 # counterparts are appropriately setup as symlinks ones to the others.
 	@$(call MESSAGE,"Sanity check in overlays $(call qstrip,$(BR2_ROOTFS_OVERLAY))")
 	support/scripts/check-merged \
-		--type overlay \
-		$(if $(BR2_ROOTFS_MERGED_USR),--merged-usr) \
-		$(if $(BR2_ROOTFS_MERGED_BIN),--merged-bin) \
+		-t overlay \
+		$(if $(BR2_ROOTFS_MERGED_USR),-u) \
+		$(if $(BR2_ROOTFS_MERGED_BIN),-b) \
 		$(call qstrip,$(BR2_ROOTFS_OVERLAY))
 
 	$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index 7c79500046..b5a1a5e80d 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -25,9 +25,9 @@ endif
 
 define SKELETON_CUSTOM_CONFIGURE_CMDS
 	support/scripts/check-merged \
-		--type skeleton \
-		$(if $(BR2_ROOTFS_MERGED_USR),--merged-usr) \
-		$(if $(BR2_ROOTFS_MERGED_BIN),--merged-bin) \
+		-t skeleton \
+		$(if $(BR2_ROOTFS_MERGED_USR),-u) \
+		$(if $(BR2_ROOTFS_MERGED_BIN),-b) \
 		$(SKELETON_CUSTOM_PATH)
 endef
 
diff --git a/support/scripts/check-merged b/support/scripts/check-merged
index bff6ae2b7b..447abfd815 100755
--- a/support/scripts/check-merged
+++ b/support/scripts/check-merged
@@ -23,9 +23,9 @@
 #                   to /usr/bin)
 #
 # Input:
-#   --type TYPE     the type of root to check: 'skeleton' or 'overlay'
-#   --merged-usr    check for merged /usr
-#   --merged-bin    check for merged /usr/bin
+#   -t TYPE         the type of root to check: 'skeleton' or 'overlay'
+#   -u              check for merged /usr
+#   -b              check for merged /usr/bin
 #   $*:             the root directories (skeleton, overlays) to check
 # Output:
 #   stdout:         the list of non-compliant paths (empty if compliant).
@@ -34,31 +34,16 @@
 #   !0:             if any directory to check is improperly merged
 #
 
-opts="type:,merged-usr,merged-bin"
-ARGS="$(getopt -n check-merged -o "" -l "${opts}" -- "${@}")" || exit 1
-eval set -- "${ARGS}"
-
 type=
 merged_usr=false
 merged_bin=false
-while :; do
-	case "${1}" in
-	(--type)
-		type="${2}"
-		shift 2
-		;;
-	(--merged-usr)
-		merged_usr=true
-		shift
-		;;
-	(--merged-bin)
-		merged_bin=true
-		shift
-		;;
-	(--)
-		shift
-		break
-		;;
+while getopts "t:ub" OPT; do
+	case "${OPT}" in
+	(t)	type="${OPTARG}";;
+	(u)	merged_usr=true;;
+	(b)	merged_bin=true;;
+	(:)	printf "option %s expects a mandatory argument\n" "${OPTARG}"; exit 1;;
+	(\?)	printf "unknown option %s\n" "${OPTARG}"; exit 1;;
 	esac
 done
 
-- 
2.51.1

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

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

* Re: [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt
  2025-11-21 16:34 [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt Yann E. MORIN via buildroot
@ 2025-11-22 12:31 ` Julien Olivain via buildroot
  2025-11-22 12:48 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 3+ messages in thread
From: Julien Olivain via buildroot @ 2025-11-22 12:31 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot, Thomas Petazzoni

On 21/11/2025 17:34, Yann E. MORIN via buildroot wrote:
> Commit 1187c34d88e1 (support/scripts: move merged-usr errors message
> into check-merged-usr.sh) introduced the use of getopt to parse its
> options; doing so allowed to use long option (with two leading dashes),
> which is more descriptive than the usual one-character options.
> 
> However, getopt is part of util-linux; it is not a shell built-in.
> util-linux is not a prerequisite of Buildroot, so we may end up running
> on a system where it is missing.
> 
> We could add host-util-linux as a dependency when the system does not
> provide getopt, but that's not very nice; even though host-skeleton 
> does
> not need to check for merged-bin for now, it does not need getopt, and
> thus we could add host-util-linux (which depends on host-skeleton) as a
> dependency of skeleton-custom. But that will not be tenable over the
> long run, especially if/when we do a merged-bin in host dir.
> 
> Requiring that util-linux be installed system-wide is not nice either;
> it's an additional requirement on the host.
> 
> We can do like we do in the oter scripts, though: use the shell 
> built-in
> getopts. Its usage is slightly different, and does not support long
> options. As it's just for use in an intenral script, we can live with
> the less descriptive options, though.
> 
> Switch to using getopts, it removes the need for a new host dependency.
> 
> Fixes: 1187c34d88e16191753bfd0c3b4cd2f914813f31
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Applied to master, thanks.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt
  2025-11-21 16:34 [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt Yann E. MORIN via buildroot
  2025-11-22 12:31 ` Julien Olivain via buildroot
@ 2025-11-22 12:48 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Petazzoni via buildroot @ 2025-11-22 12:48 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

Hello Yann,

On Fri, 21 Nov 2025 17:34:53 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Commit 1187c34d88e1 (support/scripts: move merged-usr errors message
> into check-merged-usr.sh) introduced the use of getopt to parse its
> options; doing so allowed to use long option (with two leading dashes),
> which is more descriptive than the usual one-character options.
> 
> However, getopt is part of util-linux; it is not a shell built-in.
> util-linux is not a prerequisite of Buildroot, so we may end up running
> on a system where it is missing.
> 
> We could add host-util-linux as a dependency when the system does not
> provide getopt, but that's not very nice; even though host-skeleton does
> not need to check for merged-bin for now, it does not need getopt, and
> thus we could add host-util-linux (which depends on host-skeleton) as a
> dependency of skeleton-custom. But that will not be tenable over the
> long run, especially if/when we do a merged-bin in host dir.
> 
> Requiring that util-linux be installed system-wide is not nice either;
> it's an additional requirement on the host.
> 
> We can do like we do in the oter scripts, though: use the shell built-in
> getopts. Its usage is slightly different, and does not support long
> options. As it's just for use in an intenral script, we can live with
> the less descriptive options, though.
> 
> Switch to using getopts, it removes the need for a new host dependency.
> 
> Fixes: 1187c34d88e16191753bfd0c3b4cd2f914813f31
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Thanks a lot for the patch! I didn't review it as it was already
applied, but anyway the changes look fairly mechanism and
straightforward. Thanks for the quick reaction!

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] 3+ messages in thread

end of thread, other threads:[~2025-11-22 12:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-21 16:34 [Buildroot] [PATCH v2] support/scripts/check-merged: use getopts instead of getopt Yann E. MORIN via buildroot
2025-11-22 12:31 ` Julien Olivain via buildroot
2025-11-22 12:48 ` Thomas Petazzoni via buildroot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox