[SSSD] [PATCH] CI: Improve usability of ci script

Roland Mainz rmainz at redhat.com
Thu Sep 11 17:42:32 UTC 2014



----- Original Message -----
> From: "Lukas Slebodnik" <lslebodn at redhat.com>
> To: "Development of the System Security Services Daemon" <sssd-devel at lists.fedorahosted.org>
> Sent: Thursday, September 11, 2014 10:36:23 AM
> Subject: Re: [SSSD] [PATCH] CI: Improve usability of ci script
> 
> On (05/09/14 18:58), Michal Židek wrote:
> >On 09/05/2014 04:18 PM, Lukas Slebodnik wrote:
> >>On (05/09/14 16:11), Michal Židek wrote:
> >>>On 09/05/2014 11:37 AM, Lukas Slebodnik wrote:
> >>>>ehlo,
> >>>>
> >>>>CI scrip can be now executed from different directory than sssd git root.
> >>>>
> >>>>[build at host ci_dir]$ ../contrib/ci/run -n
> >>>>autoreconf:             success  00:00:12 ci_dir/ci-autoreconf.log
> >>>>DEBUG BUILD:                              ci_dir/ci-build-debug
> >>>>configure:              success  00:00:14
> >>>>ci_dir/ci-build-debug/ci-configure.log
> >>>>make-tests:             success  00:01:03
> >>>>ci_dir/ci-build-debug/ci-make-tests.log
> >>>>make-check-valgrind:    failure  00:00:48
> >>>>ci_dir/ci-build-debug/ci-make-check-valgrind.log
> >>>>SUCCESS
> >>>>
> >>>>Resolves:
> >>>>https://fedorahosted.org/sssd/ticket/2429
> >>>>
> >>>>http://sssd-ci.duckdns.org/logs/job/0/41/summary.html
> >>>>
> >>>>Patch is attached.
> >>>>
> >>>>LS
> >>>>
> >>>
> >>>This patch does not apply cleanly on current master. Please rebase.
> >>>
> >>Done.
> >>
> >>LS
> >>
> >>
> >
> >Hi Lukas,
> >
> >please update the comment for function disppath() because
> >it now works with CWD_DIR and not BASE_DIR variable.
> >
> New version attached.

Changes look good to me...
... r=rmainz at nrubsig.org

Some (minor) nits (not needed for now):
1. Please use "source", not ".", e.g. turn...
-- snip --
. shared_variables.sh
-- snip --
... into ...
-- snip --
source shared_variables.sh
-- snip --

The reason is that "source" is more readable and is an alias for "command ." which allows a more gracefull exit if the source file cannot be found or executed.


2. Double-quotes around variable expansion are sometimes missing (consistency!):
-- snip --
-            -e 's/@PRERELEASE_VERSION@//g' contrib/sssd.spec.in |
+            -e 's/@PRERELEASE_VERSION@//g' $SRC_DIR/contrib/sssd.spec.in |
-- snip --
... should be...
-- snip --

-            -e 's/@PRERELEASE_VERSION@//g' contrib/sssd.spec.in |
+            -e 's/@PRERELEASE_VERSION@//g' "$SRC_DIR/contrib/sssd.spec.in" |
-- snip --

... and ...

-- snip --
+    LOG_DIR="$LOG_DIR/$dir"
     cd "$dir"
     "$@"
     cd ..
+    LOG_DIR=$LOG_DIR_BACKUP
-- snip --
... should be ...
-- snip --
+    LOG_DIR="$LOG_DIR/$dir"
     cd "$dir"
     "$@"
     cd ..
+    LOG_DIR="$LOG_DIR_BACKUP"
-- snip --


3. Not your fault, not your change... but I noticed it:
-- snip --
     printf "%-$((TITLE_WIDTH + RESULT_WIDTH))s%s\n" \
            "${id^^} BUILD:" "`disppath \"\$dir\"`"
-- snip --
... this won't always work because %s in printf(1) is required to count in bytes, not characters... so if you have multibyte characters in a string it can cut them in the middle of a character and create a stream of invalid character byte sequences... ;-(
... a solution is not easy for bash ... the POSIX standard added "Ls" to printf(1) to deal with the dilemma:
-- snip --
$ ksh -c 'printf "|%4.4Ls|\n" "$(printf "\u[20ac]\u[20ad]\u[20ae]\n")"'
| €₭₮|
$ ksh -c 'printf "|%2.2Ls|\n" "$(printf "\u[20ac]\u[20ad]\u[20ae]\n")"'
|€₭|
-- snip --
... but neither bash's builtin printf(1) not GNU printf(1) implement it yet... ;-(

Workaround may be to use typeset -L/-R

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) rmainz at redhat.com
  \__\/\/__/  IPA/Kerberos5 team
  /O /==\ O\  
 (;O/ \/ \O;)
 



More information about the sssd-devel mailing list