Re: [Freeipa-devel] ds-migrate feature enhancements
On 11/24/2014 11:24 PM, Alan Evans wrote:
I am in the midst of preparing for a migration from OpenLDAP to FreeIPA.
ds-migrate wasn't going to fill all of my needs so I thought I would use it
for most and then make up some LDIF's and massage them to do the last bit
of migration.
I have instead decided to extend ds-migrate and I think that my features
might be of use to others so I would like to contribute them.
Great idea! :-) IMO, enhancing FreeIPA migration capability and making it more
even more pleasant experience for user users is a good goal.
Before I get
too for I wanted to get some input from the community.
Here are MY original goals:
* Migrate ssh public keys
The openssh-lpk schema is used in my tree so objectClass: ldapPublicKey
attribute: sshPublicKey
* Migrate disabled accounts as disabled
We 'disable' usere by setting their shadowExpire to a date in the past
and setting their shell to /bin/false
I realized that the ssh-public key problem is more generally an attribute
mapping problem and dealing with disabled users could be more generalized
too.
Here are instead the new features I would provide.
* Attribute mapping
Feature should check the new syntax exists and is the same as the old
syntax (perhaps further check for compatible syntax)
--user-attribute-map=oldAttribute=newAttribute
--group-attribute-map=foo=bar
Should I drop user/group and just make it --attribute-map and apply it to
both?
Should certain attributes be mapped by default, i.e.
sshPublicKey=ipaSSHPubKey (this means we also need to ignore the
objectClass ldapPublicKey by default) Maybe make a separate switch
--with-ssh-keys that automatically adds a map and an ignore?
If the mapping sshPublicKey - ipaSSHPubKey is clear, I would do it by default
and maybe add a switch to skip these advanced migrations. Just make sure the
expected format matches (ipa requires all 3 pieces of the public key, not just
the blob)
I think it would be very useful to combine user attribute mapping with the
ideas from
https://fedorahosted.org/freeipa/ticket/3709
i.e. filling attribute with values from original entry or a default. This may
lead to following syntax:
--user-attribute-default sn=notdefined --user-attribute-default
description=User with cn $cn
which would only use the pattern if attribute is not defined in original entry
and
--user-attribute-map sn=$cn
which would fill overwrite the attribute even if it was defined in the original
entry.
* Handling disabled users
1. How to identify disabled users?
a. shadowExpire now()
--use-disable-shadow-expire
How would you like to disable users then? With nsAccountLock attribute?
Wouldn't it be better to instead map shadowExpire to krbPrincipalExpiration
attribute which should have sematically the same meaning?
b. loginShell is one of configurable shells
--use-disable-login-shell
--disabled-shell=/bin/false --disabled-shell=/sbin/nologin (these
Is this really a general mechanism? I think Kerberos principal expiration is
the way to go:
# ipa user-mod fbar --principal-expiration 2014010100Z
# ssh fbar@`hostname`
f...@vm-067.idm.lab.bos.redhat.com's password:
Permission denied, please try again.
# tail /var/log/krb5kdc.log
...
Nov 26 04:56:51 vm-067.idm.lab.bos.redhat.com krb5kdc[19615](info): AS_REQ (6
etypes {18 17 16 23 25 26}) 10.16.78.67: CLIENT EXPIRED:
f...@idm.lab.bos.redhat.com for
krbtgt/idm.lab.bos.redhat@idm.lab.bos.redhat.com, Client's entry in
database has expired
It is respected by Kerberos authentication and SSSD logins, at minimum.
two would be the defaults)
c. nsAccountLocked (though that would be straight copied by the
migrator anyway
+1 for copying.
d. From Open DJ the attribute ds-pwp-account-disabled can be used to
identify disabled users
(are there others?)
2. What do do with disabled users (in my case migrate and disable)
a. Migrate them and don't touch nsAccountLocked
b. Migrate them and set nsAccountLocked = true
--disable-users
c. Do not migrate them
--skip-disabled-users
d. Which is the default? Migrate and disable? If so which are the
default methods for identifying them? All methods?
I may be missing something, but to me following would make sense:
- properly migrate and fill krbPrincipalExpiration and nsACcountLock attributes
- optionally implement --skip-disabled-users. Though it may be challenging to
implement the is_user_disabled function right so that it works on all sort of
DS schemes.
So is there anything I'm missing? Any suggestions on the switches? I'm not
entirely sure I like them the way they are.
I have code to cover about 60% of the above already. The user-attr-map
feature is working and the --disabled-users and disabled-shells options are
working.
Regards,
-Alan
HTH,
Martin
___
Freeipa-devel mailing list
Re: [Freeipa-devel] [PATCH] 0033 Use singular in help metavars + update man pages.
On 25/11/14 15:44, David Kupka wrote: On 11/25/2014 03:23 PM, Martin Basti wrote: On 25/11/14 13:16, David Kupka wrote: On 11/25/2014 09:57 AM, David Kupka wrote: On 11/25/2014 09:51 AM, David Kupka wrote: On 11/24/2014 03:59 PM, Martin Basti wrote: On 24/11/14 15:54, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4695 IMO this is one of two reasonable ways how to fix this ticket. The other one is to change just the manual page but it seems more consistent to use singular for metavars everywhere. I like this approach. But IMO we should instead of You ... form in help, this message, as we use with forwarders This option can be used multiple times Martin^2 Our manual pages are not exactly unified in that but let's stay with the majority. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Wrong patch, sorry. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Extending help messages too. NACK, you edited wrong option --no-reverseDo not create reverse DNS zone. This option can be used multiple times Bad line, thanks for noticing, fixed patches attached. ./make-lint * Module ipaserver.install.ipa_replica_prepare ipaserver/install/ipa_replica_prepare.py:59: [E0001(syntax-error), ] invalid syntax) * Module ipa-replica-prepare install/tools/ipa-replica-prepare:21: [E0611(no-name-in-module), ] No name 'ipa_replica_prepare' in module 'ipaserver.install') -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
On 11/12/2014 03:30 PM, Petr Spacek wrote: On 24.7.2014 11:00, Petr Spacek wrote: On 27.2.2014 15:19, Lukas Slebodnik wrote: ehlo, I did some reviews of bind-dyndb-ldap last week and it was little bit annoying to export special CFLAGS for bind9 header files. It can be automatically detected in configure script using utility isc-config. Attached patch should improve this and CFLAGS needn't be exported. Kind NACK. It would be valuable to test if isc/errno2result.h header is present and throw appropriate error. Current check with isc-config.sh only will pass if you have bind-devel package installed but you are missing bind-lite-devel package. I have a question: How +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99 works? Will it take user-defined CFLAGS into account? I would like to place user-defined flags at the end of the list so you can easily override settings given by autotools. Thank you for clarification :-) I will be really happy to commit complete fix. Thank you for cleaning this autotools mess! This version actually works. Previous version did not take CFLAGS from isc-config.sh into account during libdns version check so it actually did not work at all :-) Please review it (and send me a modified patch if you see a problem). Thank you for your time! ACK. No need to export CPPFLAGS any more! Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0033 Use singular in help metavars + update man pages.
On 11/26/2014 12:23 PM, Martin Basti wrote: On 25/11/14 15:44, David Kupka wrote: On 11/25/2014 03:23 PM, Martin Basti wrote: On 25/11/14 13:16, David Kupka wrote: On 11/25/2014 09:57 AM, David Kupka wrote: On 11/25/2014 09:51 AM, David Kupka wrote: On 11/24/2014 03:59 PM, Martin Basti wrote: On 24/11/14 15:54, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4695 IMO this is one of two reasonable ways how to fix this ticket. The other one is to change just the manual page but it seems more consistent to use singular for metavars everywhere. I like this approach. But IMO we should instead of You ... form in help, this message, as we use with forwarders This option can be used multiple times Martin^2 Our manual pages are not exactly unified in that but let's stay with the majority. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Wrong patch, sorry. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Extending help messages too. NACK, you edited wrong option --no-reverseDo not create reverse DNS zone. This option can be used multiple times Bad line, thanks for noticing, fixed patches attached. ./make-lint * Module ipaserver.install.ipa_replica_prepare ipaserver/install/ipa_replica_prepare.py:59: [E0001(syntax-error), ] invalid syntax) * Module ipa-replica-prepare install/tools/ipa-replica-prepare:21: [E0611(no-name-in-module), ] No name 'ipa_replica_prepare' in module 'ipaserver.install') Fixed. -- David Kupka From 3e4ff8de6670260d04db6cc5c5234d9797a78c8f Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Mon, 24 Nov 2014 08:49:05 -0500 Subject: [PATCH] Use singular in help metavars + update man pages. https://fedorahosted.org/freeipa/ticket/4695 --- install/tools/ipa-dns-install| 8 install/tools/ipa-replica-install| 5 +++-- install/tools/ipa-server-install | 8 +--- install/tools/man/ipa-dns-install.1 | 3 ++- install/tools/man/ipa-replica-install.1 | 3 ++- install/tools/man/ipa-replica-prepare.1 | 3 ++- install/tools/man/ipa-server-install.1 | 3 ++- ipaserver/install/ipa_replica_prepare.py | 8 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index 7d9bf6a8b223b586e7923137abec557036f650da..967057e1afae11873d2cd116d4385b0d79da8e14 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -43,16 +43,16 @@ def parse_options(): sensitive=True, help=admin password) parser.add_option(-d, --debug, dest=debug, action=store_true, default=False, help=print debugging information) -parser.add_option(--ip-address, dest=ip_addresses, +parser.add_option(--ip-address, dest=ip_addresses, metavar=IP_ADDRESS, default=[], action=append, - type=ip, ip_local=True, help=Master Server IP Address) + type=ip, ip_local=True, help=Master Server IP Address. This option can be used multiple times) parser.add_option(--forwarder, dest=forwarders, action=append, type=ip, help=Add a DNS forwarder. This option can be used multiple times) parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) parser.add_option(--reverse-zone, dest=reverse_zones, - default=[], action=append, - help=The reverse DNS zone to use) + default=[], action=append, metavar=REVERSE_ZONE, + help=The reverse DNS zone to use. This option can be used multiple times) parser.add_option(--no-reverse, dest=no_reverse, action=store_true, default=False, help=Do not create new reverse DNS zone) parser.add_option(--no-dnssec-validation, dest=no_dnssec_validation, action=store_true, diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index 409f1f61b42ccb2a4f2053689a7452f4fe344b4d..20d833d72d550ccaec2b56230a52d310185977ec 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -69,7 +69,7 @@ def parse_options(): default=False, help=configure a dogtag KRA) basic_group.add_option(--ip-address, dest=ip_addresses, type=ip, ip_local=True, action=append, default=[], - help=Replica server IP Address) + help=Replica server IP Address. This option can be used multiple times, metavar=IP_ADDRESS) basic_group.add_option(-p, --password, dest=password,
Re: [Freeipa-devel] [PATCH] 0033 Use singular in help metavars + update man pages.
On 26/11/14 12:37, David Kupka wrote: On 11/26/2014 12:23 PM, Martin Basti wrote: On 25/11/14 15:44, David Kupka wrote: On 11/25/2014 03:23 PM, Martin Basti wrote: On 25/11/14 13:16, David Kupka wrote: On 11/25/2014 09:57 AM, David Kupka wrote: On 11/25/2014 09:51 AM, David Kupka wrote: On 11/24/2014 03:59 PM, Martin Basti wrote: On 24/11/14 15:54, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4695 IMO this is one of two reasonable ways how to fix this ticket. The other one is to change just the manual page but it seems more consistent to use singular for metavars everywhere. I like this approach. But IMO we should instead of You ... form in help, this message, as we use with forwarders This option can be used multiple times Martin^2 Our manual pages are not exactly unified in that but let's stay with the majority. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Wrong patch, sorry. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Extending help messages too. NACK, you edited wrong option --no-reverseDo not create reverse DNS zone. This option can be used multiple times Bad line, thanks for noticing, fixed patches attached. ./make-lint * Module ipaserver.install.ipa_replica_prepare ipaserver/install/ipa_replica_prepare.py:59: [E0001(syntax-error), ] invalid syntax) * Module ipa-replica-prepare install/tools/ipa-replica-prepare:21: [E0611(no-name-in-module), ] No name 'ipa_replica_prepare' in module 'ipaserver.install') Fixed. Thanks! ACK -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On 11/25/2014 07:53 PM, Martin Basti wrote: On 12/11/14 16:34, Petr Spacek wrote: On 25.2.2014 15:05, Lukas Slebodnik wrote: On (25/02/14 09:54), Petr Spacek wrote: On 24.2.2014 18:56, Lukas Slebodnik wrote: On (24/02/14 16:48), Petr Spacek wrote: Hello, Drop unnecessary #define _BSD_SOURCE. -- Petr^2 Spacek From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 24 Feb 2014 16:48:09 +0100 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,8 +15,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE - #include isc/util.h #include string.h #include stdlib.h -- 1.8.5.3 Simo is an author (according to git blame) He defined this macro due to function setenv from man setenv: NAME setenv - change or add an environment variable SYNOPSIS #include stdlib.h int setenv(const char *name, const char *value, int overwrite); int unsetenv(const char *name); Feature Test Macro Requirements for glibc (see feature_test_macros(7)): setenv(), unsetenv(): _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included header file stdlib.h. I tested only on fedora 20. It can be used on the other distributions. I would rather let this macro as is. Wow, I didn't expect that somebody will spend time on this :-) See build logs from Fedora 21 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log You should have noticed this in the 1st mail. Because it is difference between removing unnecessary macro and depprecated usage of macro. /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Werror=cpp] # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE Patches with 'the right' solution are welcome. I'm not going to spend more time on this. Attached patch should fix the warning in the 'proper' way, I hope. Without this patch the warning constantly pops up on Fedora 21. Works for me, I haven't had warning there. ACK. Works for me, too. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0306] Improve info messages about number of defined/loaded zones
On 11/07/2014 01:33 PM, Petr Spacek wrote: Hello, Improve info messages about number of defined/loaded zones. ACK. The new message looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0287] Re-initialize NSS database after otptoken plugin tests
On 11/21/2014 11:47 AM, Tomas Babej wrote: Hi, OTP token tests do not properly reinitialize the NSS db, thus making subsequent xmlrpc tests fail on SSL cert validation. Make sure NSS db is re-initalized in the teardown method. https://fedorahosted.org/freeipa/ticket/4748 Note for reviewers: Requires Petr^3's pytest patchset, which I am pushing right now. Thank you! ACK, pushed to master: 792ff0c0c40ddd1583c6789c8f34382c050d3e92 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0676 Ignore ipap11helper/setup.py in doctests
A fix in test configuration. Pushed as one-liner to master: 4e99663379b333e2dd851f06a786c705c3e07e4b I've opened https://fedorahosted.org/freeipa/ticket/4769 to fix this a better way; either in upstream pytest (if they deem projects with multiple setup.py files as important enough), or in our own plugin. -- Petr³ From 6e5114f7def1cd60e8d6511cf5129b21552bbfc0 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 26 Nov 2014 06:13:34 +0100 Subject: [PATCH] Ignore ipap11helper/setup.py in doctests Pytest imports all modules when running doctests. The setup.py runs code on import, and raises an exception, depending on globa connand-line arguments, so it needs to be ignored. Also, pytest dislikes multiple top-level modules with the same name (setup in this case). Again ignoring is the way to go. --- ipatests/pytest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/ipatests/pytest.ini b/ipatests/pytest.ini index 3531482d266758b2c31b9775994b16b92bc23bcf..275593682673f25baf75a64ffd90ef84a80d3c4d 100644 --- a/ipatests/pytest.ini +++ b/ipatests/pytest.ini @@ -23,3 +23,4 @@ addopts = --doctest-modules --ignore=install/share/copy-schema-to-ca.py --ignore=install/share/wsgi.py --ignore=ipapython/py_default_encoding/setup.py + --ignore=ipapython/ipap11helper/setup.py -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0309] Fix crash caused by interaction between forward and master zones
On 11/25/2014 07:07 PM, Martin Basti wrote: On 25/11/14 18:11, Petr Spacek wrote: Hello, Fix crash caused by interaction between forward and master zones. LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns object were incorrectly processed using update_zone() in cases where forward zone sub.example.com. existed in LDAP as object idnsName=sub.example.com, cn=dns. https://fedorahosted.org/bind-dyndb-ldap/ticket/145 Tomas and Martin^2, please review it ASAP. Thank you! Works for me, IPA tests passed, can you Tomas check the code before push? ACK. The patch looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0308] Improve detection of BIND 9 isc__errno2result header file
On 11/25/2014 07:48 PM, Martin Basti wrote: On 12/11/14 16:11, Petr Spacek wrote: Hello, Improve detection of BIND 9 isc__errno2result header file. This header file is not in standard distribution so normal isc-config.sh detection is not enough. With this patch, ./configure should work even without explicit CFLAGS and it should also detect that bind-devel or bind-lite-devel packages are missing. Works for me ACK. Works for me, too. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0307] Send DNS NOTIFY message after any modification to the zone
On 07/11/14 15:34, Petr Spacek wrote: Hello, Send DNS NOTIFY message after any modification to the zone. https://fedorahosted.org/bind-dyndb-ldap/ticket/144 Works for me. But don't push it before Tomas check the code please. Martin^2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0310] Fix misleading error message about forward zones on reconnect
On 11/25/2014 07:25 PM, Martin Basti wrote: On 25/11/14 18:27, Petr Spacek wrote: Hello, Fix misleading error message about forward zones on reconnect. Previously the plugin could log 'already exist' error after successful reconnection to LDAP for each active forward zone. Now it prints message: forward zone 'fw.example.com': loaded Log looks better now, ACK if Tomas has no objections. ACK. Looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Release notes of FreeIPA 4.1.2
Hi all, FreeIPA 4.1.2 release was created yesterday. The only missing step is sending an announce mail. Here's release notes draft: - http://www.freeipa.org/page/Releases/4.1.2 Feel free to amend. I'll wait 2 hours and then send it. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] ipa-managed-entries prompts for password with -p option and incorrect passowrd
On 24/11/14 04:43, Gabe Alford wrote: Hello, I have a fix for https://fedorahosted.org/freeipa/ticket/4089 thanks, Gabe Thank you! ACK -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0033 Use singular in help metavars + update man pages.
On 11/26/2014 01:04 PM, Martin Basti wrote: On 26/11/14 12:37, David Kupka wrote: Fixed. Thanks! ACK Pushed to ipa-4-1: 2f8c4e7b165609706a4af9f680579c0e47edaeca Pushed to master: 3a6d714bb229f8dd68ae219d94283f05cf57a6d7 -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] ipa-managed-entries prompts for password with -p option and incorrect passowrd
On 11/26/2014 02:17 PM, Martin Basti wrote: On 24/11/14 04:43, Gabe Alford wrote: Hello, I have a fix for https://fedorahosted.org/freeipa/ticket/4089 thanks, Gabe Thank you! ACK Pushed to master: 45dbd12d8886ca2025bcab5b10ec5e004af3d9ab -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
On (12/11/14 15:30), Petr Spacek wrote: On 24.7.2014 11:00, Petr Spacek wrote: On 27.2.2014 15:19, Lukas Slebodnik wrote: ehlo, I did some reviews of bind-dyndb-ldap last week and it was little bit annoying to export special CFLAGS for bind9 header files. It can be automatically detected in configure script using utility isc-config. Attached patch should improve this and CFLAGS needn't be exported. Kind NACK. It would be valuable to test if isc/errno2result.h header is present and throw appropriate error. Current check with isc-config.sh only will pass if you have bind-devel package installed but you are missing bind-lite-devel package. I have a question: How +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99 works? Will it take user-defined CFLAGS into account? I would like to place user-defined flags at the end of the list so you can easily override settings given by autotools. Thank you for clarification :-) I will be really happy to commit complete fix. Thank you for cleaning this autotools mess! This version actually works. Previous version did not take CFLAGS from isc-config.sh into account during libdns version check so it actually did not work at all :-) Please review it (and send me a modified patch if you see a problem). Thank you for your time! -- Petr^2 Spacek From 4b17099abe2169ddb86b24e53cd2769b76f3ea2d Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lsleb...@redhat.com Date: Tue, 25 Feb 2014 10:46:50 +0100 Subject: [PATCH] Improve detection of BIND 9 header files and necessary CFLAGS. BIND 9 header files can be stored in non-default path (/usr/include/bind9). The isc-config.sh utility can provide necessary CFLAGS. --- configure.ac | 43 ++- contrib/bind-dyndb-ldap.spec | 1 - 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index d471038ada54c07dcfc211c8a2572850e3b28205..c985908c760c974f7c02b6fa3d183e839bbeb9ad 100644 --- a/configure.ac +++ b/configure.ac @@ -15,14 +15,6 @@ m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) AC_PROG_CC AC_PROG_LIBTOOL -# Checks for libraries. -AC_CHECK_LIB([dns], [dns_name_init], [], - AC_MSG_ERROR([Install BIND9 development files])) -AC_CHECK_LIB([ldap], [ldap_initialize], [], - AC_MSG_ERROR([Install OpenLDAP development files])) -AC_CHECK_LIB([krb5], [krb5_cc_initialize], [], - AC_MSG_ERROR([Install Kerberos 5 development files])) - # Checks for header files. AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h]) @@ -47,6 +39,39 @@ AC_TRY_COMPILE([ [CFLAGS=$SAVED_CFLAGS AC_MSG_RESULT([no])]) +# Get CFLAGS from isc-config.sh +AC_ARG_VAR([BIND9_CFLAGS], + [C compiler flags for bind9, overriding isc-config.sh]) +AC_SUBST(BIND9_CFLAGS) + +dnl do not override enviroment variables BIND9_CFLAGS +if test -z $BIND9_CFLAGS; then ^ What is a purpose of this condition. IIRC AC_SUBST(BIND9_CFLAGS) should allow you to override BIND9_CFLAGS from command line. If it does not wok I need to fix it. LS ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0677 test_integration: Use python-pytest-multihost
Hello,
I have split FreeIPA's multi-host testing infrastructure into a separate
project. It is temoprarily available at:
https://github.com/encukou/pytest-multihost
and I will move it to fedorahosted as soon as it's approved:
https://fedorahosted.org/fedora-infrastructure/ticket/4605
RPMs for Fedora 20..rawhide and EPEL 7 are available in COPR:
https://copr.fedoraproject.org/coprs/pviktori/pytest-plugins/
This patch contains the necessary changes to FreeIPA. The tests
themselves are almost unchanged. FreeIPA specific parts (most
importantly, logging and environ-based configuration) are also left in.
--
Petr³
From 16778cff44ce1d271334032a41a02ccabad566d0 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 13 Nov 2014 16:23:56 +0100
Subject: [PATCH] test_integration: Use python-pytest-multihost
The core integration testing functionality was split into a separate
project. Use this project, and configure it for FreeIPA.
The mh (multihost) fixture is made available for integration tests.
Configuration based on environment variables is moved into a separate
module, to ease eventual deprecation.
---
freeipa.spec.in| 2 +-
ipatests/ipa-test-config | 5 +-
ipatests/ipa-test-task | 2 +
ipatests/pytest_plugins/integration.py | 117 +++---
ipatests/test_integration/base.py | 11 +-
ipatests/test_integration/config.py| 419 +++
ipatests/test_integration/env_config.py| 359 +
ipatests/test_integration/host.py | 238 +--
ipatests/test_integration/tasks.py | 2 +-
ipatests/test_integration/test_caless.py | 35 +-
.../test_forced_client_reenrollment.py | 6 +-
ipatests/test_integration/test_trust.py| 2 +-
ipatests/test_integration/transport.py | 443 -
ipatests/test_integration/util.py | 10 -
14 files changed, 524 insertions(+), 1127 deletions(-)
create mode 100644 ipatests/test_integration/env_config.py
delete mode 100644 ipatests/test_integration/transport.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9b12c20899e729cedacdee470f8f2b13250af4e0..f4218d4098204403aa86a66070439be3724461db 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -310,7 +310,7 @@ Requires: pytest = 2.6
Requires: python-paste
Requires: python-coverage
Requires: python-polib
-Requires: python-paramiko = 1.7.7
+Requires: python-pytest-multihost = 0.2
Conflicts: %{alt_name}-tests
Obsoletes: %{alt_name}-tests %{version}
diff --git a/ipatests/ipa-test-config b/ipatests/ipa-test-config
index dc94b8afb8afd6f24f0806a2fc2c74d445f2d336..6a3101f63ee5e16f675849e3390e91c39350326e 100755
--- a/ipatests/ipa-test-config
+++ b/ipatests/ipa-test-config
@@ -25,7 +25,7 @@ import argparse
import json
from ipalib.constants import FQDN
-from ipatests.test_integration import config
+from ipatests.test_integration import config, env_config
def main(argv):
@@ -92,7 +92,8 @@ def main(argv):
import yaml
return yaml.safe_dump(conf.to_dict(), default_flow_style=False)
else:
-return config.env_to_script(get_object(conf, args).to_env(**kwargs))
+env = get_object(conf, args).to_env(**kwargs)
+return env_config.env_to_script(env)
def get_object(conf, args):
diff --git a/ipatests/ipa-test-task b/ipatests/ipa-test-task
index 612974549363277fdfe101734cf9defc59c99ab8..d89af841de9f8558ca620989fb665e6f3e2c573c 100755
--- a/ipatests/ipa-test-task
+++ b/ipatests/ipa-test-task
@@ -248,6 +248,8 @@ class TaskRunner(object):
args = self.get_parser().parse_args(argv)
self.config = config.Config.from_env(os.environ)
+if not self.config:
+raise EnvironmentError('Multihost environment not configured')
logs_to_collect = {}
diff --git a/ipatests/pytest_plugins/integration.py b/ipatests/pytest_plugins/integration.py
index 5329e5190a78b37b3881fb3a5b7bd6b0c5ad215b..a6c09518ff4602d31eb37a9cbc27be3ae752ea29 100644
--- a/ipatests/pytest_plugins/integration.py
+++ b/ipatests/pytest_plugins/integration.py
@@ -24,10 +24,13 @@
import shutil
import pytest
+from pytest_multihost import make_multihost_fixture
from ipapython import ipautil
from ipapython.ipa_log_manager import log_mgr
-from ipatests.test_integration.config import get_global_config
+from ipatests.test_integration import tasks
+from ipatests.test_integration.config import Config
+from ipatests.test_integration.env_config import get_global_config
log = log_mgr.get_logger(__name__)
@@ -147,74 +150,86 @@ def integration_logs(class_integration_logs, request):
@pytest.yield_fixture(scope='class')
-def integration_config(request, class_integration_logs):
-Integration test Config object
+def mh(request, class_integration_logs):
+
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On (12/11/14 16:34), Petr Spacek wrote: On 25.2.2014 15:05, Lukas Slebodnik wrote: On (25/02/14 09:54), Petr Spacek wrote: On 24.2.2014 18:56, Lukas Slebodnik wrote: On (24/02/14 16:48), Petr Spacek wrote: Hello, Drop unnecessary #define _BSD_SOURCE. -- Petr^2 Spacek From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 24 Feb 2014 16:48:09 +0100 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,8 +15,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE - #include isc/util.h #include string.h #include stdlib.h -- 1.8.5.3 Simo is an author (according to git blame) He defined this macro due to function setenv from man setenv: NAME setenv - change or add an environment variable SYNOPSIS #include stdlib.h int setenv(const char *name, const char *value, int overwrite); int unsetenv(const char *name); Feature Test Macro Requirements for glibc (see feature_test_macros(7)): setenv(), unsetenv(): _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included header file stdlib.h. I tested only on fedora 20. It can be used on the other distributions. I would rather let this macro as is. Wow, I didn't expect that somebody will spend time on this :-) See build logs from Fedora 21 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log You should have noticed this in the 1st mail. Because it is difference between removing unnecessary macro and depprecated usage of macro. /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Werror=cpp] # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE Patches with 'the right' solution are welcome. I'm not going to spend more time on this. Attached patch should fix the warning in the 'proper' way, I hope. Without this patch the warning constantly pops up on Fedora 21. -- Petr^2 Spacek From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Wed, 12 Nov 2014 16:30:56 +0100 Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with _POSIX_C_SOURCE. See https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes --- src/krb5_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE +#define _POSIX_C_SOURCE 200112L /* setenv */ I'm not sure wheather it is good idea to define special value. It should be autodetected. One way could be to test available extensions in configure time AC_USE_SYSTEM_EXTENSIONS. or use _BSD_SOURCE conditioanally. diff --git a/configure.ac b/configure.ac index 9e33116..95a1440 100644 --- a/configure.ac +++ b/configure.ac @@ -30,7 +30,7 @@ AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h]) AC_TYPE_SIZE_T # Checks for library functions. -AC_CHECK_FUNCS([memset strcasecmp strncasecmp]) +AC_CHECK_FUNCS([memset strcasecmp strncasecmp setenv]) # Check if build chain supports symbol visibility AC_MSG_CHECKING([for -fvisibility=hidden compiler flag]) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d178720..8ce11b8 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,10 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include config.h +#ifndef HAVE_SETENV #define _BSD_SOURCE +#endif /* HAVE_SETENV */ #include isc/util.h #include string.h LS ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On 26.11.2014 16:47, Lukas Slebodnik wrote: On (12/11/14 16:34), Petr Spacek wrote: On 25.2.2014 15:05, Lukas Slebodnik wrote: On (25/02/14 09:54), Petr Spacek wrote: On 24.2.2014 18:56, Lukas Slebodnik wrote: On (24/02/14 16:48), Petr Spacek wrote: Hello, Drop unnecessary #define _BSD_SOURCE. -- Petr^2 Spacek From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 24 Feb 2014 16:48:09 +0100 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,8 +15,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE - #include isc/util.h #include string.h #include stdlib.h -- 1.8.5.3 Simo is an author (according to git blame) He defined this macro due to function setenv from man setenv: NAME setenv - change or add an environment variable SYNOPSIS #include stdlib.h int setenv(const char *name, const char *value, int overwrite); int unsetenv(const char *name); Feature Test Macro Requirements for glibc (see feature_test_macros(7)): setenv(), unsetenv(): _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included header file stdlib.h. I tested only on fedora 20. It can be used on the other distributions. I would rather let this macro as is. Wow, I didn't expect that somebody will spend time on this :-) See build logs from Fedora 21 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log You should have noticed this in the 1st mail. Because it is difference between removing unnecessary macro and depprecated usage of macro. /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Werror=cpp] # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE Patches with 'the right' solution are welcome. I'm not going to spend more time on this. Attached patch should fix the warning in the 'proper' way, I hope. Without this patch the warning constantly pops up on Fedora 21. -- Petr^2 Spacek From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Wed, 12 Nov 2014 16:30:56 +0100 Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with _POSIX_C_SOURCE. See https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes --- src/krb5_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE +#define _POSIX_C_SOURCE 200112L /* setenv */ I'm not sure wheather it is good idea to define special value. It should be autodetected. One way could be to test available extensions in configure time AC_USE_SYSTEM_EXTENSIONS. or use _BSD_SOURCE conditioanally. diff --git a/configure.ac b/configure.ac index 9e33116..95a1440 100644 --- a/configure.ac +++ b/configure.ac @@ -30,7 +30,7 @@ AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h]) AC_TYPE_SIZE_T # Checks for library functions. -AC_CHECK_FUNCS([memset strcasecmp strncasecmp]) +AC_CHECK_FUNCS([memset strcasecmp strncasecmp setenv]) # Check if build chain supports symbol visibility AC_MSG_CHECKING([for -fvisibility=hidden compiler flag]) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d178720..8ce11b8 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,10 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include config.h +#ifndef HAVE_SETENV #define _BSD_SOURCE +#endif /* HAVE_SETENV */ #include isc/util.h #include string.h Hello, thank you for your patch but I'm going to stick to standard POSIX way. NACK -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
On 26.11.2014 15:57, Lukas Slebodnik wrote: On (12/11/14 15:30), Petr Spacek wrote: On 24.7.2014 11:00, Petr Spacek wrote: On 27.2.2014 15:19, Lukas Slebodnik wrote: ehlo, I did some reviews of bind-dyndb-ldap last week and it was little bit annoying to export special CFLAGS for bind9 header files. It can be automatically detected in configure script using utility isc-config. Attached patch should improve this and CFLAGS needn't be exported. Kind NACK. It would be valuable to test if isc/errno2result.h header is present and throw appropriate error. Current check with isc-config.sh only will pass if you have bind-devel package installed but you are missing bind-lite-devel package. I have a question: How +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99 works? Will it take user-defined CFLAGS into account? I would like to place user-defined flags at the end of the list so you can easily override settings given by autotools. Thank you for clarification :-) I will be really happy to commit complete fix. Thank you for cleaning this autotools mess! This version actually works. Previous version did not take CFLAGS from isc-config.sh into account during libdns version check so it actually did not work at all :-) Please review it (and send me a modified patch if you see a problem). Thank you for your time! -- Petr^2 Spacek From 4b17099abe2169ddb86b24e53cd2769b76f3ea2d Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lsleb...@redhat.com Date: Tue, 25 Feb 2014 10:46:50 +0100 Subject: [PATCH] Improve detection of BIND 9 header files and necessary CFLAGS. BIND 9 header files can be stored in non-default path (/usr/include/bind9). The isc-config.sh utility can provide necessary CFLAGS. --- configure.ac | 43 ++- contrib/bind-dyndb-ldap.spec | 1 - 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index d471038ada54c07dcfc211c8a2572850e3b28205..c985908c760c974f7c02b6fa3d183e839bbeb9ad 100644 --- a/configure.ac +++ b/configure.ac @@ -15,14 +15,6 @@ m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) AC_PROG_CC AC_PROG_LIBTOOL -# Checks for libraries. -AC_CHECK_LIB([dns], [dns_name_init], [], -AC_MSG_ERROR([Install BIND9 development files])) -AC_CHECK_LIB([ldap], [ldap_initialize], [], -AC_MSG_ERROR([Install OpenLDAP development files])) -AC_CHECK_LIB([krb5], [krb5_cc_initialize], [], -AC_MSG_ERROR([Install Kerberos 5 development files])) - # Checks for header files. AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h]) @@ -47,6 +39,39 @@ AC_TRY_COMPILE([ [CFLAGS=$SAVED_CFLAGS AC_MSG_RESULT([no])]) +# Get CFLAGS from isc-config.sh +AC_ARG_VAR([BIND9_CFLAGS], + [C compiler flags for bind9, overriding isc-config.sh]) +AC_SUBST(BIND9_CFLAGS) + +dnl do not override enviroment variables BIND9_CFLAGS +if test -z $BIND9_CFLAGS; then ^ What is a purpose of this condition. IIRC AC_SUBST(BIND9_CFLAGS) should allow you to override BIND9_CFLAGS from command line. Don's ask me, it was in your original version of the patch :-) -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On (26/11/14 16:51), Petr Spacek wrote: On 26.11.2014 16:47, Lukas Slebodnik wrote: On (12/11/14 16:34), Petr Spacek wrote: On 25.2.2014 15:05, Lukas Slebodnik wrote: On (25/02/14 09:54), Petr Spacek wrote: On 24.2.2014 18:56, Lukas Slebodnik wrote: On (24/02/14 16:48), Petr Spacek wrote: Hello, Drop unnecessary #define _BSD_SOURCE. -- Petr^2 Spacek From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 24 Feb 2014 16:48:09 +0100 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,8 +15,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE - #include isc/util.h #include string.h #include stdlib.h -- 1.8.5.3 Simo is an author (according to git blame) He defined this macro due to function setenv from man setenv: NAME setenv - change or add an environment variable SYNOPSIS #include stdlib.h int setenv(const char *name, const char *value, int overwrite); int unsetenv(const char *name); Feature Test Macro Requirements for glibc (see feature_test_macros(7)): setenv(), unsetenv(): _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included header file stdlib.h. I tested only on fedora 20. It can be used on the other distributions. I would rather let this macro as is. Wow, I didn't expect that somebody will spend time on this :-) See build logs from Fedora 21 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log You should have noticed this in the 1st mail. Because it is difference between removing unnecessary macro and depprecated usage of macro. /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Werror=cpp] # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE Patches with 'the right' solution are welcome. I'm not going to spend more time on this. Attached patch should fix the warning in the 'proper' way, I hope. Without this patch the warning constantly pops up on Fedora 21. -- Petr^2 Spacek From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Wed, 12 Nov 2014 16:30:56 +0100 Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with _POSIX_C_SOURCE. See https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes --- src/krb5_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE +#define _POSIX_C_SOURCE 200112L /* setenv */ I'm not sure wheather it is good idea to define special value. It should be autodetected. One way could be to test available extensions in configure time AC_USE_SYSTEM_EXTENSIONS. or use _BSD_SOURCE conditioanally. diff --git a/configure.ac b/configure.ac index 9e33116..95a1440 100644 --- a/configure.ac +++ b/configure.ac @@ -30,7 +30,7 @@ AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h]) AC_TYPE_SIZE_T # Checks for library functions. -AC_CHECK_FUNCS([memset strcasecmp strncasecmp]) +AC_CHECK_FUNCS([memset strcasecmp strncasecmp setenv]) # Check if build chain supports symbol visibility AC_MSG_CHECKING([for -fvisibility=hidden compiler flag]) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d178720..8ce11b8 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,7 +15,10 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include config.h +#ifndef HAVE_SETENV #define _BSD_SOURCE +#endif /* HAVE_SETENV */ #include isc/util.h #include string.h Hello, thank you for your patch but I'm going to stick to standard POSIX way. NACK You don't know which version of posix is defined on machine. Hardcoded value is as bad. LS ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On 26.11.2014 13:04, Tomas Hozza wrote: On 11/25/2014 07:53 PM, Martin Basti wrote: On 12/11/14 16:34, Petr Spacek wrote: On 25.2.2014 15:05, Lukas Slebodnik wrote: On (25/02/14 09:54), Petr Spacek wrote: On 24.2.2014 18:56, Lukas Slebodnik wrote: On (24/02/14 16:48), Petr Spacek wrote: Hello, Drop unnecessary #define _BSD_SOURCE. -- Petr^2 Spacek From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 24 Feb 2014 16:48:09 +0100 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -15,8 +15,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#define _BSD_SOURCE - #include isc/util.h #include string.h #include stdlib.h -- 1.8.5.3 Simo is an author (according to git blame) He defined this macro due to function setenv from man setenv: NAME setenv - change or add an environment variable SYNOPSIS #include stdlib.h int setenv(const char *name, const char *value, int overwrite); int unsetenv(const char *name); Feature Test Macro Requirements for glibc (see feature_test_macros(7)): setenv(), unsetenv(): _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included header file stdlib.h. I tested only on fedora 20. It can be used on the other distributions. I would rather let this macro as is. Wow, I didn't expect that somebody will spend time on this :-) See build logs from Fedora 21 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log You should have noticed this in the 1st mail. Because it is difference between removing unnecessary macro and depprecated usage of macro. /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Werror=cpp] # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE Patches with 'the right' solution are welcome. I'm not going to spend more time on this. Attached patch should fix the warning in the 'proper' way, I hope. Without this patch the warning constantly pops up on Fedora 21. Works for me, I haven't had warning there. ACK. Pushed to master: 8ad9965136ab15f14cdb356a81a141575b2a84aa -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0309] Fix crash caused by interaction between forward and master zones
On 26.11.2014 13:33, Tomas Hozza wrote: On 11/25/2014 07:07 PM, Martin Basti wrote: On 25/11/14 18:11, Petr Spacek wrote: Hello, Fix crash caused by interaction between forward and master zones. LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns object were incorrectly processed using update_zone() in cases where forward zone sub.example.com. existed in LDAP as object idnsName=sub.example.com, cn=dns. https://fedorahosted.org/bind-dyndb-ldap/ticket/145 Tomas and Martin^2, please review it ASAP. Thank you! Works for me, IPA tests passed, can you Tomas check the code before push? ACK. Pushed to master: 584f9ceeef131145feb32a741a8f5dbc04b9a2cd -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0306] Improve info messages about number of defined/loaded zones
On 26.11.2014 13:07, Tomas Hozza wrote: On 11/07/2014 01:33 PM, Petr Spacek wrote: Hello, Improve info messages about number of defined/loaded zones. ACK. The new message looks good. Pushed to master: eb600df6af932292e0a15817710cfc674f5c952b -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
On 26.11.2014 12:33, Tomas Hozza wrote: On 11/12/2014 03:30 PM, Petr Spacek wrote: On 24.7.2014 11:00, Petr Spacek wrote: On 27.2.2014 15:19, Lukas Slebodnik wrote: ehlo, I did some reviews of bind-dyndb-ldap last week and it was little bit annoying to export special CFLAGS for bind9 header files. It can be automatically detected in configure script using utility isc-config. Attached patch should improve this and CFLAGS needn't be exported. Kind NACK. It would be valuable to test if isc/errno2result.h header is present and throw appropriate error. Current check with isc-config.sh only will pass if you have bind-devel package installed but you are missing bind-lite-devel package. I have a question: How +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99 works? Will it take user-defined CFLAGS into account? I would like to place user-defined flags at the end of the list so you can easily override settings given by autotools. Thank you for clarification :-) I will be really happy to commit complete fix. Thank you for cleaning this autotools mess! This version actually works. Previous version did not take CFLAGS from isc-config.sh into account during libdns version check so it actually did not work at all :-) Please review it (and send me a modified patch if you see a problem). Thank you for your time! ACK. Pushed to master: 2af6e66e251a0d142b8fa05d7ad5a95fb9946e3e -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0310] Fix misleading error message about forward zones on reconnect
On 26.11.2014 14:13, Tomas Hozza wrote: On 11/25/2014 07:25 PM, Martin Basti wrote: On 25/11/14 18:27, Petr Spacek wrote: Hello, Fix misleading error message about forward zones on reconnect. Previously the plugin could log 'already exist' error after successful reconnection to LDAP for each active forward zone. Now it prints message: forward zone 'fw.example.com': loaded Log looks better now, ACK if Tomas has no objections. ACK. Pushed to master: a0ef923d7074a2aff2089f6affcda94843fd9455 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0308] Improve detection of BIND 9 isc__errno2result header file
On 26.11.2014 13:39, Tomas Hozza wrote: On 11/25/2014 07:48 PM, Martin Basti wrote: On 12/11/14 16:11, Petr Spacek wrote: Hello, Improve detection of BIND 9 isc__errno2result header file. This header file is not in standard distribution so normal isc-config.sh detection is not enough. With this patch, ./configure should work even without explicit CFLAGS and it should also detect that bind-devel or bind-lite-devel packages are missing. Works for me ACK. Pushed to master: 3e364c72f79e3cec69fc4c3263cd0bccbc467bf5 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0036] Add missing python files to Makefile
Hello, Wondering if I could get a review. Updated patch attached. Thanks, Gabe On Tue, Nov 11, 2014 at 7:21 AM, Gabe Alford redhatri...@gmail.com wrote: Hello, Fix for https://fedorahosted.org/freeipa/ticket/4700 Thanks, Gabe From 4af61cbaafdadb1b338601699d160bd4a15db551 Mon Sep 17 00:00:00 2001 From: Gabe redhatri...@gmail.com Date: Wed, 26 Nov 2014 16:45:30 -0700 Subject: [PATCH] Add missing python files to Makefiles https://fedorahosted.org/freeipa/ticket/4700 --- ipaserver/install/Makefile.am | 38 +-- ipaserver/install/plugins/Makefile.am | 17 +--- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/ipaserver/install/Makefile.am b/ipaserver/install/Makefile.am index 9fcad4e77c93cf44ed5fcf3ff793233ba35482c1..4209f6cbec35f55f0a19988514e47fa7664b2666 100644 --- a/ipaserver/install/Makefile.am +++ b/ipaserver/install/Makefile.am @@ -3,20 +3,36 @@ NULL = appdir = $(pythondir)/ipaserver app_PYTHON = \ __init__.py \ + adtrustinstance.py \ bindinstance.py \ cainstance.py \ - dsinstance.py \ - ipaldap.py \ - krbinstance.py \ - httpinstance.py \ - ntpinstance.py \ - adtrustinstance.py \ + certs.py \ + dnskeysyncinstance.py \ + dogtaginstance.py \ + dsinstance.py \ + httpinstance.py \ + installutils.py \ + ipa_backup.py \ + ipa_cacert_manage.py \ + ipa_kra_install.py \ + ipa_ldap_updater.py \ + ipa_otptoken_import.py \ + ipa_replica_prepare.py \ + ipa_restore.py \ + ipa_server_certinstall.py \ + krainstance.py \ + krbinstance.py \ + ldapupdate.py \ + memcacheinstance.py \ + ntpinstance.py \ + odsexporterinstance.py \ + opendnssecinstance.py \ + otpdinstance.py \ + replication.py \ + schemaupdate.py \ service.py \ - installutils.py \ - replication.py \ - certs.py \ -ldapupdate.py \ -certmonger.py \ + sysupgrade.py \ + upgradeinstance.py \ $(NULL) EXTRA_DIST = \ diff --git a/ipaserver/install/plugins/Makefile.am b/ipaserver/install/plugins/Makefile.am index d651297ac141b0f05831e7fabbb9b561cdd239c7..f998da9c2f4c5aee4bc98ab3ddb75e747e8eb897 100644 --- a/ipaserver/install/plugins/Makefile.am +++ b/ipaserver/install/plugins/Makefile.am @@ -3,17 +3,20 @@ NULL = appdir = $(pythondir)/ipaserver/install app_PYTHON = \ __init__.py \ - baseupdate.py \ + adtrust.py \ + baseupdate.py \ + ca_renewal_master.py \ + dns.py \ fix_replica_agreements.py \ rename_managed.py \ - dns.py \ updateclient.py \ - update_services.py \ - update_anonymous_aci.py \ - update_pacs.py \ - update_referint.py \ - ca_renewal_master.py \ + update_idranges.py \ + update_managed_permissions.py \ + update_pacs.py \ + update_referint.py \ + update_services.py \ update_uniqueness.py \ + upload_cacrt.py \ $(NULL) EXTRA_DIST = \ -- 2.0.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
