[PATCH] 5-make-release-commit: fix rstheader

[PATCH] 5-make-release-commit: fix rstheader

[christian.ehrhardt]

From: Christian Ehrhardt <christian.ehrhardt@canonical.com>

The printf magic fails trying to interpret "-" as option and thereby breaking
the top level headers.
   printf: -%: invalid option
   printf: usage: printf [-v var] format [arguments]

Instead of relying printf expansion, fall back to the more trivial
printf n times which works with any char.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 5-make-release-commit | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/5-make-release-commit b/5-make-release-commit
index 374154f..00acf92 100755
--- a/5-make-release-commit
+++ b/5-make-release-commit
@@ -29,7 +29,7 @@ function rstheader()
 
 	echo
 	echo "${msg}"
-	printf "${rchar}%.0s" $(seq 1 ${#msg})
+	for i in $(seq 1 ${#msg}); do printf "${rchar}"; done
 	# break after line plus one empty line
 	echo
 	echo
-- 
2.34.1

Re: [PATCH] 5-make-release-commit: fix rstheader

[David Marchand]

On Fri, Jan 7, 2022 at 10:50 AM <christian.ehrhardt@canonical.com> wrote:
>
> From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>
> The printf magic fails trying to interpret "-" as option and thereby breaking
> the top level headers.
>    printf: -%: invalid option
>    printf: usage: printf [-v var] format [arguments]

That's getopt that caught the -.
You could also fix by prefixing with --:
-       printf "${rchar}%.0s" $(seq 1 ${#msg})
+       printf -- "${rchar}%.0s" $(seq 1 ${#msg})

>
> Instead of relying printf expansion, fall back to the more trivial
> printf n times which works with any char.
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  5-make-release-commit | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/5-make-release-commit b/5-make-release-commit
> index 374154f..00acf92 100755
> --- a/5-make-release-commit
> +++ b/5-make-release-commit
> @@ -29,7 +29,7 @@ function rstheader()
>
>         echo
>         echo "${msg}"
> -       printf "${rchar}%.0s" $(seq 1 ${#msg})
> +       for i in $(seq 1 ${#msg}); do printf "${rchar}"; done
>         # break after line plus one empty line
>         echo
>         echo

Either form works for me.
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [PATCH] 5-make-release-commit: fix rstheader

[Christian Ehrhardt]

On Fri, Jan 7, 2022 at 11:00 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Fri, Jan 7, 2022 at 10:50 AM <christian.ehrhardt@canonical.com> wrote:
> >
> > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >
> > The printf magic fails trying to interpret "-" as option and thereby breaking
> > the top level headers.
> >    printf: -%: invalid option
> >    printf: usage: printf [-v var] format [arguments]
>
> That's getopt that caught the -.
> You could also fix by prefixing with --:
> -       printf "${rchar}%.0s" $(seq 1 ${#msg})
> +       printf -- "${rchar}%.0s" $(seq 1 ${#msg})

I know, but since it broke us once I wanted to make it dead-simple
(also less printf magic) by just looping around printing a char once
every time.

> >
> > Instead of relying printf expansion, fall back to the more trivial
> > printf n times which works with any char.
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >  5-make-release-commit | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/5-make-release-commit b/5-make-release-commit
> > index 374154f..00acf92 100755
> > --- a/5-make-release-commit
> > +++ b/5-make-release-commit
> > @@ -29,7 +29,7 @@ function rstheader()
> >
> >         echo
> >         echo "${msg}"
> > -       printf "${rchar}%.0s" $(seq 1 ${#msg})
> > +       for i in $(seq 1 ${#msg}); do printf "${rchar}"; done
> >         # break after line plus one empty line
> >         echo
> >         echo
>
> Either form works for me.

Thanks for the review, yeah to me either way is fine as well.
If there are strong opinions we can switch to yours, otherwise keep
mine as suggested.

> Reviewed-by: David Marchand <david.marchand@redhat.com>
>
>
> --
> David Marchand
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

Re: [PATCH] 5-make-release-commit: fix rstheader

[Kevin Traynor]

On 07/01/2022 09:50, christian.ehrhardt@canonical.com wrote:
> From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> 
> The printf magic fails trying to interpret "-" as option and thereby breaking
> the top level headers.
>     printf: -%: invalid option
>     printf: usage: printf [-v var] format [arguments]
> 
> Instead of relying printf expansion, fall back to the more trivial
> printf n times which works with any char.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>   5-make-release-commit | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/5-make-release-commit b/5-make-release-commit
> index 374154f..00acf92 100755
> --- a/5-make-release-commit
> +++ b/5-make-release-commit
> @@ -29,7 +29,7 @@ function rstheader()
>   
>   	echo
>   	echo "${msg}"
> -	printf "${rchar}%.0s" $(seq 1 ${#msg})
> +	for i in $(seq 1 ${#msg}); do printf "${rchar}"; done
>   	# break after line plus one empty line
>   	echo
>   	echo
> 

Acked-by: Kevin Traynor <ktraynor@redhat.com>

Re: [PATCH] 5-make-release-commit: fix rstheader

[Christian Ehrhardt]

On Mon, Jan 10, 2022 at 3:51 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> On 07/01/2022 09:50, christian.ehrhardt@canonical.com wrote:
> > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >
> > The printf magic fails trying to interpret "-" as option and thereby breaking
> > the top level headers.
> >     printf: -%: invalid option
> >     printf: usage: printf [-v var] format [arguments]
> >
> > Instead of relying printf expansion, fall back to the more trivial
> > printf n times which works with any char.
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >   5-make-release-commit | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/5-make-release-commit b/5-make-release-commit
> > index 374154f..00acf92 100755
> > --- a/5-make-release-commit
> > +++ b/5-make-release-commit
> > @@ -29,7 +29,7 @@ function rstheader()
> >
> >       echo
> >       echo "${msg}"
> > -     printf "${rchar}%.0s" $(seq 1 ${#msg})
> > +     for i in $(seq 1 ${#msg}); do printf "${rchar}"; done
> >       # break after line plus one empty line
> >       echo
> >       echo
> >
>
> Acked-by: Kevin Traynor <ktraynor@redhat.com>


Thank you both for the review, pushed to the stable-scripts git now.


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd