Github messages for voidlinux
 help / color / mirror / Atom feed
* [PR PATCH] 99-pkglint-subpkgs: simplify and correct
@ 2020-12-05  1:41 sgn
  2020-12-05  3:44 ` [PR PATCH] [Updated] " sgn
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: sgn @ 2020-12-05  1:41 UTC (permalink / raw)
  To: ml

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

There is a new pull request by sgn against master on the void-packages repository

https://github.com/sgn/void-packages 99-pkglint-subpkgs-fix
https://github.com/void-linux/void-packages/pull/26950

99-pkglint-subpkgs: simplify and correct
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

A patch file from https://github.com/void-linux/void-packages/pull/26950.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-99-pkglint-subpkgs-fix-26950.patch --]
[-- Type: text/x-diff, Size: 2323 bytes --]

From 19d6ecf481f071313e9819490b3c931a32a0d9c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Sat, 5 Dec 2020 08:13:56 +0700
Subject: [PATCH] 99-pkglint-subpkgs: simplify and correct

As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547
---
 common/hooks/pre-pkg/99-pkglint-subpkgs.sh | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index add7fdd4a07..0bfc72d11f0 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -18,21 +18,22 @@ hook() {
 
     subpkgs=$(get_subpkgs)
 
-    subpackages="${subpackages// /$'\n'}"
-
     # Sort the strings so they can be compare for equality
-    subpkgs="$(printf "%s\\n" "$subpkgs" | sort)"
-    subpackages="$(printf "%s\\n" "$subpackages" | sort)"
+    subpkgs="$(printf "%s\\n" $subpkgs | sort)"
+    subpackages="$(printf "%s\\n" $subpackages | sort)"
 
     if [ "$subpackages" = "$subpkgs" ]; then
         return 0
     fi
 
     # XXX: Make the sed call work when subpackages has multiple lines
-    # this can be done with grep with perl regexp (-P) but chroot-grep
-    # is compiled without it
-    matches="$(sed -n 's/subpackages.*"\(.*\)"[^"]*$/\1/p' $XBPS_SRCPKGDIR/$pkgname/template \
-        | tr " " "\n" | sort)"
+    matches="$(sed -n -e 's/subpackages.*"\(.*\)"[^"]*$/\1/p' \
+            -e '/subpackages[^"]*"[^"]*$/,/"/{
+                /"/!p
+                /subpackages/s/.*"//p
+                s/".*//p
+            }' $XBPS_SRCPKGDIR/$pkgname/template |
+        tr " " "\n" | sort)"
 
     for s in $subpkgs; do
         grep -q "^$s$" <<< "$matches" ||

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

* Re: [PR PATCH] [Updated] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
@ 2020-12-05  3:44 ` sgn
  2020-12-07  0:03 ` [PR PATCH] [Closed]: " sgn
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-05  3:44 UTC (permalink / raw)
  To: ml

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

There is an updated pull request by sgn against master on the void-packages repository

https://github.com/sgn/void-packages 99-pkglint-subpkgs-fix
https://github.com/void-linux/void-packages/pull/26950

99-pkglint-subpkgs: simplify and correct
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

A patch file from https://github.com/void-linux/void-packages/pull/26950.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-99-pkglint-subpkgs-fix-26950.patch --]
[-- Type: text/x-diff, Size: 2336 bytes --]

From c0f4118b4ff188d26511bde38a7cf563bc46c8e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Sat, 5 Dec 2020 08:13:56 +0700
Subject: [PATCH] 99-pkglint-subpkgs: correct for multiline subpackages

As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547
---
 common/hooks/pre-pkg/99-pkglint-subpkgs.sh | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index add7fdd4a07..0bfc72d11f0 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -18,21 +18,22 @@ hook() {
 
     subpkgs=$(get_subpkgs)
 
-    subpackages="${subpackages// /$'\n'}"
-
     # Sort the strings so they can be compare for equality
-    subpkgs="$(printf "%s\\n" "$subpkgs" | sort)"
-    subpackages="$(printf "%s\\n" "$subpackages" | sort)"
+    subpkgs="$(printf "%s\\n" $subpkgs | sort)"
+    subpackages="$(printf "%s\\n" $subpackages | sort)"
 
     if [ "$subpackages" = "$subpkgs" ]; then
         return 0
     fi
 
     # XXX: Make the sed call work when subpackages has multiple lines
-    # this can be done with grep with perl regexp (-P) but chroot-grep
-    # is compiled without it
-    matches="$(sed -n 's/subpackages.*"\(.*\)"[^"]*$/\1/p' $XBPS_SRCPKGDIR/$pkgname/template \
-        | tr " " "\n" | sort)"
+    matches="$(sed -n -e 's/subpackages.*"\(.*\)"[^"]*$/\1/p' \
+            -e '/subpackages[^"]*"[^"]*$/,/"/{
+                /"/!p
+                /subpackages/s/.*"//p
+                s/".*//p
+            }' $XBPS_SRCPKGDIR/$pkgname/template |
+        tr " " "\n" | sort)"
 
     for s in $subpkgs; do
         grep -q "^$s$" <<< "$matches" ||

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

* Re: [PR PATCH] [Closed]: 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
  2020-12-05  3:44 ` [PR PATCH] [Updated] " sgn
