Re: [devel-distro] [PATCH] reports.mk: Generate rpm and srpms lists

Re: [devel-distro] [PATCH] reports.mk: Generate rpm and srpms lists

[Michael Shigorin]

[-- Attachment #1: Type: text/plain, Size: 2144 bytes --]

On Fri, Apr 16, 2021 at 03:34:11PM +0700, Anton Midyukov wrote:
> --- a/reports.mk
> +++ b/reports.mk
> +	@cat $(BUILDLOG) | grep -E 'chroot/.in/[^/]*.rpm' | cut -d' ' -f 1 | tr -d "'"'`' | \
> +		rev | cut -d'/' -f 1 | rev | sort -u > "$(REPORTDIR)/list-rpms.txt"
> +	@cat $(BUILDLOG) | grep -E 'chroot/.in/[^/]*.rpm' | cut -d' ' -f 1 | tr -d "'"'`' | \
> +		xargs rpm -qp --queryformat '%{sourcerpm}\n' | sort -u > "$(REPORTDIR)/list-srpms.txt"

Типовой useless use of cat(1): можно же делать сразу

grep ... < $(BUILDLOG) | cut ...

:-)

Заодно причесал и выделил отдельной целью reports/packages,
она может быть самоценна.

PS: ещё немножко посмотрел этот скрипт -- не настаиваю на мерже
последнего патча из приложенных трёх, но мне кажется, что так
чуточку предпочтительней и на двадцатую долю секунды быстрей:

[ прогон для разогрева дискового кэша ]
$ time sh reports1.sh  
sh reports1.sh  0,72s user 0,13s system 109% cpu 0,768 total
$ time bash reports2.sh
bash reports2.sh  0,65s user 0,12s system 106% cpu 0,723 total
$ md5sum list-*
e592b01a6c506685aac3b1026eb92b93  list-rpms.1.txt
e592b01a6c506685aac3b1026eb92b93  list-rpms.2.txt
aef452a7062c2be24b0e23ba418e160b  list-srpms.1.txt
aef452a7062c2be24b0e23ba418e160b  list-srpms.2.txt
$ head reports*
==> reports1.sh <==
BUILDLOG=$HOME/out/alt-server-20210428-x86_64.iso.log; REPORTDIR=$TMP;
grep -E 'chroot/.in/[^/]*.rpm' < $BUILDLOG | cut -d' ' -f 1 | tr -d "'"'`' | sed 's,^.*/,,' | sort -u > "$REPORTDIR/list-rpms.1.txt"
grep -E 'chroot/.in/[^/]*.rpm' < $BUILDLOG | cut -d' ' -f 1 | tr -d "'"'`' | xargs rpm -qp --queryformat '%{sourcerpm}\n' | sort -u > "$REPORTDIR/list-srpms.1.txt"

==> reports2.sh <==
BUILDLOG=$HOME/out/alt-server-20210428-x86_64.iso.log; REPORTDIR=$TMP; grep -E 'chroot/.in/[^/]*.rpm' < $BUILDLOG | cut -d' ' -f 1 | tr -d "'"'`' |tee /dev/stderr 2> >(sed 's,^.*/,,' | sort -u > "$REPORTDIR/list-rpms.2.txt") | xargs rpm -qp --queryformat '%{sourcerpm}\n' | sort -u > "$REPORTDIR/list-srpms.2.txt"

PPS: когда копипаст махровый лезет -- пиши, гляну.

-- 
 ---- WBR, Michael Shigorin / http://altlinux.org
  ------ http://opennet.ru / http://anna-news.info

[-- Attachment #2: 0001-reports.mk-factor-out-reports-packages.patch --]
[-- Type: text/x-patch, Size: 1723 bytes --]

>From 14ecaee2cbb65ef5e37b379181f9bcd25c35c455 Mon Sep 17 00:00:00 2001
From: Michael Shigorin <mike@altlinux.org>
Date: Wed, 28 Apr 2021 14:48:59 +0300
Subject: [PATCH 1/3] reports.mk: factor out reports/packages

...making it fit 80 columns while at that
to avoid horizontal spaghetti code.
---
 reports.mk | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/reports.mk b/reports.mk
index 9efa4f885..0e44c4471 100644
--- a/reports.mk
+++ b/reports.mk
@@ -26,7 +26,8 @@ define report_body
 fi; }
 endef
 
-all: reports/targets reports/scripts reports/cleanlog reports/contents
+all: reports/targets reports/scripts reports/cleanlog \
+	reports/contents reports/packages
 	@if [ -n "$(IMAGE_OUTPATH)" ]; then \
 		rm -fr "$(LOGDIR)/$(IMAGE_OUTFILE).reports"; \
 		cp -a "$(REPORTDIR)" "$(LOGDIR)/$(IMAGE_OUTFILE).reports"; \
@@ -75,10 +76,16 @@ reports/contents: reports/prep
 			echo "reports.mk: missing isoinfo" >&2; \
 		fi; \
 	esac
