On 05/25/2015 11:44 AM, Lukas Slebodnik wrote:
A)
The 1st patch is not necessary.
CI passed without 1st patch
http://sssd-ci.duckdns.org/logs/job/15/89/summary.html (fedora 20 failed due to
dyndnd-test)
B) You are misusing autoconf macro AM_CONDITIONAL.
It shoudl be used just in case you want to have "if conditions" in automake
files. ENABLE_INTGCHECK
Please try to look into documentation of autoconf macros
AC_SUBST
AC_DEFINE (AC_DEFINE_UNQUOTED)
AM_CONDITIONAL
In most cases you just need just AC_SUBST for HAVE_$sth
or just AC_CHECK_PROG.
Yes this can make things simpler and more reliable. I remade the configuration
to just use shell variables, as AC_SUBST is not necessary, because we don't
use these in any configure-processed files (e.g. Makefile.am's).
Please see the first attached patch to see what I mean.
This also reminds me you can squas
"small external" files into src/external/intgcheck.m4
It does not worth to have one file just with a line AC_CHECK_PROG
src/external/fakeroot.m4 | 2 +
src/external/intgcheck.m4 | 23 ++++
src/external/ldap.m4 | 6 +
src/external/pytest.m4 | 3 +
Regarding this, wouldn't it make the corresponding macros somewhat harder to
find? We have a few short files under src/external with clear names and
contents that doesn't quite fit anywhere and I thought that rather neat.
I can stuff fakeroot and py.test checks into intgcheck.m4 without much harm, I
think. However, would it really be good to move slapd and ldapmodify checking
out of ldap.m4? Wouldn't it be confusing?
@see
diff --git a/src/external/pytest.m4 b/src/external/pytest.m4
index 00f28e5..b6305d1 100644
--- a/src/external/pytest.m4
+++ b/src/external/pytest.m4
@@ -1,3 +1,2 @@
AC_PATH_PROG([PYTEST], [py.test])
-AC_SUBST([PYTEST])
-AM_CONDITIONAL([HAVE_PYTEST], [test x"$PYTEST" != x])
+AC_SUBST([HAVE_PYTEST], [test x"$PYTEST" != x])
Hmm, would the last line really work? To me it looks like not, am I missing
something?
C) We can simplify make target intgcheck
It significantly improve user experince.
(the same as "make distcheck" or make tests in samba)
Ah, good catch, yes. Thank you, Lukas. However, leaving the configure option
named "--enable-intgcheck" will be confusing, so I tried to rename it to
something more relevant to what it will be doing now. It still feels a bit
awkward, but perhaps it's good enough.
Please take a look at the second attached patch, for illustration.
D) please find better name for "src/tests/intg/misc.py"
I think "misc.py" is perfectly fine, but since our previous discussion on
similar matter stalled, please suggest a name and I'll name it as you say and
we'll be done with it.
I will file other tickets with improvements after pushing patch to
master.
Thank you, Lukas.
BTW there is a failed intgtest
http://sssd-ci.duckdns.org/logs/job/15/90/fedora20/ci-build-debug/ci-make...
It would be good to prevent such fialures. I've already sent patches to fix
negcache-test and SSSDConfig-test. We do not need to have another problematic test
Argh, I've seen this, wanted to fix it and forgot. Can't reproduce it at the
moment, but I'll keep trying. Meanwhile, do you have any ideas of what might
be causing this?
Thank you.
Nick