@ 2020-12-07  0:03 ` sgn
  2020-12-07  9:03 ` [PR REVIEW] " Piraty
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-07  0:03 UTC (permalink / raw)
  To: ml

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

There's a closed pull request on the void-packages repository

99-pkglint-subpkgs: simplify and correct
https://github.com/void-linux/void-packages/pull/26950

Description:
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (2 preceding siblings ...)
  2020-12-07  9:03 ` [PR REVIEW] " Piraty
@ 2020-12-07  9:03 ` Piraty
  2020-12-07 11:29 ` sgn
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Piraty @ 2020-12-07  9:03 UTC (permalink / raw)
  To: ml

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

New review comment by Piraty on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r537336338

Comment:
isn't that comment obsolete with your fix? 

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
  2020-12-05  3:44 ` [PR PATCH] [Updated] " sgn
  2020-12-07  0:03 ` [PR PATCH] [Closed]: " sgn
@ 2020-12-07  9:03 ` Piraty
  2020-12-07  9:03 ` Piraty
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Piraty @ 2020-12-07  9:03 UTC (permalink / raw)
  To: ml

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

New review comment by Piraty on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r537335135

Comment:
`printf '%s\n' $subpkgs`

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (3 preceding siblings ...)
  2020-12-07  9:03 ` Piraty
@ 2020-12-07 11:29 ` sgn
  2020-12-07 11:34 ` [PR PATCH] [Updated] " sgn
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-07 11:29 UTC (permalink / raw)
  To: ml

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

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r537433395

Comment:
Ah, yes, I'll remove it, too.

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

* Re: [PR PATCH] [Updated] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (4 preceding siblings ...)
  2020-12-07 11:29 ` sgn
@ 2020-12-07 11:34 ` sgn
  2020-12-07 12:59 ` sgn
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-07 11:34 UTC (permalink / raw)
  To: ml

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

There is an updated pull request by sgn against master on the void-packages repository

https://github.com/sgn/void-packages 99-pkglint-subpkgs-fix
https://github.com/void-linux/void-packages/pull/26950

99-pkglint-subpkgs: simplify and correct
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

A patch file from https://github.com/void-linux/void-packages/pull/26950.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-99-pkglint-subpkgs-fix-26950.patch --]
[-- Type: text/x-diff, Size: 2335 bytes --]

From 1627bf46a48ba6a87e0a0cdf926d50e582ff874f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Sat, 5 Dec 2020 08:13:56 +0700
Subject: [PATCH] 99-pkglint-subpkgs: correct for multiline subpackages

As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547
---
 common/hooks/pre-pkg/99-pkglint-subpkgs.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index add7fdd4a07..53e4e697e21 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -18,21 +18,21 @@ hook() {
 
     subpkgs=$(get_subpkgs)
 
-    subpackages="${subpackages// /$'\n'}"
-
     # Sort the strings so they can be compare for equality
-    subpkgs="$(printf "%s\\n" "$subpkgs" | sort)"
-    subpackages="$(printf "%s\\n" "$subpackages" | sort)"
+    subpkgs="$(printf '%s\n' $subpkgs | sort)"
+    subpackages="$(printf '%s\n' $subpackages | sort)"
 
     if [ "$subpackages" = "$subpkgs" ]; then
         return 0
     fi
 
-    # XXX: Make the sed call work when subpackages has multiple lines
-    # this can be done with grep with perl regexp (-P) but chroot-grep
-    # is compiled without it
-    matches="$(sed -n 's/subpackages.*"\(.*\)"[^"]*$/\1/p' $XBPS_SRCPKGDIR/$pkgname/template \
-        | tr " " "\n" | sort)"
+    matches="$(sed -n -e 's/subpackages.*"\(.*\)"[^"]*$/\1/p' \
+            -e '/subpackages[^"]*"[^"]*$/,/"/{
+                /"/!p
+                /subpackages/s/.*"//p
+                s/".*//p
+            }' $XBPS_SRCPKGDIR/$pkgname/template |
+        tr ' ' '\n' | sort)"
 
     for s in $subpkgs; do
         grep -q "^$s$" <<< "$matches" ||

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

* Re: 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (5 preceding siblings ...)
  2020-12-07 11:34 ` [PR PATCH] [Updated] " sgn
