[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