[PATCH] Get rid of "grep: warning: stray \ before /"

[Coiby Xu]

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185

grep (3.8) warnings when running the unit tests or running
"kdumpctl reset-crashkernel" on >= F39,
    # unit tests
    Examples:
      1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the given
release
         When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64

         1.1) WARNING: There was output to stderr but not found expectation

                stderr: grep: warning: stray \ before /

              # spec/kdumpctl_general_spec.sh:169-172

    # kdumpctl reset-crashkernel
    grep: warning: stray \ before /
    kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for
kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to
take effect.

This warning can be reproduced by
  echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E
"^kernel=.*$_release(\/\w+)?\"$"

This patch removes unneeded backslash. It also adds a test for
systemd-boot path. And for simplification, Parameters:dynamic is now
used to generate test data dynamically.

Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths")
Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
---
 kdumpctl                      |  2 +-
 spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 45277a79..30eb27dd 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
 
 	# Insert '/' before '+' to cope with grep's EREs
 	_release=${_release//+/\\+}
-	_grubby_kernel_str=$(grubby --info ALL | grep -E
"^kernel=.*$_release(\/\w+)?\"$")
+	_grubby_kernel_str=$(grubby --info ALL | grep -E
"^kernel=.*$_release(/\w+)?\"$")
 	_kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
 	if [[ -z $_kernel_path ]]; then
 		ddebug "kernel $_release doesn't exist"
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
index 243aeca3..f69924f6 100644
--- a/spec/kdumpctl_general_spec.sh
+++ b/spec/kdumpctl_general_spec.sh
@@ -156,16 +156,25 @@ Describe 'kdumpctl'
 	End
 
 	Describe '_find_kernel_path_by_release()'
+		# When the array length changes, the Parameters:dynamic should change as well
+		kernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64
/boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64
/boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux)
+		kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k
vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64)
+
 		grubby() {
-			echo -e
'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"'
+			for key in "${!kernel_paths[@]}"; do
+				echo "kernel=\"${kernel_paths[$key]}\""
+			done
 		}
 
-		Parameters
-			# parameter          answer
-			vmlinuz-6.2.11-200.fc37.x86_64	/boot/vmlinuz-6.2.11-200.fc37.x86_64
-			vmlinuz-5.14.0-322.el9.aarch64	/boot/vmlinuz-5.14.0-322.el9.aarch64
-			vmlinuz-5.14.0-316.el9.aarch64+64k	/boot/vmlinuz-5.14.0-316.el9.aarch64+64k
+		Parameters:dynamic
+			# Due to a bug [1] in shellspec, hardcode the loop number instead of using the
+			# array length
+			# [1] https://github.com/shellspec/shellspec/issues/259
+			for key in {0..3}; do
+				%data "${kernels[$key]}" "${kernel_paths[$key]}"
+			done
 		End
+
 		It 'returns the kernel path for the given release'
 			When call _find_kernel_path_by_release "$1"
 			The output should equal "$2"
-- 
2.43.0

[Philipp Rudo]

Hi Coiby,

On Tue, 30 Jan 2024 14:39:38 +0800
Coiby Xu <coxu(a)redhat.com&gt; wrote:

>  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185
>  
>  grep (3.8) warnings when running the unit tests or running
>  "kdumpctl reset-crashkernel" on >= F39,
>      # unit tests
>      Examples:
>        1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the given
> release
>           When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64
>  
>           1.1) WARNING: There was output to stderr but not found expectation
>  
>                  stderr: grep: warning: stray \ before /
>  
>                # spec/kdumpctl_general_spec.sh:169-172
>  
>      # kdumpctl reset-crashkernel
>      grep: warning: stray \ before /
>      kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for
> kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to
> take effect.
>  
>  This warning can be reproduced by
>    echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E
> "^kernel=.*$_release(\/\w+)?\"$"
>  
>  This patch removes unneeded backslash. It also adds a test for
>  systemd-boot path. And for simplification, Parameters:dynamic is now
>  used to generate test data dynamically.
>  
>  Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths")
>  Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
>  ---
>   kdumpctl                      |  2 +-
>   spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------
>   2 files changed, 16 insertions(+), 7 deletions(-)
>  
>  diff --git a/kdumpctl b/kdumpctl
>  index 45277a79..30eb27dd 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
>   
>   	# Insert '/' before '+' to cope with grep's EREs
>   	_release=${_release//+/\\+}
>  -	_grubby_kernel_str=$(grubby --info ALL | grep -E
> "^kernel=.*$_release(\/\w+)?\"$")
>  +	_grubby_kernel_str=$(grubby --info ALL | grep -E
> "^kernel=.*$_release(/\w+)?\"$")
>   	_kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
>   	if [[ -z $_kernel_path ]]; then
>   		ddebug "kernel $_release doesn't exist"
>  diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
>  index 243aeca3..f69924f6 100644
>  --- a/spec/kdumpctl_general_spec.sh
>  +++ b/spec/kdumpctl_general_spec.sh
>  @@ -156,16 +156,25 @@ Describe 'kdumpctl'
>   	End
>   
>   	Describe '_find_kernel_path_by_release()'
>  +		# When the array length changes, the Parameters:dynamic should change as well
>  +		kernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64
> /boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64
> /boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux)
>  +		kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k
> vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64)
>  + 
all in all the patch looks good. Only the two lines above are extremely
hard to read because they are so long. Could you change formatting so
every line only contains a single kernel. I believe that will improve
readability a lot.