-	@cat $(BUILDLOG) | grep -E 'chroot/.in/[^/]*.rpm' | cut -d' ' -f 1 | tr -d "'"'`' | \
-		rev | cut -d'/' -f 1 | rev | sort -u > "$(REPORTDIR)/list-rpms.txt"
-	@cat $(BUILDLOG) | grep -E 'chroot/.in/[^/]*.rpm' | cut -d' ' -f 1 | tr -d "'"'`' | \
-		xargs rpm -qp --queryformat '%{sourcerpm}\n' | sort -u > "$(REPORTDIR)/list-srpms.txt"
+
+reports/packages: reports/prep
+	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
+		cut -d' ' -f 1 | tr -d "'"'`' | \
+		rev | cut -d'/' -f 1 | rev | \
+		sort -u > "$(REPORTDIR)/list-rpms.txt"
+	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
+		cut -d' ' -f 1 | tr -d "'"'`' | \
+		xargs rpm -qp --queryformat '%{sourcerpm}\n' | \
+		sort -u > "$(REPORTDIR)/list-srpms.txt"
 
 else
 all:; @:
-- 
2.25.4


[-- Attachment #3: 0002-reports.mk-streamline-basedir-1-equivalent.patch --]
[-- Type: text/x-patch, Size: 1047 bytes --]

>From 5eca87b3314d00a11009a5b5f3b57e1a17dec949 Mon Sep 17 00:00:00 2001
From: Michael Shigorin <mike@altlinux.org>
Date: Wed, 28 Apr 2021 14:50:06 +0300
Subject: [PATCH 2/3] reports.mk: streamline basedir(1) equivalent

That "clever hack" with two rev(1) invocations
seems pretty stupid to me: too much processes
and processing instead of simple prefix removal.

One can also have that as ${var##*/} when dealing
with a shell variable and avoiding extra forks, BTW.
---
 reports.mk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/reports.mk b/reports.mk
index 0e44c4471..7b81e785a 100644
--- a/reports.mk
+++ b/reports.mk
@@ -79,8 +79,7 @@ reports/contents: reports/prep
 
 reports/packages: reports/prep
 	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
-		cut -d' ' -f 1 | tr -d "'"'`' | \
-		rev | cut -d'/' -f 1 | rev | \
+		cut -d' ' -f 1 | tr -d "'"'`' | sed 's,^.*/,,' | \
 		sort -u > "$(REPORTDIR)/list-rpms.txt"
 	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
 		cut -d' ' -f 1 | tr -d "'"'`' | \
-- 
2.25.4


[-- Attachment #4: 0003-reports.mk-use-process-substitution-to-dedup.patch --]
[-- Type: text/x-patch, Size: 1346 bytes --]

>From 20642ae4aa3847403d3462e74ace4c00ca3240b3 Mon Sep 17 00:00:00 2001
From: Michael Shigorin <mike@altlinux.org>
Date: Wed, 28 Apr 2021 15:06:57 +0300
Subject: [PATCH 3/3] reports.mk: use process substitution to dedup

The first half of both pipes was clearly a copy-pasted
initial logfile processing; the file can be of considerable
size (e.g. several megabytes) so it might be slightly more
efficient and cool (but a bit more arcane) to use bash(1)'s
process substitution along with good ol' tee(1) like this:

$ echo -e '1\n2\n3' |tee /dev/stderr 2> >(grep 2 >STDERR) |grep 1 >STDOUT
$ head STD*
==> STDERR <==
2

==> STDOUT <==
1
---
 reports.mk | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/reports.mk b/reports.mk
index 7b81e785a..247c722b5 100644
--- a/reports.mk
+++ b/reports.mk
@@ -78,11 +78,10 @@ reports/contents: reports/prep
 	esac
 
 reports/packages: reports/prep
-	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
-		cut -d' ' -f 1 | tr -d "'"'`' | sed 's,^.*/,,' | \
-		sort -u > "$(REPORTDIR)/list-rpms.txt"
 	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
 		cut -d' ' -f 1 | tr -d "'"'`' | \
+		tee /dev/stderr 2> >(sed 's,^.*/,,' | \
+			sort -u > "$(REPORTDIR)/list-rpms.txt") | \
 		xargs rpm -qp --queryformat '%{sourcerpm}\n' | \
 		sort -u > "$(REPORTDIR)/list-srpms.txt"
 
-- 
2.25.4

Re: [devel-distro] [PATCH] reports.mk: Generate rpm and srpms lists

[Michael Shigorin]

[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]

On Wed, Apr 28, 2021 at 03:38:53PM +0300, Michael Shigorin wrote:
> PS: ещё немножко посмотрел этот скрипт -- не настаиваю на мерже
> последнего патча из приложенных трёх, но

> >From 20642ae4aa3847403d3462e74ace4c00ca3240b3 Mon Sep 17 00:00:00 2001
> From: Michael Shigorin <mike@altlinux.org>
> Date: Wed, 28 Apr 2021 15:06:57 +0300
> Subject: [PATCH 3/3] reports.mk: use process substitution to dedup
> 
> The first half of both pipes was clearly a copy-pasted
> initial logfile processing; the file can be of considerable
> size (e.g. several megabytes) so it might be slightly more
> efficient and cool (but a bit more arcane) to use bash(1)'s

Сам же написал -- bash; а SHELL по умолчанию будет sh.

> process substitution along with good ol' tee(1) like this:
> 
> --- a/reports.mk
> +++ b/reports.mk
>  
>  reports/packages: reports/prep
> -	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
> -		cut -d' ' -f 1 | tr -d "'"'`' | sed 's,^.*/,,' | \
> -		sort -u > "$(REPORTDIR)/list-rpms.txt"
>  	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
>  		cut -d' ' -f 1 | tr -d "'"'`' | \
> +		tee /dev/stderr 2> >(sed 's,^.*/,,' | \
> +			sort -u > "$(REPORTDIR)/list-rpms.txt") | \
>  		xargs rpm -qp --queryformat '%{sourcerpm}\n' | \
>  		sort -u > "$(REPORTDIR)/list-srpms.txt"

По идее, на стадии report.mk нам уже нет дела до трассировки,
которая взводится в lib/report.mk накруткой на SHELL;
понадобится ещё вот такая строчка сперва:

reports/packages: SHELL = /bin/bash

Можно свести с предыдущим коммитом (если его забирать),
чтоб сразу был исправный.

-- 
 ---- WBR, Michael Shigorin / http://altlinux.org
  ------ http://opennet.ru / http://anna-news.info

[-- Attachment #2: 0001-reports.mk-provide-bashism-with-bash.patch --]
[-- Type: text/x-patch, Size: 814 bytes --]

>From 9c4e1fee862c7c80f2fae2bd290a25b5b3d315f9 Mon Sep 17 00:00:00 2001
From: Michael Shigorin <mike@altlinux.org>
Date: Wed, 28 Apr 2021 16:29:46 +0300
Subject: [PATCH] reports.mk: provide bashism with bash

Wrote that it's a bashism myself and neglected
to specify the proper shell, sigh.

This SHELL manipulation could ruin tracing
piggybacked onto it within lib/report.mk
but it's too late at this stage anyways.
---
 reports.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/reports.mk b/reports.mk
index 247c722b5..f6020ce3c 100644
--- a/reports.mk
+++ b/reports.mk
@@ -77,6 +77,7 @@ reports/contents: reports/prep
 		fi; \
 	esac
 
+reports/packages: SHELL = /bin/bash
 reports/packages: reports/prep
 	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
 		cut -d' ' -f 1 | tr -d "'"'`' | \
-- 
2.25.4

Re: [devel-distro] [PATCH] reports.mk: Generate rpm and srpms lists

[Антон Мидюков]

28.04.2021 20:52, Michael Shigorin пишет:
> On Wed, Apr 28, 2021 at 03:38:53PM +0300, Michael Shigorin wrote:
>> PS: ещё немножко посмотрел этот скрипт -- не настаиваю на мерже
>> последнего патча из приложенных трёх, но
> 
>> >From 20642ae4aa3847403d3462e74ace4c00ca3240b3 Mon Sep 17 00:00:00 2001
>> From: Michael Shigorin <mike@altlinux.org>
>> Date: Wed, 28 Apr 2021 15:06:57 +0300
>> Subject: [PATCH 3/3] reports.mk: use process substitution to dedup
>>
>> The first half of both pipes was clearly a copy-pasted
>> initial logfile processing; the file can be of considerable
>> size (e.g. several megabytes) so it might be slightly more
>> efficient and cool (but a bit more arcane) to use bash(1)'s
> 
> Сам же написал -- bash; а SHELL по умолчанию будет sh.
> 
>> process substitution along with good ol' tee(1) like this:
>>
>> --- a/reports.mk
>> +++ b/reports.mk
>>  
>>  reports/packages: reports/prep
>> -	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
>> -		cut -d' ' -f 1 | tr -d "'"'`' | sed 's,^.*/,,' | \
>> -		sort -u > "$(REPORTDIR)/list-rpms.txt"
>>  	@grep -E 'chroot/.in/[^/]*.rpm' < $(BUILDLOG) | \
>>  		cut -d' ' -f 1 | tr -d "'"'`' | \
>> +		tee /dev/stderr 2> >(sed 's,^.*/,,' | \
>> +			sort -u > "$(REPORTDIR)/list-rpms.txt") | \
>>  		xargs rpm -qp --queryformat '%{sourcerpm}\n' | \
>>  		sort -u > "$(REPORTDIR)/list-srpms.txt"
> 
> По идее, на стадии report.mk нам уже нет дела до трассировки,
> которая взводится в lib/report.mk накруткой на SHELL;
> понадобится ещё вот такая строчка сперва:
> 
> reports/packages: SHELL = /bin/bash
> 
> Можно свести с предыдущим коммитом (если его забирать),
> чтоб сразу был исправный.
> 

Забрал, все четыре коммита.


-- 
С уважением, Антон Мидюков <antohami@altlinux.org>