| Message ID | 20250417102130.800-1-leimaohui@fujitsu.com |
|---|---|
| State | Accepted, archived |
| Commit | 198ae02439fb3c4146bfa05edbea30dfe3bad445 |
| Headers | show |
| Series | gzip: upgrade 1.13 -> 1.14 | expand |
On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org <leimaohui=fujitsu.com@lists.openembedded.org> wrote: > +Subject: [PATCH] tests/reference: fix invalid option of od issue > + > +ptest issue: > +| od: invalid option -- 'A' > +| BusyBox v1.37.0 () multi-call binary. Instead of working around busybox od limitations with a patch, ptest package should rdepend on coreutils to pull in the real original od. Alex
On Tue, 2025-04-22 at 17:24 +0200, Alexander Kanavin via lists.openembedded.org wrote: > On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org > <leimaohui=fujitsu.com@lists.openembedded.org> wrote: > > +Subject: [PATCH] tests/reference: fix invalid option of od issue > > + > > +ptest issue: > > +| od: invalid option -- 'A' > > +| BusyBox v1.37.0 () multi-call binary. > > Instead of working around busybox od limitations with a patch, ptest > package should rdepend on coreutils to pull in the real original od. Agreed, I've been meaning to reply to say exactly this! Cheers, Richard
On 4/22/25 8:36 AM, Richard Purdie via lists.openembedded.org wrote: > On Tue, 2025-04-22 at 17:24 +0200, Alexander Kanavin via > lists.openembedded.org wrote: >> On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org >> <leimaohui=fujitsu.com@lists.openembedded.org> wrote: >>> +Subject: [PATCH] tests/reference: fix invalid option of od issue >>> + >>> +ptest issue: >>> +| od: invalid option -- 'A' >>> +| BusyBox v1.37.0 () multi-call binary. >> >> Instead of working around busybox od limitations with a patch, ptest >> package should rdepend on coreutils to pull in the real original od. > > Agreed, I've been meaning to reply to say exactly this! > Consider other side of this as well. Many embedded systems do use busybox heavily, imposing dependencies for tests like this would mean ptests are less relevant for such usecases since bringing coreutils might also shunt other busybox utilities along the way. I would ask this patch to be proposed to gzip upstream first though. > Cheers, > > Richard > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#215233): https://lists.openembedded.org/g/openembedded-core/message/215233 > Mute This Topic: https://lists.openembedded.org/mt/112310990/1997914 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Wed, 23 Apr 2025 at 00:51, Khem Raj <raj.khem@gmail.com> wrote: > Consider other side of this as well. > Many embedded systems do use busybox heavily, imposing dependencies for > tests like this would mean ptests are less relevant for such usecases > since bringing coreutils might also shunt other busybox utilities along > the way. > > I would ask this patch to be proposed to gzip upstream first though. Nearly all upstreams run these tests with a GNU userspace environment on some classic binary distro. It's not that we impose GNU on ptests, it's that we don't want to deviate from upstream expectations, as doing that only results in upstream unknowingly introducing regressions and us fixing them after the fact, with no guarantee that upstreams will take the fixes. Version updates are hard enough as it is. I can kinda see the point in fixing ptests to work with musl, but fixing them to work with busybox is a step too far. There's a lot of ptests that already pull in various GNU pieces, so adding gzip to that set will not make it any worse. Alex
On 4/22/25 4:40 PM, Alexander Kanavin wrote: > On Wed, 23 Apr 2025 at 00:51, Khem Raj <raj.khem@gmail.com> wrote: >> Consider other side of this as well. >> Many embedded systems do use busybox heavily, imposing dependencies for >> tests like this would mean ptests are less relevant for such usecases >> since bringing coreutils might also shunt other busybox utilities along >> the way. >> >> I would ask this patch to be proposed to gzip upstream first though. > > Nearly all upstreams run these tests with a GNU userspace environment > on some classic binary distro. It's not that we impose GNU on ptests, > it's that we don't want to deviate from upstream expectations, as > doing that only results in upstream unknowingly introducing > regressions and us fixing them after the fact, with no guarantee that > upstreams will take the fixes. Version updates are hard enough as it > is. That is fine, and I can empathize. OEs usecases are unlike classic binary distros, otherwise thats what users would use. We need to be mindful of end users too. A bit of extra portability helps us, if we can get upstreams to support that then it helps our usecase. > > I can kinda see the point in fixing ptests to work with musl, but > fixing them to work with busybox is a step too far. There's a lot of > ptests that already pull in various GNU pieces, so adding gzip to that > set will not make it any worse. > > Alex
> -----邮件原件----- > 发件人: openembedded-core@lists.openembedded.org > <openembedded-core@lists.openembedded.org> 代表 Alexander Kanavin via > lists.openembedded.org > 发送时间: 2025年4月22日 23:25 > 收件人: Lei, Maohui/雷 茂慧 <leimaohui@fujitsu.com> > 抄送: Denys Dmytriyenko <denis@denix.org>; > openembedded-core@lists.openembedded.org > 主题: Re: [OE-core] [PATCH] gzip: upgrade 1.13 -> 1.14 > > On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org > <leimaohui=fujitsu.com@lists.openembedded.org> wrote: > > +Subject: [PATCH] tests/reference: fix invalid option of od issue > > + > > +ptest issue: > > +| od: invalid option -- 'A' > > +| BusyBox v1.37.0 () multi-call binary. > > Instead of working around busybox od limitations with a patch, ptest package > should rdepend on coreutils to pull in the real original od. I will continue this patch on behalf of my colleague Thank you, I know add rdepend on coreutils is the usual approach taken by oe-core for such cases. But have you noticed that gzip source code recommended to use perl instead of "od" if " If using "od" is not portable enough" right before the error point $ vi gzip-1.14/ tests/reference 42 # Ensure that compressing these simple strings always produces the same bytes. 43 # If using "od" is not portable enough, consider using this: 44 # perl -ne 'printf "%02x ", ord($_) for split //' Liu Yiding > > Alex
On Wed, 2025-04-23 at 03:53 +0000, Yiding Liu (Fujitsu) wrote: > > > > -----邮件原件----- > > 发件人: openembedded-core@lists.openembedded.org > > <openembedded-core@lists.openembedded.org> 代表 Alexander Kanavin via > > lists.openembedded.org > > 发送时间: 2025年4月22日 23:25 > > 收件人: Lei, Maohui/雷 茂慧 <leimaohui@fujitsu.com> > > 抄送: Denys Dmytriyenko <denis@denix.org>; > > openembedded-core@lists.openembedded.org > > 主题: Re: [OE-core] [PATCH] gzip: upgrade 1.13 -> 1.14 > > > > On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org > > <leimaohui=fujitsu.com@lists.openembedded.org> wrote: > > > +Subject: [PATCH] tests/reference: fix invalid option of od issue > > > + > > > +ptest issue: > > > +| od: invalid option -- 'A' > > > +| BusyBox v1.37.0 () multi-call binary. > > > > Instead of working around busybox od limitations with a patch, ptest package > > should rdepend on coreutils to pull in the real original od. > I will continue this patch on behalf of my colleague > > Thank you, I know add rdepend on coreutils is the usual approach taken by oe-core for such cases. > But have you noticed that gzip source code recommended to use perl instead of "od" if " If using "od" > is not portable enough" right before the error point > > > $ vi gzip-1.14/ tests/reference > 42 # Ensure that compressing these simple strings always produces the same bytes. > 43 # If using "od" is not portable enough, consider using this: > 44 # perl -ne 'printf "%02x ", ord($_) for split //' It is a maintainability question. Is carrying a patch doing this worth the effort of doing so? It adds maintenance overhead for us and I'm not sure we gain any significant benefits from it. I think I'd prefer just to add a ptest package specific dependency on coreutils rather than carrying a patch we could never upstream. We should add a comment about why it is needed. Mentioning the issue to upstream might able be helpful as they might change their test if they know it does cause downstreams some challenges. Cheers, Richard
> -----Original Message----- > From: Richard Purdie <richard.purdie@linuxfoundation.org> > Sent: 2025年4月24日 18:43 > To: Liu, Yiding/刘 乙丁 <liuyd.fnst@fujitsu.com>; alex.kanavin@gmail.com; > Khem Raj <raj.khem@gmail.com> > Cc: Denys Dmytriyenko <denis@denix.org>; > openembedded-core@lists.openembedded.org > Subject: Re: 回复: [OE-core] [PATCH] gzip: upgrade 1.13 -> 1.14 > > On Wed, 2025-04-23 at 03:53 +0000, Yiding Liu (Fujitsu) wrote: > > > > > > > -----邮件原件----- > > > 发件人: openembedded-core@lists.openembedded.org > > > <openembedded-core@lists.openembedded.org> 代表 Alexander Kanavin > via > > > lists.openembedded.org > > > 发送时间: 2025年4月22日 23:25 > > > 收件人: Lei, Maohui/雷 茂慧 <leimaohui@fujitsu.com> > > > 抄送: Denys Dmytriyenko <denis@denix.org>; > > > openembedded-core@lists.openembedded.org > > > 主题: Re: [OE-core] [PATCH] gzip: upgrade 1.13 -> 1.14 > > > > > > On Thu, 17 Apr 2025 at 12:21, leimaohui via lists.openembedded.org > > > <leimaohui=fujitsu.com@lists.openembedded.org> wrote: > > > > +Subject: [PATCH] tests/reference: fix invalid option of od issue > > > > + > > > > +ptest issue: > > > > +| od: invalid option -- 'A' > > > > +| BusyBox v1.37.0 () multi-call binary. > > > > > > Instead of working around busybox od limitations with a patch, ptest > > > package should rdepend on coreutils to pull in the real original od. > > I will continue this patch on behalf of my colleague > > > > Thank you, I know add rdepend on coreutils is the usual approach taken by > oe-core for such cases. > > But have you noticed that gzip source code recommended to use perl instead of > "od" if " If using "od" > > is not portable enough" right before the error point > > > > > > $ vi gzip-1.14/ tests/reference > > 42 # Ensure that compressing these simple strings always produces the same > bytes. > > 43 # If using "od" is not portable enough, consider using this: > > 44 # perl -ne 'printf "%02x ", ord($_) for split //' > > It is a maintainability question. Is carrying a patch doing this worth the effort of > doing so? It adds maintenance overhead for us and I'm not sure we gain any > significant benefits from it. > > I think I'd prefer just to add a ptest package specific dependency on coreutils > rather than carrying a patch we could never upstream. We should add a > comment about why it is needed. > > Mentioning the issue to upstream might able be helpful as they might change > their test if they know it does cause downstreams some challenges. Thank you Please refer to V2 patch and I have reported to upstream that we have met this issue > > Cheers, > > Richard
diff --git a/meta/recipes-extended/gzip/gzip-1.14/0001-tests-reference-fix-invalid-option-of-od-issue.patch b/meta/recipes-extended/gzip/gzip-1.14/0001-tests-reference-fix-invalid-option-of-od-issue.patch new file mode 100644 index 0000000000..cc7ddbef5a --- /dev/null +++ b/meta/recipes-extended/gzip/gzip-1.14/0001-tests-reference-fix-invalid-option-of-od-issue.patch @@ -0,0 +1,36 @@ +From fe8f7d75ca16e8c1c53b210dcdf130b91b57c252 Mon Sep 17 00:00:00 2001 +From: Lei Maohui <leimaohui@fujitsu.com> +Date: Thu, 17 Apr 2025 01:15:06 +0000 +Subject: [PATCH] tests/reference: fix invalid option of od issue + +ptest issue: +| od: invalid option -- 'A' +| BusyBox v1.37.0 () multi-call binary. + +replace "od" with "perl" according to the comments before error point +|# Ensure that compressing these simple strings always produces the same bytes. +|# If using "od" is not portable enough, consider using this: +|# perl -ne 'printf "%02x ", ord($_) for split //' +|for i in '' a b c yyy zzzzzzzzzzz; do +| echo $i: $(printf %s "$i" | gzip | od -An -tx1 | tr -d '\n') +|done + +Upstream-Status: Inappropriate [oe-core specific] +Signed-off-by: Lei Maohui <leimaohui@fujitsu.com> +--- + tests/reference | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tests/reference b/tests/reference +index add962e..bfd6ad3 100755 +--- a/tests/reference ++++ b/tests/reference +@@ -43,7 +43,7 @@ EOF + # If using "od" is not portable enough, consider using this: + # perl -ne 'printf "%02x ", ord($_) for split //' + for i in '' a b c yyy zzzzzzzzzzz; do +- echo $i: $(printf %s "$i" | gzip | od -An -tx1 | tr -d '\n') ++ echo $i: $(printf %s "$i" | gzip | perl -ne 'printf "%02x ", ord($_) for split //') + done > out || framework_failure_ + + compare exp out || fail=1 diff --git a/meta/recipes-extended/gzip/gzip-1.13/wrong-path-fix.patch b/meta/recipes-extended/gzip/gzip-1.14/wrong-path-fix.patch similarity index 76% rename from meta/recipes-extended/gzip/gzip-1.13/wrong-path-fix.patch rename to meta/recipes-extended/gzip/gzip-1.14/wrong-path-fix.patch index 7f9e249de8..4d5e7a8e02 100644 --- a/meta/recipes-extended/gzip/gzip-1.13/wrong-path-fix.patch +++ b/meta/recipes-extended/gzip/gzip-1.14/wrong-path-fix.patch @@ -1,4 +1,7 @@ -fix MakeMaker issues with using wrong SHELL/GREP +From eda9b1d08c517acbdc5b26c24c94a3985f29c749 Mon Sep 17 00:00:00 2001 +From: Ming Liu <ming.liu@windriver.com> +Date: Fri, 21 Nov 2014 04:50:57 -0500 +Subject: [PATCH] fix MakeMaker issues with using wrong SHELL/GREP A set of substitution is being processed to all target scripts with sed by replacing some key words with the detected values at configure time, this @@ -12,16 +15,15 @@ instead. Signed-off-by: Ming Liu <ming.liu@windriver.com> Upstream-Status: Pending - --- Makefile.am | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am -index 4f51b61..80a5ddf 100644 +index 23e0d3e..3455878 100644 --- a/Makefile.am +++ b/Makefile.am -@@ -93,8 +93,7 @@ SUFFIXES = .in +@@ -95,8 +95,7 @@ SUFFIXES = .in .in: $(AM_V_GEN)rm -f $@-t $@ \ && sed \ @@ -31,6 +33,3 @@ index 4f51b61..80a5ddf 100644 -e "s|'gzip'|$(GZIP_TRANSFORMED)|g" \ -e "s|'zdiff'|$(ZDIFF_TRANSFORMED)|g" \ -e "s|'zgrep'|$(ZGREP_TRANSFORMED)|g" \ --- -2.7.4 - diff --git a/meta/recipes-extended/gzip/gzip_1.13.bb b/meta/recipes-extended/gzip/gzip_1.14.bb similarity index 80% rename from meta/recipes-extended/gzip/gzip_1.13.bb rename to meta/recipes-extended/gzip/gzip_1.14.bb index fd846b30a5..f12dcb1e77 100644 --- a/meta/recipes-extended/gzip/gzip_1.13.bb +++ b/meta/recipes-extended/gzip/gzip_1.14.bb @@ -6,11 +6,13 @@ LICENSE = "GPL-3.0-or-later" SRC_URI = "${GNU_MIRROR}/gzip/${BP}.tar.gz \ file://run-ptest \ + file://0001-tests-reference-fix-invalid-option-of-od-issue.patch \ " SRC_URI:append:class-target = " file://wrong-path-fix.patch" LIC_FILES_CHKSUM = "file://COPYING;md5=1ebbd3e34237af26da5dc08a4e440464 \ - file://gzip.h;beginline=8;endline=20;md5=6e47caaa630e0c8bf9f1bc8d94a8ed0e" + file://gzip.h;beginline=8;endline=20;md5=a22158dc3dd3f5cf6e5a556940a49212 \ + " PROVIDES:append:class-native = " gzip-replacement-native" @@ -33,9 +35,10 @@ do_install_ptest() { -e 's/^Makefile: ..*/Makefile: /' \ -e 's,--sysroot=${STAGING_DIR_TARGET},,g' \ -e 's|${DEBUG_PREFIX_MAP}||g' \ + -e 's|${BUILD_LDFLAGS}||g' \ -e 's:${HOSTTOOLS_DIR}/::g' \ -e 's:${BASE_WORKDIR}/${MULTIMACH_TARGET_SYS}::g' \ ${B}/tests/Makefile > ${D}${PTEST_PATH}/src/tests/Makefile } -SRC_URI[sha256sum] = "20fc818aeebae87cdbf209d35141ad9d3cf312b35a5e6be61bfcfbf9eddd212a" +SRC_URI[sha256sum] = "613d6ea44f1248d7370c7ccdeee0dd0017a09e6c39de894b3c6f03f981191c6b"
fix issue: |ERROR: gzip-1.14-r0 do_package_qa: QA Issue: File /usr/lib/gzip/ptest/src/tests/Makefile in package gzip-ptest contains reference to TMPDIR ildpaths] |ERROR: gzip-1.14-r0 do_package_qa: Fatal QA errors were found, failing task. error point in /usr/lib/gzip/ptest/src/tests/Makefile of gzip-ptest: |BUILD_LDFLAGS = -L/gzip/1.14/recipe-sysroot-native/usr/lib -L/gzip/1.14/recipe-sysroot-native/lib | -Wl,--enable-new-dtags -Wl,-rpath-link,/gzip/1.14/recipe-sysroot-native/usr/lib -Wl,-r |path-link,/gzip/1.14/recipe-sysroot-native/lib -Wl,-rpath,/gzip/1.14/recipe-sysroot-native/usr/lib | -Wl,-rpath,/gzip/1.14/recipe-sysroot-native/lib -Wl,-O1 -Wl,--allow-shlib-undefined -Wl,--dynamic-linker=/mnt |/test/build_auh/tmp/sysroots-uninative/x86_64-linux/lib/ld-linux-x86-64.so.2 -pthread ptest pass after delete BUILD_LDFLAGS in Makefile ptest result: |============================================================================ |Testsuite summary for gzip 1.14 |============================================================================ |# TOTAL: 30 |# PASS: 28 |# SKIP: 2 |# XFAIL: 0 |# FAIL: 0 |# XPASS: 0 |# ERROR: 0 License-Update: copyright years update and simplifies compliance by replacing physical contact instructions with a permanent web reference Signed-off-by: Lei Maohui <leimaohui@fujitsu.com> --- ...rence-fix-invalid-option-of-od-issue.patch | 36 +++++++++++++++++++ .../wrong-path-fix.patch | 13 ++++--- .../gzip/{gzip_1.13.bb => gzip_1.14.bb} | 7 ++-- 3 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 meta/recipes-extended/gzip/gzip-1.14/0001-tests-reference-fix-invalid-option-of-od-issue.patch rename meta/recipes-extended/gzip/{gzip-1.13 => gzip-1.14}/wrong-path-fix.patch (76%) rename meta/recipes-extended/gzip/{gzip_1.13.bb => gzip_1.14.bb} (80%)