@ 2020-12-07 12:59 ` sgn
  2020-12-07 12:59 ` sgn
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-07 12:59 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#issuecomment-739903041

Comment:
I tentative to add this change:
```
diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index 53e4e697e2..b932d122b6 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -32,13 +32,17 @@ hook() {
                 /subpackages/s/.*"//p
                 s/".*//p
             }' $XBPS_SRCPKGDIR/$pkgname/template |
-        tr ' ' '\n' | sort)"
+        tr '\v\t\r\n' '    ')"
 
     for s in $subpkgs; do
-        grep -q "^$s$" <<< "$matches" ||
-            msg_warn "${s}_package() defined but will never be built.\n"
+        case " $matches " in
+            *" $s "*) ;;
+            *) msg_warn "${s}_package() defined but will never be built.\n" ;;
+        esac
     done
 
-    grep -q "^$pkgname$" <<< "$matches" &&
-        msg_warn "$pkgname is sourcepkg but is in subpackages=.\n" || :
+    case " $matches " in
+        *" $pkgname "*)
+            msg_warn "$pkgname is sourcepkg but is in subpackages=.\n" ;;
+    esac
 }
```

On looping over 100 subpkgs, the new code show a reduction from 286ms to 4ms.

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

* Re: 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (6 preceding siblings ...)
  2020-12-07 12:59 ` sgn
@ 2020-12-07 12:59 ` sgn
  2020-12-09 18:29 ` [PR REVIEW] " ericonr
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-07 12:59 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#issuecomment-739903041

Comment:
I tentative to add this change:
```diff
diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index 53e4e697e2..b932d122b6 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -32,13 +32,17 @@ hook() {
                 /subpackages/s/.*"//p
                 s/".*//p
             }' $XBPS_SRCPKGDIR/$pkgname/template |
-        tr ' ' '\n' | sort)"
+        tr '\v\t\r\n' '    ')"
 
     for s in $subpkgs; do
-        grep -q "^$s$" <<< "$matches" ||
-            msg_warn "${s}_package() defined but will never be built.\n"
+        case " $matches " in
+            *" $s "*) ;;
+            *) msg_warn "${s}_package() defined but will never be built.\n" ;;
+        esac
     done
 
-    grep -q "^$pkgname$" <<< "$matches" &&
-        msg_warn "$pkgname is sourcepkg but is in subpackages=.\n" || :
+    case " $matches " in
+        *" $pkgname "*)
+            msg_warn "$pkgname is sourcepkg but is in subpackages=.\n" ;;
+    esac
 }
```

On looping over 100 subpkgs, the new code show a reduction from 286ms to 4ms.

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (7 preceding siblings ...)
  2020-12-07 12:59 ` sgn
@ 2020-12-09 18:29 ` ericonr
  2020-12-10  0:13 ` [PR PATCH] [Updated] " sgn
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: ericonr @ 2020-12-09 18:29 UTC (permalink / raw)
  To: ml

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

New review comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r539545878

Comment:
If you can think of a comment to put here, that'd be nice. Making sense of this sed command is quite hard.

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

