Re: [pacman-dev] [PATCH 2/3] libmakepkg: use readelf instead of file for finding ELF file types
> > --- a/scripts/libmakepkg/tidy/strip.sh.in
> > +++ b/scripts/libmakepkg/tidy/strip.sh.in
> > @@ -111,22 +111,20 @@ tidy_strip() {
> >
> > local binary strip_flags
> > find . -type f -perm -u+w -print0 2>/dev/null | while IFS=
> > read -rd '' binary ; do
> > - case "$(file -bi "$binary")" in
> > - *application/x-sharedlib*) # Libraries (.so)
> > + case "$(LC_ALL=C readelf -h "$binary" 2>/dev/null)" in
>
> More squelching of errors.
The error redirection to null is because readelf outputs an error when
the file it is run on isn't an ELF file at all, and obviously that error
is unwanted.
> Incidentally, is the output of this, documented, or can it arbitrarily
> change in the future? file is guaranteed to emit output in the
> RFC-documented IANA media type format.
These file types and their specification in the file header are part of
the ELF spec, and the readelf output has remained consistent, at least
the part that matters to us, and the relevant part is the same across 4
different implementations, and I doubt there will be some change of the
word 'Type:' being used, given that the ELF specification refers to those
file types as 'types'.
[pacman-dev] [PATCH 3/3] remove mention of file as dependency and from build system
Signed-off-by: Ethan Sommer
---
build-aux/edit-script.sh.in | 1 -
configure.ac| 12
meson.build | 9 -
scripts/makepkg.sh.in | 4 +---
4 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/build-aux/edit-script.sh.in b/build-aux/edit-script.sh.in
index 661c22d5..6ed563be 100644
--- a/build-aux/edit-script.sh.in
+++ b/build-aux/edit-script.sh.in
@@ -19,7 +19,6 @@ mode=$3
-e "s|@TEMPLATE_DIR[@]|@TEMPLATE_DIR@|g" \
-e "s|@DEBUGSUFFIX[@]|@DEBUGSUFFIX@|g" \
-e "s|@INODECMD[@]|@INODECMD@|g" \
- -e "s|@FILECMD[@]|@FILECMD@|g" \
"$input" >"$output"
if [[ $mode ]]; then
diff --git a/configure.ac b/configure.ac
index e59f82e9..137f30e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -228,18 +228,6 @@ PKG_CHECK_VAR(bashcompdir, [bash-completion],
[completionsdir], ,
PKG_CHECK_MODULES(LIBARCHIVE, [libarchive >= 3.0.0], ,
AC_MSG_ERROR([*** libarchive >= 3.0.0 is needed to compile pacman!]))
-# Check file for seccomp
-if test "x$with_file_seccomp" = "xauto"; then
- file_version="$(file --version| sed -n 's/^file-\(.*\)/\1/p')"
- AX_COMPARE_VERSION([$file_version], [ge], [5.38],
[with_file_seccomp=yes])
-fi
-if test "x$with_file_seccomp" = "xyes"; then
- FILECMD="file -S"
-else
- FILECMD="file"
-fi
-AC_SUBST(FILECMD)
-
# Check for OpenSSL
have_openssl=no
have_nettle=no
diff --git a/meson.build b/meson.build
index 2c9185a6..f149548d 100644
--- a/meson.build
+++ b/meson.build
@@ -219,19 +219,11 @@ config_h = configure_file(
configuration : conf)
add_project_arguments('-include', 'config.h', language : 'c')
-filecmd = 'file'
inodecmd = 'stat -c \'%i %n\''
strip_binaries = '--strip-all'
strip_shared = '--strip-unneeded'
strip_static = '--strip-debug'
-file_seccomp = get_option('file-seccomp')
-# meson-git has find_program('file', required: false, version: '>=5.38')
-filever = run_command('sh', '-c', 'file --version | sed -n
"s/^file-\(.*\)/\\1/p"').stdout()
-if file_seccomp.enabled() or ( file_seccomp.auto() and
filever.version_compare('>= 5.38') )
- filecmd = 'file -S'
-endif
-
os = host_machine.system()
if os.startswith('darwin')
inodecmd = '/usr/bin/stat -f \'%i %N\''
@@ -266,7 +258,6 @@ substs.set('BUILDSCRIPT', BUILDSCRIPT)
substs.set('TEMPLATE_DIR', get_option('makepkg-template-dir'))
substs.set('DEBUGSUFFIX', get_option('debug-suffix'))
substs.set('INODECMD', inodecmd)
-substs.set('FILECMD', filecmd)
substs.set('LIBMAKEPKGDIR', LIBMAKEPKGDIR)
substs.set('STRIP_BINARIES', strip_binaries)
substs.set('STRIP_SHARED', strip_shared)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 2deb61da..06d36f6b 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -26,15 +26,13 @@
# makepkg uses quite a few external programs during its execution. You
# need to have at least the following installed for makepkg to function:
-# awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, file, find
(findutils),
+# awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils),
# gettext, gpg, grep, gzip, sed, tput (ncurses), xz
# gettext initialization
export TEXTDOMAIN='pacman-scripts'
export TEXTDOMAINDIR='@localedir@'
-# file -i does not work on Mac OSX unless legacy mode is set
-export COMMAND_MODE='legacy'
# Ensure CDPATH doesn't screw with our cd calls
unset CDPATH
# Ensure GREP_OPTIONS doesn't screw with our grep calls
--
2.23.0
[pacman-dev] [PATCH 1/3] libmakepkg: use extraction commands instead of file to find archive type
Previously, to determine which command we should use to extract an
archive, we would run file and match the output against our list of
possible extraction commands
Instead, run the archive through each extraction command's -t (--test)
flag, if this succeeds then we know that the command is able to extract
the file and is the one to use
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/source/file.sh.in | 39
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/scripts/libmakepkg/source/file.sh.in
b/scripts/libmakepkg/source/file.sh.in
index 7297a1c6..faace79b 100644
--- a/scripts/libmakepkg/source/file.sh.in
+++ b/scripts/libmakepkg/source/file.sh.in
@@ -96,35 +96,18 @@ extract_file() {
fi
# do not rely on extension for file type
- local file_type=$(@FILECMD@ -bizL -- "$file")
- local ext=${file##*.}
local cmd=''
- case "$file_type" in
-
*application/x-tar*|*application/zip*|*application/x-zip*|*application/x-cpio*)
- cmd="bsdtar" ;;
- *application/x-gzip*|*application/gzip*)
- case "$ext" in
- gz|z|Z) cmd="gzip" ;;
- *) return;;
- esac ;;
- *application/x-bzip*)
- case "$ext" in
- bz2|bz) cmd="bzip2" ;;
- *) return;;
- esac ;;
- *application/x-xz*)
- case "$ext" in
- xz) cmd="xz" ;;
- *) return;;
- esac ;;
- *)
- # See if bsdtar can recognize the file
- if bsdtar -tf "$file" -q '*' &>/dev/null; then
- cmd="bsdtar"
- else
- return 0
- fi ;;
- esac
+ if bsdtar -tf "$file" -q '*'; then
+ cmd='bsdtar'
+ elif gzip -t "$file"; then
+ cmd='gzip'
+ elif bzip2 -t "$file"; then
+ cmd='bzip2'
+ elif xz -t "$file"; then
+ cmd='xz'
+ else
+ return 0
+ fi &>/dev/null
local ret=0
msg2 "$(gettext "Extracting %s with %s")" "$file" "$cmd"
--
2.23.0
[pacman-dev] [PATCH 2/3] libmakepkg: use readelf instead of file for finding ELF file types
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/tidy/strip.sh.in | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/scripts/libmakepkg/tidy/strip.sh.in
b/scripts/libmakepkg/tidy/strip.sh.in
index 1bd810f0..301d1989 100644
--- a/scripts/libmakepkg/tidy/strip.sh.in
+++ b/scripts/libmakepkg/tidy/strip.sh.in
@@ -111,22 +111,20 @@ tidy_strip() {
local binary strip_flags
find . -type f -perm -u+w -print0 2>/dev/null | while IFS= read
-rd '' binary ; do
- case "$(file -bi "$binary")" in
- *application/x-sharedlib*) # Libraries (.so)
+ case "$(LC_ALL=C readelf -h "$binary" 2>/dev/null)" in
+ *Type:*'DYN (Shared object file)'*) # Libraries
(.so) or Relocatable binaries
strip_flags="$STRIP_SHARED";;
- *application/x-archive*)# Libraries (.a)
- strip_flags="$STRIP_STATIC";;
- *application/x-object*)
- case "$binary" in
- *.ko) #
Kernel module
-
strip_flags="$STRIP_SHARED";;
- *)
- continue;;
- esac;;
- *application/x-executable*) # Binaries
+ *Type:*'EXEC (Executable file)'*) # Binaries
strip_flags="$STRIP_BINARIES";;
- *application/x-pie-executable*) # Relocatable
binaries
- strip_flags="$STRIP_SHARED";;
+ *Type:*'REL (Relocatable file)'*) # Libraries
(.a) or objects
+ if ar t "$binary" &>/dev/null; then #
Libraries (.a)
+ strip_flags="$STRIP_STATIC"
+ elif [[ $binary = *'.ko' ]]; then #
Kernel module
+ strip_flags="$STRIP_SHARED"
+ else
+ continue
+ fi
+ ;;
*)
continue ;;
esac
--
2.23.0
[pacman-dev] [PATCH] repo-add: use wc -c on stdin instead of file to avoid use of cut
Redirect file to stdin so wc -c doesn't print a file name that needs to
be stripped.
Signed-off-by: Ethan Sommer
---
scripts/repo-add.sh.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index caf1232d..7b2a9edb 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -273,7 +273,7 @@ db_write_entry() {
error "$(gettext "Cannot use armored signatures for
packages: %s")" "$pkgfile.sig"
return 1
fi
- pgpsigsize=$(wc -c "$pkgfile.sig" | cut -d' ' -f1)
+ pgpsigsize=$(wc -c < "$pkgfile.sig")
if (( pgpsigsize > 16384 )); then
error "$(gettext "Invalid package signature file
'%s'.")" "$pkgfile.sig"
return 1
@@ -282,7 +282,7 @@ db_write_entry() {
pgpsig=$(base64 "$pkgfile.sig" | tr -d '\n')
fi
- csize=$(wc -c "$pkgfile" | cut -d' ' -f1)
+ csize=$(wc -c < "$pkgfile")
# compute checksums
msg2 "$(gettext "Computing checksums...")"
--
2.23.0
[pacman-dev] [PATCH v2] makepkg: replaces sed in-place with built in substitution
Reads PKGBUILD into an array and replaces the pkgver and pkgrel with
bash parameter substitution, then uses shell redirection to write to to
the file. Because shell redirection follows symlinks, this accomplishes
the same thing as the previous default of using the GNU-specific
--follow-symlinks sed flag.
Removes SEDPATH and SEDINPLACEFLAGS from the build systems as they are
not used elsewhere.
Signed-off-by: Ethan Sommer
---
build-aux/edit-script.sh.in | 2 --
configure.ac| 11 ---
meson.build | 11 ---
meson_options.txt | 3 ---
scripts/Makefile.am | 2 --
scripts/makepkg.sh.in | 8 +---
6 files changed, 5 insertions(+), 32 deletions(-)
diff --git a/build-aux/edit-script.sh.in b/build-aux/edit-script.sh.in
index 640d32f8..7423a223 100644
--- a/build-aux/edit-script.sh.in
+++ b/build-aux/edit-script.sh.in
@@ -20,8 +20,6 @@ mode=$3
-e "s|@DEBUGSUFFIX[@]|@DEBUGSUFFIX@|g" \
-e "s|@INODECMD[@]|@INODECMD@|g" \
-e "s|@FILECMD[@]|@FILECMD@|g" \
- -e "s|@SEDINPLACEFLAGS[@]|@SEDINPLACEFLAGS@|g" \
- -e "s|@SEDPATH[@]|@SEDPATH@|g" \
-e "s|@configure_input[@]|Generated from ${input##*/}; do not edit by
hand.|g" \
"$input" >"$output"
diff --git a/configure.ac b/configure.ac
index 305432b3..e59f82e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -369,7 +369,6 @@ AC_CHECK_MEMBERS([struct statfs.f_flags],,,[[#include
GCC_VISIBILITY_CC
# Host-dependant definitions
-DEFAULT_SEDINPLACEFLAGS=" --follow-symlinks -i"
INODECMD="stat -c '%i %n'"
STRIP_BINARIES="--strip-all"
STRIP_SHARED="--strip-unneeded"
@@ -377,30 +376,21 @@ STRIP_STATIC="--strip-debug"
case "${host_os}" in
*bsd*)
INODECMD="stat -f '%i %N'"
- DEFAULT_SEDINPLACEFLAGS=" -i \"\""
;;
darwin*)
host_os_darwin=yes
INODECMD="/usr/bin/stat -f '%i %N'"
- DEFAULT_SEDINPLACEFLAGS=" -i ''"
STRIP_BINARIES=""
STRIP_SHARED="-S"
STRIP_STATIC="-S"
;;
esac
AM_CONDITIONAL([DARWIN], test "x$host_os_darwin" = "xyes")
-AC_PATH_PROGS([SEDPATH], [sed], [sed], [/usr/bin$PATH_SEPARATOR/bin] )
AC_SUBST(INODECMD)
AC_SUBST(STRIP_BINARIES)
AC_SUBST(STRIP_SHARED)
AC_SUBST(STRIP_STATIC)
-# Flags for sed in place
-if test "${SEDINPLACEFLAGS+set}" != "set"; then
-SEDINPLACEFLAGS="${DEFAULT_SEDINPLACEFLAGS}"
-fi
-AC_ARG_VAR(SEDINPLACEFLAGS, [flags for sed, overriding the default])
-
# Variables plugged into makepkg.conf
CARCH="${host%%-*}"
CHOST="${host}"
@@ -576,7 +566,6 @@ ${PACKAGE_NAME}:
Architecture : ${CARCH}
Host Type : ${CHOST}
File inode command : ${INODECMD}
-In-place sed command : ${SEDPATH} ${SEDINPLACEFLAGS}
File seccomp command : ${FILECMD}
libalpm version: ${LIB_VERSION}
diff --git a/meson.build b/meson.build
index 8c296cb8..36f87ed2 100644
--- a/meson.build
+++ b/meson.build
@@ -221,7 +221,6 @@ config_h = configure_file(
add_project_arguments('-include', 'config.h', language : 'c')
filecmd = 'file'
-default_sedinplaceflags = ' --follow-symlinks -i'
inodecmd = 'stat -c \'%i %n\''
strip_binaries = '--strip-all'
strip_shared = '--strip-unneeded'
@@ -237,18 +236,11 @@ endif
os = host_machine.system()
if os.startswith('darwin')
inodecmd = '/usr/bin/stat -f \'%i %N\''
- default_sedinplaceflags = ' -i \'\''
strip_binaries = ''
strip_shared = '-s'
strip_static = '-s'
elif os.contains('bsd') or os == 'dragonfly'
inodecmd = 'stat -f \'%i %N\''
- default_sedinplaceflags = ' -i \'\''
-endif
-
-sedinplaceflags = get_option('sedinplaceflags')
-if sedinplaceflags == 'auto'
- sedinplaceflags = default_sedinplaceflags
endif
chost = run_command(cc, '-dumpmachine').stdout().strip()
@@ -277,8 +269,6 @@ substs.set('TEMPLATE_DIR',
get_option('makepkg-template-dir'))
substs.set('DEBUGSUFFIX', get_option('debug-suffix'))
substs.set('INODECMD', inodecmd)
substs.set('FILECMD', filecmd)
-substs.set('SEDINPLACEFLAGS', sedinplaceflags)
-substs.set('SEDPATH', SED.path())
substs.set('LIBMAKEPKGDIR', LIBMAKEPKGDIR)
substs.set('STRIP_BINARIES', strip_binaries)
substs.set('STRIP_SHARED', strip_shared)
@@ -430,7 +420
[pacman-dev] [PATCH] makepkg: replace sed in-place with built-in substitution
Read PKGBUILD into an array and replace the pkgver and pkgrel with
bash parameter substitution, then use shell redirection to write to to
the file. Because shell redirection follows symlinks, this accomplishes
the same thing as the previous default of using the GNU-specific
--follow-symlinks sed flag.
Remove SEDPATH and SEDINPLACEFLAGS from the build systems as they are
not used elsewhere.
---
build-aux/edit-script.sh.in | 2 --
configure.ac| 11 ---
meson.build | 11 ---
meson_options.txt | 3 ---
scripts/Makefile.am | 2 --
scripts/makepkg.sh.in | 7 ---
6 files changed, 4 insertions(+), 32 deletions(-)
diff --git a/build-aux/edit-script.sh.in b/build-aux/edit-script.sh.in
index 640d32f8..7423a223 100644
--- a/build-aux/edit-script.sh.in
+++ b/build-aux/edit-script.sh.in
@@ -20,8 +20,6 @@ mode=$3
-e "s|@DEBUGSUFFIX[@]|@DEBUGSUFFIX@|g" \
-e "s|@INODECMD[@]|@INODECMD@|g" \
-e "s|@FILECMD[@]|@FILECMD@|g" \
- -e "s|@SEDINPLACEFLAGS[@]|@SEDINPLACEFLAGS@|g" \
- -e "s|@SEDPATH[@]|@SEDPATH@|g" \
-e "s|@configure_input[@]|Generated from ${input##*/}; do not edit by
hand.|g" \
"$input" >"$output"
diff --git a/configure.ac b/configure.ac
index 305432b3..e59f82e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -369,7 +369,6 @@ AC_CHECK_MEMBERS([struct statfs.f_flags],,,[[#include
GCC_VISIBILITY_CC
# Host-dependant definitions
-DEFAULT_SEDINPLACEFLAGS=" --follow-symlinks -i"
INODECMD="stat -c '%i %n'"
STRIP_BINARIES="--strip-all"
STRIP_SHARED="--strip-unneeded"
@@ -377,30 +376,21 @@ STRIP_STATIC="--strip-debug"
case "${host_os}" in
*bsd*)
INODECMD="stat -f '%i %N'"
- DEFAULT_SEDINPLACEFLAGS=" -i \"\""
;;
darwin*)
host_os_darwin=yes
INODECMD="/usr/bin/stat -f '%i %N'"
- DEFAULT_SEDINPLACEFLAGS=" -i ''"
STRIP_BINARIES=""
STRIP_SHARED="-S"
STRIP_STATIC="-S"
;;
esac
AM_CONDITIONAL([DARWIN], test "x$host_os_darwin" = "xyes")
-AC_PATH_PROGS([SEDPATH], [sed], [sed], [/usr/bin$PATH_SEPARATOR/bin] )
AC_SUBST(INODECMD)
AC_SUBST(STRIP_BINARIES)
AC_SUBST(STRIP_SHARED)
AC_SUBST(STRIP_STATIC)
-# Flags for sed in place
-if test "${SEDINPLACEFLAGS+set}" != "set"; then
-SEDINPLACEFLAGS="${DEFAULT_SEDINPLACEFLAGS}"
-fi
-AC_ARG_VAR(SEDINPLACEFLAGS, [flags for sed, overriding the default])
-
# Variables plugged into makepkg.conf
CARCH="${host%%-*}"
CHOST="${host}"
@@ -576,7 +566,6 @@ ${PACKAGE_NAME}:
Architecture : ${CARCH}
Host Type : ${CHOST}
File inode command : ${INODECMD}
-In-place sed command : ${SEDPATH} ${SEDINPLACEFLAGS}
File seccomp command : ${FILECMD}
libalpm version: ${LIB_VERSION}
diff --git a/meson.build b/meson.build
index 8c296cb8..36f87ed2 100644
--- a/meson.build
+++ b/meson.build
@@ -221,7 +221,6 @@ config_h = configure_file(
add_project_arguments('-include', 'config.h', language : 'c')
filecmd = 'file'
-default_sedinplaceflags = ' --follow-symlinks -i'
inodecmd = 'stat -c \'%i %n\''
strip_binaries = '--strip-all'
strip_shared = '--strip-unneeded'
@@ -237,18 +236,11 @@ endif
os = host_machine.system()
if os.startswith('darwin')
inodecmd = '/usr/bin/stat -f \'%i %N\''
- default_sedinplaceflags = ' -i \'\''
strip_binaries = ''
strip_shared = '-s'
strip_static = '-s'
elif os.contains('bsd') or os == 'dragonfly'
inodecmd = 'stat -f \'%i %N\''
- default_sedinplaceflags = ' -i \'\''
-endif
-
-sedinplaceflags = get_option('sedinplaceflags')
-if sedinplaceflags == 'auto'
- sedinplaceflags = default_sedinplaceflags
endif
chost = run_command(cc, '-dumpmachine').stdout().strip()
@@ -277,8 +269,6 @@ substs.set('TEMPLATE_DIR',
get_option('makepkg-template-dir'))
substs.set('DEBUGSUFFIX', get_option('debug-suffix'))
substs.set('INODECMD', inodecmd)
substs.set('FILECMD', filecmd)
-substs.set('SEDINPLACEFLAGS', sedinplaceflags)
-substs.set('SEDPATH', SED.path())
substs.set('LIBMAKEPKGDIR', LIBMAKEPKGDIR)
substs.set('STRIP_BINARIES', strip_binaries)
substs.set('STRIP_SHARED', strip_shared)
@@ -430,7 +420,6 @@ message('\n'.join([
' Architecture: @0@'.format(carch),
' Host Type : @0@'.format(chost),
' File inode command : @0@'.format(inodecmd),
- ' In-place sed command: @0@ @1@'.format(SED.path(), sedinplaceflags),
' File seccomp command: @0@'.format(filecmd),
' libalpm version : @0@'.format(libalpm_version),
' pacman version : @0@'.format(PACKAGE_VERSION),
diff --git a/meson_options.txt b/meson_options.txt
index 2b92ca1a..4d8cc300 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -54,6 +54,3 @@ option('i18n', type : 'boolean', value : true,
# tools
option('file-seccomp', type: 'feature', value: 'auto',
description: 'deter
[pacman-dev] [PATCH v2] libmakepkg: fix empty arguments in parseopts
Previously parseopts checked if there was an argument by checking
that the string was non-empty, resulting in empty arguments being
incorrectly considered non-existent. This change makes parseopts check
if arguments exist at all, rather than checking that they are non-empty
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 8
test/scripts/parseopts_test.sh | 5 -
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index 38ff7f62..698f2acb 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -103,7 +103,7 @@ parseopts() {
OPTRET+=("-$opt" "${1:i+1}")
break
# if we're at the end,
grab the the next positional, if it exists
- elif (( i == ${#1} - 1
)) && [[ $2 ]]; then
+ elif (( i == ${#1} - 1
&& $# > 1 )); then
OPTRET+=("-$opt" "$2")
shift
break
@@ -144,7 +144,7 @@ parseopts() {
case $? in
0)
# parse failure
- if [[ $optarg ]]; then
+ if [[ $1 = *=* ]]; then
printf "${0##*/}:
$(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2
OPTRET=(--)
return 1
@@ -155,10 +155,10 @@ parseopts() {
;;
1)
# --longopt=optarg
- if [[ $optarg ]]; then
+ if [[ $1 = *=* ]]; then
OPTRET+=("--$opt"
"$optarg")
# --longopt optarg
- elif [[ $2 ]]; then
+ elif (( $# > 1 )); then
OPTRET+=("--$opt" "$2" )
shift
# parse failure
diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh
index 8f1ea1f3..3d706be8 100755
--- a/test/scripts/parseopts_test.sh
+++ b/test/scripts/parseopts_test.sh
@@ -31,7 +31,7 @@ tap_parse() {
unset OPTRET
}
-tap_plan 54
+tap_plan 56
# usage: tap_parse test-params...
# a failed tap_parse will match only the end of options marker '--'
@@ -117,4 +117,7 @@ tap_parse '--opt= --opt=foo --opt --' 4 --opt= --opt=foo
--opt
# short opt with and without optional arg, and non-option arg
tap_parse '-b=foo -A -b -- foo' 5 -bfoo -Ab foo
+# all possible ways to specify empty optargs
+tap_parse '--key --pkg -p --' 7 --key '' --pkg='' -p ''
+
tap_finish
--
2.24.0
[pacman-dev] [PATCH v4] libmakepkg: add optional argument support to parseopts
Adds a "?" suffix that can be used to indicate that an option's argument is
optional.
This allows options to have a default behaviour when the user doesn't
specify one, e.g.: --color=[when] being able to behave like --color=auto
when only --color is passed
Options with optional arguments given on the command line will be returned
in the form "--opt=optarg" and "-o=optarg". Despite that not being the
syntax for passing an argument with a shortopt (trying to pass -o=foo
would make -o's argument "=foo"), this is done to allow the caller to split
the option and its optarg easily
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 116 +++-
test/scripts/parseopts_test.sh | 12 ++-
2 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index c056cb1e..42540438 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -18,16 +18,23 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# A getopt_long-like parser which portably supports longopts and
-# shortopts with some GNU extensions. It does not allow for options
-# with optional arguments. For both short and long opts, options
-# requiring an argument should be suffixed with a colon. After the
-# first argument containing the short opts, any number of valid long
-# opts may be be passed. The end of the options delimiter must then be
-# added, followed by the user arguments to the calling program.
+# shortopts with some GNU extensions. For both short and long opts,
+# options requiring an argument should be suffixed with a colon, and
+# options with optional arguments should be suffixed with a question
+# mark. After the first argument containing the short opts, any number
+# of valid long opts may be be passed. The end of the options delimiter
+# must then be added, followed by the user arguments to the calling
+# program.
+#
+# Options with optional arguments will be returned as "--longopt=optarg"
+# for longopts, or "-o=optarg" for shortopts. This isn't actually a valid
+# way to pass an optional argument with a shortopt on the command line,
+# but is done by parseopts to enable the caller script to split the option
+# and its optarg easily.
#
# Recommended Usage:
-# OPT_SHORT='fb:z'
-# OPT_LONG=('foo' 'bar:' 'baz')
+# OPT_SHORT='fb:zq?'
+# OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
# if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
# exit 1
# fi
@@ -49,29 +56,30 @@ parseopts() {
longoptmatch() {
local o longmatch=()
for o in "${longopts[@]}"; do
- if [[ ${o%:} = "$1" ]]; then
+ if [[ ${o%[:?]} = "$1" ]]; then
longmatch=("$o")
break
fi
- [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
+ [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
done
case ${#longmatch[*]} in
1)
- # success, override with opt and return arg req
(0 == none, 1 == required)
- opt=${longmatch%:}
- if [[ $longmatch = *: ]]; then
- return 1
- else
- return 0
- fi ;;
+ # success, override with opt and return arg req
(0 == none, 1 == required, 2 == optional)
+ opt=${longmatch%[:?]}
+ case $longmatch in
+ *:) return 1 ;;
+ *\?) return 2 ;;
+ *) return 0 ;;
+ esac
+ ;;
0)
# fail, no match found
return 255 ;;
*)
# fail, ambiguous match
printf "${0##*/}: $(gettext "option '%s' is
ambiguous; possibilities:")" "--$1"
- printf " '%s'" "${longmatch[@]%:}"
+ printf " '%s'" "${longmatch[@]%[:?]}"
printf '\n'
return 2
Re: [pacman-dev] [PATCH v3] libmakepkg: add optional argument support to parseopts
> Again... devils advocate. You give an example of '--colour=auto' being > equivalent to '--color'. Why would the default when the options is not > specified not be default in the codebase? Fair enough, another example could be if you wanted to allow specifying a custom log file for whatever reason, one could make -L/--log have an optional argument to specify what file to output the log to, and if no argument is given use the default log file that makepkg currently chooses. > Why not follow the GNU extension to getopt and use '::' for option > arguments instead of '?' The existing code is built around checking for and removing one trailing character from an option, it'd require both more code change and more code all-around to deal with checking for the same character either once or twice than to just check for a different single trailing character.
Re: [pacman-dev] [PATCH] libmakepkg: fix empty arguments in parseopts
> When parsing options, is there any case where an empty argument is > functionally different from a missing one? > > I.e. what is the justification for considering empty arguments? makepkg --config='' --clean ==> ERROR: --clean not found. Aborting... I think that this isn't correct and makes it less clear than it should be why the error is happening, i.e. the option parsing is incorrect when the options passed should be completely valid to the parser, and only the program itself should consider them incorrect.
[pacman-dev] [PATCH] libmakepkg: fix empty arguments in parseopts
Previously parseopts checked if there was an argument by checking
that the string was non-empty, resulting in empty arguments being
incorrectly considered non-existent. This change makes parseopts check
if arguments exist at all, rather than checking that they are non-empty
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 8
test/scripts/parseopts_test.sh | 5 -
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index c056cb1e..8fb862d8 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -102,7 +102,7 @@ parseopts() {
OPTRET+=("${1:i+1}")
break
# if we're at the end, grab the
the next positional, if it exists
- elif (( i == ${#1} - 1 )) && [[
$2 ]]; then
+ elif (( i == ${#1} - 1 && $# >
1 )); then
OPTRET+=("$2")
shift
break
@@ -121,7 +121,7 @@ parseopts() {
case $? in
0)
# parse failure
- if [[ $optarg ]]; then
+ if [[ $1 = *=* ]]; then
printf "${0##*/}:
$(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2
OPTRET=(--)
return 1
@@ -132,10 +132,10 @@ parseopts() {
;;
1)
# --longopt=optarg
- if [[ $optarg ]]; then
+ if [[ $1 = *=* ]]; then
OPTRET+=("--$opt"
"$optarg")
# --longopt optarg
- elif [[ $2 ]]; then
+ elif (( $# > 1 )); then
OPTRET+=("--$opt" "$2" )
shift
# parse failure
diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh
index 9674c6a6..1d76f1ad 100755
--- a/test/scripts/parseopts_test.sh
+++ b/test/scripts/parseopts_test.sh
@@ -31,7 +31,7 @@ tap_parse() {
unset OPTRET
}
-tap_plan 50
+tap_plan 52
# usage: tap_parse test-params...
# a failed tap_parse will match only the end of options marker '--'
@@ -111,4 +111,7 @@ tap_parse '--force --' 2 --force
# exact match on possible stem (opt has optarg)
tap_parse '--clean foo --' 3 --clean=foo
+# all possible ways to specify empty optargs
+tap_parse '--key --pkg -p --' 7 --key '' --pkg='' -p ''
+
tap_finish
--
2.23.0
[pacman-dev] [PATCH v3] libmakepkg: add optional argument support to parseopts
Adds a "?" suffix that can be used to indicate that an option's argument is
optional.
This allows options to have a default behaviour when the user doesn't
specify one, e.g.: --color=[when] being able to behave like --color=auto
when only --color is passed
Options with optional arguments given on the command line will be returned
in the form "--opt=optarg" and "-o=optarg". Despite that not being the
syntax for passing an argument with a shortopt (trying to pass -o=foo
would make -o's argument "=foo"), this is done to allow the caller to split
the option and its optarg easily
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 116 +++-
test/scripts/parseopts_test.sh | 12 ++-
2 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index c056cb1e..42540438 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -18,16 +18,23 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# A getopt_long-like parser which portably supports longopts and
-# shortopts with some GNU extensions. It does not allow for options
-# with optional arguments. For both short and long opts, options
-# requiring an argument should be suffixed with a colon. After the
-# first argument containing the short opts, any number of valid long
-# opts may be be passed. The end of the options delimiter must then be
-# added, followed by the user arguments to the calling program.
+# shortopts with some GNU extensions. For both short and long opts,
+# options requiring an argument should be suffixed with a colon, and
+# options with optional arguments should be suffixed with a question
+# mark. After the first argument containing the short opts, any number
+# of valid long opts may be be passed. The end of the options delimiter
+# must then be added, followed by the user arguments to the calling
+# program.
+#
+# Options with optional arguments will be returned as "--longopt=optarg"
+# for longopts, or "-o=optarg" for shortopts. This isn't actually a valid
+# way to pass an optional argument with a shortopt on the command line,
+# but is done by parseopts to enable the caller script to split the option
+# and its optarg easily.
#
# Recommended Usage:
-# OPT_SHORT='fb:z'
-# OPT_LONG=('foo' 'bar:' 'baz')
+# OPT_SHORT='fb:zq?'
+# OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
# if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
# exit 1
# fi
@@ -49,29 +56,30 @@ parseopts() {
longoptmatch() {
local o longmatch=()
for o in "${longopts[@]}"; do
- if [[ ${o%:} = "$1" ]]; then
+ if [[ ${o%[:?]} = "$1" ]]; then
longmatch=("$o")
break
fi
- [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
+ [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
done
case ${#longmatch[*]} in
1)
- # success, override with opt and return arg req
(0 == none, 1 == required)
- opt=${longmatch%:}
- if [[ $longmatch = *: ]]; then
- return 1
- else
- return 0
- fi ;;
+ # success, override with opt and return arg req
(0 == none, 1 == required, 2 == optional)
+ opt=${longmatch%[:?]}
+ case $longmatch in
+ *:) return 1 ;;
+ *\?) return 2 ;;
+ *) return 0 ;;
+ esac
+ ;;
0)
# fail, no match found
return 255 ;;
*)
# fail, ambiguous match
printf "${0##*/}: $(gettext "option '%s' is
ambiguous; possibilities:")" "--$1"
- printf " '%s'" "${longmatch[@]%:}"
+ printf " '%s'" "${longmatch[@]%[:?]}"
printf '\n'
return 2
[pacman-dev] [PATCH v2] libmakepkg: add optional argument support to parseopts
Adds a "?" suffix that can be used to indicate that an option's argument is
optional.
This allows options to have a default behaviour when the user doesn't
specify one, e.g.: --color=[when] being able to behave like --color=auto
when only --color is passed
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 110 +++-
test/scripts/parseopts_test.sh | 12 ++-
2 files changed, 77 insertions(+), 45 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index c056cb1e..9a215648 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -18,16 +18,17 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# A getopt_long-like parser which portably supports longopts and
-# shortopts with some GNU extensions. It does not allow for options
-# with optional arguments. For both short and long opts, options
-# requiring an argument should be suffixed with a colon. After the
-# first argument containing the short opts, any number of valid long
-# opts may be be passed. The end of the options delimiter must then be
-# added, followed by the user arguments to the calling program.
+# shortopts with some GNU extensions. For both short and long opts,
+# options requiring an argument should be suffixed with a colon, and
+# options with optional arguments should be suffixed with a question
+# mark. After the first argument containing the short opts, any number
+# of valid long opts may be be passed. The end of the options delimiter
+# must then be added, followed by the user arguments to the calling
+# program.
#
# Recommended Usage:
-# OPT_SHORT='fb:z'
-# OPT_LONG=('foo' 'bar:' 'baz')
+# OPT_SHORT='fb:zq?'
+# OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
# if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
# exit 1
# fi
@@ -49,29 +50,30 @@ parseopts() {
longoptmatch() {
local o longmatch=()
for o in "${longopts[@]}"; do
- if [[ ${o%:} = "$1" ]]; then
+ if [[ ${o%[:?]} = "$1" ]]; then
longmatch=("$o")
break
fi
- [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
+ [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
done
case ${#longmatch[*]} in
1)
- # success, override with opt and return arg req
(0 == none, 1 == required)
- opt=${longmatch%:}
- if [[ $longmatch = *: ]]; then
- return 1
- else
- return 0
- fi ;;
+ # success, override with opt and return arg req
(0 == none, 1 == required, 2 == optional)
+ opt=${longmatch%[:?]}
+ case $longmatch in
+ *:) return 1 ;;
+ *\?) return 2 ;;
+ *) return 0 ;;
+ esac
+ ;;
0)
# fail, no match found
return 255 ;;
*)
# fail, ambiguous match
printf "${0##*/}: $(gettext "option '%s' is
ambiguous; possibilities:")" "--$1"
- printf " '%s'" "${longmatch[@]%:}"
+ printf " '%s'" "${longmatch[@]%[:?]}"
printf '\n'
return 254 ;;
esac >&2
@@ -87,32 +89,47 @@ parseopts() {
for (( i = 1; i < ${#1}; i++ )); do
opt=${1:i:1}
- # option doesn't exist
- if [[ $shortopts != *$opt* ]]; then
- printf "${0##*/}: $(gettext
"invalid option") -- '%s'\n" "$opt" >&2
- OPTRET=(--)
- return 1
- fi
-
- OPT
[pacman-dev] [PATCH] libmakepkg: add optional argument support to parseopts
Signed-off-by: Ethan Sommer
---
scripts/libmakepkg/util/parseopts.sh.in | 110 +++-
test/scripts/parseopts_test.sh | 12 ++-
2 files changed, 77 insertions(+), 45 deletions(-)
diff --git a/scripts/libmakepkg/util/parseopts.sh.in
b/scripts/libmakepkg/util/parseopts.sh.in
index c056cb1e..9a215648 100644
--- a/scripts/libmakepkg/util/parseopts.sh.in
+++ b/scripts/libmakepkg/util/parseopts.sh.in
@@ -18,16 +18,17 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# A getopt_long-like parser which portably supports longopts and
-# shortopts with some GNU extensions. It does not allow for options
-# with optional arguments. For both short and long opts, options
-# requiring an argument should be suffixed with a colon. After the
-# first argument containing the short opts, any number of valid long
-# opts may be be passed. The end of the options delimiter must then be
-# added, followed by the user arguments to the calling program.
+# shortopts with some GNU extensions. For both short and long opts,
+# options requiring an argument should be suffixed with a colon, and
+# options with optional arguments should be suffixed with a question
+# mark. After the first argument containing the short opts, any number
+# of valid long opts may be be passed. The end of the options delimiter
+# must then be added, followed by the user arguments to the calling
+# program.
#
# Recommended Usage:
-# OPT_SHORT='fb:z'
-# OPT_LONG=('foo' 'bar:' 'baz')
+# OPT_SHORT='fb:zq?'
+# OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
# if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
# exit 1
# fi
@@ -49,29 +50,30 @@ parseopts() {
longoptmatch() {
local o longmatch=()
for o in "${longopts[@]}"; do
- if [[ ${o%:} = "$1" ]]; then
+ if [[ ${o%[:?]} = "$1" ]]; then
longmatch=("$o")
break
fi
- [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
+ [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
done
case ${#longmatch[*]} in
1)
- # success, override with opt and return arg req
(0 == none, 1 == required)
- opt=${longmatch%:}
- if [[ $longmatch = *: ]]; then
- return 1
- else
- return 0
- fi ;;
+ # success, override with opt and return arg req
(0 == none, 1 == required, 2 == optional)
+ opt=${longmatch%[:?]}
+ case $longmatch in
+ *:) return 1 ;;
+ *\?) return 2 ;;
+ *) return 0 ;;
+ esac
+ ;;
0)
# fail, no match found
return 255 ;;
*)
# fail, ambiguous match
printf "${0##*/}: $(gettext "option '%s' is
ambiguous; possibilities:")" "--$1"
- printf " '%s'" "${longmatch[@]%:}"
+ printf " '%s'" "${longmatch[@]%[:?]}"
printf '\n'
return 254 ;;
esac >&2
@@ -87,32 +89,47 @@ parseopts() {
for (( i = 1; i < ${#1}; i++ )); do
opt=${1:i:1}
- # option doesn't exist
- if [[ $shortopts != *$opt* ]]; then
- printf "${0##*/}: $(gettext
"invalid option") -- '%s'\n" "$opt" >&2
- OPTRET=(--)
- return 1
- fi
-
- OPTRET+=("-$opt")
- # option requires optarg
- if [[ $shortopts = *$opt:* ]]; then
- # if we're not at the end of
the option chunk, the rest i
