[Freeipa-devel] [freeipa PR#718][comment] configure: fix AC_CHECK_LIB usage
URL: https://github.com/freeipa/freeipa/pull/718 Title: #718: configure: fix AC_CHECK_LIB usage lslebodn commented: """ A little bit offtopic. It is interesting that you look for the function `main` in libkrad. Because it does not exist there. I've just noticed it. ``` sh# nm --defined-only --dynamic /usr/lib64/libkrad.so A HIDDEN A krad_0_MIT 2540 T krad_attr_name2num 25d0 T krad_attr_num2name 2780 T krad_attrset_add 2850 T krad_attrset_add_number 2a00 T krad_attrset_copy 28b0 T krad_attrset_del 26a0 T krad_attrset_free 2990 T krad_attrset_get 2630 T krad_attrset_new 3310 T krad_client_free 32a0 T krad_client_new 3380 T krad_client_send 36c0 T krad_code_name2num 3750 T krad_code_num2name 3940 T krad_packet_bytes_needed 3f70 T krad_packet_decode_request 4040 T krad_packet_decode_response 41a0 T krad_packet_encode 39c0 T krad_packet_free 4220 T krad_packet_get_attr 41e0 T krad_packet_get_code 3b20 T krad_packet_new_request 3e10 T krad_packet_new_response ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/718#issuecomment-294845871 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#718][comment] configure: fix AC_CHECK_LIB usage
URL: https://github.com/freeipa/freeipa/pull/718 Title: #718: configure: fix AC_CHECK_LIB usage lslebodn commented: """ >It does not explicitly say what constitutes an unspecified value though. ```, , ``` or ``` , [] ,``` are considered as unspecified values and therefore default action was used. BTW it is not peculiar behaviour because most project define custom LIBS using the 3rd argument and not after invocation of macro. e.g. ``` AC_CHECK_HEADER(krad.h, [], [AC_MSG_ERROR([krad.h not found])]) -AC_CHECK_LIB(krad, main, [], [AC_MSG_ERROR([libkrad not found])]) +AC_CHECK_LIB(krad, main, [KRAD_LIBS="-lkrad"], [AC_MSG_ERROR([libkrad not found])]) -KRAD_LIBS="-lkrad" ``` And link to online documentation https://www.gnu.org/software/autoconf/manual/autoconf.html#Libraries """ See the full comment at https://github.com/freeipa/freeipa/pull/718#issuecomment-294844095 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#718][comment] configure: fix AC_CHECK_LIB usage
URL: https://github.com/freeipa/freeipa/pull/718 Title: #718: configure: fix AC_CHECK_LIB usage lslebodn commented: """ >It does not explicitly say what constitutes an unspecified value though. ```[]``` means default action. BTW it is not peculiar behaviour because most project define custom LIBS using the 3rd argument and not after invocation of macro. e.g. ``` AC_CHECK_HEADER(krad.h, [], [AC_MSG_ERROR([krad.h not found])]) -AC_CHECK_LIB(krad, main, [], [AC_MSG_ERROR([libkrad not found])]) +AC_CHECK_LIB(krad, main, [KRAD_LIBS="-lkrad"], [AC_MSG_ERROR([libkrad not found])]) -KRAD_LIBS="-lkrad" ``` And link to online documentation https://www.gnu.org/software/autoconf/manual/autoconf.html#Libraries """ See the full comment at https://github.com/freeipa/freeipa/pull/718#issuecomment-294844095 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [bind-dyndb-ldap PR#16][comment] spec: remove unnecessary bind-pkcs11 dependency
URL: https://github.com/freeipa/bind-dyndb-ldap/pull/16 Title: #16: spec: remove unnecessary bind-pkcs11 dependency lslebodn commented: """ I am not sure whether change to "requires" is ideal. Because after installation of freeipa-dns with this spec file system will contain packages: `bind` and `bind-pkcs11`. And I doubt both will be running in the same time. rpm 4.13 is in fedora 24+ and contains boolean dependencies http://rpm.org/user_doc/more_dependencies.html#boolean-dependencies """ See the full comment at https://github.com/freeipa/bind-dyndb-ldap/pull/16#issuecomment-292711267 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#623][comment] client install: do not assume /etc/krb5.conf.d exists
URL: https://github.com/freeipa/freeipa/pull/623
Title: #623: client install: do not assume /etc/krb5.conf.d exists
lslebodn commented:
"""
On (28/03/17 04:08), Christian Heimes wrote:
>The ipa-certauth plugin now starts to rely on the existence of
>```/etc/krb5.conf.d```:
>
>```
>%config(noreplace) %{_sysconfdir}/krb5.conf.d/ipa-certauth
>```
>
The upstream spec file is fedora/rhel spec files and fedora+rhel have
`%{_sysconfdir}/krb5.conf.d/`. I cannot see any problem.
>**Practicality beats purity**, let's make ```/etc/krb5.conf.d``` part of the
>offical FreeIPA configuation settings on all IPA enrolled systems.
>
But neither debian nor arch linux/opensuse have this directory(or any other)
included by default in `/etc/krb5.conf`.
I would like to see standard directory for krb5 snippet files.
But that should be solved in distribution. And just used by freeipa.
LS
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/623#issuecomment-289738839
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#623][comment] client install: do not assume /etc/krb5.conf.d exists
URL: https://github.com/freeipa/freeipa/pull/623 Title: #623: client install: do not assume /etc/krb5.conf.d exists lslebodn commented: """ FYI: `/etc/krb5.conf.d` is not default include directory it is fedora/el7 specific. debian testing has MIT kerberos 1.15 and `/etc/krb5.conf.d` does not exist there as is not included in /etc/krb5.conf. So +1 for @HonzaCholasta approach. """ See the full comment at https://github.com/freeipa/freeipa/pull/623#issuecomment-289583003 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#623][comment] client install: do not assume /etc/krb5.conf.d exists
URL: https://github.com/freeipa/freeipa/pull/623 Title: #623: client install: do not assume /etc/krb5.conf.d exists lslebodn commented: """ FYI: `/etc/krb5.conf.d` is not default include directory it is fedora/el7 specific. debian testing has MIT kerberos 1.15 and `/etc/krb5.conf.d` does not exist there as is not included in /etc/krb5.conf. So +1 for @HonzaCholasta approach. """ See the full comment at https://github.com/freeipa/freeipa/pull/623#issuecomment-289583003 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#657][comment] configure: fix --disable-server with certauth plugin
URL: https://github.com/freeipa/freeipa/pull/657 Title: #657: configure: fix --disable-server with certauth plugin lslebodn commented: """ `AM_CONDITIONAL` need to be executed every time. So just `AM_CONDITIONAL([BUILD_IPA_CERTAUTH_PLUGIN]` need to be moved from sever.m4 -> configure.ac. The best would be to build somewhere after `m4_include(server.m4)` """ See the full comment at https://github.com/freeipa/freeipa/pull/657#issuecomment-289417730 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#657][comment] configure: fix --disable-server with certauth plugin
URL: https://github.com/freeipa/freeipa/pull/657 Title: #657: configure: fix --disable-server with certauth plugin lslebodn commented: """ `AM_CONDITIONAL` need to be executed every time. So just `AM_CONDITIONAL([BUILD_IPA_CERTAUTH_PLUGIN]` need to be moved from sever.m4 -> configure.ac. The best would be to build somewhere after `m4_include(server.m4)` """ See the full comment at https://github.com/freeipa/freeipa/pull/657#issuecomment-289417730 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#600][comment] CONFIGURE: Improve detection of xmlrpc_c flags
URL: https://github.com/freeipa/freeipa/pull/600 Title: #600: CONFIGURE: Improve detection of xmlrpc_c flags lslebodn commented: """ On (15/03/17 08:14), MartinBasti wrote: >Commit message amended before pushed > Thank you; I was busy with other tasks. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/600#issuecomment-286785306 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#599][comment] CONFIGURE: Improve error messages for optional dependencies
URL: https://github.com/freeipa/freeipa/pull/599 Title: #599: CONFIGURE: Improve error messages for optional dependencies lslebodn commented: """ @tiran one more time: The approach in PR #502 was not accepted in upstream discussion https://www.redhat.com/archives/freeipa-devel/2017-March/msg00307.html """ See the full comment at https://github.com/freeipa/freeipa/pull/599#issuecomment-286738321 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#599][comment] CONFIGURE: Improve error messages for optional dependencies
URL: https://github.com/freeipa/freeipa/pull/599 Title: #599: CONFIGURE: Improve error messages for optional dependencies lslebodn commented: """ >NACK, you are changing the spirit of the accepted PR #502. The approach PR #502 was not accepted in upstream discussion https://www.redhat.com/archives/freeipa-devel/2017-March/msg00307.html and moreover @HonzaCholasta was not against in PR #502 https://github.com/freeipa/freeipa/pull/502#issuecomment-286724681 """ See the full comment at https://github.com/freeipa/freeipa/pull/599#issuecomment-286735020 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#600][comment] CONFIGURE: Improve detection of xmlrpc_c flags
URL: https://github.com/freeipa/freeipa/pull/600 Title: #600: CONFIGURE: Improve detection of xmlrpc_c flags lslebodn commented: """ @tjaalton It should simplify you work on debian """ See the full comment at https://github.com/freeipa/freeipa/pull/600#issuecomment-286733233 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#600][opened] CONFIGURE: Improve detection of xmlrpc_c flags
URL: https://github.com/freeipa/freeipa/pull/600 Author: lslebodn Title: #600: CONFIGURE: Improve detection of xmlrpc_c flags Action: opened PR body: """ The pkg-config files for xmlrpc_c libraries are shipped just in fedora/rhel due to downstream patch. Debian does not have pkg-config files for xmlrpc_c. Therefore we need to fallback to older method of detection XMLRPC_*FLAGS which was reverted by the commit 1e0143c159134337a00a91d4ae64e614f72da62e """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/600/head:pr600 git checkout pr600 From bfd587d38726a60a346977366b997056529f3774 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:25 +0100 Subject: [PATCH] CONFIGURE: Improve detection of xmlrpc_c flags The pkg-config files for xmlrpc_c libraries are shipped just in fedora/rhel due to downstream patch. Debian does not have pkg-config files for xmlrpc_c. Therefore we need to fallback to older method of detection XMLRPC_*FLAGS which was reverted by the commit 1e0143c159134337a00a91d4ae64e614f72da62e --- configure.ac | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 4d7a846..d99be0c 100644 --- a/configure.ac +++ b/configure.ac @@ -171,7 +171,20 @@ PKG_CHECK_MODULES([SASL], [libsasl2]) dnl --- dnl - Check for XMLRPC-C dnl --- -PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util]) +PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util], [], + [try_xmlrpc_fallback=true]) +if test x"$try_xmlrpc_fallback" = xtrue; then +XMLRPC_LIBS= +AC_CHECK_HEADER([xmlrpc-c/base.h], [], +[AC_MSG_ERROR([xmlrpc-c/base.h not found])]) + +AC_CHECK_LIB([xmlrpc_client], [xmlrpc_client_init2], + [XMLRPC_LIBS="-lxmlrpc -lxmlrpc_client -lxmlrpc_util"]) +if test "x$XMLRPC_LIBS" = "x" ; then +AC_MSG_ERROR([xmlrpc-c not found]) +fi +AC_SUBST(XMLRPC_LIBS) +fi dnl --- dnl - Check for libintl -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#599][synchronized] CONFIGURE: Improve error messages for optional dependencies
URL: https://github.com/freeipa/freeipa/pull/599
Author: lslebodn
Title: #599: CONFIGURE: Improve error messages for optional dependencies
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/599/head:pr599
git checkout pr599
From 9bfd00b011432de9d03ffa6264ecd41e0c9255fe Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 15 Mar 2017 13:45:21 +0100
Subject: [PATCH] CONFIGURE: Improve error messages for optional dependencies
https://www.redhat.com/archives/freeipa-devel/2017-March/msg00307.html
---
configure.ac | 49 +++--
1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/configure.ac b/configure.ac
index 4d7a846..d47421e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -381,25 +381,19 @@ AC_SUBST([i18ntests])
AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
- AS_HELP_STRING([--enable-pylint],
- [Require pylint. Default is autodetection with
- "python -m pylint".]),
+ AS_HELP_STRING([--disable-pylint],
+ [skip Pylint in make lint target
+This feature is optional and aimed for checking issues in python code.
+You can skip this check wich configure time option --disable-pylint.]),
[PYLINT=$enableval],
- [PYLINT=check]
+ [PYLINT=yes]
)
-
if test x$PYLINT != xno; then
AC_MSG_CHECKING([for Pylint])
$PYTHON -m pylint --version >/dev/null 2>&1
if test "$?" != "0"; then
-if test x$PYLINT = xcheck; then
-PYLINT=no
-AC_MSG_NOTICE([cannot find optional pylint for $PYTHON])
-else
-AC_MSG_ERROR([cannot find pylint for $PYTHON])
-fi
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
else
-PYLINT=yes
AC_MSG_RESULT([yes])
fi
fi
@@ -409,29 +403,16 @@ AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
AC_ARG_WITH([jslint],
AS_HELP_STRING([--with-jslint=[FILE]],
- [path to JavaScript linter. Default is autodetection of
- utility "jsl" ]),
-[JSLINT="$withval"],
-[JSLINT=check]
-)
-
-AS_CASE([$JSLINT],
-[yes], [AC_PATH_PROG([JSLINT], [jsl], [missing])
-if test $JSLINT = missing; then
-AC_MSG_FAILURE([jsl is missing])
-fi],
-[no], [],
-[check], [AC_PATH_PROG([JSLINT], [jsl], [no])],
-dnl user setting
-[if ! test -f "$JSLINT"; then
-AC_MSG_RESULT([$JSLINT non-existing])
-AC_MSG_FAILURE([invalid value $JSLINT for jsl])
- fi
- if ! test -x "$JSLINT"; then
-AC_MSG_RESULT([$JSLINT non-executable])
-AC_MSG_FAILURE([invalid value $JSLINT for jsl])
- fi]
+ [path to JavaScript linter.
+This feature is optional and aimed for web ui developers.
+You can skip this check wich configure time option --without-jslint]),
+dnl --without-jslint will set JSLINT=no
+[JSLINT=$with_jslint],
+[AC_PATH_PROG([JSLINT], [jsl])]
)
+if test "x${JSLINT}" == "x"; then
+ AC_MSG_ERROR([cannot find JS lint])
+fi
AC_SUBST([JSLINT])
AM_CONDITIONAL([WITH_JSLINT], [test "x${JSLINT}" != "xno"])
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#599][opened] CONFIGURE: Improve error messages for optional dependencies
URL: https://github.com/freeipa/freeipa/pull/599
Author: lslebodn
Title: #599: CONFIGURE: Improve error messages for optional dependencies
Action: opened
PR body:
"""
https://www.redhat.com/archives/freeipa-devel/2017-March/msg00307.html
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/599/head:pr599
git checkout pr599
From ac15eb331ee1f0984709aa62338aec1b9a25e0b7 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 15 Mar 2017 13:45:21 +0100
Subject: [PATCH] CONFIGURE: Improve error messages for optional dependencies
https://www.redhat.com/archives/freeipa-devel/2017-March/msg00307.html
---
configure.ac | 52 +++-
1 file changed, 15 insertions(+), 37 deletions(-)
diff --git a/configure.ac b/configure.ac
index 4d7a846..39a4b2c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -381,25 +381,19 @@ AC_SUBST([i18ntests])
AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
- AS_HELP_STRING([--enable-pylint],
- [Require pylint. Default is autodetection with
- "python -m pylint".]),
+ AS_HELP_STRING([--disable-pylint],
+ [skip Pylint in make lint target
+This feature is optional and aimed for checking issues in python code.
+You can skip this check wich configure time option --disable-pylint.]),
[PYLINT=$enableval],
- [PYLINT=check]
+ [PYLINT=yes]
)
-
if test x$PYLINT != xno; then
AC_MSG_CHECKING([for Pylint])
$PYTHON -m pylint --version >/dev/null 2>&1
if test "$?" != "0"; then
-if test x$PYLINT = xcheck; then
-PYLINT=no
-AC_MSG_NOTICE([cannot find optional pylint for $PYTHON])
-else
-AC_MSG_ERROR([cannot find pylint for $PYTHON])
-fi
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
else
-PYLINT=yes
AC_MSG_RESULT([yes])
fi
fi
@@ -409,29 +403,16 @@ AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
AC_ARG_WITH([jslint],
AS_HELP_STRING([--with-jslint=[FILE]],
- [path to JavaScript linter. Default is autodetection of
- utility "jsl" ]),
-[JSLINT="$withval"],
-[JSLINT=check]
-)
-
-AS_CASE([$JSLINT],
-[yes], [AC_PATH_PROG([JSLINT], [jsl], [missing])
-if test $JSLINT = missing; then
-AC_MSG_FAILURE([jsl is missing])
-fi],
-[no], [],
-[check], [AC_PATH_PROG([JSLINT], [jsl], [no])],
-dnl user setting
-[if ! test -f "$JSLINT"; then
-AC_MSG_RESULT([$JSLINT non-existing])
-AC_MSG_FAILURE([invalid value $JSLINT for jsl])
- fi
- if ! test -x "$JSLINT"; then
-AC_MSG_RESULT([$JSLINT non-executable])
-AC_MSG_FAILURE([invalid value $JSLINT for jsl])
- fi]
+ [path to JavaScript linter.
+This feature is optional and aimed for web ui developers.
+You can skip this check wich configure time option --without-jslint]),
+dnl --without-jslint will set JSLINT=no
+[JSLINT=$with_jslint],
+[AC_PATH_PROG([JSLINT], [jsl])]
)
+if test "x${JSLINT}" == "x"; then
+ AC_MSG_ERROR([cannot find JS lint])
+fi
AC_SUBST([JSLINT])
AM_CONDITIONAL([WITH_JSLINT], [test "x${JSLINT}" != "xno"])
@@ -544,9 +525,6 @@ echo "
source code location: ${srcdir}
compiler: ${CC}
cflags: ${CFLAGS}
-Python: ${PYTHON}
-pylint: ${PYLINT}
-jslint: ${JSLINT}
LDAP libs:${LDAP_LIBS}
OpenSSL crypto libs: ${CRYPTO_LIBS}
KRB5 libs:${KRB5_LIBS}"
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (15/03/17 05:32), Petr Vobornik wrote: >In any case spending so much time discussing so minor change is a waste of >time. I'd push it. > Will you accept patch whith improves error messages? I can send it in few minutes; I do not want to creat PR which will be rejected. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286728710 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (15/03/17 05:17), Jan Cholasta wrote: >@lslebodn, nobody said that this has to be the last lint build related patch >ever, we can change the behavior later, even on top of this PR. I would rather >push this now and continue the discussion / submit additional PRs after 4.5 is >released. > But it would be good to have patch/approach in **official release** which was result of upstream discussion. Does it mean that I should improve error messages myself? LS """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286726459 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (14/03/17 16:05), Christian Heimes wrote: >https://github.com/freeipa/freeipa/pull/502#issue-209980292 > >two thumbs up, one heart, no thumbs down > My naive assumption was that discussion about apprach was moved to freipa-devel. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286721796 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ >PR #593 is not related to default yes; It is about something else. Current version does not fix concerns; because default should be yes as it was discussed in https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html I looks like upstream discussion is useless. And nobody cares about other distributions then fedora/rhel which can parse recommendation form upstream spec file. I am really disappointed from such upstream unfriendly approach. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286718251 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ >PR #593 is not related to default yes; It is about something else. Current version does not fix concerns; because default should be yes as it was discussed in https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html I looks like upstream discussion is useless. And nobody cares about other distributions then them fedora/rhel which can parse recommendation form upstream spec file. I am really disappointed from such upstream unfriendly approach. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286718251 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ > This PR makes packaging IPA 4.5 on RHEL 7 easier for me, so thumbs up from me. I understand it is more convenient to have less extra configure options in rhel; But it was discussed on upstream mailing list and better error messages would give such hints to everyone. https://www.redhat.com/archives/freeipa-devel/2017-March/msg00308.html @HonzaCholasta it would be good if you add comment also to upstream discussion; if you prefer autodetection. It would be good if result of discussion is the same as pushed patch. https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html Current version will not persuade other distributions(debian; openSUSE) to run pylint as part of build. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286719902 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ > This PR makes packaging IPA 4.5 on RHEL 7 easier for me, so thumbs up from me. I understand it is more convenient to have less extra configure options in rhel; But it was discussed on upstream mailing list and better error messages would give such hints to everyone. https://www.redhat.com/archives/freeipa-devel/2017-March/msg00308.html @HonzaCholasta it would be good if you add comment also to upstream discussion; if you prefer autodetection. It would be good if result of discussion is the same as pushed patch. https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html Current version will not persuade other distributions(debian; openSUSE) to run pylint as part of build. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286719902 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ >PR #593 is not related to default yes; It is about something else. Current version does not fix concerns; because default should be yes as it was discussed in https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html I looks like upstream discussion is useless. And nobody cares about other distributions then them fedora/rhel which can parse recommendation form upstream spec file. I am really disappointed from such upstream unfriendly approach. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286718251 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#596][comment] spec file: support client-only build
URL: https://github.com/freeipa/freeipa/pull/596
Title: #596: spec file: support client-only build
lslebodn commented:
"""
On (15/03/17 02:32), Jan Cholasta wrote:
>I can't, both are required to build client components (`ipa-getkeytab`
>specifically), moving them to the server section would break client-only RPM
>build.
>
Sorry for confusion. I misinterpreted changes in diff.
The real change was moving following part to server only build
```
# %{_unitdir}, %{_tmpfilesdir}
BuildRequires: systemd
# systemd-tmpfiles which is executed from make install requires apache user
BuildRequires: httpd
```
But git diff decided to show moving of other lines
LS
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/596#issuecomment-286694086
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#591][comment] spec file: add unconditional python-setuptools BuildRequires
URL: https://github.com/freeipa/freeipa/pull/591 Title: #591: spec file: add unconditional python-setuptools BuildRequires lslebodn commented: """ Post push comment. This is a reason why there was configure time check in **rejected** PR#494. Spec file change is not very upstream friendly. https://github.com/freeipa/freeipa/pull/494/commits/a1321abbdb2cf510e0f36c65d2af0fb6329d2e23 """ See the full comment at https://github.com/freeipa/freeipa/pull/591#issuecomment-286685603 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#596][comment] spec file: support client-only build
URL: https://github.com/freeipa/freeipa/pull/596 Title: #596: spec file: support client-only build lslebodn commented: """ You can also move few dependencies to server only build ``` BuildRequires: libini_config-devel BuildRequires: cyrus-sasl-devel ``` Check spec file changes in **rejected** PR https://github.com/freeipa/freeipa/pull/494/files otherwise LGTM """ See the full comment at https://github.com/freeipa/freeipa/pull/596#issuecomment-286683090 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (14/03/17 09:56), Christian Heimes wrote: >The PR got three +1 / heart and not -1. I propose to get it merged for 4.5 >today. > I cannot see 3-times +1 in this PR. I can see just 2 reviewers in top right corner * lslebodn requested changes * tomaskrizek approved these changes Is there any reason why upstream discussion is not accepted? https://www.redhat.com/archives/freeipa-devel/2017-March/msg00371.html LS """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-286530703 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#414][comment] SPEC: Update SELinux file context of ipa-otpd
URL: https://github.com/freeipa/freeipa/pull/414 Title: #414: SPEC: Update SELinux file context of ipa-otpd lslebodn commented: """ On (10/03/17 00:28), Martin Babinsky wrote: >@lslebodn CI complains that your changes produce an invalid specfile. Can you >please fix this. A better question, is there a demand to have this workaround >in the spec file? > It could be fixed; but there are more SELinux bugs now. So it does not worth IMHO. Feel free to close. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/414#issuecomment-285621154 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ I am still expect some comment from @rcritten LS """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283938711 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (01/03/17 22:37), Jan Cholasta wrote: >I tend to agree with @lslebodn, but I don't have a strong opinion on this. I >noticed a couple of issues though: > >* `--without-jslint` does not seem to work correctly: > ``` > $ ./configure --without-jslint > ... > IPA Server 4.4.90.dev201703020634+git3a29b47 > > ... > jslint: /usr/bin/jsl > ... > ``` > >* In `freeipa.spec.in`, when `with_lint` is not defined, lint should be >disabled, so `--disable-pylint` and `--without-jslint` should be passed to >`%configure`. > This is exactly a reason why the simplest solution would be to exend error messages without changing default. Default "yes" should encourage downstream packagers[1,2] to run make lint and catch issues with backported(downstream only) patches. BTW there were also some version of packages for opensuse[2] but they were removed. [1] https://anonscm.debian.org/cgit/pkg-freeipa/freeipa.git [2] https://en.opensuse.org/Portal:FreeIPA """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283582564 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ On (01/03/17 09:39), Petr Vobornik wrote: >+1 Reasoning for not skipping linters was that reviewer or patch author can >forget to run those. This problem was solved by travis checks. > ATM nothing force reviewer/author to run lint. `makerpms.sh` does not call `make lint` and it is not a dependency of `make all`. configure script just remind developer to install pylint/jslint. (or disable configure time check) Which is a huge difference. If you think that developers should not/needn't have installed pylint by default then it's your decission. But I cannot see a good reason for removing this reminder. my 2 cents """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283457251 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ > But you're right we should update the wiki pages to mention the new defaults. Such change require broader discussion. e.g. I know that @rcritten had strong opinion about pylint usage in past. """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283337442 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ It was explained on IRC > < cheimes> lslebodn: Your proposal is missing the point of the ticket. It > doesn't not simplify > building, but rather improves error messages. The whole point > of the ticket is a more > pleasant experience for outsiders that are not FreeIPA core > contributors. But there is main problem with this PR. The design document expected these option enabled by default. http://www.freeipa.org/page/V4/Build_system_refactoring This is a reason why I mentioned to log just a hint for optional *lint dependencies. And ticket https://pagure.io/freeipa/issue/6604 says that options were made optional a moth ago master: *5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint *3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for jslint *b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283335089 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ Please add explanation to the thumb down """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283309329 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#501][comment] C compilation fixes and hardening
URL: https://github.com/freeipa/freeipa/pull/501 Title: #501: C compilation fixes and hardening lslebodn commented: """ I know it's closed. It was just a kindly reminder to address obvious problems as part of review process. """ See the full comment at https://github.com/freeipa/freeipa/pull/501#issuecomment-283299475 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ Writing hint together with error is the simplest solution. And still remind developers to install pylint/jslint. e.g. ``` diff --git a/configure.ac b/configure.ac index af41f5e3a..c02dd1fe4 100644 --- a/configure.ac +++ b/configure.ac @@ -384,7 +384,10 @@ if test x$PYLINT != xno; then AC_MSG_CHECKING([for Pylint]) $PYTHON -m pylint --version > /dev/null if test "$?" != "0"; then -AC_MSG_ERROR([cannot find pylint for $PYTHON]) +AC_MSG_ERROR([cannot find pylint for $PYTHON +This feature is optional and aimed for developers. You can skip this check +wich configure time option --disable-pylint +]) else AC_MSG_RESULT([yes]) fi ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283298387 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#501][comment] C compilation fixes and hardening
URL: https://github.com/freeipa/freeipa/pull/501 Title: #501: C compilation fixes and hardening lslebodn commented: """ FYI; it is far far away from best practice to modify `CFLAGS` and configure time; unless you test compiler options. Such change should be in makefile `AM_CFLAGS` In the future, try to address comments in PR (#364) rather then ignoring them and then fixing in new PR """ See the full comment at https://github.com/freeipa/freeipa/pull/501#issuecomment-283296048 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#502][comment] Make pylint and jsl optional
URL: https://github.com/freeipa/freeipa/pull/502 Title: #502: Make pylint and jsl optional lslebodn commented: """ May I know why the default was changed without design discussion? IIRC pscacek intentionally enabled it by default. Much better approach would be to print hint at configure time detection that it is optional and can be disabled. Code wise; the change is too complicated (too many nested if condition ...) which does not improve readability of configure.ac """ See the full comment at https://github.com/freeipa/freeipa/pull/502#issuecomment-283294552 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#472][comment] Packaging: Add placeholder packages
URL: https://github.com/freeipa/freeipa/pull/472 Title: #472: Packaging: Add placeholder packages lslebodn commented: """ NACK for downstream patch. The intentin of build system refactoring was make packaging in downstream simpler. """ See the full comment at https://github.com/freeipa/freeipa/pull/472#issuecomment-281935052 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#472][comment] Packaging: Add placeholder packages
URL: https://github.com/freeipa/freeipa/pull/472 Title: #472: Packaging: Add placeholder packages lslebodn commented: """ NACK for downstream patch. The intentin of build system refactoring was make packaging in downstream simpler. """ See the full comment at https://github.com/freeipa/freeipa/pull/472#issuecomment-281935052 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#472][comment] Packaging: Add placeholder packages
URL: https://github.com/freeipa/freeipa/pull/472 Title: #472: Packaging: Add placeholder packages lslebodn commented: """ FYI: At the moment, `make check` run just C-based unit test and all of them are optional. If required dependency is not found at configure time then test is not build/executed. The reason is that required dependencies are not in some downstream distributions and IIRC python-wheel is not there either. """ See the full comment at https://github.com/freeipa/freeipa/pull/472#issuecomment-281933816 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#472][comment] Packaging: Add placeholder packages
URL: https://github.com/freeipa/freeipa/pull/472 Title: #472: Packaging: Add placeholder packages lslebodn commented: """ I agree with @MartinBasti it is not a unit test. IMHO better approach is to test it in CI/travis/... """ See the full comment at https://github.com/freeipa/freeipa/pull/472#issuecomment-281929628 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ >Thanks for your contribution. I added your patch to my PR. On my system I ran >into a minor issue. >Some C99 types like uint8_t were not defined and I had to >include stdint.h. This change is not enough; there is still warning: ``` ipa_pwd_ntlm.c: In function 'encode_nt_key': ipa_pwd_ntlm.c:58:5: warning: implicit declaration of function 'strlen' [-Wimplicit-function-declaration] il = strlen(newPasswd); ^ ipa_pwd_ntlm.c:58:10: warning: incompatible implicit declaration of built-in function 'strlen' il = strlen(newPasswd); ^ ``` The latest version is a small improvement; but there are still problems/small issues because this PR was created with intention to use tox. I know you are busy. So I wrote client-only implementation from scratch. This PR is superseded by #494 """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-281606346 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (22/02/17 06:16), Simo Sorce wrote: >So this is the reasoning and why I am approving this PR and not #494. > >When you build all components, including server bits, tests are installed, >therefore when we build just client bits tets that are relevant to client bits >also need to be installed for consistency. > >Any switch should default to the same behavior regardless of whether server >build is enabled. It is confusing if the --with[out]-[ipa]tests switch changes >default based on a different switch passed to configure. > >As far as I understand this PR maintains the same default for either server or >client only builds, so it gets my approval. > Neither of python packages which I mention in #494 package unit test in fedora. So there is not a reason to package them by default for client only build. And integration tests require server therefore must not be installed by default with client-only build. Result: This PR has wrong default for instalation of ipatests with client-only build. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-281687875 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 01:52), Tomas Krizek wrote: >@lslebodn My bad, there was some leftover stuff that `git clean -dfx` didn't >clear for some reason. > >Nevertheless, this does work and allows a client only, as well as installing >tests with `--with-tests` option. The mock build when run with >`--without=server` does install less dependencies. > >But I'm not acking, because of the controversy with the `--with-tests` option >(see #364). > @tomaskrizek FYI `rpmbuild` accepts also parameter `--without server` but it is not simple to pass it through `make rpms` and it would not check minimal dependencies in spec file. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281686378 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 03:24), Christian Heimes wrote: >python-requests is a bad example because it suffers from the same issue as IPA. > >A better example is any other modern Python project like cryptography. It runs >tests with installed files, not in-tree files. > hmm; I probably missed something. ``` sh$ rpm -ql rpm -ql python3-cryptography | grep test /usr/share/doc/python3-cryptography/docs/development/test-vectors.rst ``` ``` sh$ wget --content-disposition https://github.com/pyca/cryptography/archive/1.7.2.tar.gz 2017-02-22 14:10:00 (9.86 MB/s) - ‘cryptography-1.7.2.tar.gz’ saved [27131190] sh$ tar -xzf cryptography-1.7.2.tar.gz sh$ find cryptography-1.7.2/ -name "*test*" cryptography-1.7.2/vectors/cryptography_vectors/keywrap/kwtestvectors cryptography-1.7.2/vectors/cryptography_vectors/hashes/whirlpool/iso-test-vectors.txt cryptography-1.7.2/vectors/cryptography_vectors/asymmetric/Traditional_OpenSSL_Serialization/testrsa.pem cryptography-1.7.2/vectors/cryptography_vectors/asymmetric/Traditional_OpenSSL_Serialization/testrsa-encrypted.pem cryptography-1.7.2/vectors/cryptography_vectors/asymmetric/DER_Serialization/testrsa.der cryptography-1.7.2/tests cryptography-1.7.2/tests/test_x509_revokedcertbuilder.py cryptography-1.7.2/tests/test_x509_ext.py cryptography-1.7.2/tests/test_x509_crlbuilder.py cryptography-1.7.2/tests/test_x509.py cryptography-1.7.2/tests/test_warnings.py cryptography-1.7.2/tests/test_utils.py cryptography-1.7.2/tests/test_interfaces.py cryptography-1.7.2/tests/test_fernet.py cryptography-1.7.2/tests/test_cryptography_utils.py cryptography-1.7.2/tests/hypothesis/test_padding.py cryptography-1.7.2/tests/hypothesis/test_fernet.py cryptography-1.7.2/tests/hazmat/primitives/twofactor/test_totp.py cryptography-1.7.2/tests/hazmat/primitives/twofactor/test_hotp.py cryptography-1.7.2/tests/hazmat/primitives/test_x963kdf.py cryptography-1.7.2/tests/hazmat/primitives/test_x963_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_serialization.py cryptography-1.7.2/tests/hazmat/primitives/test_seed.py cryptography-1.7.2/tests/hazmat/primitives/test_scrypt.py cryptography-1.7.2/tests/hazmat/primitives/test_rsa.py cryptography-1.7.2/tests/hazmat/primitives/test_pbkdf2hmac_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_pbkdf2hmac.py cryptography-1.7.2/tests/hazmat/primitives/test_padding.py cryptography-1.7.2/tests/hazmat/primitives/test_keywrap.py cryptography-1.7.2/tests/hazmat/primitives/test_kbkdf_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_kbkdf.py cryptography-1.7.2/tests/hazmat/primitives/test_idea.py cryptography-1.7.2/tests/hazmat/primitives/test_hmac_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_hmac.py cryptography-1.7.2/tests/hazmat/primitives/test_hkdf_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_hkdf.py cryptography-1.7.2/tests/hazmat/primitives/test_hashes.py cryptography-1.7.2/tests/hazmat/primitives/test_hash_vectors.py cryptography-1.7.2/tests/hazmat/primitives/test_ec.py cryptography-1.7.2/tests/hazmat/primitives/test_dsa.py cryptography-1.7.2/tests/hazmat/primitives/test_dh.py cryptography-1.7.2/tests/hazmat/primitives/test_constant_time.py cryptography-1.7.2/tests/hazmat/primitives/test_concatkdf.py cryptography-1.7.2/tests/hazmat/primitives/test_cmac.py cryptography-1.7.2/tests/hazmat/primitives/test_ciphers.py cryptography-1.7.2/tests/hazmat/primitives/test_cast5.py cryptography-1.7.2/tests/hazmat/primitives/test_camellia.py cryptography-1.7.2/tests/hazmat/primitives/test_blowfish.py cryptography-1.7.2/tests/hazmat/primitives/test_block.py cryptography-1.7.2/tests/hazmat/primitives/test_asym_utils.py cryptography-1.7.2/tests/hazmat/primitives/test_arc4.py cryptography-1.7.2/tests/hazmat/primitives/test_aes.py cryptography-1.7.2/tests/hazmat/primitives/test_3des.py cryptography-1.7.2/tests/hazmat/bindings/test_openssl.py cryptography-1.7.2/tests/hazmat/bindings/test_commoncrypto.py cryptography-1.7.2/tests/hazmat/backends/test_openssl.py cryptography-1.7.2/tests/hazmat/backends/test_multibackend.py cryptography-1.7.2/tests/hazmat/backends/test_commoncrypto.py cryptography-1.7.2/tests/hazmat/backends/test_backendinit.py cryptography-1.7.2/tests/conftest.py cryptography-1.7.2/docs/development/test-vectors.rst ``` and unit test are exeuted as part of rpm-build. ``` http://pkgs.fedoraproject.org/cgit/rpms/python-cryptography.git/tree/python-cryptography.spec#n133 ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281666168 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 03:24), Christian Heimes wrote: >python-requests is a bad example because it suffers from the same issue as IPA. > >A better example is any other modern Python project like cryptography. It runs >tests with installed files, not in-tree files. > I check few other quite new projects which were written by RH python guys. https://admin.fedoraproject.org/pkgdb/package/rpms/devassistant/ https://admin.fedoraproject.org/pkgdb/package/rpms/python-pytest-multihost/ They run unit tests as part of build process and unit tests are not installed. But maybe I was not just lucky enough to find modern Python project. Anyway `ipatests` are installed by default with freeipa. If you want to use non-defalt option for client-only build then it is possible to install `ipatests` as well. Thank you for your comments. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281649262 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ >Thanks for your contribution. I added your patch to my PR. On my system I ran >into a minor issue. >Some C99 types like uint8_t were not defined and I had to >include stdint.h. This change is not enough; there is still warning: ``` ipa_pwd_ntlm.c: In function 'encode_nt_key': ipa_pwd_ntlm.c:58:5: warning: implicit declaration of function 'strlen' [-Wimplicit-function-declaration] il = strlen(newPasswd); ^ ipa_pwd_ntlm.c:58:10: warning: incompatible implicit declaration of built-in function 'strlen' il = strlen(newPasswd); ^ ``` >By the way I'm just going to ignore your snidely and snarky comment. No problem. I am going to forget that my proposal for compromise was ignored for 12 days. The latest version is a small improvement; but there are still problems/small issues because this PR was created with intention to use tox. I know you are busy. So I wrote client-only implementation from scratch. This PR is superseded by #494 """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-281606346 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 03:04), Christian Heimes wrote: >You are aware that your example code checks the wrong code? It is testing >in-tree sources, not the actual sources that get packaged and installed. > Yes, because unit tests are not usually installed with package. e.g. `rpm -ql python3-requests | grep tests` and unit tests are executed as part of build http://pkgs.fedoraproject.org/cgit/rpms/python-requests.git/tree/python-requests.spec#n158 And I know that your use-case is different. Therefore there is a configure time option `--with-ipatests` LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281639897 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494
Title: #494: Support client-only build
lslebodn commented:
"""
On (22/02/17 02:51), Christian Heimes wrote:
>You assumption is incorrect. ```ipatests``` does not depend on
>```ipaserver```,
>https://github.com/freeipa/freeipa/blob/master/ipatests/setup.py#L61
>
>```
>install_requires=[
>"cryptography",
>"dnspython",
>"gssapi",
>"ipaclient",
>"ipalib",
>"ipaplatform",
>"ipapython",
>"nose",
>"polib",
>"pyldap",
>"pytest",
>"pytest_multihost",
>"python-nss",
>"six",
>],
>```
>
>Only some subcomponents of ```ipatests``` do depend on the ```ipaserver```
>package or a running server for integration tests,
>https://github.com/freeipa/freeipa/blob/master/ipatests/setup.py#L77
>
>```
>extras_require={
>"integration": ["dbus-python", "pyyaml", "ipaserver"],
>"ipaserver": ["ipaserver"],
>"webui": ["selenium", "pyyaml", "ipaserver"],
>"xmlrpc": ["ipaserver"],
>}
>```
>
Packagers can run unit tests in-tree. And that's a usual way
how packagers run unit tests.
e.g.
```
PYTHONPATH=$PWD/ \
$PYTHON ./ipatests/ipa-run-tests -vvv --tb=native \
$PWD/ipatests/test_ipaclient/ \
$PWD/ipatests/test_ipalib \
$PWD/ipatests/test_ipapython \
$PWD/ipatests/test_util.py \
$PWD/ipatests/util.py
```
Tox is a special case.
Therefore installation of tests is disabled for `--disable-server`
But for tox it is possible to overrride it.
e.g. `./configure --disable-server --with-tests`
LS
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/494#issuecomment-281636358
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 02:09), Christian Heimes wrote: >There are two reasons we decided on ```--without-ipatests```: > >* ```--with-tests``` / ```--without-tests``` is technically not correct. We >still compile C tests. The flag is about the component ```ipatests```, so >let's call it ```--without-ipatests```. >* ```--with-ipatests``` / ```--without-ipatests``` is only relevant for >downstream packaging to make the life of a packager a bit easier. FreeIPA is >an upstream first project. The default settings for configure should be >convenient and user-friendly for upstream developers and users. > `without-tests` was changed to `without-ipatests` freeip-4.4 has a weird build system and all downstream packages had to do many tricks/workaround to install it an package. The intention of build-refactoring was to make packaging as simple as possible. The purpose of client only build https://fedorahosted.org/freeipa/ticket/6517 Is to allow package just client parts on distriutions which does not have systemd or they do not want to depend on systemd. Because ipa-client install just configure sssd, certmonger which still can be compiled without systemd support. So the `--disable-server` must disable all parts which requires anything with server dependencies. Therefore it disable js-lint, pylint and installation of ipatest. There is a still possiblility to enable them with client-only build. e.g. `./configure --disable-server --with-ipatests --enable-pylint` >The final decision has been made. > The decission was made that there will be `--without-ipatests` for tox use-case. Becasue tox use-case is not a client only build. Therefore explicit enabling `ipatests` is required for tox use-case. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281633229 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 02:23), Christian Heimes wrote: >tiran requested changes on this pull request. > >see comments > >> -CFLAGS="$bck_cflags" >- >-LIBPDB_NAME="" >-AC_CHECK_LIB([samba-passdb], >- [make_pdb_method], >- [LIBPDB_NAME="samba-passdb"; HAVE_LIBPDB=1], >- [LIBPDB_NAME="pdb"], >- [$SAMBA40EXTRA_LIBPATH]) >- >-if test "x$LIB_PDB_NAME" = "xpdb" ; then >- AC_CHECK_LIB([$LIBPDB_NAME], >- [make_pdb_method], >- [HAVE_LIBPDB=1], >- [AC_MSG_ERROR([Neither libpdb nor libsamba-passdb does have >make_pdb_method])], >- [$SAMBA40EXTRA_LIBPATH]) >+AC_MSG_CHECKING($(basename $PYTHON) module setuptools ) > >Please put this in a separate PR. This is not related to --disable-server. > refactoring/cleaning is requred for minimising dependencies before split. Otherwise git log would be confusing. >+AM_CONDITIONAL([ENABLE_SERVER], [test x"$enable_server" = xyes]) >+if test x"$enable_server" = xyes; then >+m4_include([server.m4]) >+fi >+ >+AC_ARG_WITH([tests], >+[AC_HELP_STRING([--with-tests], > >NACK, ```without-ipatests``` > already changed. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281631097 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 02:16), Christian Heimes wrote: >NACK on 42fb9b1c > >* Either use ```--with-ipaplatform=redhat``` on CentOS >* Or implement a proper way to fill ipaplatfrom from ```/etc/os-relase``` >value ```ID_LIKE```, >https://www.freedesktop.org/software/systemd/man/os-release.html > ID_LIKE is multivalue on centos; it cannot be used. ``` sh# cat /etc/os-release NAME="CentOS Linux" VERSION="7 (Core)" ID="centos" ID_LIKE="rhel fedora" VERSION_ID="7" PRETTY_NAME="CentOS Linux 7 (Core)" ANSI_COLOR="0;31" CPE_NAME="cpe:/o:centos:centos:7" HOME_URL="https://www.centos.org/"; BUG_REPORT_URL="https://bugs.centos.org/"; CENTOS_MANTISBT_PROJECT="CentOS-7" CENTOS_MANTISBT_PROJECT_VERSION="7" REDHAT_SUPPORT_PRODUCT="centos" REDHAT_SUPPORT_PRODUCT_VERSION="7" ``` >Either way, this should be handled by a separate PR and not mixed with >client-only builds. > The purpose of client only build is to make life of packars simpler. This patch improves UX so it need to be part of this PR. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281630224 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][synchronized] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Author: lslebodn Title: #494: Support client-only build Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/494/head:pr494 git checkout pr494 From b4e0d5ed62bfdb09e1a329e35a15e8cb138026ab Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:08 +0100 Subject: [PATCH 01/14] CONFIGURE: Decrease dependency on libini_config libini_config is used only in ipa-getkeytab and it uses only functions from libini_config-1.1 sh$ objdump -p /usr/sbin/ipa-getkeytab | grep INI_CONFIG 0x00acdc20 0x00 04 INI_CONFIG_1.1.0 There is not any reason ho have dependency for higher version and lower dependency will allow to build client only on older distributions. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 44dc11b..246803f 100644 --- a/configure.ac +++ b/configure.ac @@ -265,7 +265,7 @@ AC_SUBST(LIBINTL_LIBS) dnl --- dnl - Check for libini_config dnl --- -PKG_CHECK_MODULES([INI], [ini_config >= 1.2.0]) +PKG_CHECK_MODULES([INI], [ini_config >= 1.1.0]) dnl --- dnl - Check for systemd directories From be1e3f5b8764e03355a45b52a6fb0df9c0b408d8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:20 +0100 Subject: [PATCH 02/14] CONFIGURE: Properly detect libpopt on el7 libpopt added pkg-config file in 1.16 but there are still distributions which has older version of library (el6, el7). And new features from libpopt are not used anywhere. Configure should try to detect as much as possible and users should not use workarounds with explicitely enabled variables as parameters e.g. ./configure POPT_LIBS="-lpopt " --- configure.ac | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 246803f..0a23fd2 100644 --- a/configure.ac +++ b/configure.ac @@ -235,7 +235,13 @@ PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap >= 1.13.90]) dnl --- dnl - Check for POPT dnl --- -PKG_CHECK_MODULES([POPT], [popt]) +POPT_LIBS= +PKG_CHECK_MODULES([POPT], [popt], [], +[AC_CHECK_HEADER([popt.h], [], [AC_MSG_ERROR([popt.h not found])]) + AC_CHECK_LIB([popt], [poptGetContext], [POPT_LIBS="-lpopt"]) + AC_SUBST(POPT_LIBS) +] +) dnl --- dnl - Check for SASL From 811080737684c2e1fcab425616ec0a6f7d5a2dee Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:25 +0100 Subject: [PATCH 03/14] CONFIGURE: Improve detection of xmlrpc_c flags The pkg-config files for xmlrpc_c libraries are shipped just in fedora/rhel due to downstream patch. Debian does not have pkg-config files for xmlrpc_c. Therefore we need to fallback to older method of detection XMLRPC_*FLAGS which was reverted by the commit 1e0143c159134337a00a91d4ae64e614f72da62e --- configure.ac | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 0a23fd2..821ae21 100644 --- a/configure.ac +++ b/configure.ac @@ -251,7 +251,20 @@ PKG_CHECK_MODULES([SASL], [libsasl2]) dnl --- dnl - Check for XMLRPC-C dnl --- -PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util]) +PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util], [], + [try_xmlrpc_fallback=true]) +if test x"$try_xmlrpc_fallback" = xtrue; then +XMLRPC_LIBS= +AC_CHECK_HEADER([xmlrpc-c/base.h], [], +[AC_MSG_ERROR([xmlrpc-c/base.h not found])]) + +AC_CHECK_LIB([xmlrpc_client], [xmlrpc_client_init2], + [XMLRPC_LIBS="-lxmlrpc -lxmlrpc_client -lxmlrpc_util"]) +if test "x$XMLRPC_LIBS" = "x" ; then +AC_MSG_ERROR([xmlrpc-c not found]) +fi +AC_SUBST(XMLRPC_LIBS) +fi dnl --- dnl - Check for libintl From 34ebfdef2ca8578a6345de508c7dda1ce9c46ae8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:31 +0100 Subject: [PATCH 04/14] CONFIGURE: Remove manual detection of libintl The gettext provided macro AM_GNU_GETTEXT checks for required header file "libintl.h" and also provide variable with linker flags LTLIBINTL. The detection is more robus an platform independent. It can also detect situ
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 01:43), Christian Heimes wrote: >NACK on aece4c3c > >We compromised on ```--without-ipatests``` with installation of ipatests >defaulting to true. The compromose was already ACKed by @simo5 > Default is true; because --enable-server has default value true. So NACK should not count. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281623863 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ BTW I tested client-only build on fedora24, fedora25, fedora rawhide, epel7, debian stable, debian testing, debian unstable """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281611039 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][comment] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Title: #494: Support client-only build lslebodn commented: """ On (22/02/17 00:59), Tomas Krizek wrote: >I'm not able to run autoreconf, it fails with the following error: > >``` >configure.ac:447: error: required file 'init/tmpfilesd/Makefile.in' not found >asn1/Makefile.am: installing './depcomp' >parallel-tests: installing './test-driver' >autoreconf: automake failed with exit status: 1 >``` I cannot see such file in git :-( ``` sh$ git clean -fdx sh$ ls init/ ipa-dnskeysyncd ipa-ods-exporter Makefile.am systemd ``` and it isn't in configure either ``` sh$ grep "/tmpfilesd" configure.ac *.m4 sh$ $echo $? 1 ``` LS """ See the full comment at https://github.com/freeipa/freeipa/pull/494#issuecomment-281610715 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ >Thanks for your contribution. I added your patch to my PR. On my system I ran >into a minor issue. >Some C99 types like uint8_t were not defined and I had to >include stdint.h. This change is not enough; there is still warning: ``` ipa_pwd_ntlm.c: In function 'encode_nt_key': ipa_pwd_ntlm.c:58:5: warning: implicit declaration of function 'strlen' [-Wimplicit-function-declaration] il = strlen(newPasswd); ^ ipa_pwd_ntlm.c:58:10: warning: incompatible implicit declaration of built-in function 'strlen' il = strlen(newPasswd); ^ ``` >By the way I'm just going to ignore your snidely and snarky comment. No problem. I am going to forget that my proposal for compromise was ignored for 12 days. The latest version is a small improvement; but there are still problems small issues because this PR was not create with intention to use tox. I know you are busy. So I wrote client-only implementation from scratch. This PR is superseded by #494 """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-281606346 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ >Thanks for your contribution. I added your patch to my PR. On my system I ran >into a minor issue. >Some C99 types like uint8_t were not defined and I had to >include stdint.h. This change is not enough; there is still warning: ``` ipa_pwd_ntlm.c: In function 'encode_nt_key': ipa_pwd_ntlm.c:58:5: warning: implicit declaration of function 'strlen' [-Wimplicit-function-declaration] il = strlen(newPasswd); ^ ipa_pwd_ntlm.c:58:10: warning: incompatible implicit declaration of built-in function 'strlen' il = strlen(newPasswd); ^ ``` >By the way I'm just going to ignore your snidely and snarky comment. No problem. I am going to forget that my proposal for compromise was ignored for 12 days. The latest version is a small improvement; but there are still problems small issues because this PR was not create with intention to use tox. I know you are busy. So I wrote client-only implementation from scratch. This PR is superseded by #494 """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-281606346 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#494][opened] Support client-only build
URL: https://github.com/freeipa/freeipa/pull/494 Author: lslebodn Title: #494: Support client-only build Action: opened PR body: """ How to test: * autoreconf -if * ./configure --disable-server * make srpms * mock --rebuild dist/rpms/freeipa-4.4.90.*.src.rpm --resultdir . * mock --rebuild dist/rpms/freeipa-4.4.90.*.src.rpm --resultdir . --without=server """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/494/head:pr494 git checkout pr494 From b4e0d5ed62bfdb09e1a329e35a15e8cb138026ab Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:08 +0100 Subject: [PATCH 01/14] CONFIGURE: Decrease dependency on libini_config libini_config is used only in ipa-getkeytab and it uses only functions from libini_config-1.1 sh$ objdump -p /usr/sbin/ipa-getkeytab | grep INI_CONFIG 0x00acdc20 0x00 04 INI_CONFIG_1.1.0 There is not any reason ho have dependency for higher version and lower dependency will allow to build client only on older distributions. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 44dc11b..246803f 100644 --- a/configure.ac +++ b/configure.ac @@ -265,7 +265,7 @@ AC_SUBST(LIBINTL_LIBS) dnl --- dnl - Check for libini_config dnl --- -PKG_CHECK_MODULES([INI], [ini_config >= 1.2.0]) +PKG_CHECK_MODULES([INI], [ini_config >= 1.1.0]) dnl --- dnl - Check for systemd directories From be1e3f5b8764e03355a45b52a6fb0df9c0b408d8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:20 +0100 Subject: [PATCH 02/14] CONFIGURE: Properly detect libpopt on el7 libpopt added pkg-config file in 1.16 but there are still distributions which has older version of library (el6, el7). And new features from libpopt are not used anywhere. Configure should try to detect as much as possible and users should not use workarounds with explicitely enabled variables as parameters e.g. ./configure POPT_LIBS="-lpopt " --- configure.ac | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 246803f..0a23fd2 100644 --- a/configure.ac +++ b/configure.ac @@ -235,7 +235,13 @@ PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap >= 1.13.90]) dnl --- dnl - Check for POPT dnl --- -PKG_CHECK_MODULES([POPT], [popt]) +POPT_LIBS= +PKG_CHECK_MODULES([POPT], [popt], [], +[AC_CHECK_HEADER([popt.h], [], [AC_MSG_ERROR([popt.h not found])]) + AC_CHECK_LIB([popt], [poptGetContext], [POPT_LIBS="-lpopt"]) + AC_SUBST(POPT_LIBS) +] +) dnl --- dnl - Check for SASL From 811080737684c2e1fcab425616ec0a6f7d5a2dee Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:25 +0100 Subject: [PATCH 03/14] CONFIGURE: Improve detection of xmlrpc_c flags The pkg-config files for xmlrpc_c libraries are shipped just in fedora/rhel due to downstream patch. Debian does not have pkg-config files for xmlrpc_c. Therefore we need to fallback to older method of detection XMLRPC_*FLAGS which was reverted by the commit 1e0143c159134337a00a91d4ae64e614f72da62e --- configure.ac | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 0a23fd2..821ae21 100644 --- a/configure.ac +++ b/configure.ac @@ -251,7 +251,20 @@ PKG_CHECK_MODULES([SASL], [libsasl2]) dnl --- dnl - Check for XMLRPC-C dnl --- -PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util]) +PKG_CHECK_MODULES([XMLRPC], [xmlrpc xmlrpc_client xmlrpc_util], [], + [try_xmlrpc_fallback=true]) +if test x"$try_xmlrpc_fallback" = xtrue; then +XMLRPC_LIBS= +AC_CHECK_HEADER([xmlrpc-c/base.h], [], +[AC_MSG_ERROR([xmlrpc-c/base.h not found])]) + +AC_CHECK_LIB([xmlrpc_client], [xmlrpc_client_init2], + [XMLRPC_LIBS="-lxmlrpc -lxmlrpc_client -lxmlrpc_util"]) +if test "x$XMLRPC_LIBS" = "x" ; then +AC_MSG_ERROR([xmlrpc-c not found]) +fi +AC_SUBST(XMLRPC_LIBS) +fi dnl --- dnl - Check for libintl From 34ebfdef2ca8578a6345de508c7dda1ce9c46ae8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 22 Feb 2017 09:39:31 +0100 Subject: [PATCH 04/14] CONFIGURE
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ It looks like @tiran does not want to move this PR forward. So here is a link to patch which does not misuse client-only build and add configure time option to install ipatests. https://paste.fedoraproject.org/paste/~u7iDljnMTvinxmLFoc25V5M1UNdIGYhyRLivL9gydE=/ As part of this work I found out that 4 dependencies can be simply moved to server only build: `libuuid`, `DIRSRV`, `libsss_idmap`, `libsss_nss_idmap`. Result => Client only build require less C-dependencies. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-280675665 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (17/02/17 01:12), Petr Vobornik wrote: >I still fail to see why we should care about `make dist` with `configure >--disable-server` this is not a combination of options which should be used >together, there is no point in it except theoretical exercise. > `make dist` works >> then I will need to send PR to fix client-only build and it will not install >> ipatests with --disable-server. > >Why? The main problem with `client-only` build and `ipatests` is that there are not test for it. * all integration test assume that server will be installed first and not that server is already available somewhere. * unit test should be run as part of build (e.g. `make check`) So there is not any reason to install `ipatests` for client only build. Client only build was tested just in downstream :-( LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-280621474 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (16/02/17 02:30), Christian Heimes wrote: >Lukas, you are wasting both my and your precious time with a needless >bike-shedding discussion about semantics. The ```--disable-server``` option >skips all parts of the build process that are only relevant for server and not >relevant for client. ```ipatests``` is relevant for both, therefore it stays. > It's not bikeshadig. You are adding HACKs to freeipa build system. If this PR will be pushed with cuurent state then I will need to send PR to fix client-only build and it will not install ipatests with --disable-server. I proposed you a compromise few times. Thank you very much for ignoring it. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-280433054 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (16/02/17 00:57), Christian Heimes wrote: >You are missing the point. Obviously tests are an important part of building. >I can't test the client bits when ipatests is not available. > You missed that the name of this PR is "Client-only builds with --disable-server" So this PR *MUST* implement client-only build. Your use-case is different and you need to realize that freeIPA does not have a *unit* test for client bits. All client parts are tested by integration tests which require server. >Let's do small, incremental improvements. I need the client-only builds ASAP >(EOW, next week tops) for some container stuff. Client-only RPMs can be >implemented later in beta phase for 4.5. > I have never requested anything related to "Client-only RPMs" I always mentioned "make install" Let's do the correct change from semantic POV. If you want to install tests with client-only build then please add new configure time option for this purpose And do not misuse option "--disable-server" Misusing options is a bad/hacky approch and we need to clean hacks from freeIPA and not create new one. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-280277782 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (14/02/17 02:29), Christian Heimes wrote: >I'm following a different design and development philosophy. In my experience >an iterative approach with small, incremental improvements is often better and >faster than striving for 100% perfect PRs. Large and feature complete PRs take >more time than evolutionary steps. > I have never wrote anythig against this philosophy. All small chages can make sense from semantical point of view. Misussing names/options for different use-case just create a big mess and confuse other people. >Please review this PR under three viewpoints: > >* Does it contribute to resolving ticket >https://fedorahosted.org/freeipa/ticket/6517 ? client only build and --disable-server is the same thing (at least from "make install" POV) I have never required changes to spec file. >* Does it enable future changes to solve the ticket? If you will not install ipatests (if there is a way to not install ipatest) then it will enable future changes to solve the ticket. Because solving ticket6517 would be just writing right spec file. ATM it does not enable future changes to solve the ticket. >* Does it break any code or feature that is currently present? [1] Yes, it install server related options even though they should not be installed Summary: You should realize that the name of PR is "Client-only builds with --disable-server" and your use-case is not pure client only build. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279693909 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (13/02/17 05:05), Christian Heimes wrote: >I'm following the development principals of **minimum viable product**. This >PR solves a critical use case for me. With the PR I can build FreeIPA client >packages in a lean and clean build container. Without the >```--disable-server``` flag I'm forced to bloat my build env with lots of >additional dependencies and then throw away all the extra stuff. > My comments are about semantic of this option. `--disable-server` should disable all parts which depends on server. I know that your use case is a little bit different but I do not like misusing of `--disable-server` for different use-cases (from semantic POV) That's the reason why I proposed compromise/alternative solution for installing `ipatests` which needn't be tight together with `--disable-server`. >My changes don't solve https://fedorahosted.org/freeipa/ticket/6517 to its >full extend. The PR provides enough of >https://fedorahosted.org/freeipa/ticket/6517 to enable me to finish some time >critical as soon as possible. RPM packaging changes and ipatests improvements >for client-only builds can be implemented another time. I consider these >changes sugar coating (aka stretch goals). > One more time; it will be solved with my proposed change to `ipatests` + small tweak to spec file (due to python2/3 changes) That is exactly way how I tested it. A little bit hacky way but works for testing: https://paste.fedoraproject.org/556868/48699519 LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279405767 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (13/02/17 04:32), Christian Heimes wrote: >No, the test runner should either detect missing packages and skip tests >automatically, or should grow an option to load and execute client tests only. >It's a separate issue. > I have a different opinion. It is not a separate issue. For me, the name of configure option is crystall clear. It should not install anything related to daemon part; even thought it is test. Maybe we can add another option to install tests (--with-tests?? +default yes) It would work for your use-case and still allow old `CLIENT_ONLY` build (equivalent to 4.4) Or you can propose another compromise. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279381495 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (13/02/17 03:56), Christian Heimes wrote: >Two reasons > >1. ```make install``` >2. I need ipatests to be part of the build process in order to get a Python >package for tox later. > OK, thank you for explanation. Then we should install just tests from directory `ipatests` which does not require daemon for execution. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279368656 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (13/02/17 03:08), Christian Heimes wrote: >Packaging is a different issue. The PR does not provide RPM packaging for >client-only build. It merely implements configuration and building without >server components. > I mentioned old version of `CLIENT_ONLY` build because I consider it as a referential implementation. And `ipa tests` were not installed in 4.4 for client only build. >For client-only builds I need ipatests to run part of the test suite to verify >client code. Test suites ```test_ipapython, test_ipalib, test_pkcs10``` >without ```test_ipalib.test_rpc``` work without ```ipaserver```. > I expected a little bit more details. Do you need to run `make install` and then run tests in installed directory? Or how do you want to "run part of the test suite". Because if you needn't run "make install" for your use-case then my proposed patch would work. BTW `ipatests` will still be part of tarball and/or git. You can run them even thought they will not be installed with `make install` LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279362399 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (13/02/17 01:25), Christian Heimes wrote: >@lslebodn it works even better without your proposed changes. Parts >```ipatests``` work fine for ```--disable-server``` builds. I need the package >to run tests. > The old version (4.4) of `CLIENT_ONLY` build did not package ipatests. Could you describe a reason/use-case for installing `ipatests` without server? LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279349836 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ It works quite good with the following change ``` diff --git a/Makefile.am b/Makefile.am index 311f6121f..13e5a87b0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,13 +1,13 @@ ACLOCAL_AMFLAGS = -I m4 if ENABLE_SERVER -SERVER_SUBDIRS = daemons init install ipaserver +SERVER_SUBDIRS = daemons init install ipaserver ipatests else SERVER_SUBDIRS = endif IPACLIENT_SUBDIRS = ipaclient ipalib ipapython SUBDIRS = asn1 util client contrib po \ - $(IPACLIENT_SUBDIRS) ipaplatform ipatests $(SERVER_SUBDIRS) + $(IPACLIENT_SUBDIRS) ipaplatform $(SERVER_SUBDIRS) MOSTLYCLEANFILES = ipasetup.pyc ipasetup.pyo \ ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-279003293 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ On (10/02/17 06:11), Petr Vobornik wrote: >I still think that this use case needs to be documented in >http://www.freeipa.org/page/V4/Build_system_refactoring#How_to_Use . > >IMHO `make dist` can fail with --disable-server. Use case for `make dist` is >releasing and there is no point to do release without server bitw. But make >sure to document it and test that `make dist` works without it ;) . > Petr FYI: make dist should include all files into tarball even though configure was invoked with parameter --disable-server. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-278952140 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ See inline comment and issue above; otherwise LGTM. """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-278405529 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ `make dist` failed if configure was executed with `--disable-server` ``` make[3]: Leaving directory '/workdir/freeipa/po' make[2]: Leaving directory '/workdir/freeipa/po' (cd daemons && make top_distdir=../freeipa-4.4.90.dev201702081623+git9da17b545 distdir=../freeipa-4.4.90.dev201702081623+git9da17b545/daemons \ am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir) make[2]: Entering directory '/workdir/freeipa/daemons' make[2]: *** No rule to make target 'distdir'. Stop. make[2]: Leaving directory '/workdir/freeipa/daemons' make[1]: *** [Makefile:707: distdir] Error 1 make[1]: Leaving directory '/workdir/freeipa' make: *** [Makefile:806: dist] Error 2 ``` Do we care? """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-278377302 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#414][opened] SPEC: Update SELinux file context of ipa-otpd
URL: https://github.com/freeipa/freeipa/pull/414
Author: lslebodn
Title: #414: SPEC: Update SELinux file context of ipa-otpd
Action: opened
PR body:
"""
The ipa-otpd binary was moved but SELinux was not updated.
This is a workaround for developers who develop on platforms
with older SELinux policy.
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/414/head:pr414
git checkout pr414
From c41e4d7ca48d32661a9acc24a02ef7868f711705 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Sat, 28 Jan 2017 10:25:39 +0100
Subject: [PATCH] SPEC: Update SELinux file context of ipa-otpd
The ipa-otpd binary was moved but SELinux was not updated.
This is a workaround for developers who develop on platforms
with older SELinux policy.
---
freeipa.spec.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index a7e05f3..b044e2f 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -243,6 +243,7 @@ Requires(pre): systemd-units
Requires(post): systemd-units
Requires: selinux-policy >= %{selinux_policy_version}
Requires(post): selinux-policy-base >= %{selinux_policy_version}
+Requires(post): coreutils
Requires: slapi-nis >= %{slapi_nis_version}
Requires: pki-ca >= 10.3.5-6
Requires: pki-kra >= 10.3.5-6
@@ -933,6 +934,8 @@ fi
/bin/systemctl reload-or-try-restart dbus
/bin/systemctl reload-or-try-restart oddjobd
+%post server
+chcon system_u:object_r:ipa_otpd_exec_t:s0 /usr/libexec/ipa/ipa-otpd
%posttrans server
# don't execute upgrade and restart of IPA when server is not installed
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][comment] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389 Title: #389: Fix build in mock lslebodn commented: """ @tiran or @pvomacka, @tomaskrizek It would be good if you could test/ack the latest version of the patch. Because currently it is not possible to build freeIPA with upstream spec file in mock; which blocks reasonable testing with static analysers """ See the full comment at https://github.com/freeipa/freeipa/pull/389#issuecomment-273147279 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ I fine with ignoring python related parts; but it should be documented. But you might ask other freeIPA developers. (maybe on freeipa-devel) """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-273135814 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#364][comment] Client-only builds with --disable-server
URL: https://github.com/freeipa/freeipa/pull/364 Title: #364: Client-only builds with --disable-server lslebodn commented: """ I think it would be simpler to read the code without to many `AM_COND_IF()`. IMHO it would be simpler to move C-related part to separate file (e.g. `server_daemons.m4`) an conditionally include the file with `m4_include`. I checked it very briefly; I might miss something. How do you want to handle python server part? """ See the full comment at https://github.com/freeipa/freeipa/pull/364#issuecomment-273126212 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][comment] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389 Title: #389: Fix build in mock lslebodn commented: """ I updated doc help do jslint in latest version """ See the full comment at https://github.com/freeipa/freeipa/pull/389#issuecomment-271931555 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][synchronized] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389
Author: lslebodn
Title: #389: Fix build in mock
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/389/head:pr389
git checkout pr389
From 496c47a4549b327f28d8cd6466af3a520dc0797d Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 17:08:30 +0100
Subject: [PATCH 1/3] CONFIGURE: Fix detection of pylint
If configure script was executed with --enable-pylint then
it behaved the same as --disable-pylint. It does not make
any sense.
Resolves:
https://fedorahosted.org/freeipa/ticket/6604
---
configure.ac | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index e8a4701..c84c1bc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -446,16 +446,18 @@ AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
AS_HELP_STRING([--disable-pylint],
[skip Pylint in make lint target]),
- [PYLINT=no],
- [PYLINT=yes
- AC_MSG_CHECKING([for Pylint])
- $PYTHON -m pylint --version > /dev/null
- if test "$?" != "0"; then
- AC_MSG_ERROR([cannot find pylint for $PYTHON])
- fi
- AC_MSG_RESULT([yes])
- ]
+ [PYLINT=$enableval],
+ [PYLINT=yes]
)
+if test x$PYLINT != xno; then
+AC_MSG_CHECKING([for Pylint])
+$PYTHON -m pylint --version > /dev/null
+if test "$?" != "0"; then
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
+else
+AC_MSG_RESULT([yes])
+fi
+fi
AC_SUBST([PYLINT])
AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
From 1a4cbc1528b248ea6060e32f0f741ebd09b82e07 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 18:14:49 +0100
Subject: [PATCH 2/3] CONFIGURE: Update help message for jslint
Resolves:
https://fedorahosted.org/freeipa/ticket/6604
---
configure.ac | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index c84c1bc..6cd3a89 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,11 +463,12 @@ AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
AC_ARG_WITH([jslint],
- AS_HELP_STRING([--with-jslint=path to jsl],
- [path to JavaScript lint]),
+AS_HELP_STRING([--with-jslint=[FILE]],
+ [path to JavaScript linter. Default is autodetection of
+ utility "jsl" ]),
dnl --without-jslint will set JSLINT=no
- [JSLINT=$with_jslint],
- [AC_PATH_PROG([JSLINT], [jsl])]
+[JSLINT=$with_jslint],
+[AC_PATH_PROG([JSLINT], [jsl])]
)
if test "x${JSLINT}" == "x"; then
AC_MSG_ERROR([cannot find JS lint])
From eda6c2a147cb5c1927cefee7aa69b6b5a761ba83 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 15:02:09 +0100
Subject: [PATCH 3/3] SPEC: Fix build in mock
Neither pylint nor jsl is installed by default because rpm macro with_lint
is not defined in spec file. However, configure script tried to
find pylint/jsl anyway.
checking for Pylint... /usr/bin/python2: No module named pylint
configure: error: cannot find pylint for /usr/bin/python2
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
Resolves:
https://fedorahosted.org/freeipa/ticket/6604
---
freeipa.spec.in | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index c4420a0..99820d1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -10,6 +10,12 @@
# lint is not executed during rpmbuild
# %%global with_lint 1
+%if 0%{?with_lint}
+%global enable_pylint_option --enable-pylint
+%else
+%global enable_pylint_option --disable-pylint
+%global without_jslint_option --without-jslint
+%endif
%global alt_name ipa
%if 0%{?rhel}
@@ -778,7 +784,10 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python2}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
+
# -Onone is workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1398405
%make_build -Onone
@@ -793,7 +802,9 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python3}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
popd
%endif # with_python3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][synchronized] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389
Author: lslebodn
Title: #389: Fix build in mock
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/389/head:pr389
git checkout pr389
From c28b3c5f5ea7aedfe8d67143c569760b8d2d851a Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 17:08:30 +0100
Subject: [PATCH 1/2] BUILD: Fix detection of pylint
If configure script was executed with --enable-pylint then
it behaved the same as --disable-pylint. It does not make
any sense.
---
configure.ac | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index e8a4701..c84c1bc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -446,16 +446,18 @@ AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
AS_HELP_STRING([--disable-pylint],
[skip Pylint in make lint target]),
- [PYLINT=no],
- [PYLINT=yes
- AC_MSG_CHECKING([for Pylint])
- $PYTHON -m pylint --version > /dev/null
- if test "$?" != "0"; then
- AC_MSG_ERROR([cannot find pylint for $PYTHON])
- fi
- AC_MSG_RESULT([yes])
- ]
+ [PYLINT=$enableval],
+ [PYLINT=yes]
)
+if test x$PYLINT != xno; then
+AC_MSG_CHECKING([for Pylint])
+$PYTHON -m pylint --version > /dev/null
+if test "$?" != "0"; then
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
+else
+AC_MSG_RESULT([yes])
+fi
+fi
AC_SUBST([PYLINT])
AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
From d1c0d0e777b504cbc33e0db09b2c3e0c66cf0846 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 15:02:09 +0100
Subject: [PATCH 2/2] SPEC: Fix build in mock
Neither pylint nor jsl is installed by default because rpm macro with_lint
is not defined in spec file. However, configure script tried to
find pylint/jsl anyway.
checking for Pylint... /usr/bin/python2: No module named pylint
configure: error: cannot find pylint for /usr/bin/python2
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
---
freeipa.spec.in | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index c4420a0..99820d1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -10,6 +10,12 @@
# lint is not executed during rpmbuild
# %%global with_lint 1
+%if 0%{?with_lint}
+%global enable_pylint_option --enable-pylint
+%else
+%global enable_pylint_option --disable-pylint
+%global without_jslint_option --without-jslint
+%endif
%global alt_name ipa
%if 0%{?rhel}
@@ -778,7 +784,10 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python2}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
+
# -Onone is workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1398405
%make_build -Onone
@@ -793,7 +802,9 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python3}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
popd
%endif # with_python3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][synchronized] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389
Author: lslebodn
Title: #389: Fix build in mock
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/389/head:pr389
git checkout pr389
From b847c8f98655d6b6099b47052aa89c279929bf29 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 17:08:30 +0100
Subject: [PATCH 1/2] BUILD: Fix detection of pylint
If configure script was executed with --enable-pylint then
it behaved the same as --disable-pylint. It does not make
any sense.
---
configure.ac | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index e8a4701..c706018 100644
--- a/configure.ac
+++ b/configure.ac
@@ -446,16 +446,18 @@ AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
AS_HELP_STRING([--disable-pylint],
[skip Pylint in make lint target]),
- [PYLINT=no],
- [PYLINT=yes
- AC_MSG_CHECKING([for Pylint])
- $PYTHON -m pylint --version > /dev/null
- if test "$?" != "0"; then
- AC_MSG_ERROR([cannot find pylint for $PYTHON])
- fi
- AC_MSG_RESULT([yes])
- ]
+ [PYLINT=$enableval],
+ [PYLINT=no]
)
+if test x$PYLINT != no; then
+AC_MSG_CHECKING([for Pylint])
+$PYTHON -m pylint --version > /dev/null
+if test "$?" != "0"; then
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
+else
+AC_MSG_RESULT([yes])
+fi
+fi
AC_SUBST([PYLINT])
AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
From 4f62f8d77cd0a5ac16bbdbbc86103f231a1ca343 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 15:02:09 +0100
Subject: [PATCH 2/2] SPEC: Fix build in mock
Neither pylint nor jsl is installed by default because rpm macro with_lint
is not defined in spec file. However, configure script tried to
find pylint/jsl anyway.
checking for Pylint... /usr/bin/python2: No module named pylint
configure: error: cannot find pylint for /usr/bin/python2
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
---
configure.ac| 2 +-
freeipa.spec.in | 15 +--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index c706018..1616f31 100644
--- a/configure.ac
+++ b/configure.ac
@@ -447,7 +447,7 @@ AC_ARG_ENABLE([pylint],
AS_HELP_STRING([--disable-pylint],
[skip Pylint in make lint target]),
[PYLINT=$enableval],
- [PYLINT=no]
+ [PYLINT=yes]
)
if test x$PYLINT != no; then
AC_MSG_CHECKING([for Pylint])
diff --git a/freeipa.spec.in b/freeipa.spec.in
index c4420a0..99820d1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -10,6 +10,12 @@
# lint is not executed during rpmbuild
# %%global with_lint 1
+%if 0%{?with_lint}
+%global enable_pylint_option --enable-pylint
+%else
+%global enable_pylint_option --disable-pylint
+%global without_jslint_option --without-jslint
+%endif
%global alt_name ipa
%if 0%{?rhel}
@@ -778,7 +784,10 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python2}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
+
# -Onone is workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1398405
%make_build -Onone
@@ -793,7 +802,9 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python3}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
popd
%endif # with_python3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#389][opened] Fix build in mock
URL: https://github.com/freeipa/freeipa/pull/389
Author: lslebodn
Title: #389: Fix build in mock
Action: opened
PR body:
"""
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/389/head:pr389
git checkout pr389
From b847c8f98655d6b6099b47052aa89c279929bf29 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 17:08:30 +0100
Subject: [PATCH 1/2] BUILD: Fix detection of pylint
If configure script was executed with --enable-pylint then
it behaved the same as --disable-pylint. It does not make
any sense.
---
configure.ac | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index e8a4701..c706018 100644
--- a/configure.ac
+++ b/configure.ac
@@ -446,16 +446,18 @@ AM_CONDITIONAL([WITH_POLINT], [test "x${enable_i18ntests}" == "xyes"])
AC_ARG_ENABLE([pylint],
AS_HELP_STRING([--disable-pylint],
[skip Pylint in make lint target]),
- [PYLINT=no],
- [PYLINT=yes
- AC_MSG_CHECKING([for Pylint])
- $PYTHON -m pylint --version > /dev/null
- if test "$?" != "0"; then
- AC_MSG_ERROR([cannot find pylint for $PYTHON])
- fi
- AC_MSG_RESULT([yes])
- ]
+ [PYLINT=$enableval],
+ [PYLINT=no]
)
+if test x$PYLINT != no; then
+AC_MSG_CHECKING([for Pylint])
+$PYTHON -m pylint --version > /dev/null
+if test "$?" != "0"; then
+AC_MSG_ERROR([cannot find pylint for $PYTHON])
+else
+AC_MSG_RESULT([yes])
+fi
+fi
AC_SUBST([PYLINT])
AM_CONDITIONAL([WITH_PYLINT], [test "x${PYLINT}" != "xno"])
From 19a0a3b8c5e4855d047d098262c1a44a76ea5ebf Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Wed, 11 Jan 2017 15:02:09 +0100
Subject: [PATCH 2/2] SPEC: Fix build in mock
Neither pylint nor jsl is installed by default because rpm macro with_lint
is not defined in spec file. However, configure script tried to
find pylint/jsl anyway.
checking for Pylint... /usr/bin/python2: No module named pylint
configure: error: cannot find pylint for /usr/bin/python2
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
Bad exit status from /var/tmp/rpm-tmp.2GAFh4 (%build)
---
freeipa.spec.in | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index c4420a0..99820d1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -10,6 +10,12 @@
# lint is not executed during rpmbuild
# %%global with_lint 1
+%if 0%{?with_lint}
+%global enable_pylint_option --enable-pylint
+%else
+%global enable_pylint_option --disable-pylint
+%global without_jslint_option --without-jslint
+%endif
%global alt_name ipa
%if 0%{?rhel}
@@ -778,7 +784,10 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python2}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
+
# -Onone is workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1398405
%make_build -Onone
@@ -793,7 +802,9 @@ find \
! -name '*.pyo' -a \
-type f -exec grep -qsm1 '^#!.*\bpython' {} \; \
-exec sed -i -e '1 s|^#!.*\bpython[^ ]*|#!%{__python3}|' {} \;
-%configure --with-vendor-suffix=-%{release}
+%configure --with-vendor-suffix=-%{release} \
+ %{enable_pylint_option} \
+ %{?without_jslint_option}
popd
%endif # with_python3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#238][comment] Build system refactoring phase 8: update translation system
URL: https://github.com/freeipa/freeipa/pull/238 Title: #238: Build system refactoring phase 8: update translation system lslebodn commented: """ >Your patch adds `config.rpath`. Is it necessary to include the file in source >control? certmonger and sssd use the file but don't have it in git. Following patch fixes this issue http://paste.fedoraproject.org/484212/47049414/ """ See the full comment at https://github.com/freeipa/freeipa/pull/238#issuecomment-261516489 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#236][comment] Build phase 7: cleanup
URL: https://github.com/freeipa/freeipa/pull/236 Title: #236: Build phase 7: cleanup lslebodn commented: """ Thank you very much for removing problematic patch. Do not forget to remove it from "Build phase 8" :-) as well A) there are few conflicts => patch need to be rebased B) I do not think that @tiran objection about rpm in makerpms.sh is valid but he should reply. Otherwise ACK after rebase Nice work and thank you very much for it. BTW is there a plan to have "make dictcheck" functional? """ See the full comment at https://github.com/freeipa/freeipa/pull/236#issuecomment-260704741 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#238][comment] Build system refactoring phase 8: update translation system
URL: https://github.com/freeipa/freeipa/pull/238 Title: #238: Build system refactoring phase 8: update translation system lslebodn commented: """ The following files should be removed from git `m4/gettext.m4 m4/iconv.m4 m4/lib-ld.m4 m4/lib-link.m4 m4/lib-prefix.m4 m4/nls.m4 m4/po.m4`. The latest versions should be used by autoreconf after installing build dependency `gettext-devel`. Otherwise it's possible that tarball might contain outdated versions in future. """ See the full comment at https://github.com/freeipa/freeipa/pull/238#issuecomment-260663677 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#238][comment] Build system refactoring phase 8: update translation system
URL: https://github.com/freeipa/freeipa/pull/238 Title: #238: Build system refactoring phase 8: update translation system lslebodn commented: """ and also ``` Making install in po make[1]: Entering directory '/home/user/freeipa/po' make[1]: *** No rule to make target 'install'. Stop. make[1]: Leaving directory '/home/user/freeipa/po' make: *** [Makefile:595: install-recursive] Error 1 ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/238#issuecomment-260662849 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#236][comment] Build phase 7: cleanup
URL: https://github.com/freeipa/freeipa/pull/236 Title: #236: Build phase 7: cleanup lslebodn commented: """ >Hi Lukas. Given there is no technical justification to have it I'm going to >remove these. Simple is better than complex. I am sorry I do not agree. The technical justification was explained in previous comments few time. The $(NULL) at the end of list makes patches much simpler and easier to read. The purpose of refactoring is make code simpler but also easier to **maintain/review** """ See the full comment at https://github.com/freeipa/freeipa/pull/236#issuecomment-260289586 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#236][comment] Build phase 7: cleanup
URL: https://github.com/freeipa/freeipa/pull/236 Title: #236: Build phase 7: cleanup lslebodn commented: """ You should the opposite of the last patch "Build: clean-up spurious NULL variables from Makefile.am files". The NULL should be added to each list. NACK to the last patch. It does not simplify anything and it makes diff more complicated for adding new entries """ See the full comment at https://github.com/freeipa/freeipa/pull/236#issuecomment-259989321 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#208][comment] Tests: Fix integration sudo test
URL: https://github.com/freeipa/freeipa/pull/208 Title: #208: Tests: Fix integration sudo test lslebodn commented: """ All versions of Fedora have sudo 1.8.18. And thank you very much for nice/verbose explanation in commit message ACK """ See the full comment at https://github.com/freeipa/freeipa/pull/208#issuecomment-258096257 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ On (20/10/16 10:29), Petr Špaček wrote: >@lslebodn I'm really trying to explain this but I'm still not able to get the >point across. >> My concerns are related purely to C-code. > >Please understand that IPA client consists of components in C as well from >components in Python, and also non-programatic components like translation >machinery etc. > Correct me If I am wrong. The configure script is and will be used just for detection of build dependencies for C-code. So here I cannot see a conflict with my proposal. >We certainly can create m4 include maze I cannot see a reason why it would be a maze Detection of client build dependencies will be done in main configure.ac Detection of server dependencies would be done in `daemons/configure.ac` `./ipatests/man/configure.ac` and `./install/configure.ac` does not have any C-related dependencies and `./asn1/configure.ac` is required by client code. IMHO ans1c/ can be moved to client/asn1c/ (but that's offtopic) There would not be much includes as I showed in POC >and force maintainer to always use `grep` before he finds particular part in >the build system. maintainers do not use grep for finding build dependencies. The most common use case is just to run configure script and wait for reported errors. pkg-config returns nice error messages. And then add new build dependency. However, C related code is not changed very often and even more C-related dependencies are added much less often. But if new depenendecy will be added to server part then it will be much simpler to review whether build dependency is added to the right section if server dependencies will be in separate file. >Unfortunatlly, even the m4 maze will not solve the problem that client-only >build of C binaries simply do not constitute working IPA client. >Integration with other Python components is necessary to get the client to >work. > Totally agree. But python components is not related to checking of build time dependencies for C-code. It should be solved in different PR IMHO, this PR should not complicate client-only build just for C-binaries. >The end goal is to fold all of hand-made Makefile and SPEC file scripts to the >build system, so in the end, it should help with porting to other arches - >there will be just one place where changes need to be done, instead of three. > Agree, but here I cannot see any conflict with my proposal. >I hope that it clears up why it is not useful to insist on keeping current >pieces as they were before. The design document was sent to freeipa-devel >mailing list in this thread: >https://www.redhat.com/archives/freeipa-devel/2016-October/msg00134.html >Please discuss conceptual questions on the mailing list so we can get >attention of other FreeIPA developers and avoid need to point people one by >one to this PR. > I read the desing page and there is mentioned that autotools will be used for C-part as an implementation and configure script should have the option --enable-server which default yes. I could not find any contradiction between my proposal and desing page. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-256120734 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ We (lslebodn and pspacek) agree that we disagree about maintainability of client_only in one monolithic configure.ac. But we agreed that the refactoring of build system[1] is more important. I will close my eyes and you can push the patch-set. :-) [1] http://www.freeipa.org/page/V4/Build_system_refactoring """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-255404139 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ On (21/10/16 00:41), Petr Špaček wrote: >> Correct me If I am wrong. The configure script is and will be used >just for detection of build dependencies for C-code. > >Maybe this is why we do not understand each other. Autotools will orchestrate >the complete build including configuration of Python (and other) components. >This is what I meant by `Use autotools suite to orchestrate the build on the >top-level` in the design document. If it is confusing I'm happy to improve it, >just tell me what you want to see there. > I cannot see a problem here. Design page just say that you will be able to optionally build and install python2 and/or python3. And setuptools instead of distutils will manage that part. I cannot see any conflict with detecting build time dependencies for C-code with enabled server and without enabled server (client_only) >This PR starts to use configure to create symlinks in ipaplatform Python >module and more things are comming. If you look at >http://www.freeipa.org/page/V4/Build_system_refactoring#Configuration you will >see that configure is going contain switches for Python components as well as >for Javascript components. I.e. there is nothing like C-only configure >anymore. > Design page says something else. `./configure --without-python2 --without-python3 --without-pylint --without-jslint` . But I do not care about these options. My intention is to have a simple way how to implement "--enable-server/--disable-server". Merging everything to the one big configure script will not simplify it. It will just complicate it. And the purpose of refactoring is to make things simpler and easily maintainable. """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-255320295 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ On (20/10/16 10:29), Petr Špaček wrote: >@lslebodn I'm really trying to explain this but I'm still not able to get the >point across. >> My concerns are related purely to C-code. > >Please understand that IPA client consists of components in C as well from >components in Python, and also non-programatic components like translation >machinery etc. Correct me If I am wrong. The configure script is and will be used just for detection of build dependencies for C-code. So here I cannot see a conflict with my proposal. >We certainly can create m4 include maze I cannot see a reason why it would be a maze Detection of client build dependencies will be done in main configure.ac Detection of server dependencies would be done in `daemons/configure.ac` `./ipatests/man/configure.ac` and `./install/configure.ac` does not have any C-related dependencies and `./asn1/configure.ac` is required by client code. IMHO ans1c/ can be moved to client/asn1c/ (but that's offtopic) There would not be much includes as I showed in POC >and force maintainer to always use `grep` before he finds particular part in >the build system. maintainers do not use grep for finding build dependencies. The most common use case is just to run configure script and wait for reported errors. pkg-config returns nice error messages. And then add new build dependency. However, C related code is not changed very often and even more C-related dependencies are added much less often. But if new depenendecy will be added to server part then it will be much simpler to review whether build dependency is added to the right section if server dependencies will be in separate file. >Unfortunatlly, even the m4 maze will not solve the problem that client-only >build of C binaries simply do not constitute working IPA client. >Integration with other Python components is necessary to get the client to >work. Totally agree. But python components is not related to checking of build time dependencies for C-code. It should be solved in different PR IMHO, this PR should not complicate client-only build just for C-binaries. >The end goal is to fold all of hand-made Makefile and SPEC file scripts to the >build system, so in the end, it should help with porting to other arches - >there will be just one place +where changes need to be done, instead of three. Agree, but here I cannot see any conflict with my proposal. >I hope that it clears up why it is not useful to insist on keeping current >pieces as they were before. The design document was sent to freeipa-devel >mailing list in this thread: >https://www.redhat.com/archives/freeipa-devel/2016-October/msg00134.html >Please discuss conceptual questions on the mailing list so we can get >attention of other FreeIPA developers and avoid need to point people one by >one to this PR. I read the desing page and there is mentioned that autotools will be used for C-part as an implementation and configure script should have the option --enable-server which default yes. I could not find any contradiction between my proposal and desing page. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-255313933 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ On (20/10/16 10:29), Petr Špaček wrote: >@lslebodn I'm really trying to explain this but I'm still not able to get the >point across. >> My concerns are related purely to C-code. > >Please understand that IPA client consists of components in C as well from >components in Python, and also non-programatic components like translation >machinery etc. > Correct me If I am wrong. The configure script is and will be used just for detection of build dependencies for C-code. So here I cannot see a conflict with my proposal. >We certainly can create m4 include maze I cannot see a reason why it would be a maze Detection of client build dependencies will be done in main configure.ac Detection of server dependencies would be done in `daemons/configure.ac` `./ipatests/man/configure.ac` and `./install/configure.ac` does not have any C-related dependencies and `./asn1/configure.ac` is required by client code. IMHO ans1c/ can be moved to client/asn1c/ (but that's offtopic) There would not be much includes as I showed in POC >and force maintainer to always use `grep` before he finds particular part in >the build system. maintainers do not use grep for finding build dependencies. The most common use case is just to run configure script and wait for reported errors. pkg-config returns nice error messages. And then add new build dependency. However, C related code is not changed very often and even more C-related dependencies are added much less often. But if new depenendecy will be added to server part then it will be much simpler to review whether build dependency is added to the right section if server dependencies will be in separate file. >Unfortunatlly, even the m4 maze will not solve the problem that client-only >build of C binaries simply do not constitute working IPA client. >Integration with other Python components is necessary to get the client to >work. > Totally agree. But python components is not related to checking of build time dependencies for C-code. It should be solved in different PR IMHO, this PR should not complicate client-only build just for C-binaries. >The end goal is to fold all of hand-made Makefile and SPEC file scripts to the >build system, so in the end, it should help with porting to other arches - >there will be just one place +where changes need to be done, instead of three. > Agree, but here I cannot see any conflict with my proposal. >I hope that it clears up why it is not useful to insist on keeping current >pieces as they were before. The design document was sent to freeipa-devel >mailing list in this thread: >https://www.redhat.com/archives/freeipa-devel/2016-October/msg00134.html >Please discuss conceptual questions on the mailing list so we can get >attention of other FreeIPA developers and avoid need to point people one by >one to this PR. > I read the desing page and there is mentioned that autotools will be used for C-part as an implementation and configure script should have the option --enable-server which default yes. I could not find any contradiction between my proposal and desing page. LS """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-255313933 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171
Title: #171: Build system cleanup phase 2
lslebodn commented:
"""
> 08:53 < pspacek> lslebodn: I would appreciate patch showing the nicer
> solution to me. I do not think it
> is that easy as you claim, especially when we count in that
> client_only build depends heavily on spec file, which is
> not usable in Debian at all, which makes
> client-only-purely-upstream-point completely moot.
Currently it is possible to build just a client part with following steps. Ant
it has nothing to do with downstream specfile
```
make version-update
cd client; ../autogen.sh --prefix=%{_usr} --sysconfdir=%{_sysconfdir}
--localstatedir=%{_localstatedir} --libdir=%{_libdir} --mandir=%{_mandir}; cd ..
make client-install
```
As you can see configure script is executed only in client sub-directory.
Here I am not interested in python part of client code.
My concerns are pure related to C-code.
But after merging everything to the one big configure script it would not be
possible
without patching a master.
And you also wanted to see some patches with nicer solution
here is a POC:
```
AC_ARG_WITH([server],
[AC_HELP_STRING([--with-server],
[Whether to build with server support [yes]]
)
],
[],
with_server=yes
)
if test x"$with_server" = xyes; then
AC_SUBST(HAVE_SERVER)
AC_DEFINE_UNQUOTED(HAVE_SERVER, 1, [Build with server support])
m4_include([./install/configure.ac.inc]]
m4_include([./daemons/configure.ac.inc]]
m4_include([./asn1/configure.acac.inc]]
fi
m4_include([./client/configure.ac])
AM_CONDITIONAL([BUILD_SERVER], [test x"$with_server" = xyes])
```
Or If you you can merge configure part into main configure script rather then
`m4_include([./client/configure.ac])`
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/171#issuecomment-255108446
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171
Title: #171: Build system cleanup phase 2
lslebodn commented:
"""
> 08:53 < pspacek> lslebodn: I would appreciate patch showing the nicer
> solution to me. I do not think it
> is that easy as you claim, especially when we count in that
> client_only build depends heavily on spec file, which is
> not usable in Debian at all, which makes
> client-only-purely-upstream-point completely moot.
Currently it is possible to build just a client part with following steps. Ant
it has nothing to do with downstream specfile
```
make version-update
cd client; ../autogen.sh --prefix=%{_usr} --sysconfdir=%{_sysconfdir}
--localstatedir=%{_localstatedir} --libdir=%{_libdir} --mandir=%{_mandir}; cd ..
make client-install
```
As you can see configure script is executed only in client sub-directory.
Here I am not interested in python part of client code.
My concerns are pure related to C-code.
But after merging everything to the one big configure script it would not be
possible
without patching a master.
And you also wanted to see some patches with nicer solution
here is a POC:
```
AC_ARG_WITH([server],
[AC_HELP_STRING([--with-server],
[Whether to build with server support [yes]]
)
],
[],
with_server=yes
)
if test x"$with_server" = xyes; then
AC_SUBST(HAVE_SERVER)
AC_DEFINE_UNQUOTED(HAVE_SERVER, 1, [Build with server support])
m4_include([./install/configure.ac.inc]]
m4_include([./daemons/configure.ac.inc]]
m4_include([./asn1/configure.acac.inc]]
fi
m4_include([./client/configure.ac])
AM_CONDITIONAL([BUILD_SERVER], [test x"$with_server" = xyes])
```
Or If you you can merge configure part into main configure script rather then
`m4_include([./client/configure.ac])`
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/171#issuecomment-255108446
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171
Title: #171: Build system cleanup phase 2
lslebodn commented:
"""
> 08:53 < pspacek> lslebodn: I would appreciate patch showing the nicer
> solution to me. I do not think it
> is that easy as you claim, especially when we count in that
> client_only build depends heavily on spec file, which is
> not usable in Debian at all, which makes
> client-only-purely-upstream-point completely moot.
Currently it is possible to build just a client part with following steps. Ant
it has nothing to do with downstream specfile
```
make version-update
cd client; ../autogen.sh --prefix=%{_usr} --sysconfdir=%{_sysconfdir}
--localstatedir=%{_localstatedir} --libdir=%{_libdir} --mandir=%{_mandir}; cd ..
make client-install
```
As you can see configure script is executed only in client sub-directory.
Here I am not interested in python part of client code.
My concerns are pure related to C-code.
But after merging everything to the one big configure script it would not be
possible
without patching a master.
And you also wanted to see some patches with nicer solution
here is a POC:
```
AC_ARG_WITH([server],
[AC_HELP_STRING([--with-server],
[Whether to build with server support [yes]]
)
],
[],
with_server=yes
)
if test x"$with_server" = xyes; then
AC_SUBST(HAVE_SERVER)
AC_DEFINE_UNQUOTED(HAVE_SERVER, 1, [Build with server support])
m4_include([./install/configure.ac.inc]]
m4_include([./daemons/configure.ac.inc]]
m4_include([./asn1/configure.acac.inc]]
fi
m4_include([./client/configure.ac])
AM_CONDITIONAL([BUILD_SERVER], [test x"$with_server" = xyes])
```
Or If you you can merge configure part into main configure script rather then
`m4_include([./client/configure.ac])`
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/171#issuecomment-255108446
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#171][comment] Build system cleanup phase 2
URL: https://github.com/freeipa/freeipa/pull/171 Title: #171: Build system cleanup phase 2 lslebodn commented: """ > Please note that current client-only build is quite broken and requires heavy > machinery in downstream spec file so it is hard to speak about any reusage. That's the problem of ugly downstream spec file. This is a pure upstream discussion and how to make at least part of freeIPA more portable rather then closing doors for other platforms/distributions without systemd. There are still people on debian which does not use systemd and want to use sssd and ipa-client. The current patch-set would not create more portable client code. It would would just make it much harder and would require more downstream patches which is not very upstream friendly approach. @pspacek, it would be good if you could comment my proposal with including configure snippets from subdirectories. `m4_include` works like a magic and if conditions around them would make much nicer configure script. """ See the full comment at https://github.com/freeipa/freeipa/pull/171#issuecomment-255043715 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