* Re: [PR PATCH] [Updated] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (8 preceding siblings ...)
  2020-12-09 18:29 ` [PR REVIEW] " ericonr
@ 2020-12-10  0:13 ` sgn
  2020-12-10  0:14 ` [PR REVIEW] " sgn
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-10  0:13 UTC (permalink / raw)
  To: ml

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

There is an updated pull request by sgn against master on the void-packages repository

https://github.com/sgn/void-packages 99-pkglint-subpkgs-fix
https://github.com/void-linux/void-packages/pull/26950

99-pkglint-subpkgs: simplify and correct
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

A patch file from https://github.com/void-linux/void-packages/pull/26950.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-99-pkglint-subpkgs-fix-26950.patch --]
[-- Type: text/x-diff, Size: 2690 bytes --]

From cbfd1892f3c0e604df33c68f3984ba6eead41b57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Sat, 5 Dec 2020 08:13:56 +0700
Subject: [PATCH] 99-pkglint-subpkgs: correct for multiline subpackages

As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547
---
 common/hooks/pre-pkg/99-pkglint-subpkgs.sh | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index add7fdd4a07..afa1ed92b17 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -18,21 +18,27 @@ hook() {
 
     subpkgs=$(get_subpkgs)
 
-    subpackages="${subpackages// /$'\n'}"
-
     # Sort the strings so they can be compare for equality
-    subpkgs="$(printf "%s\\n" "$subpkgs" | sort)"
-    subpackages="$(printf "%s\\n" "$subpackages" | sort)"
+    subpkgs="$(printf '%s\n' $subpkgs | sort)"
+    subpackages="$(printf '%s\n' $subpackages | sort)"
 
     if [ "$subpackages" = "$subpkgs" ]; then
         return 0
     fi
 
-    # XXX: Make the sed call work when subpackages has multiple lines
-    # this can be done with grep with perl regexp (-P) but chroot-grep
-    # is compiled without it
-    matches="$(sed -n 's/subpackages.*"\(.*\)"[^"]*$/\1/p' $XBPS_SRCPKGDIR/$pkgname/template \
-        | tr " " "\n" | sort)"
+    # sed supports comment but let put them here
+    # 1: print everything between pairs of <""> in subpackages[+]?="..."
+    # 2: multiline subpackages="...\n..."
+    # 2.1: For anyline in the middle, aka no <"> exists, print them
+    # 2.2: For the first line, print everything after <">
+    # 2.3: For last line, print everything before <">
+    matches="$(sed -n -e 's/subpackages.*"\(.*\)"[^"]*$/\1/p' \
+            -e '/subpackages[^"]*"[^"]*$/,/"/{
+                /"/!p
+                /subpackages/s/.*"//p
+                s/".*//p
+            }' $XBPS_SRCPKGDIR/$pkgname/template |
+        tr ' ' '\n' | sort)"
 
     for s in $subpkgs; do
         grep -q "^$s$" <<< "$matches" ||

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (9 preceding siblings ...)
  2020-12-10  0:13 ` [PR PATCH] [Updated] " sgn
@ 2020-12-10  0:14 ` sgn
  2020-12-10  0:27 ` ericonr
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-10  0:14 UTC (permalink / raw)
  To: ml

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

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r539745639

Comment:
> If you can think of a comment to put here, that'd be nice.

Explanation added.

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

* Re: [PR REVIEW] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (10 preceding siblings ...)
  2020-12-10  0:14 ` [PR REVIEW] " sgn
@ 2020-12-10  0:27 ` ericonr
  2020-12-10  0:39 ` [PR PATCH] [Updated] " sgn
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: ericonr @ 2020-12-10  0:27 UTC (permalink / raw)
  To: ml

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

New review comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#discussion_r539750677

Comment:
```suggestion
    # sed supports comment but let's put them here
    # 1: print everything between pairs of <""> in subpackages[+]?="..."
    # 2: multiline subpackages="...\n..."
    # 2.1: For any line in the middle, i.e., no <"> exists, print it
    # 2.2: For the first line, print everything after <">
    # 2.3: For last line, print everything before <">
```

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

* Re: [PR PATCH] [Updated] 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (11 preceding siblings ...)
  2020-12-10  0:27 ` ericonr
