[PATCH] Fix random dependency issue (#5)

2 replies

[PATCH] Fix random dependency issue (#5)

From: Aaron G
To: dev
kiss(1) will sometimes not install certain dependencies. This is caused
by a loop which installs dependencies from the kiss binary cache,
instead of rebuilding. The loop iterates the positional parameters
(packages to be build) and shifts the positional parameters for every
package installed from the binary cache. The mistake is, that the first
positional parameter being shifted away, might not be the same package
as the one of the current loop iteration. For example: If the packages
"dep2", "dep1" and "foopackage" are to be build, while only "dep1" has a
cached binary, the second loop iteration will shift once, falsely
removing "dep2" from the packages which still need to be build.

This patch uses a separate variable, to properly keep track of the
remaining packages to be build.
---
 kiss | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kiss b/kiss
index 67f37f9..0585b88 100755
--- a/kiss
+++ b/kiss
@@ -627,15 +627,17 @@ pkg_build() {
     # Install any pre-built dependencies if they exist in the binary
     # directory and are up to date.
     for pkg do
-        ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg" && {
+        if ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg"; then
             log "$pkg" "Found pre-built binary, installing"
             (KISS_FORCE=1 args i "$tar_file")
-
-            # Remove the now installed package from the build list.
-            shift
-        }
+        else
+           remaining_pkgs=" $remaining_pkgs $pkg"
     done

+    # See [1] at top of script.
+    # shellcheck disable=2046,2086
+    set -- $remaining_pkgs
+
     for pkg do pkg_sources "$pkg"; done
     pkg_verify "$@"

--
2.31.1

Re: [PATCH] Fix random dependency issue (#5)

From: Aaron G
To: dev
Forgot the closing fi -.-

---
 kiss | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kiss b/kiss
index 0585b88..787a154 100755
--- a/kiss
+++ b/kiss
@@ -632,6 +632,7 @@ pkg_build() {
             (KISS_FORCE=1 args i "$tar_file")
         else
 	    remaining_pkgs=" $remaining_pkgs $pkg"
+	fi
     done
 
     # See [1] at top of script.
-- 
2.31.1
3 replies

[PATCH v2] Fix random dependency issue (#5)

From: Aaron G
To: dev
Related GitHub issue: https://github.com/kiss-community/kiss/issues/5

kiss(1) will sometimes not install certain dependencies. This is caused
by a loop which installs dependencies from the kiss binary cache,
instead of rebuilding. The loop iterates the positional parameters
(packages to be build) and shifts the positional parameters for every
package installed from the binary cache. The mistake is, that the first
positional parameter being shifted away, might not be the same package
as the one of the current loop iteration. For example: If the packages
"dep2", "dep1" and "foopackage" are to be build, while only "dep1" has a
cached binary, the second loop iteration will shift once, falsely
removing "dep2" from the packages which still need to be build.

This patch uses a separate variable, to properly keep track of the
remaining packages to be build.
---
 kiss | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/kiss b/kiss
index 67f37f9..7efa59b 100755
--- a/kiss
+++ b/kiss
@@ -627,15 +627,18 @@ pkg_build() {
     # Install any pre-built dependencies if they exist in the binary
     # directory and are up to date.
     for pkg do
-        ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg" && {
+        if ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg"; then
             log "$pkg" "Found pre-built binary, installing"
             (KISS_FORCE=1 args i "$tar_file")
-
-            # Remove the now installed package from the build list.
-            shift
-        }
+        else
+            remaining_pkgs=" $remaining_pkgs $pkg"
+        fi
     done
 
+    # See [1] at top of script.
+    # shellcheck disable=2046,2086
+    set -- $remaining_pkgs
+
     for pkg do pkg_sources "$pkg"; done
     pkg_verify "$@"
 
-- 
2.31.1
1 reply

Re: [PATCH v2] Fix random dependency issue (#5)

From: Dilyn Corner
To: Dev


Apr 17, 2021, 7:48 PM by aarong@disroot.org:

> Related GitHub issue: https://github.com/kiss-community/kiss/issues/5
>
> kiss(1) will sometimes not install certain dependencies. This is caused
> by a loop which installs dependencies from the kiss binary cache,
> instead of rebuilding. The loop iterates the positional parameters
> (packages to be build) and shifts the positional parameters for every
> package installed from the binary cache. The mistake is, that the first
> positional parameter being shifted away, might not be the same package
> as the one of the current loop iteration. For example: If the packages
> "dep2", "dep1" and "foopackage" are to be build, while only "dep1" has a
> cached binary, the second loop iteration will shift once, falsely
> removing "dep2" from the packages which still need to be build.
>
> This patch uses a separate variable, to properly keep track of the
> remaining packages to be build.
> ---
>  kiss | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kiss b/kiss
> index 67f37f9..7efa59b 100755
> --- a/kiss
> +++ b/kiss
> @@ -627,15 +627,18 @@ pkg_build() {
>  # Install any pre-built dependencies if they exist in the binary
>  # directory and are up to date.
>  for pkg do
> -        ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg" && {
> +        if ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg"; then
>  log "$pkg" "Found pre-built binary, installing"
>  (KISS_FORCE=1 args i "$tar_file")
> -
> -            # Remove the now installed package from the build list.
> -            shift
> -        }
> +        else
> +            remaining_pkgs=" $remaining_pkgs $pkg"
> +        fi
>  done
>  
> +    # See [1] at top of script.
> +    # shellcheck disable=2046,2086
> +    set -- $remaining_pkgs
> +
>  for pkg do pkg_sources "$pkg"; done
>  pkg_verify "$@"
>  
> -- 
> 2.31.1
>

What an odd yet obvious corner case. 

I'll get to testing it out to see if I can break it, if anybody else would like to do the same! 

Great work on this. 

Dilyn

1 reply

Re: [PATCH v2] Fix random dependency issue (#5)

From: Dilyn Corner
To: dev
On Sat Apr 17, 2021 at 7:48 PM EDT, Aaron G wrote:
> Related GitHub issue: https://github.com/kiss-community/kiss/issues/5
>
> kiss(1) will sometimes not install certain dependencies. This is caused
> by a loop which installs dependencies from the kiss binary cache,
> instead of rebuilding. The loop iterates the positional parameters
> (packages to be build) and shifts the positional parameters for every
> package installed from the binary cache. The mistake is, that the first
> positional parameter being shifted away, might not be the same package
> as the one of the current loop iteration. For example: If the packages
> "dep2", "dep1" and "foopackage" are to be build, while only "dep1" has a
> cached binary, the second loop iteration will shift once, falsely
> removing "dep2" from the packages which still need to be build.
>
> This patch uses a separate variable, to properly keep track of the
> remaining packages to be build.
> ---
> kiss | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kiss b/kiss
> index 67f37f9..7efa59b 100755
> --- a/kiss
> +++ b/kiss
> @@ -627,15 +627,18 @@ pkg_build() {
> # Install any pre-built dependencies if they exist in the binary
> # directory and are up to date.
> for pkg do
> - ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg" && {
> + if ! contains "$explicit_build" "$pkg" && pkg_cache "$pkg"; then
> log "$pkg" "Found pre-built binary, installing"
> (KISS_FORCE=1 args i "$tar_file")
> -
> - # Remove the now installed package from the build list.
> - shift
> - }
> + else
> + remaining_pkgs=" $remaining_pkgs $pkg"
> + fi
> done
>  
> + # See [1] at top of script.
> + # shellcheck disable=2046,2086
> + set -- $remaining_pkgs
> +
> for pkg do pkg_sources "$pkg"; done
> pkg_verify "$@"
>  
> --
> 2.31.1

This has been merged. Thank you for doing this!

Dilyn