Besides this tiny nit
Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;

>   		grubby() {
>  -			echo -e
> 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"'
>  +			for key in "${!kernel_paths[@]}"; do
>  +				echo "kernel=\"${kernel_paths[$key]}\""
>  +			done
>   		}
>   
>  -		Parameters
>  -			# parameter          answer
>  -			vmlinuz-6.2.11-200.fc37.x86_64	/boot/vmlinuz-6.2.11-200.fc37.x86_64
>  -			vmlinuz-5.14.0-322.el9.aarch64	/boot/vmlinuz-5.14.0-322.el9.aarch64
>  -			vmlinuz-5.14.0-316.el9.aarch64+64k	/boot/vmlinuz-5.14.0-316.el9.aarch64+64k
>  +		Parameters:dynamic
>  +			# Due to a bug [1] in shellspec, hardcode the loop number instead of using the
>  +			# array length
>  +			# [1] https://github.com/shellspec/shellspec/issues/259
>  +			for key in {0..3}; do
>  +				%data "${kernels[$key]}" "${kernel_paths[$key]}"
>  +			done
>   		End
>  +
>   		It 'returns the kernel path for the given release'
>   			When call _find_kernel_path_by_release "$1"
>   			The output should equal "$2"

[Coiby Xu]

On Tue, Jan 30, 2024 at 10:04:06AM +0100, Philipp Rudo wrote:
> Hi Coiby,
>
> On Tue, 30 Jan 2024 14:39:38 +0800
> Coiby Xu <coxu(a)redhat.com&gt; wrote:
>
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185
> >
> > grep (3.8) warnings when running the unit tests or running
> > "kdumpctl reset-crashkernel" on >= F39,
> >     # unit tests
> >     Examples:
> >       1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the
> given release
> >          When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64
> >
> >          1.1) WARNING: There was output to stderr but not found expectation
> >
> >                 stderr: grep: warning: stray \ before /
> >
> >               # spec/kdumpctl_general_spec.sh:169-172
> >
> >     # kdumpctl reset-crashkernel
> >     grep: warning: stray \ before /
> >     kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for
> kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to
> take effect.
> >
> > This warning can be reproduced by
> >   echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E
> "^kernel=.*$_release(\/\w+)?\"$"
> >
> > This patch removes unneeded backslash. It also adds a test for
> > systemd-boot path. And for simplification, Parameters:dynamic is now
> > used to generate test data dynamically.
> >
> > Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths")
> > Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
> > ---
> >  kdumpctl                      |  2 +-
> >  spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------
> >  2 files changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index 45277a79..30eb27dd 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
> >
> >  	# Insert '/' before '+' to cope with grep's EREs
> >  	_release=${_release//+/\\+}
> > -	_grubby_kernel_str=$(grubby --info ALL | grep -E
> "^kernel=.*$_release(\/\w+)?\"$")
> > +	_grubby_kernel_str=$(grubby --info ALL | grep -E
> "^kernel=.*$_release(/\w+)?\"$")
> >  	_kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
> >  	if [[ -z $_kernel_path ]]; then
> >  		ddebug "kernel $_release doesn't exist"
> > diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
> > index 243aeca3..f69924f6 100644
> > --- a/spec/kdumpctl_general_spec.sh
> > +++ b/spec/kdumpctl_general_spec.sh
> > @@ -156,16 +156,25 @@ Describe 'kdumpctl'
> >  	End
> >
> >  	Describe '_find_kernel_path_by_release()'
> > +		# When the array length changes, the Parameters:dynamic should change as well
> > +		kernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64
> /boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64
> /boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux)
> > +		kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k
> vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64)
> > +
>
> all in all the patch looks good. Only the two lines above are extremely
> hard to read because they are so long. Could you change formatting so
> every line only contains a single kernel. I believe that will improve
> readability a lot.
>
> Besides this tiny nit
> Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt; 
Patch merged with the suggestion of formatting the code applied, thanks!

-- 
Best regards,
Coiby