[SSSD] Re: [PATCH] SPEC: Remove unnecessary requirements
On (21/01/16 17:30), Sumit Bose wrote:
>On Thu, Jan 21, 2016 at 05:09:52PM +0100, Lukas Slebodnik wrote:
>> ehlo,
>>
>> reason is explained in commit message.
>> The intention for this patch was to a simplify spec file
>> (at least a little bit :-)
>>
>> If we want to have some requirements for version of lib{ldb,tdb}
>> then it would be better to have minimal required version in
>> BuildRequires + configure detection of libraries.
>> But it might be outdated if we use some ver ldb features.
>>
>> LS
>
>> From 275b260be7c4365668a7adeceee6e9e089ae2f42 Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik
>> Date: Thu, 21 Jan 2016 16:55:11 +0100
>> Subject: [PATCH] SPEC: Remove unnecessary requirements
>>
>> We do not need to requires specific version of libldb
>> or libtdb because it is automatically detected from
>> binary/library dependencies. We also need never version
>> of that libraries as it was specified in spec file.
>>
>> e.g.
>> sh$ rpm -q --requires sssd-common | grep -E "TDB|LDB"
>> libldb.so.1(LDB_0.9.10)(64bit)
>> libtdb.so.1(TDB_1.2.1)(64bit)
>>
>> Our public libraries also provides version definitions
>> therefore rpm can also detect minimal required version.
>>
>> sh$ rpm -q --requires sssd-common | grep -E "IDMAP"
>> libsss_idmap.so.0(SSS_IDMAP_0.4)(64bit)
>> sh$ $rpm -q --requires python-libsss_nss_idmap | grep -E "IDMAP"
>> libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.0.1)(64bit)
>> sh$ rpm -q --requires sssd-ipa | grep -E "HBAC"
>> libipa_hbac.so.0(IPA_HBAC_0.0.1)(64bit)
>
>I agree with the ldb and tbd changes.
>
>But iirc the other explicit version dependencies should take care that
>the sssd packages are always install with the same version. E.g. if you
>have a system with
>
>libsss_nss_idmap-1.13.3-1.fc23.x86_64 and
>python3-libsss_nss_idmap-1.13.3-1.fc23.x86_64
>
>installed and call
>
>dnf update python3-libsss_nss_idmap-1.13.3-1.fc23.x86_64
>
>becasue there is a newer 1.13.3-2 SSSD release, I would expect that dnf
>only updates python3-libsss_nss_idmap to 1.13.3-2 if you remove
>"Requires: libsss_nss_idmap = %{version}-%{release}" form the package
>definition because the library version dependency
>"libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.0.1)" is still satisfied by
>libsss_nss_idmap-1.13.3-1.fc23.x86_64. As a result you have
>
>libsss_nss_idmap-1.13.3-1.fc23.x86_64 and
>python3-libsss_nss_idmap-1.13.3-2.fc23.x86_64
>
You might looks like irritating but if you want to update just one specific
package then it will safe a badwith and speed-up process.
But I do not insist on the change.
Meanwhile I found a redundant dependency on sssd-common-pac.
sssd -> sssd-ipa -> sssd-common-pac
-> sssd-ad -> sssd-common-pac
-> sssd-common-pac
I want to remove as much as possible :-)
Updated version is attached.
LS
>From 870eba7bfd10b726fce9162afe2484f01c4bcd90 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Thu, 21 Jan 2016 16:55:11 +0100
Subject: [PATCH] SPEC: Remove unnecessary requirements
We do not need to requires specific version of libldb
or libtdb because it is automatically detected from
binary/library dependencies. We also need never version
of that libraries as it was specified in spec file.
e.g.
sh$ rpm -q --requires sssd-common | grep -E "TDB|LDB"
libldb.so.1(LDB_0.9.10)(64bit)
libtdb.so.1(TDB_1.2.1)(64bit)
There is also redundant dependency on sssd-common-pac
sssd -> sssd-ipa -> sssd-common-pac
-> sssd-ad -> sssd-common-pac
-> sssd-common-pac
sh$ rpm -q --whatrequires sssd-common-pac
sssd-ipa-1.13.3-1.fc23.x86_64
sssd-ad-1.13.3-1.fc23.x86_64
sssd-1.13.3-1.fc23.x86_64
---
contrib/sssd.spec.in | 3 ---
1 file changed, 3 deletions(-)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index
76066de9ac8b8ac1f9b14bf3bc787169d317fe2f..73bc4b7a62c9bb43a4d1637f3d2d7e8dc85ac83e
100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -81,7 +81,6 @@ Requires: sssd-common = %{version}-%{release}
Requires: sssd-ldap = %{version}-%{release}
Requires: sssd-krb5 = %{version}-%{release}
Requires: sssd-ipa = %{version}-%{release}
-Requires: sssd-common-pac = %{version}-%{release}
Requires: sssd-ad = %{version}-%{release}
Requires: sssd-proxy = %{version}-%{release}
%if (0%{?with_python3} == 1)
@@ -177,8 +176,6 @@ the existing back ends.
Summary: Common files for the SSSD
Group: Applications/System
License: GPLv3+
-Requires: libldb >= 0.9.3
-Requires: libtdb >= 1.1.3
Requires: sssd-client%{?_isa} = %{version}-%{release}
Requires: libsss_sudo = %{version}-%{release}
Requires: libsss_idmap = %{version}-%{release}
--
2.5.0
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] SPEC: Remove unnecessary requirements
On Thu, Jan 21, 2016 at 05:09:52PM +0100, Lukas Slebodnik wrote:
> ehlo,
>
> reason is explained in commit message.
> The intention for this patch was to a simplify spec file
> (at least a little bit :-)
>
> If we want to have some requirements for version of lib{ldb,tdb}
> then it would be better to have minimal required version in
> BuildRequires + configure detection of libraries.
> But it might be outdated if we use some ver ldb features.
>
> LS
> From 275b260be7c4365668a7adeceee6e9e089ae2f42 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik
> Date: Thu, 21 Jan 2016 16:55:11 +0100
> Subject: [PATCH] SPEC: Remove unnecessary requirements
>
> We do not need to requires specific version of libldb
> or libtdb because it is automatically detected from
> binary/library dependencies. We also need never version
> of that libraries as it was specified in spec file.
>
> e.g.
> sh$ rpm -q --requires sssd-common | grep -E "TDB|LDB"
> libldb.so.1(LDB_0.9.10)(64bit)
> libtdb.so.1(TDB_1.2.1)(64bit)
>
> Our public libraries also provides version definitions
> therefore rpm can also detect minimal required version.
>
> sh$ rpm -q --requires sssd-common | grep -E "IDMAP"
> libsss_idmap.so.0(SSS_IDMAP_0.4)(64bit)
> sh$ $rpm -q --requires python-libsss_nss_idmap | grep -E "IDMAP"
> libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.0.1)(64bit)
> sh$ rpm -q --requires sssd-ipa | grep -E "HBAC"
> libipa_hbac.so.0(IPA_HBAC_0.0.1)(64bit)
I agree with the ldb and tbd changes.
But iirc the other explicit version dependencies should take care that
the sssd packages are always install with the same version. E.g. if you
have a system with
libsss_nss_idmap-1.13.3-1.fc23.x86_64 and
python3-libsss_nss_idmap-1.13.3-1.fc23.x86_64
installed and call
dnf update python3-libsss_nss_idmap-1.13.3-1.fc23.x86_64
becasue there is a newer 1.13.3-2 SSSD release, I would expect that dnf
only updates python3-libsss_nss_idmap to 1.13.3-2 if you remove
"Requires: libsss_nss_idmap = %{version}-%{release}" form the package
definition because the library version dependency
"libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.0.1)" is still satisfied by
libsss_nss_idmap-1.13.3-1.fc23.x86_64. As a result you have
libsss_nss_idmap-1.13.3-1.fc23.x86_64 and
python3-libsss_nss_idmap-1.13.3-2.fc23.x86_64
installed. This should not cause any harm but looks a bit irritating
imo.
bye,
Sumit
> ---
> contrib/sssd.spec.in | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> index
> 76066de9ac8b8ac1f9b14bf3bc787169d317fe2f..44e57600797e4e98cc241d24f5f7264906dda9be
> 100644
> --- a/contrib/sssd.spec.in
> +++ b/contrib/sssd.spec.in
> @@ -177,11 +177,8 @@ the existing back ends.
> Summary: Common files for the SSSD
> Group: Applications/System
> License: GPLv3+
> -Requires: libldb >= 0.9.3
> -Requires: libtdb >= 1.1.3
> Requires: sssd-client%{?_isa} = %{version}-%{release}
> Requires: libsss_sudo = %{version}-%{release}
> -Requires: libsss_idmap = %{version}-%{release}
> Conflicts: sssd < %{version}-%{release}
> %if (0%{?use_systemd} == 1)
> Requires(post): systemd-units systemd-sysv
> @@ -369,7 +366,6 @@ License: GPLv3+
> Conflicts: sssd < %{version}-%{release}
> Requires: sssd-common = %{version}-%{release}
> Requires: sssd-krb5-common = %{version}-%{release}
> -Requires: libipa_hbac = %{version}-%{release}
> Requires: bind-utils
> Requires: sssd-common-pac = %{version}-%{release}
>
> @@ -444,7 +440,6 @@ Utility library to validate FreeIPA HBAC rules for
> authorization requests
> Summary: Python2 bindings for the FreeIPA HBAC Evaluator library
> Group: Development/Libraries
> License: LGPLv3+
> -Requires: libipa_hbac = %{version}-%{release}
> Provides: libipa_hbac-python = %{version}-%{release}
> Obsoletes: libipa_hbac-python < 1.12.90
>
> @@ -457,7 +452,6 @@ used by Python applications.
> Summary: Python3 bindings for the FreeIPA HBAC Evaluator library
> Group: Development/Libraries
> License: LGPLv3+
> -Requires: libipa_hbac = %{version}-%{release}
>
> %description -n python3-libipa_hbac
> The python3-libipa_hbac contains the bindings so that libipa_hbac can be
> @@ -487,7 +481,6 @@ Utility library for SID based lookups
> Summary: Python2 bindings for libsss_nss_idmap
> Group: Development/Libraries
> License: LGPLv3+
> -Requires: libsss_nss_idmap = %{version}-%{release}
> Provides: libsss_nss_idmap-python = %{version}-%{release}
> Obsoletes: libsss_nss_idmap-python < 1.12.90
>
> @@ -500,7 +493,6 @@ be used by Python applications.
> Summary: Python3 bindings for libsss_nss_idmap
> Group: Development/Libraries
> License: LGPLv3+
> -Requires: libsss_nss_idmap = %{version}-%{release}
>
> %description -n python3-libsss_nss_idmap
> The python3-libsss_nss_idmap contains the bindings so that libsss_nss_idmap
> can
> --
> 2.5.0
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.f
[SSSD] [PATCH] SPEC: Remove unnecessary requirements
ehlo,
reason is explained in commit message.
The intention for this patch was to a simplify spec file
(at least a little bit :-)
If we want to have some requirements for version of lib{ldb,tdb}
then it would be better to have minimal required version in
BuildRequires + configure detection of libraries.
But it might be outdated if we use some ver ldb features.
LS
>From 275b260be7c4365668a7adeceee6e9e089ae2f42 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Thu, 21 Jan 2016 16:55:11 +0100
Subject: [PATCH] SPEC: Remove unnecessary requirements
We do not need to requires specific version of libldb
or libtdb because it is automatically detected from
binary/library dependencies. We also need never version
of that libraries as it was specified in spec file.
e.g.
sh$ rpm -q --requires sssd-common | grep -E "TDB|LDB"
libldb.so.1(LDB_0.9.10)(64bit)
libtdb.so.1(TDB_1.2.1)(64bit)
Our public libraries also provides version definitions
therefore rpm can also detect minimal required version.
sh$ rpm -q --requires sssd-common | grep -E "IDMAP"
libsss_idmap.so.0(SSS_IDMAP_0.4)(64bit)
sh$ $rpm -q --requires python-libsss_nss_idmap | grep -E "IDMAP"
libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.0.1)(64bit)
sh$ rpm -q --requires sssd-ipa | grep -E "HBAC"
libipa_hbac.so.0(IPA_HBAC_0.0.1)(64bit)
---
contrib/sssd.spec.in | 8
1 file changed, 8 deletions(-)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index
76066de9ac8b8ac1f9b14bf3bc787169d317fe2f..44e57600797e4e98cc241d24f5f7264906dda9be
100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -177,11 +177,8 @@ the existing back ends.
Summary: Common files for the SSSD
Group: Applications/System
License: GPLv3+
-Requires: libldb >= 0.9.3
-Requires: libtdb >= 1.1.3
Requires: sssd-client%{?_isa} = %{version}-%{release}
Requires: libsss_sudo = %{version}-%{release}
-Requires: libsss_idmap = %{version}-%{release}
Conflicts: sssd < %{version}-%{release}
%if (0%{?use_systemd} == 1)
Requires(post): systemd-units systemd-sysv
@@ -369,7 +366,6 @@ License: GPLv3+
Conflicts: sssd < %{version}-%{release}
Requires: sssd-common = %{version}-%{release}
Requires: sssd-krb5-common = %{version}-%{release}
-Requires: libipa_hbac = %{version}-%{release}
Requires: bind-utils
Requires: sssd-common-pac = %{version}-%{release}
@@ -444,7 +440,6 @@ Utility library to validate FreeIPA HBAC rules for
authorization requests
Summary: Python2 bindings for the FreeIPA HBAC Evaluator library
Group: Development/Libraries
License: LGPLv3+
-Requires: libipa_hbac = %{version}-%{release}
Provides: libipa_hbac-python = %{version}-%{release}
Obsoletes: libipa_hbac-python < 1.12.90
@@ -457,7 +452,6 @@ used by Python applications.
Summary: Python3 bindings for the FreeIPA HBAC Evaluator library
Group: Development/Libraries
License: LGPLv3+
-Requires: libipa_hbac = %{version}-%{release}
%description -n python3-libipa_hbac
The python3-libipa_hbac contains the bindings so that libipa_hbac can be
@@ -487,7 +481,6 @@ Utility library for SID based lookups
Summary: Python2 bindings for libsss_nss_idmap
Group: Development/Libraries
License: LGPLv3+
-Requires: libsss_nss_idmap = %{version}-%{release}
Provides: libsss_nss_idmap-python = %{version}-%{release}
Obsoletes: libsss_nss_idmap-python < 1.12.90
@@ -500,7 +493,6 @@ be used by Python applications.
Summary: Python3 bindings for libsss_nss_idmap
Group: Development/Libraries
License: LGPLv3+
-Requires: libsss_nss_idmap = %{version}-%{release}
%description -n python3-libsss_nss_idmap
The python3-libsss_nss_idmap contains the bindings so that libsss_nss_idmap can
--
2.5.0
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH v2] intg: Add more LDAP tests
On 01/21/2016 02:53 PM, Nikolai Kondrashov wrote: Hi Jakub, Michal, everyone, On 11/11/2015 07:03 PM, Michal Židek wrote: I modified the "squashing" patch to remove the workaround for failing memcache. SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned). CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests +def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn, From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch -def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn, These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet. What shall we do with them? Nick Hi Nick, I have this in my todo list, but I did not started with it yet. I will probably not get to it anytime soon. Feel free to create a ticket and assign it to me. We can triage it next week. Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: disable midpoint refresh for netgroups if ptask refresh, is enabled
On (21/01/16 15:40), Jakub Hrozek wrote:
>On Thu, Jan 21, 2016 at 02:55:51PM +0100, Lukas Slebodnik wrote:
>> On (20/01/16 11:09), Pavel Březina wrote:
>> >On 01/19/2016 02:20 PM, Michal Židek wrote:
>> >>On 01/19/2016 02:07 PM, Pavel Březina wrote:
>> >>>On 01/19/2016 01:13 PM, Michal Židek wrote:
>> On 01/19/2016 12:28 PM, Pavel Březina wrote:
>> >On 01/19/2016 12:24 PM, Michal Židek wrote:
>> >>On 01/19/2016 11:56 AM, Pavel Březina wrote:
>> >>>On 01/19/2016 02:08 AM, Michal Židek wrote:
>> Hello,
>>
>> this patch applies on top of the
>> patch from thred:
>> [SSSD] [PATCH] NSS: Refresh also netgroup cache if needed
>>
>> It (re-)fixes the ticket:
>> https://fedorahosted.org/sssd/ticket/2102
>>
>> I separated them to give this one special attention :)
>> and also because I am not sure if it is a good solution or
>> not.
>>
>> I would like pbrezina or preichl to review
>> this one, because they were the original
>> reviewer/author for the fix, so maybe they
>> still remember why it was done that way.
>>
>> Michal
>> >>>
>> >>>Nack.
>> >>>
>> if (ret == EOK) {
>> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid,
>> returning..\n");
>> return EOK;
>> } else if (ret == EAGAIN &&
>> dctx->domain->refresh_expired_interval
>> && req_type == SSS_DP_NETGR) {
>> /* Skip midpoint refresh if background refresh is
>> enabled
>> * (for netgroups only)
>> */
>> return EOK;
>> } else if (ret != EAGAIN && ret != ENOENT) {
>> DEBUG(SSSDBG_CRIT_FAILURE, "Error checking cache: %d\n",
>> ret);
>> goto error;
>> }
>> >>>
>> >>>Please, return is_refresh_on_bg function and extend it to return true
>> >>>also for users and groups and merge the condition with the first
>> >>>one so
>> >>>debug message is visible, like so:
>> >>>
>> >>>if (ret == EOK || (ret == EAGAIN && is_refreshed_on_bg(...)) {
>> >>> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid, returning..\n");
>> >>> return EOK;
>> >>>}
>> >>>
>> >>>Maybe you can merge those two patches together..
>> >>
>> >>I wanted this one only in master. Do you think it should be
>> >>backported as well? I wanted only necessary changes in downstream.
>> >>
>> >>Michal
>> >
>> >I think it should be backported since "Tests showed that having
>> >midpoint
>> >refresh enabled may actually slow down netgroup request occasionally."
>> >See:
>> >https://fedorahosted.org/sssd/ticket/2102
>> >
>> >I don't remember thought on which tests is this statement based on...
>>
>> Ok, I merged the two then. Attached is the merged patch for master and
>> 1.13.
>>
>> Michal
>>
>> 0001-NSS-do-not-skip-cache-check-for-netgoups.patch
>>
>>
>> From c4209629af439f3ead805b7c89ee9ac36c6264a6 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?=
>> Date: Mon, 18 Jan 2016 22:02:55 +0100
>> Subject: [PATCH] NSS: do not skip cache check for netgoups
>>
>> When refresh_expired_interval was not zero,
>> the NSS responder only refreshed netgroup cache
>> using background periodic task and ignored
>> SYSDB_CACHE_EXPIRE attribute.
>>
>> With this behaviour it was impossible to
>> get new netgroup from remote server even
>> after sss_cache tool was used to expire
>> existing entry in the cache.
>> ---
>> src/responder/nss/nsssrv_cmd.c | 48
>> +-
>> 1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/src/responder/nss/nsssrv_cmd.c
>> b/src/responder/nss/nsssrv_cmd.c
>> index d6ac9dc..1f24d1b 100644
>> --- a/src/responder/nss/nsssrv_cmd.c
>> +++ b/src/responder/nss/nsssrv_cmd.c
>> @@ -579,10 +579,9 @@ static int nss_cmd_getpw_send_reply(struct
>> nss_dom_ctx *dctx, bool filter)
>> return EOK;
>> }
>>
>> -/* Currently only refreshing expired netgroups is supported. */
>> static bool
>> is_refreshed_on_bg(enum sss_dp_acct_type req_type,
>> - enum sss_dp_acct_type refresh_expired_interval)
>> + uint32_t refresh_expired_interval)
>> {
>> if (refresh_expired_interval == 0) {
>> return false;
>> @@ -590,6 +589,8 @@ is_refreshed_on_bg(enum sss_dp_acct_type req_type,
>>
>> switch (req_type) {
>> case SSS_DP_NETGR:
>> +case SSS_DP_USER:
>> +case SSS_DP_GROUP:
>> return true;
>> >>
[SSSD] Re: [PATCHES] UTIL: Provide varargs version of debug_fn
On Sat, 2016-01-16 at 12:33 +0100, Lukas Slebodnik wrote: > On (15/01/16 16:09), Simo Sorce wrote: > >On Fri, 2016-01-15 at 12:44 +0100, Lukas Slebodnik wrote: > >> On (15/01/16 12:03), Pavel Březina wrote: > >> >On 01/12/2016 10:15 AM, Lukas Slebodnik wrote: > >> >>ehlo, > >> >> > >> >>The main reason for these patch was to improve > >> >>recently added logging to hbac. > >> >> > >> >>Side effect of these change is improvement for libldb > >> >>and libsemanage (6th patch) > >> >> > >> >>4th patch is not API/ABI change because > >> >>such version has not beeen released yet. > >> >>If you do not like change in hbac callback > >> >>hbac_debug_fn_t then we should also remove > >> >>because it is too internal then we should > >> >>remove also the first two arguments. > >> >>"file", "line" also leaks internal data from libhbac. > >> >>Removing the first two arguments would be almost > >> >>consistent callbacks in libldb and libsemanage. > >> >> > >> >>LS > >> > > >> >Hi, > >> >I'm getting following errors: > >> > > >> >In file included from > >> >/home/pbrezina/workspace/sssd/src/python/pyhbac.c:26:0: > >> >/home/pbrezina/workspace/sssd/src/providers/ipa/ipa_hbac.h:54:0: error: > >> >"SSS_ATTRIBUTE_PRINTF" redefined [-Werror] > >> > #define SSS_ATTRIBUTE_PRINTF(a1, a2) __attribute__((format(printf, a1, > >> > a2))) > >> > ^ > >> >In file included from > >> >/home/pbrezina/workspace/sssd/src/python/pyhbac.c:24:0: > >> >/home/pbrezina/workspace/sssd/src/util/util.h:62:0: note: this is the > >> >location of the previous definition > >> > #define SSS_ATTRIBUTE_PRINTF(a1, a2) __attribute__ ((format (printf, a1, > >> >a2))) > >> > > >> I might add another #ifdef guard but I decided to rename macro > >> SSS_ATTRIBUTE_PRINTF -> HBAC_ATTRIBUTE_PRINTF > > > > > >Why is debug_fn being called directly in hbac/semanage/etc.. insted of > >being called through a common macro ? > > > libldb, libsemanage ... provide function which allows you to register > your own debug function > > e.g. > libldb has ldb_set_debug and we register ldb_debug_messages wih prototype >void ldb_debug_messages(void *context, enum ldb_debug_level level, >const char *fmt, va_list ap) > > However you cannot call DEBUG macro with agruments:"const char *fmt, va_list > ap" > So you need to call "vasprintf(&message, fmt, ap)" and then log message > and then log message + release message; it is inefficient. > > The same situation is with libhbac and libsemanage. > > Does this answer your question? Yes. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: disable midpoint refresh for netgroups if ptask refresh, is enabled
On Thu, Jan 21, 2016 at 02:55:51PM +0100, Lukas Slebodnik wrote:
> On (20/01/16 11:09), Pavel Březina wrote:
> >On 01/19/2016 02:20 PM, Michal Židek wrote:
> >>On 01/19/2016 02:07 PM, Pavel Březina wrote:
> >>>On 01/19/2016 01:13 PM, Michal Židek wrote:
> On 01/19/2016 12:28 PM, Pavel Březina wrote:
> >On 01/19/2016 12:24 PM, Michal Židek wrote:
> >>On 01/19/2016 11:56 AM, Pavel Březina wrote:
> >>>On 01/19/2016 02:08 AM, Michal Židek wrote:
> Hello,
>
> this patch applies on top of the
> patch from thred:
> [SSSD] [PATCH] NSS: Refresh also netgroup cache if needed
>
> It (re-)fixes the ticket:
> https://fedorahosted.org/sssd/ticket/2102
>
> I separated them to give this one special attention :)
> and also because I am not sure if it is a good solution or
> not.
>
> I would like pbrezina or preichl to review
> this one, because they were the original
> reviewer/author for the fix, so maybe they
> still remember why it was done that way.
>
> Michal
> >>>
> >>>Nack.
> >>>
> if (ret == EOK) {
> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid,
> returning..\n");
> return EOK;
> } else if (ret == EAGAIN &&
> dctx->domain->refresh_expired_interval
> && req_type == SSS_DP_NETGR) {
> /* Skip midpoint refresh if background refresh is
> enabled
> * (for netgroups only)
> */
> return EOK;
> } else if (ret != EAGAIN && ret != ENOENT) {
> DEBUG(SSSDBG_CRIT_FAILURE, "Error checking cache: %d\n",
> ret);
> goto error;
> }
> >>>
> >>>Please, return is_refresh_on_bg function and extend it to return true
> >>>also for users and groups and merge the condition with the first
> >>>one so
> >>>debug message is visible, like so:
> >>>
> >>>if (ret == EOK || (ret == EAGAIN && is_refreshed_on_bg(...)) {
> >>> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid, returning..\n");
> >>> return EOK;
> >>>}
> >>>
> >>>Maybe you can merge those two patches together..
> >>
> >>I wanted this one only in master. Do you think it should be
> >>backported as well? I wanted only necessary changes in downstream.
> >>
> >>Michal
> >
> >I think it should be backported since "Tests showed that having
> >midpoint
> >refresh enabled may actually slow down netgroup request occasionally."
> >See:
> >https://fedorahosted.org/sssd/ticket/2102
> >
> >I don't remember thought on which tests is this statement based on...
>
> Ok, I merged the two then. Attached is the merged patch for master and
> 1.13.
>
> Michal
>
> 0001-NSS-do-not-skip-cache-check-for-netgoups.patch
>
>
> From c4209629af439f3ead805b7c89ee9ac36c6264a6 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?=
> Date: Mon, 18 Jan 2016 22:02:55 +0100
> Subject: [PATCH] NSS: do not skip cache check for netgoups
>
> When refresh_expired_interval was not zero,
> the NSS responder only refreshed netgroup cache
> using background periodic task and ignored
> SYSDB_CACHE_EXPIRE attribute.
>
> With this behaviour it was impossible to
> get new netgroup from remote server even
> after sss_cache tool was used to expire
> existing entry in the cache.
> ---
> src/responder/nss/nsssrv_cmd.c | 48
> +-
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/src/responder/nss/nsssrv_cmd.c
> b/src/responder/nss/nsssrv_cmd.c
> index d6ac9dc..1f24d1b 100644
> --- a/src/responder/nss/nsssrv_cmd.c
> +++ b/src/responder/nss/nsssrv_cmd.c
> @@ -579,10 +579,9 @@ static int nss_cmd_getpw_send_reply(struct
> nss_dom_ctx *dctx, bool filter)
> return EOK;
> }
>
> -/* Currently only refreshing expired netgroups is supported. */
> static bool
> is_refreshed_on_bg(enum sss_dp_acct_type req_type,
> - enum sss_dp_acct_type refresh_expired_interval)
> + uint32_t refresh_expired_interval)
> {
> if (refresh_expired_interval == 0) {
> return false;
> @@ -590,6 +589,8 @@ is_refreshed_on_bg(enum sss_dp_acct_type req_type,
>
> switch (req_type) {
> case SSS_DP_NETGR:
> +case SSS_DP_USER:
> +case SSS_DP_GROUP:
> return true;
> default:
> return false;
> @@ -753,31 +754,30 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
> get_dp_name_and_id(dctx->cmdctx, dc
[SSSD] Re: [PATCH] Abstract and improve connection credential handling
On Wed, 2016-01-20 at 16:38 +0100, Lukas Slebodnik wrote:
> On (19/01/16 15:38), Simo Sorce wrote:
> >On Tue, 2016-01-19 at 10:34 -0500, Simo Sorce wrote:
> >> On Tue, 2016-01-19 at 11:23 +0100, Lukas Slebodnik wrote:
> >> [...]
> >> > >+#endif /* __SSSD_UTIL_SELINUX_H__ */
> >> > BTW will we need this header file if we make
> >> > struct cli_creds opaque?
> >>
> >> Replying to both your mails here:
> >> Making cli_creds opaque requires using a pointer and dealing with
> >> allocating it, I guess I can do that, the headers file would still be
> >> needed in order to avoid huge ifdefs around the functions that implement
> >> handling SELinux stuff. It makes the code a lot more readable and
> >> searchable.
> >>
> >> Simo.
> >>
> >
> >Attached find patch that changes the code so that cli_creds is now
> >opaque.
> >This *should* work w/o the patch that changes the headers but I haven't
> >tested it w/o that patch yet.
> >
> I had an issue to build it correctly with ifp responder.
> src/responder/ifp/ifpsrv_util.c: In function ‘ifp_req_create’:
> src/responder/ifp/ifpsrv_util.c:64:30: error: passing argument 1 of
> ‘check_allowed_uids’ makes pointer from integer without a cast
> [-Werror=int-conversion]
> ret = check_allowed_uids(dbus_req->client,
> ^
> In file included from ../sssd/src/responder/ifp/ifp_private.h:27:0,
> from ../sssd/src/responder/ifp/ifpsrv_util.c:27:
> src/responder/common/responder.h:334:9: note: expected ‘struct cli_creds *’
> but argument is of type ‘int64_t {aka long int}’
> errno_t check_allowed_uids(struct cli_creds *creds,
> ^
>
> ifp responder uses different way to obtain uid. I changed back the prototype
> of
> check_allowed_uids.
> Here is a diff on to of your patch.
>
> diff --git a/src/responder/common/responder.h
> b/src/responder/common/responder.h
> index 419a863..3b70b69 100644
> --- a/src/responder/common/responder.h
> +++ b/src/responder/common/responder.h
> @@ -331,8 +331,7 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx,
> const char *csv_string,
> size_t *_uid_count, uid_t **_uids);
>
> uid_t client_euid(struct cli_creds *creds);
> -errno_t check_allowed_uids(struct cli_creds *creds,
> - size_t allowed_uids_count,
> +errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
> uid_t *allowed_uids);
>
> struct tevent_req *
> diff --git a/src/responder/common/responder_common.c
> b/src/responder/common/responder_common.c
> index 27193ce..6ac1ea2 100644
> --- a/src/responder/common/responder_common.c
> +++ b/src/responder/common/responder_common.c
> @@ -138,8 +138,7 @@ uid_t client_euid(struct cli_creds *creds)
> return cli_creds_get_uid(creds);
> }
>
> -errno_t check_allowed_uids(struct cli_creds *creds,
> - size_t allowed_uids_count,
> +errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
> uid_t *allowed_uids)
> {
> size_t c;
> @@ -149,7 +148,7 @@ errno_t check_allowed_uids(struct cli_creds *creds,
> }
>
> for (c = 0; c < allowed_uids_count; c++) {
> -if (client_euid(creds) == allowed_uids[c]) {
> +if (uid == allowed_uids[c]) {
> return EOK;
> }
> }
> @@ -449,12 +448,13 @@ static void accept_fd_handler(struct tevent_context *ev,
> return;
> }
>
> -ret = check_allowed_uids(cctx->creds, rctx->allowed_uids_count,
> +ret = check_allowed_uids(client_euid(cctx->creds),
> rctx->allowed_uids_count,
> rctx->allowed_uids);
> if (ret != EOK) {
> if (ret == EACCES) {
> -DEBUG(SSSDBG_CRIT_FAILURE, "Access denied for uid [%d].\n",
> -(int)client_euid(cctx->creds));
> +DEBUG(SSSDBG_CRIT_FAILURE,
> + "Access denied for uid [%"SPRIuid"].\n",
> + client_euid(cctx->creds));
> } else {
> DEBUG(SSSDBG_OP_FAILURE, "check_allowed_uids failed.\n");
> }
> diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
> index 5fe3e6b..bfc534f 100644
> --- a/src/responder/pam/pamsrv_cmd.c
> +++ b/src/responder/pam/pamsrv_cmd.c
> @@ -1001,7 +1001,7 @@ static bool is_uid_trusted(struct cli_creds *creds,
> return true;
> }
>
> -ret = check_allowed_uids(creds, trusted_uids_count, trusted_uids);
> +ret = check_allowed_uids(client_euid(creds), trusted_uids_count,
> trusted_uids);
> if (ret == EOK) return true;
>
> return false;
> @@ -1091,13 +1091,13 @@ static int pam_forwarder(struct cli_ctx *cctx, int
> pam_cmd)
> }
> pd = preq->pd;
>
> -preq->is_uid_trusted = is_uid_trusted(&cctx->creds,
> +preq->is_uid_trusted = is_uid_trusted(cctx->creds,
>
[SSSD] Re: [PATCH] NSS: disable midpoint refresh for netgroups if ptask refresh, is enabled
On (20/01/16 11:09), Pavel Březina wrote:
>On 01/19/2016 02:20 PM, Michal Židek wrote:
>>On 01/19/2016 02:07 PM, Pavel Březina wrote:
>>>On 01/19/2016 01:13 PM, Michal Židek wrote:
On 01/19/2016 12:28 PM, Pavel Březina wrote:
>On 01/19/2016 12:24 PM, Michal Židek wrote:
>>On 01/19/2016 11:56 AM, Pavel Březina wrote:
>>>On 01/19/2016 02:08 AM, Michal Židek wrote:
Hello,
this patch applies on top of the
patch from thred:
[SSSD] [PATCH] NSS: Refresh also netgroup cache if needed
It (re-)fixes the ticket:
https://fedorahosted.org/sssd/ticket/2102
I separated them to give this one special attention :)
and also because I am not sure if it is a good solution or
not.
I would like pbrezina or preichl to review
this one, because they were the original
reviewer/author for the fix, so maybe they
still remember why it was done that way.
Michal
>>>
>>>Nack.
>>>
if (ret == EOK) {
DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid,
returning..\n");
return EOK;
} else if (ret == EAGAIN &&
dctx->domain->refresh_expired_interval
&& req_type == SSS_DP_NETGR) {
/* Skip midpoint refresh if background refresh is
enabled
* (for netgroups only)
*/
return EOK;
} else if (ret != EAGAIN && ret != ENOENT) {
DEBUG(SSSDBG_CRIT_FAILURE, "Error checking cache: %d\n",
ret);
goto error;
}
>>>
>>>Please, return is_refresh_on_bg function and extend it to return true
>>>also for users and groups and merge the condition with the first
>>>one so
>>>debug message is visible, like so:
>>>
>>>if (ret == EOK || (ret == EAGAIN && is_refreshed_on_bg(...)) {
>>> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid, returning..\n");
>>> return EOK;
>>>}
>>>
>>>Maybe you can merge those two patches together..
>>
>>I wanted this one only in master. Do you think it should be
>>backported as well? I wanted only necessary changes in downstream.
>>
>>Michal
>
>I think it should be backported since "Tests showed that having
>midpoint
>refresh enabled may actually slow down netgroup request occasionally."
>See:
>https://fedorahosted.org/sssd/ticket/2102
>
>I don't remember thought on which tests is this statement based on...
Ok, I merged the two then. Attached is the merged patch for master and
1.13.
Michal
0001-NSS-do-not-skip-cache-check-for-netgoups.patch
From c4209629af439f3ead805b7c89ee9ac36c6264a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
Date: Mon, 18 Jan 2016 22:02:55 +0100
Subject: [PATCH] NSS: do not skip cache check for netgoups
When refresh_expired_interval was not zero,
the NSS responder only refreshed netgroup cache
using background periodic task and ignored
SYSDB_CACHE_EXPIRE attribute.
With this behaviour it was impossible to
get new netgroup from remote server even
after sss_cache tool was used to expire
existing entry in the cache.
---
src/responder/nss/nsssrv_cmd.c | 48
+-
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c
b/src/responder/nss/nsssrv_cmd.c
index d6ac9dc..1f24d1b 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -579,10 +579,9 @@ static int nss_cmd_getpw_send_reply(struct
nss_dom_ctx *dctx, bool filter)
return EOK;
}
-/* Currently only refreshing expired netgroups is supported. */
static bool
is_refreshed_on_bg(enum sss_dp_acct_type req_type,
- enum sss_dp_acct_type refresh_expired_interval)
+ uint32_t refresh_expired_interval)
{
if (refresh_expired_interval == 0) {
return false;
@@ -590,6 +589,8 @@ is_refreshed_on_bg(enum sss_dp_acct_type req_type,
switch (req_type) {
case SSS_DP_NETGR:
+case SSS_DP_USER:
+case SSS_DP_GROUP:
return true;
default:
return false;
@@ -753,31 +754,30 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
get_dp_name_and_id(dctx->cmdctx, dctx->domain, req_type,
opt_name, opt_id,
&name, &id);
-/* if we have any reply let's check cache validity, but ignore
netgroups
- * if refresh_expired_interval is set (which implies that another
method
- * is used to refresh netgroups)
- */
+
[SSSD] Re: [PATCH v2] intg: Add more LDAP tests
Hi Jakub, Michal, everyone, On 11/11/2015 07:03 PM, Michal Židek wrote: I modified the "squashing" patch to remove the workaround for failing memcache. SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned). CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests +def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn, From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch -def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn, These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet. What shall we do with them? Nick ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] Do not compile generated header files
On (21/01/16 13:36), Lukas Slebodnik wrote: >On (21/01/16 11:22), Pavel Březina wrote: >>On 01/20/2016 01:54 PM, Lukas Slebodnik wrote: >>>On (20/01/16 13:09), Pavel Březina wrote: I think this is not necessary... or is there any specific reason? >>> From b24cc925e5d22b02a17b31e20d8ce4f8a3c2eb51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Fri, 15 Jan 2016 13:39:04 +0100 Subject: [PATCH 01/12] Do not compile generated header files --- Makefile.am | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index 6008b06ddf45fa8c6558876db37d9d07788ca9da..30fa4aebf3b3163323e5794fad288f2fbb055136 100644 --- a/Makefile.am +++ b/Makefile.am @@ -458,9 +458,7 @@ SSSD_RESPONDER_OBJ = \ src/responder/common/responder_utils.c \ src/responder/common/responder_cache_req.c \ src/monitor/monitor_iface_generated.c \ -src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \ -src/providers/data_provider_iface_generated.h \ src/providers/data_provider_req.c SSSD_TOOLS_OBJ = \ @@ -559,6 +557,7 @@ dist_noinst_HEADERS = \ src/util/util_sss_idmap.h \ src/monitor/monitor.h \ src/monitor/monitor_interfaces.h \ +src/monitor/monitor_iface_generated.h \ src/responder/common/responder.h \ src/responder/common/responder_packet.h \ src/responder/common/responder_sbus.h \ @@ -575,6 +574,7 @@ dist_noinst_HEADERS = \ src/responder/sudo/sudosrv_private.h \ src/responder/autofs/autofs_private.h \ src/responder/ssh/sshsrv_private.h \ +src/responder/ifp/ifp_iface_generated.h \ src/responder/ifp/ifp_private.h \ src/responder/ifp/ifp_domains.h \ src/responder/ifp/ifp_components.h \ @@ -647,6 +647,7 @@ dist_noinst_HEADERS = \ src/providers/ipa/ipa_sudo.h \ src/providers/ad/ad_srv.h \ src/providers/proxy/proxy.h \ +src/providers/data_provider_iface_generated.h \ >>>Patch make sense header files needn't be listed as a dependencies >>>for binary files (libraries). Dependency on header files should >>>be automatically generated by compiler (or libtool) IIRC. >>> >>>make distcheck passed but would you mind to move previous header files to >>>different place in dist_noinst_HEADERS. >>> >>>Here is a diff >>>diff --git a/Makefile.am b/Makefile.am >>>index 30fa4ae..12ddeba 100644 >>>--- a/Makefile.am >>>+++ b/Makefile.am >>>@@ -599,6 +599,7 @@ dist_noinst_HEADERS = \ >>> src/confdb/confdb_setup.h \ >>> src/providers/data_provider.h \ >>> src/providers/data_provider_req.h \ >>>+src/providers/data_provider_iface_generated.h \ >>> src/providers/dp_backend.h \ >>> src/providers/dp_dyndns.h \ >>> src/providers/dp_ptask_private.h \ >>>@@ -647,7 +648,6 @@ dist_noinst_HEADERS = \ >>> src/providers/ipa/ipa_sudo.h \ >>> src/providers/ad/ad_srv.h \ >>> src/providers/proxy/proxy.h \ >>>-src/providers/data_provider_iface_generated.h \ >>> src/tools/tools_util.h \ >>> src/tools/sss_sync_ops.h \ >>> src/resolv/async_resolv.h \ >>> >>> >>>I looks better src/providers/data_provider* header files are together :-) >> >>Attached. >> > >>From 7d7da72862bcf4c8ba91db3e9cd031fedd9bfd80 Mon Sep 17 00:00:00 2001 >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= >>Date: Fri, 15 Jan 2016 13:39:04 +0100 >>Subject: [PATCH 01/13] Do not compile generated header files >> >>--- >> Makefile.am | 23 +++ >> 1 file changed, 11 insertions(+), 12 deletions(-) >> >ACK > >http://sssd-ci.duckdns.org/logs/job/36/59/summary.html > master: * 7ac503a73a26abe49f9f7d175c74df705380898d LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] Do not compile generated header files
On (21/01/16 11:22), Pavel Březina wrote: >On 01/20/2016 01:54 PM, Lukas Slebodnik wrote: >>On (20/01/16 13:09), Pavel Březina wrote: >>>I think this is not necessary... or is there any specific reason? >> >>>From b24cc925e5d22b02a17b31e20d8ce4f8a3c2eb51 Mon Sep 17 00:00:00 2001 >>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= >>>Date: Fri, 15 Jan 2016 13:39:04 +0100 >>>Subject: [PATCH 01/12] Do not compile generated header files >>> >>>--- >>>Makefile.am | 23 +++ >>>1 file changed, 11 insertions(+), 12 deletions(-) >>> >>>diff --git a/Makefile.am b/Makefile.am >>>index >>>6008b06ddf45fa8c6558876db37d9d07788ca9da..30fa4aebf3b3163323e5794fad288f2fbb055136 >>> 100644 >>>--- a/Makefile.am >>>+++ b/Makefile.am >>>@@ -458,9 +458,7 @@ SSSD_RESPONDER_OBJ = \ >>> src/responder/common/responder_utils.c \ >>> src/responder/common/responder_cache_req.c \ >>> src/monitor/monitor_iface_generated.c \ >>>-src/monitor/monitor_iface_generated.h \ >>> src/providers/data_provider_iface_generated.c \ >>>-src/providers/data_provider_iface_generated.h \ >>> src/providers/data_provider_req.c >>> >>>SSSD_TOOLS_OBJ = \ >>>@@ -559,6 +557,7 @@ dist_noinst_HEADERS = \ >>> src/util/util_sss_idmap.h \ >>> src/monitor/monitor.h \ >>> src/monitor/monitor_interfaces.h \ >>>+src/monitor/monitor_iface_generated.h \ >>> src/responder/common/responder.h \ >>> src/responder/common/responder_packet.h \ >>> src/responder/common/responder_sbus.h \ >>>@@ -575,6 +574,7 @@ dist_noinst_HEADERS = \ >>> src/responder/sudo/sudosrv_private.h \ >>> src/responder/autofs/autofs_private.h \ >>> src/responder/ssh/sshsrv_private.h \ >>>+src/responder/ifp/ifp_iface_generated.h \ >>> src/responder/ifp/ifp_private.h \ >>> src/responder/ifp/ifp_domains.h \ >>> src/responder/ifp/ifp_components.h \ >>>@@ -647,6 +647,7 @@ dist_noinst_HEADERS = \ >>> src/providers/ipa/ipa_sudo.h \ >>> src/providers/ad/ad_srv.h \ >>> src/providers/proxy/proxy.h \ >>>+src/providers/data_provider_iface_generated.h \ >>Patch make sense header files needn't be listed as a dependencies >>for binary files (libraries). Dependency on header files should >>be automatically generated by compiler (or libtool) IIRC. >> >>make distcheck passed but would you mind to move previous header files to >>different place in dist_noinst_HEADERS. >> >>Here is a diff >>diff --git a/Makefile.am b/Makefile.am >>index 30fa4ae..12ddeba 100644 >>--- a/Makefile.am >>+++ b/Makefile.am >>@@ -599,6 +599,7 @@ dist_noinst_HEADERS = \ >> src/confdb/confdb_setup.h \ >> src/providers/data_provider.h \ >> src/providers/data_provider_req.h \ >>+src/providers/data_provider_iface_generated.h \ >> src/providers/dp_backend.h \ >> src/providers/dp_dyndns.h \ >> src/providers/dp_ptask_private.h \ >>@@ -647,7 +648,6 @@ dist_noinst_HEADERS = \ >> src/providers/ipa/ipa_sudo.h \ >> src/providers/ad/ad_srv.h \ >> src/providers/proxy/proxy.h \ >>-src/providers/data_provider_iface_generated.h \ >> src/tools/tools_util.h \ >> src/tools/sss_sync_ops.h \ >> src/resolv/async_resolv.h \ >> >> >>I looks better src/providers/data_provider* header files are together :-) > >Attached. > >From 7d7da72862bcf4c8ba91db3e9cd031fedd9bfd80 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= >Date: Fri, 15 Jan 2016 13:39:04 +0100 >Subject: [PATCH 01/13] Do not compile generated header files > >--- > Makefile.am | 23 +++ > 1 file changed, 11 insertions(+), 12 deletions(-) > ACK http://sssd-ci.duckdns.org/logs/job/36/59/summary.html LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] Do not compile generated header files
On 01/20/2016 01:54 PM, Lukas Slebodnik wrote: On (20/01/16 13:09), Pavel Březina wrote: I think this is not necessary... or is there any specific reason? From b24cc925e5d22b02a17b31e20d8ce4f8a3c2eb51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Fri, 15 Jan 2016 13:39:04 +0100 Subject: [PATCH 01/12] Do not compile generated header files --- Makefile.am | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index 6008b06ddf45fa8c6558876db37d9d07788ca9da..30fa4aebf3b3163323e5794fad288f2fbb055136 100644 --- a/Makefile.am +++ b/Makefile.am @@ -458,9 +458,7 @@ SSSD_RESPONDER_OBJ = \ src/responder/common/responder_utils.c \ src/responder/common/responder_cache_req.c \ src/monitor/monitor_iface_generated.c \ -src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \ -src/providers/data_provider_iface_generated.h \ src/providers/data_provider_req.c SSSD_TOOLS_OBJ = \ @@ -559,6 +557,7 @@ dist_noinst_HEADERS = \ src/util/util_sss_idmap.h \ src/monitor/monitor.h \ src/monitor/monitor_interfaces.h \ +src/monitor/monitor_iface_generated.h \ src/responder/common/responder.h \ src/responder/common/responder_packet.h \ src/responder/common/responder_sbus.h \ @@ -575,6 +574,7 @@ dist_noinst_HEADERS = \ src/responder/sudo/sudosrv_private.h \ src/responder/autofs/autofs_private.h \ src/responder/ssh/sshsrv_private.h \ +src/responder/ifp/ifp_iface_generated.h \ src/responder/ifp/ifp_private.h \ src/responder/ifp/ifp_domains.h \ src/responder/ifp/ifp_components.h \ @@ -647,6 +647,7 @@ dist_noinst_HEADERS = \ src/providers/ipa/ipa_sudo.h \ src/providers/ad/ad_srv.h \ src/providers/proxy/proxy.h \ +src/providers/data_provider_iface_generated.h \ Patch make sense header files needn't be listed as a dependencies for binary files (libraries). Dependency on header files should be automatically generated by compiler (or libtool) IIRC. make distcheck passed but would you mind to move previous header files to different place in dist_noinst_HEADERS. Here is a diff diff --git a/Makefile.am b/Makefile.am index 30fa4ae..12ddeba 100644 --- a/Makefile.am +++ b/Makefile.am @@ -599,6 +599,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ src/providers/data_provider_req.h \ +src/providers/data_provider_iface_generated.h \ src/providers/dp_backend.h \ src/providers/dp_dyndns.h \ src/providers/dp_ptask_private.h \ @@ -647,7 +648,6 @@ dist_noinst_HEADERS = \ src/providers/ipa/ipa_sudo.h \ src/providers/ad/ad_srv.h \ src/providers/proxy/proxy.h \ -src/providers/data_provider_iface_generated.h \ src/tools/tools_util.h \ src/tools/sss_sync_ops.h \ src/resolv/async_resolv.h \ I looks better src/providers/data_provider* header files are together :-) Attached. From 7d7da72862bcf4c8ba91db3e9cd031fedd9bfd80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Fri, 15 Jan 2016 13:39:04 +0100 Subject: [PATCH 01/13] Do not compile generated header files --- Makefile.am | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index 6008b06ddf45fa8c6558876db37d9d07788ca9da..12ddebae0439a8cdacb92a32a5ac382a91cfa48c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -458,9 +458,7 @@ SSSD_RESPONDER_OBJ = \ src/responder/common/responder_utils.c \ src/responder/common/responder_cache_req.c \ src/monitor/monitor_iface_generated.c \ -src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \ -src/providers/data_provider_iface_generated.h \ src/providers/data_provider_req.c SSSD_TOOLS_OBJ = \ @@ -559,6 +557,7 @@ dist_noinst_HEADERS = \ src/util/util_sss_idmap.h \ src/monitor/monitor.h \ src/monitor/monitor_interfaces.h \ +src/monitor/monitor_iface_generated.h \ src/responder/common/responder.h \ src/responder/common/responder_packet.h \ src/responder/common/responder_sbus.h \ @@ -575,6 +574,7 @@ dist_noinst_HEADERS = \ src/responder/sudo/sudosrv_private.h \ src/responder/autofs/autofs_private.h \ src/responder/ssh/sshsrv_private.h \ +src/responder/ifp/ifp_iface_generated.h \ src/responder/ifp/ifp_private.h \ src/responder/ifp/ifp_domains.h \ src/responder/ifp/ifp_components.h \ @@ -599,6 +599,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ src/providers/data_provider_req.h \ +src/providers/data_provider_iface_generated.h \ src/providers/dp_backend.h \ src/providers/dp_dyndns.h \ src/providers/dp_ptask_private.h \ @@ -659,6 +660,7 @@ dist_noinst_HEADERS = \ src/tests/cmocka/common_mock_krb5.h \ src/tests/cmo
