| Message ID | 20220308113305.39395-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | test-lib.sh: use awk instead of expr for a POSIX non integer check | expand |
On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón <carenas@gmail.com> wrote: > Restrict the glibc version to a single version number and compare it > arithmetically against the base glibc version to avoid accidentally > matching against "2.3" and better supporting versions like "2.34.9000" > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > diff --git a/t/test-lib.sh b/t/test-lib.sh > @@ -518,9 +518,9 @@ else > - if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) && > - _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} && > - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | > + awk '{ if ($2 - 2.34 < 0) exit 1 }' No need for `cut` since `awk` can accomplish the same by itself. if echo "$_GLIBC_VERSION" | awk '/^glibc / { if ($2 - 2.34 < 0) exit 1 }' should work, I would think.
On Tue, Mar 8, 2022 at 6:55 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón > <carenas@gmail.com> wrote: > > Restrict the glibc version to a single version number and compare it > > arithmetically against the base glibc version to avoid accidentally > > matching against "2.3" and better supporting versions like "2.34.9000" > > > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > > --- > > diff --git a/t/test-lib.sh b/t/test-lib.sh > > @@ -518,9 +518,9 @@ else > > - if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) && > > - _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} && > > - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null > > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) > > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | > > + awk '{ if ($2 - 2.34 < 0) exit 1 }' > > No need for `cut` since `awk` can accomplish the same by itself. > > if echo "$_GLIBC_VERSION" | awk '/^glibc / { if ($2 - 2.34 < 0) exit 1 }' > > should work, I would think. Nevermind, I forgot you want to better support "2.34.9000" matches. Though, awk should still be able to do so on its own, one would expect, but not too important.
On Tue, Mar 8, 2022 at 6:58 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > On Tue, Mar 8, 2022 at 6:55 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón > > <carenas@gmail.com> wrote: > > > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) > > > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | > > > + awk '{ if ($2 - 2.34 < 0) exit 1 }' > > > > No need for `cut` since `awk` can accomplish the same by itself. > > > > if echo "$_GLIBC_VERSION" | awk '/^glibc / { if ($2 - 2.34 < 0) exit 1 }' > > > > should work, I would think. > > Nevermind, I forgot you want to better support "2.34.9000" matches. > Though, awk should still be able to do so on its own, one would > expect, but not too important. This seems to work, though it's getting a bit verbose: awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) - 2.34 < 0) exit 1 }'
Eric Sunshine <sunshine@sunshineco.com> writes: > On Tue, Mar 8, 2022 at 6:58 PM Eric Sunshine <sunshine@sunshineco.com> wrote: >> On Tue, Mar 8, 2022 at 6:55 PM Eric Sunshine <sunshine@sunshineco.com> wrote: >> > On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón >> > <carenas@gmail.com> wrote: >> > > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) >> > > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | >> > > + awk '{ if ($2 - 2.34 < 0) exit 1 }' >> > >> > No need for `cut` since `awk` can accomplish the same by itself. >> > >> > if echo "$_GLIBC_VERSION" | awk '/^glibc / { if ($2 - 2.34 < 0) exit 1 }' >> > >> > should work, I would think. >> >> Nevermind, I forgot you want to better support "2.34.9000" matches. >> Though, awk should still be able to do so on its own, one would >> expect, but not too important. > > This seems to work, though it's getting a bit verbose: > > awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) > - 2.34 < 0) exit 1 }' If we are losing "cut" (which I think is a good thing to do), we probably can lose the pipe, too and refer to $_GLIBC_VERSION as an element in ARGV[] and make the command used as "if" condition to a single "awk" script? In general it is a good discipline to question a pipeline that preprocesses input fed to a script written in a language with full programming power like awk and perl (and to lessor extent, sed) to see if we can come up with a simpler solution without pipeline helping to solve what these languages are invented to solve, and I very much appreciate your exploration ;-) Thanks.
On Wed, Mar 09 2022, Junio C Hamano wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > >> On Tue, Mar 8, 2022 at 6:58 PM Eric Sunshine <sunshine@sunshineco.com> wrote: >>> On Tue, Mar 8, 2022 at 6:55 PM Eric Sunshine <sunshine@sunshineco.com> wrote: >>> > On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón >>> > <carenas@gmail.com> wrote: >>> > > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) >>> > > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | >>> > > + awk '{ if ($2 - 2.34 < 0) exit 1 }' >>> > >>> > No need for `cut` since `awk` can accomplish the same by itself. >>> > >>> > if echo "$_GLIBC_VERSION" | awk '/^glibc / { if ($2 - 2.34 < 0) exit 1 }' >>> > >>> > should work, I would think. >>> >>> Nevermind, I forgot you want to better support "2.34.9000" matches. >>> Though, awk should still be able to do so on its own, one would >>> expect, but not too important. >> >> This seems to work, though it's getting a bit verbose: >> >> awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) >> - 2.34 < 0) exit 1 }' > > If we are losing "cut" (which I think is a good thing to do), we > probably can lose the pipe, too and refer to $_GLIBC_VERSION as an > element in ARGV[] and make the command used as "if" condition to a > single "awk" script? > > In general it is a good discipline to question a pipeline that > preprocesses input fed to a script written in a language with full > programming power like awk and perl (and to lessor extent, sed) to > see if we can come up with a simpler solution without pipeline > helping to solve what these languages are invented to solve, and I > very much appreciate your exploration ;-) I agree :) But the first language we've got here is C. Rather than fiddle around with getconf, awk/sed etc. why not just the rather trivial: diff --git a/Makefile b/Makefile index 6f0b4b775fe..f566c9c5df2 100644 --- a/Makefile +++ b/Makefile @@ -732,6 +732,7 @@ TEST_BUILTINS_OBJS += test-parse-pathspec-file.o TEST_BUILTINS_OBJS += test-partial-clone.o TEST_BUILTINS_OBJS += test-path-utils.o TEST_BUILTINS_OBJS += test-pcre2-config.o +TEST_BUILTINS_OBJS += test-glibc-config.o TEST_BUILTINS_OBJS += test-pkt-line.o TEST_BUILTINS_OBJS += test-prio-queue.o TEST_BUILTINS_OBJS += test-proc-receive.o diff --git a/t/helper/test-glibc-config.c b/t/helper/test-glibc-config.c new file mode 100644 index 00000000000..3c3cc2a8ba5 --- /dev/null +++ b/t/helper/test-glibc-config.c @@ -0,0 +1,12 @@ +#include "test-tool.h" +#include "cache.h" + +int cmd__glibc_config(int argc, const char **argv) +{ +#ifdef __GNU_LIBRARY__ + printf("%d\n%d\n", __GLIBC__, __GLIBC_MINOR__); + return 0; +#else + return 1; +#endif +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index e6ec69cf326..c01422f5cab 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -35,6 +35,7 @@ static struct test_cmd cmds[] = { { "genrandom", cmd__genrandom }, { "genzeros", cmd__genzeros }, { "getcwd", cmd__getcwd }, + { "glibc-config", cmd__glibc_config }, { "hashmap", cmd__hashmap }, { "hash-speed", cmd__hash_speed }, { "index-version", cmd__index_version }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 20756eefdda..dc061bc7833 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -26,6 +26,7 @@ int cmd__fast_rebase(int argc, const char **argv); int cmd__genrandom(int argc, const char **argv); int cmd__genzeros(int argc, const char **argv); int cmd__getcwd(int argc, const char **argv); +int cmd__glibc_config(int argc, const char **argv); int cmd__hashmap(int argc, const char **argv); int cmd__hash_speed(int argc, const char **argv); int cmd__index_version(int argc, const char **argv); I mainly copied the pcre2-config template I added in 95ca1f987ed (grep/pcre2: better support invalid UTF-8 haystacks, 2021-01-24), which likewise would have been quite a bit more complex to do from non-C.
On Wed, Mar 9, 2022 at 12:47 PM Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > > On Tue, Mar 8, 2022 at 6:58 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > >> > On Tue, Mar 8, 2022 at 6:44 PM Carlo Marcelo Arenas Belón > >> > <carenas@gmail.com> wrote: > >> > > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) > >> > > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | > >> > > + awk '{ if ($2 - 2.34 < 0) exit 1 }' > > > > This seems to work, though it's getting a bit verbose: > > > > awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) > > - 2.34 < 0) exit 1 }' > > If we are losing "cut" (which I think is a good thing to do), we > probably can lose the pipe, too and refer to $_GLIBC_VERSION as an > element in ARGV[] and make the command used as "if" condition to a > single "awk" script? Erm, something like this perhaps? awk 'BEGIN { split(ARGV[1], v, "[ .]"); if (v[1]=="glibc" && sprintf("%s.%s", v[2], v[3]) - 2.34 < 0) exit 1 }' > In general it is a good discipline to question a pipeline that > preprocesses input fed to a script written in a language with full > programming power like awk and perl (and to lessor extent, sed) to > see if we can come up with a simpler solution without pipeline > helping to solve what these languages are invented to solve, and I > very much appreciate your exploration ;-) Yup, although in this case, the pure-awk solution may not convey the intent as easily as the original posted by Carlo which had the benefit of being perhaps easier to digest.
On Wed, Mar 9, 2022 at 3:14 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Wed, Mar 09 2022, Junio C Hamano wrote: > > Eric Sunshine <sunshine@sunshineco.com> writes: > >> This seems to work, though it's getting a bit verbose: > >> > >> awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) > >> - 2.34 < 0) exit 1 }' > > > > In general it is a good discipline to question a pipeline that > > preprocesses input fed to a script written in a language with full > > programming power like awk and perl (and to lessor extent, sed) to > > see if we can come up with a simpler solution without pipeline > > helping to solve what these languages are invented to solve, and I > > very much appreciate your exploration ;-) > > I agree :) But the first language we've got here is C. Rather than > fiddle around with getconf, awk/sed etc. why not just the rather > trivial: > > +#include "test-tool.h" > +#include "cache.h" > + > +int cmd__glibc_config(int argc, const char **argv) > +{ > +#ifdef __GNU_LIBRARY__ > + printf("%d\n%d\n", __GLIBC__, __GLIBC_MINOR__); > + return 0; > +#else > + return 1; > +#endif > +} It feels overkill to add this just for this one case which is otherwise done easily enough with existing shell tools. That said, perhaps I'm missing something, but I don't see how this solution helps us get away from the need for `expr`, `awk`, or `perl` since one of those languages would be needed to perform the arithmetic comparison (checking if glibc is >= 2.34).
On Fri, Mar 11 2022, Eric Sunshine wrote: > On Wed, Mar 9, 2022 at 3:14 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> On Wed, Mar 09 2022, Junio C Hamano wrote: >> > Eric Sunshine <sunshine@sunshineco.com> writes: >> >> This seems to work, though it's getting a bit verbose: >> >> >> >> awk '/^glibc / { split($2,v,"."); if (sprintf("%s.%s", v[1], v[2]) >> >> - 2.34 < 0) exit 1 }' >> > >> > In general it is a good discipline to question a pipeline that >> > preprocesses input fed to a script written in a language with full >> > programming power like awk and perl (and to lessor extent, sed) to >> > see if we can come up with a simpler solution without pipeline >> > helping to solve what these languages are invented to solve, and I >> > very much appreciate your exploration ;-) >> >> I agree :) But the first language we've got here is C. Rather than >> fiddle around with getconf, awk/sed etc. why not just the rather >> trivial: >> >> +#include "test-tool.h" >> +#include "cache.h" >> + >> +int cmd__glibc_config(int argc, const char **argv) >> +{ >> +#ifdef __GNU_LIBRARY__ >> + printf("%d\n%d\n", __GLIBC__, __GLIBC_MINOR__); >> + return 0; >> +#else >> + return 1; >> +#endif >> +} > > It feels overkill to add this just for this one case which is > otherwise done easily enough with existing shell tools. > > That said, perhaps I'm missing something, but I don't see how this > solution helps us get away from the need for `expr`, `awk`, or `perl` > since one of those languages would be needed to perform the arithmetic > comparison (checking if glibc is >= 2.34). In this case they're \n delimited, so we can use the shell's native whitespace splitting to $(())-compare $1 and $2. But probably better is to just amend that to call it as "test-tool libc is-glibc-2.34-or-newer" or whatever. Then just do: if (__GLIBC__ > 2 || (__GLIBC__ == 2 && 34 >= __GLIBC_MINOR__)) return 0; return 1;
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > But probably better is to just amend that to call it as "test-tool libc > is-glibc-2.34-or-newer" or whatever. Then just do: > > if (__GLIBC__ > 2 || (__GLIBC__ == 2 && 34 >= __GLIBC_MINOR__)) > return 0; > return 1; Yuck. Then we'd have yet another libc-is-glibc-2.36-or-newer option, too, in the future?
On Sat, Mar 12, 2022 at 6:21 PM Junio C Hamano <gitster@pobox.com> wrote: > > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > But probably better is to just amend that to call it as "test-tool libc > > is-glibc-2.34-or-newer" or whatever. Then just do: > > > > if (__GLIBC__ > 2 || (__GLIBC__ == 2 && 34 >= __GLIBC_MINOR__)) > > return 0; > > return 1; > > Yuck. Then we'd have yet another libc-is-glibc-2.36-or-newer > option, too, in the future? Luckily that won't be needed, as this the original version (with expr) is practically good enough even if it might be a little odd looking and incorrect for 2.4 <= glibc <= 2.9 (which are over 10 years old). $ expr 2.34 \<= "2.34.9000" 1 $ expr 2.34 \<= "" 0 Apologies for the confusion, and feel free to drop this patch Carlo
Carlo Arenas <carenas@gmail.com> writes: > On Sat, Mar 12, 2022 at 6:21 PM Junio C Hamano <gitster@pobox.com> wrote: >> >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >> > But probably better is to just amend that to call it as "test-tool libc >> > is-glibc-2.34-or-newer" or whatever. Then just do: >> > >> > if (__GLIBC__ > 2 || (__GLIBC__ == 2 && 34 >= __GLIBC_MINOR__)) >> > return 0; >> > return 1; >> >> Yuck. Then we'd have yet another libc-is-glibc-2.36-or-newer >> option, too, in the future? > > Luckily that won't be needed, as this the original version (with expr) > is practically good enough even if it might be a little odd looking > and incorrect for 2.4 <= glibc <= 2.9 (which are over 10 years old). > > $ expr 2.34 \<= "2.34.9000" > 1 > $ expr 2.34 \<= "" > 0 Yeah, that is good. What I was trying to get at was to extend Ævar's one trivially to $ test-tool libc-is-at-or-later-than 2.34 so that we can deal with $ test-tool libc-is-at-or-later-than 2.36 for free, but if we do not have to do anything, that is even better ;-)
Il giorno mar 8 mar 2022 alle ore 12:34 Carlo Marcelo Arenas Belón <carenas@gmail.com> ha scritto: > > Restrict the glibc version to a single version number and compare it > arithmetically against the base glibc version to avoid accidentally > matching against "2.3" and better supporting versions like "2.34.9000" > I didn't understand the problem. How glibc names the versions is known: https://sourceware.org/glibc/wiki/Glibc%20Timeline What is wrong with the expr statement ? + expr 2.34 '<=' 2.35 1 + expr 2.34 '<=' 2.34 1 + expr 2.34 '<=' 2.33 0 + expr 2.34 '<=' 2.32 0 + expr 2.34 '<=' 2.31 0 + expr 2.34 '<=' 2.30 0 + expr 2.34 '<=' 2.29 0 + expr 2.34 '<=' 2.28 0 + expr 2.34 '<=' 2.27 0 + expr 2.34 '<=' 2.26 0 + expr 2.34 '<=' 2.25 0 + expr 2.34 '<=' 2.24 0 + expr 2.34 '<=' 2.23 0 + expr 2.34 '<=' 2.22 0 + expr 2.34 '<=' 2.21 0 + expr 2.34 '<=' 2.20 0 + expr 2.34 '<=' 2.19 0 + expr 2.34 '<=' 2.18 0 + expr 2.34 '<=' 2.17 0 + expr 2.34 '<=' 2.16 0 + expr 2.34 '<=' 2.15 0 + expr 2.34 '<=' 2.13 > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > t/test-lib.sh | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 8e59c58e7e7..f624f87eb81 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -518,9 +518,9 @@ else > setup_malloc_check () { > MALLOC_CHECK_=3 MALLOC_PERTURB_=165 > export MALLOC_CHECK_ MALLOC_PERTURB_ > - if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) && > - _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} && > - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null > + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) > + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | > + awk '{ if ($2 - 2.34 < 0) exit 1 }' > then > g= > LD_PRELOAD="libc_malloc_debug.so.0" > -- > 2.35.1.505.g27486cd1b2d >
diff --git a/t/test-lib.sh b/t/test-lib.sh index 8e59c58e7e7..f624f87eb81 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -518,9 +518,9 @@ else setup_malloc_check () { MALLOC_CHECK_=3 MALLOC_PERTURB_=165 export MALLOC_CHECK_ MALLOC_PERTURB_ - if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) && - _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} && - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null + local _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) + if echo "$_GLIBC_VERSION" | cut -d. -f1-2 | + awk '{ if ($2 - 2.34 < 0) exit 1 }' then g= LD_PRELOAD="libc_malloc_debug.so.0"
Restrict the glibc version to a single version number and compare it arithmetically against the base glibc version to avoid accidentally matching against "2.3" and better supporting versions like "2.34.9000" Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- t/test-lib.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)