@ 2020-12-10  0:39 ` sgn
  2020-12-10 14:23 ` sgn
  2020-12-14  2:35 ` [PR PATCH] [Merged]: " sgn
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-10  0:39 UTC (permalink / raw)
  To: ml

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

There is an updated pull request by sgn against master on the void-packages repository

https://github.com/sgn/void-packages 99-pkglint-subpkgs-fix
https://github.com/void-linux/void-packages/pull/26950

99-pkglint-subpkgs: simplify and correct
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

A patch file from https://github.com/void-linux/void-packages/pull/26950.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-99-pkglint-subpkgs-fix-26950.patch --]
[-- Type: text/x-diff, Size: 2692 bytes --]

From f33a44f8a3ea572f746101b2ef5952cd9328bd31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Sat, 5 Dec 2020 08:13:56 +0700
Subject: [PATCH] 99-pkglint-subpkgs: correct for multiline subpackages

As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547
---
 common/hooks/pre-pkg/99-pkglint-subpkgs.sh | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
index add7fdd4a07..fd1fee6c445 100644
--- a/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
+++ b/common/hooks/pre-pkg/99-pkglint-subpkgs.sh
@@ -18,21 +18,27 @@ hook() {
 
     subpkgs=$(get_subpkgs)
 
-    subpackages="${subpackages// /$'\n'}"
-
     # Sort the strings so they can be compare for equality
-    subpkgs="$(printf "%s\\n" "$subpkgs" | sort)"
-    subpackages="$(printf "%s\\n" "$subpackages" | sort)"
+    subpkgs="$(printf '%s\n' $subpkgs | sort)"
+    subpackages="$(printf '%s\n' $subpackages | sort)"
 
     if [ "$subpackages" = "$subpkgs" ]; then
         return 0
     fi
 
-    # XXX: Make the sed call work when subpackages has multiple lines
-    # this can be done with grep with perl regexp (-P) but chroot-grep
-    # is compiled without it
-    matches="$(sed -n 's/subpackages.*"\(.*\)"[^"]*$/\1/p' $XBPS_SRCPKGDIR/$pkgname/template \
-        | tr " " "\n" | sort)"
+    # sed supports comment but let's put them here
+    # 1: print everything between pairs of <""> in subpackages[+]?="..."
+    # 2: multiline subpackages="...\n..."
+    # 2.1: For any line in the middle, i.e. no <"> exists, print it
+    # 2.2: For the first line, print everything after <">
+    # 2.3: For last line, print everything before <">
+    matches="$(sed -n -e 's/subpackages.*"\(.*\)"[^"]*$/\1/p' \
+            -e '/subpackages[^"]*"[^"]*$/,/"/{
+                /"/!p
+                /subpackages/s/.*"//p
+                s/".*//p
+            }' $XBPS_SRCPKGDIR/$pkgname/template |
+        tr ' ' '\n' | sort)"
 
     for s in $subpkgs; do
         grep -q "^$s$" <<< "$matches" ||

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

* Re: 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (12 preceding siblings ...)
  2020-12-10  0:39 ` [PR PATCH] [Updated] " sgn
@ 2020-12-10 14:23 ` sgn
  2020-12-14  2:35 ` [PR PATCH] [Merged]: " sgn
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-10 14:23 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/26950#issuecomment-742551907

Comment:
Anything else to improve?

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

* Re: [PR PATCH] [Merged]: 99-pkglint-subpkgs: simplify and correct
  2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
                   ` (13 preceding siblings ...)
  2020-12-10 14:23 ` sgn
@ 2020-12-14  2:35 ` sgn
  14 siblings, 0 replies; 16+ messages in thread
From: sgn @ 2020-12-14  2:35 UTC (permalink / raw)
  To: ml

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

There's a merged pull request on the void-packages repository

99-pkglint-subpkgs: simplify and correct
https://github.com/void-linux/void-packages/pull/26950

Description:
As discussing in [1], on template with "subpackages" as multilines will
report false positive on some packages will never be built.

There're multiple problems here:

- expanded "subpackages" will have an empty line if it has a newline
  inside template
- "sed" expression couldn't work with multilines "subpackages"

Let's not quote "$subpkgs" and "$subpackages" in "printf" to let the
shell do expansion and trim the empty lines for us. And rewrite the
"sed" expression to work with multilines "subpackages"

[1]: https://github.com/void-linux/void-packages/pull/26939#issuecomment-739098547

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

end of thread, other threads:[~2020-12-14  2:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-05  1:41 [PR PATCH] 99-pkglint-subpkgs: simplify and correct sgn
2020-12-05  3:44 ` [PR PATCH] [Updated] " sgn
2020-12-07  0:03 ` [PR PATCH] [Closed]: " sgn
2020-12-07  9:03 ` [PR REVIEW] " Piraty
2020-12-07  9:03 ` Piraty
2020-12-07 11:29 ` sgn
2020-12-07 11:34 ` [PR PATCH] [Updated] " sgn
2020-12-07 12:59 ` sgn
2020-12-07 12:59 ` sgn
2020-12-09 18:29 ` [PR REVIEW] " ericonr
2020-12-10  0:13 ` [PR PATCH] [Updated] " sgn
2020-12-10  0:14 ` [PR REVIEW] " sgn
2020-12-10  0:27 ` ericonr
2020-12-10  0:39 ` [PR PATCH] [Updated] " sgn
2020-12-10 14:23 ` sgn
2020-12-14  2:35 ` [PR PATCH] [Merged]: " sgn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).