Re: [Freeipa-devel] Automated Fedora update testing

[Lukas Slebodnik]

On (28/04/17 17:07), Adam Williamson wrote:
>Hi folks! I thought this might be of interest to the FreeIPA community,
>so I thought I'd write it up here in case anyone missed it elsewhere.
>
>I work on the Fedora QA team, and we have been using the openQA
>automated test system (developed by our friends at SUSE) to run various
>functional tests on Fedora composes for the last couple of years.
>
>As FreeIPA is considered a critical part of Fedora Server, we run a few
>tests that exercise FreeIPA. The tests set up a FreeIPA server, run
>some basic checks on it, and also enrol two systems as clients of the
>domain, one using the 'realm join' command directly, one using Cockpit.
>The client tests do some basic client functionality testing (getent,
>logging in as a domain user, changing passwords, etc.) and also test
>the web UI to some extent.
>
>Until recently we ran these tests only on Fedora's nightly development
>release distribution composes. Recently, though, we deployed some
>enhancements to our openQA setup that let us run tests on Fedora
>distribution updates as well, and have the results made visible through
>the Fedora update system (Bodhi). The tests are automatically run on
>any critical path package, and as of today, they are also run on any
>update containing any of a manually-tended list of FreeIPA-related
>packages:
>
>389-ds
>389-ds-base
>bind
>bind-dyndb-ldap
>certmonger
>ding-libs
>freeipa
>krb5-server
>pki-core
>sssd
>tomcat
>cockpit
>
>This means that for any Fedora update containing one of these or any
>critical path package, Fedora's openQA FreeIPA tests should run, and
>you should see the results in the Fedora update system (Bodhi). You can
>see the results in Bodhi by clicking the Automated Updates tab for any
>update. For instance, here's a recent 389-ds-base update for Fedora 26:
>
>https://bodhi.fedoraproject.org/updates/FEDORA-2017-15e2a038b2
>
>If you look at the Automated Tests tab, you can see passes for:
>
>update.server_role_deploy_domain_controller
>update.realmd_join_cockpit
>update.realmd_join_sssd
>
>indicating that this update didn't cause any problems for FreeIPA.
>Clicking on any test result will take you to the openQA page for the
>test, where you can diagnose failures and so on (explaining how to do
>this is a bit beyond the scope of this mail, please do ask me if you're
>interested!)
>
>I hope this stuff will help us avoid shipping updates that break
>FreeIPA (and other key components). If you have any questions,
>concerns, comments, or suggestions, please do ask!
>
>To anticipate one question: you can cause *all* the tests for an update
>to be re-run by editing the update in any way (you don't have to change
>the package loadout, just changing a single character in the
>description or something will do). If you think just one test result is
>bogus and want it re-run, currently, you'll have to ask someone with
>the necessary power - either me or Jan Sedlak (garretraziel on IRC).
>I'm in North America and he's in Europe, so we should have most
>timezones covered between us. We're hoping to set up a better mechanism
>for this in future.
>
>Note, if you're interested in the results for the nightly Fedora
>distribution composes, an email summary of the results for those is
>sent each time they're run to the Fedora test@ and devel@ lists, look
>for mails with "compose check report" in the subject. Any time any of
>the FreeIPA tests fails, the failure will be listed in the mail (passed
>tests are not specifically listed, just a count of them). I usually
>keep an eye on those results and analyze failures and file bugs,
>though.

Tested with sssd and it passed as well.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-8addfc0188

freeIPA has also upstream integration tests packaged in
python{2,3}-ipatests. They use pytest + python-pytest-multihost.

Will it be possible to run some of them in openQA?
e.g. test_installation.py (

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TESTING] Please test and add karma to pki-core-10.4.0-1

[Lukas Slebodnik]

On (17/03/17 12:14), Martin Babinsky wrote:
>A new update for Dogtag PKI (pki-core-10.4.0-1.fc25) landed it Fedora 25
>updates-testing yesterday.[1]
>
It was also pushed to fedora26
https://bodhi.fedoraproject.org/updates/FEDORA-2017-9cc27242c1

>I have already provided negative karma as the update broke CA clone deployment
>on FreeIPA replica install.
>
>It would be nice if you could test it and provide +1/-1 ASAP so that we can
>push it out before it hits stable and give Matthew a change to privode fixes.
>
The fastest will be if it will be unpushed by fedora maintainer
Adding mharmsen to CC.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (06/03/17 14:36), Tomas Krizek wrote:
>> I am sorry but I still did not get your point. Could you a little bit
>> ellaborate?
>In this case - build won't fail when you don't have the dependencies for
>linters.
But it will be *easier* to develop on other distributions if they run
"make lint". Therefore default should be yes.

And it there is a super-hero developers who wrote perfect code
and does not need run make lint then he/she can run
./configure --disable-pylint to skip check for installed pylint.

It would be clear enough after improving error message for missing pylint.

>> B) it is not just an optional dependency. I tried to explain in 1st mail
>> that it should be a recomended dependency.
>I agree it's recommended.
But would not be with current version of PR#502.

>> C) Could you explain how it will be easier to develop on debian/other
>> distribution if upstream does not recommend to run "make lint".
>It probably doesn't make it easier to develop for other distributions.
>But it may be easier for a new upstream contributor, when building the
>project doesn't require any extra dependencies.
>
The simplest way for new developers is to follow BUILD.txt. (use fedora :-)
And instructions there recommend to install all dependencies including pylint.
So they(new upstream contributors) would not be affected by missing pylint.
But people who would like to port freeipa on other distributions would be
affected by changing default from yes -> autodetect.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (06/03/17 14:38), Christian Heimes wrote:
>> B) it is not just an optional dependency. I tried to explain in 1st mail
>> that it should be a recomended dependency.
>
>Recommended != required
>
{Py,js}lint are not required ATM.
Just error message from configure is poorly phrased.

>Linting is a recommended tool for development. It's a totally optional
>thing for building and installing FreeIPA. The RPM spec is the best
>proof for that. Linting is not even a required tool for development. CI
>takes care of linting.
>
CI does not care of linting on other distributions.
At the moment lint is executed only on f25 in travis
(f26 of fedora rawhide, el7 are not covered)

ATM everything is fedora focused even BUILD.txt which is not ideal
from upstream POV. Therefore default checks in configure should not be focused
just for fedora.

>> C) Could you explain how it will be easier to develop on debian/other
>> distribution if upstream does not recommend to run "make lint".
>
>My PR does not discourage `make lint`.
>
But it does not recommend it; because missing pylint it is skipped with
combo "./configure && make install." in PR#502

So potential developers on other distributions needn't notice it
and with your PR they will not be able to run "make pylint"
in such situation; because the target will be disabled in case of
missing pylint.

Summary:
rcrit, jcholast[1] and lslebodn think that it is better for upstream
to have default yes.

cheimes(you) and tkrizek prefer autodetection.

Democracy works. Please do not change default value and
improve error messages with hint how to disable the optional
*lint dependencies at configure time.

LS

[1] https://github.com/freeipa/freeipa/pull/502#issuecomment-283569745

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (06/03/17 13:49), Tomas Krizek wrote:
>On 03/06/2017 01:44 PM, Lukas Slebodnik wrote:
>> On (06/03/17 13:35), Tomas Krizek wrote:
>>> On 03/03/2017 09:22 PM, Rob Crittenden wrote:
>>>> Lukas Slebodnik wrote:
>>>>> On (03/03/17 17:07), Lukas Slebodnik wrote:
>>>>>> ehlo,
>>>>>>
>>>>>> This is a small continuation fo discussin from pull request
>>>>>> "Make pylint and jsl optional" #502[1]
>>>>>>
>>>>>> Pylint and jslint are already optional because some downstream 
>>>>>> distributions
>>>>>> does not have such packages. This is a reason why desing document[2]
>>>>>> mention configuration options for disabling them.
>>>>>>   --disable-pylint --without-jslint
>>>>>>
>>>>>> Previusly (4.4) "pylint was executed" before building rpm packages.
>>>>>> This strict requirement was changed because "make lint" is executed
>>>>>> with each pull request in travis.
>>>>>>
>>>>>> It was changed in commits
>>>>>> master:
>>>>>>
>>>>>> * 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of 
>>>>>> pylint
>>>>>> * 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help 
>>>>>> message for jslint
>>>>>> * b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock
>>>>>>
>>>>>> The main intention of PR#502 [1] is to make it even more optional
>>>>>> and do not fail if pylint is not installed on machine.
>>>>>> In another words, changing default value from "yes" to "autodetect".
>>>>>> I think the main reason is that it is not obvious that it is an optional
>>>>>> dependency if you run just "./configure". But that can be improved with
>>>>>> better error message. @see attachments.
>>>> I was going to go into a history of why it was required (we pushed
>>>> broken changes into master) but in retrospect that doesn't really
>>>> matter. I've been out of mainline development for some time so don't
>>>> know your current processes, but I do have a question:
>>>>
>>>> Is it expected that ./configure && make && make install will result in
>>>> the bits in all the right places? We never had that expectation before
>>>> though I know Christian has been moving in that direction. Is that an
>>>> end goal? It would be nice for developing in-tree and pushing out micro
>>>> changes onto the current, live development system.
>>> If you provide correct paths to ./configure, yes - make && make install
>>> will place all the bits in the right places. I commonly use it with
>>> DESTDIR and sshfs, so I can develop locally and deploy to a remote
>>> machine without building RPMs.
>>>> If so, does it have checks for all the runtime dependencies or will you
>>>> still have to do a bunch of work afterward the make install?
>>> It doesn't check runtime dependencies. I install the freeipa rpms once
>>> to install dependencies and then use make && make install.
>>>> I've seen discussions about making freeIPA more accessible to the
>>>> average developer, which is good, but it is just so more complex than
>>>> the typical software because it is more about integration than most big
>>>> projects. So I don't know that this is every going to really be true.
>>>> Will it help the average dev install it? Sure, but then what? Will you
>>>> support such an install?
>>>>
>>>> If you want to disable the checks for *lint that is certainly your
>>>> prerogative but I see some downsides:
>>>>
>>>> - I used to setup new dev systems all the time and this is definitely
>>>> something I'd forget to do with some frequency
>>>> - As I understand it the checks will be executed by upstream before a
>>>> change is accepted so that's good but it adds a huge delay and the
>>>> requirement of a roundtrip to fix simple mistakes (happens all the time
>>>> in OpenStack).
>>> On-PR checks can handle this. When you need to fix a linter issue, you
>>> can install the dependencies and run make lint locally.
>>>> I think my question boils down to how many people will thi

Re: [Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (06/03/17 13:35), Tomas Krizek wrote:
>On 03/03/2017 09:22 PM, Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
>>> On (03/03/17 17:07), Lukas Slebodnik wrote:
>>>> ehlo,
>>>>
>>>> This is a small continuation fo discussin from pull request
>>>> "Make pylint and jsl optional" #502[1]
>>>>
>>>> Pylint and jslint are already optional because some downstream 
>>>> distributions
>>>> does not have such packages. This is a reason why desing document[2]
>>>> mention configuration options for disabling them.
>>>>   --disable-pylint --without-jslint
>>>>
>>>> Previusly (4.4) "pylint was executed" before building rpm packages.
>>>> This strict requirement was changed because "make lint" is executed
>>>> with each pull request in travis.
>>>>
>>>> It was changed in commits
>>>> master:
>>>>
>>>> * 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of 
>>>> pylint
>>>> * 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message 
>>>> for jslint
>>>> * b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock
>>>>
>>>> The main intention of PR#502 [1] is to make it even more optional
>>>> and do not fail if pylint is not installed on machine.
>>>> In another words, changing default value from "yes" to "autodetect".
>>>> I think the main reason is that it is not obvious that it is an optional
>>>> dependency if you run just "./configure". But that can be improved with
>>>> better error message. @see attachments.
>> I was going to go into a history of why it was required (we pushed
>> broken changes into master) but in retrospect that doesn't really
>> matter. I've been out of mainline development for some time so don't
>> know your current processes, but I do have a question:
>>
>> Is it expected that ./configure && make && make install will result in
>> the bits in all the right places? We never had that expectation before
>> though I know Christian has been moving in that direction. Is that an
>> end goal? It would be nice for developing in-tree and pushing out micro
>> changes onto the current, live development system.
>If you provide correct paths to ./configure, yes - make && make install
>will place all the bits in the right places. I commonly use it with
>DESTDIR and sshfs, so I can develop locally and deploy to a remote
>machine without building RPMs.
>> If so, does it have checks for all the runtime dependencies or will you
>> still have to do a bunch of work afterward the make install?
>It doesn't check runtime dependencies. I install the freeipa rpms once
>to install dependencies and then use make && make install.
>> I've seen discussions about making freeIPA more accessible to the
>> average developer, which is good, but it is just so more complex than
>> the typical software because it is more about integration than most big
>> projects. So I don't know that this is every going to really be true.
>> Will it help the average dev install it? Sure, but then what? Will you
>> support such an install?
>>
>> If you want to disable the checks for *lint that is certainly your
>> prerogative but I see some downsides:
>>
>> - I used to setup new dev systems all the time and this is definitely
>> something I'd forget to do with some frequency
>> - As I understand it the checks will be executed by upstream before a
>> change is accepted so that's good but it adds a huge delay and the
>> requirement of a roundtrip to fix simple mistakes (happens all the time
>> in OpenStack).
>On-PR checks can handle this. When you need to fix a linter issue, you
>can install the dependencies and run make lint locally.
>> I think my question boils down to how many people will this actually
>> benefit vs how much time will be lost resubmitting patches? I don't
>> think there is an easy answer for the first part but from my own
>> experience I'd expect fairly regularly for lint and pep8 errors.
>If someone often has this issue, the workflow can be modified to address
>it. For example, I've configured my repo to run to run pylint and pep8
>on the modified files before the commit.
>> On the other hand I guess this also will have the additional advantage
>> that make rpms will be significantly faster if you don't enable them.
>>
>> The --disable vs --without is what bugs me most about the

Re: [Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (03/03/17 17:07), Lukas Slebodnik wrote:
>ehlo,
>
>This is a small continuation fo discussin from pull request
>"Make pylint and jsl optional" #502[1]
>
>Pylint and jslint are already optional because some downstream distributions
>does not have such packages. This is a reason why desing document[2]
>mention configuration options for disabling them.
>   --disable-pylint --without-jslint
>
>Previusly (4.4) "pylint was executed" before building rpm packages.
>This strict requirement was changed because "make lint" is executed
>with each pull request in travis.
>
>It was changed in commits
>master:
>
>* 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint
>* 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for 
>jslint
>* b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock
>
>The main intention of PR#502 [1] is to make it even more optional
>and do not fail if pylint is not installed on machine.
>In another words, changing default value from "yes" to "autodetect".
>I think the main reason is that it is not obvious that it is an optional
>dependency if you run just "./configure". But that can be improved with
>better error message. @see attachments.
>
And with missing attachment :-)

LS
diff --git a/configure.ac b/configure.ac
index 31bfa8aaf..fee39fe4f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -384,7 +384,10 @@ if test x$PYLINT != xno; then
 AC_MSG_CHECKING([for Pylint])
 $PYTHON -m pylint --version > /dev/null
 if test "$?" != "0"; then
-AC_MSG_ERROR([cannot find pylint for $PYTHON])
+AC_MSG_ERROR([cannot find pylint for $PYTHON
+This feature is optional and aimed for checking issues in python code.
+You can skip this check wich configure time option --disable-pylint.
+])
 else
 AC_MSG_RESULT([yes])
 fi
@@ -402,7 +405,10 @@ dnl --without-jslint will set JSLINT=no
 [AC_PATH_PROG([JSLINT], [jsl])]
 )
 if test "x${JSLINT}" == "x"; then
-   AC_MSG_ERROR([cannot find JS lint])
+AC_MSG_ERROR([cannot find JS lint
+This feature is optional and aimed for web ui developers.
+You can skip this check wich configure time option --without-jslint
+])
 fi
 AC_SUBST([JSLINT])
 AM_CONDITIONAL([WITH_JSLINT], [test "x${JSLINT}" != "xno"])
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

ehlo,

This is a small continuation fo discussin from pull request
"Make pylint and jsl optional" #502[1]

Pylint and jslint are already optional because some downstream distributions
does not have such packages. This is a reason why desing document[2]
mention configuration options for disabling them.
   --disable-pylint --without-jslint

Previusly (4.4) "pylint was executed" before building rpm packages.
This strict requirement was changed because "make lint" is executed
with each pull request in travis.

It was changed in commits
master:

* 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint
* 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for 
jslint
* b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock

The main intention of PR#502 [1] is to make it even more optional
and do not fail if pylint is not installed on machine.
In another words, changing default value from "yes" to "autodetect".
I think the main reason is that it is not obvious that it is an optional
dependency if you run just "./configure". But that can be improved with
better error message. @see attachments.

  checking if source directory is a Git reposistory... yes
  checking for more warnings... no
  checking for Pylint... /usr/bin/python: No module named pylint
  configure: error: cannot find pylint for /usr/bin/python

Cristian wrote some explanation in pull request:
  Rational:
  pylint and jsl are not required to build FreeIPA. Both are useful
  developer tools. It's more user friendly to make both components
  optionally with default config arguments. There is no reason to
  fail building on a build system without development tools.

But there is also another opinion. pylint/jslint is not usefull just for
developers but also for packagers. I would personally encourage packagers
to run pylint as part of build. (it took just 2 minutes on my laptop 8 CPUs)
Pylint/jslint should check typical issues in downstream only patches
or with backported patches. My experience with optional dependencies for
unit tests is that packagers tend to remove them in case of failure in tests
and then forget to return back with next release because it is optional.
This is a reason why explicit ./configure --disable-pylint will be a reminder
for them to try run "make lint" with next release.

Cristian's version will not affect fedora developers because there is
recommendation to install all required dependencies for running "make lint"
  dnf builddep -b -D "with_lint 1" --spec freeipa.spec.in

However, there is not such simple way for other distributions
debian unstable[3]/ubuntu 16.04/openSUSE[4]. This is a reason why I
would prefer to keep default vaue to "yes" and just improve
error message. It will remind packagers/developers to run lint.
It does not force them to run it.

So the main question is whether we want to change default for configure
time options --enable-pylint --enable-jslint.

LS

[1] https://github.com/freeipa/freeipa/pull/502
[2] http://www.freeipa.org/page/V4/Build_system_refactoring
[3] https://anonscm.debian.org/cgit/pkg-freeipa/freeipa.git
[4] https://en.opensuse.org/Portal:FreeIPA

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

[Lukas Slebodnik]

On (28/02/17 12:17), Martin Basti wrote:
>
>
>On 28.02.2017 12:03, Petr Vobornik wrote:
>> On 02/28/2017 12:00 PM, Petr Vobornik wrote:
>> > On 02/27/2017 12:46 PM, Petr Vobornik wrote:
>> > > Hello list,
>> > > 
>> > > today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
>> > > will take place.
>> > > 
>> > > It is due to FedoraHosted sunset [2]. Both will be migrated to
>> > > pagure.io
>> > > [3].
>> > > 
>> > > During this migration it won't be possible to add new tickets and
>> > > comments to Trac or Pagure.
>> > > 
>> > > [1] https://fedorahosted.org/freeipa/
>> > > [2]
>> > > https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
>> > > [3] https://pagure.io/
>> > > 
>> > > Thank you for understanding,
>> > 
>> > Issue tracker and git repo were migrated. They can be used now.
>> > 
>> > https://pagure.io/freeipa
>> > 
>> > Additional steps will follow
>> > - redirection of old URLs to new
>> > - sync with github
>> > 
>> 
>> Also we need to setup rights for the repo.
>> 
>> I've created group 'freeipa'. My proposal is to add all people who had
>> git commit rights to the group. Set the group to have 'commit' right on
>> 'freeipa' pagure project.
>> 
>> Former admins can be added as admins to the project directly.
>> 
>> Martin2 is working on setting up sync with Git Hub:
>> - https://pagure.io/fedora-infrastructure/issue/5844
>> 
>
>and
>
>https://pagure.io/fedora-infrastructure/issue/5845
>
>Please do NOT push to old repository, for users of ipatool change your
>repositories to pagure and would be good to postpone pushing until mirroring
>to github is enabled.
>
The best is to asg on fedora-infrastructure to chown the git repo on
fedorahosted, so no one can push changes there.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] IPA-AD user authentication to Linux servers using ssh-key?

[Lukas Slebodnik]

On (30/12/16 15:02), Oucema Bellagha wrote:
>Hi folks,
>
>After establishing the trust between AD and IPA, users from AD can 
>authenticate to Linux servers using password, but I want to add a another 
>authentication method using ssh-key.
>
>I can add SSH-key to AD users by adding a new schema attribute but then the 
>ssh-key can't be managed by IPA.
>
>Is there another way to manage ssh-key for AD users from IPA ? or another 
>method simply allowing ssh-key authentication of AD users to Linux servers ?
>
If you have freeIPA >= 4.1.3 then you can try ipa overrides
https://fedorahosted.org/freeipa/ticket/4868

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] client-only FreeIPA build

[Lukas Slebodnik]

On (24/11/16 10:27), Petr Spacek wrote:
>On 23.11.2016 13:53, Lukas Slebodnik wrote:
>> On (22/11/16 11:25), Rob Crittenden wrote:
>>> Lukas Slebodnik wrote:
>>>> On (22/11/16 16:29), Petr Spacek wrote:
>>>>> On 22.11.2016 16:27, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 22.11.2016 16:04, Petr Spacek wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> the recent changes with regard to
>>>>>>> http://www.freeipa.org/page/V4/Integration_Improvements
>>>>>>> beg a question whether we should invest into supporting client-only 
>>>>>>> builds in
>>>>>>> FreeIPA build system.
>>>
>>> Note that the Integration efforts don't really apply. The client-only
>>> install is for doing client enrollment and integration can mean lots of
>>> things.
>>>
>>>>>>>
>>>>>>> Right now, FreeIPA can be built on all architectures we care about so 
>>>>>>> there is
>>>>>>> no incentive to invest into client-only build - this applies to 
>>>>>>> binary/RPM
>>>>>>> builds.
>>>>>>
>>>>>> Client-only build lowers the barrier for porting IPA to new platforms 
>>>>>> (porting
>>>>>> only client code is *much* easier than porting the whole thing), so I 
>>>>>> would
>>>>>> very much prefer if we kept it.
>>>>>
>>>>> Understood.
>>>>>
>>>> Agree about portability
>>>>
>>>> But upstream spec file needn't have such relicts.
>>>> The upstream spec file is pure fedora specific.
>>>
>>> The upstream spec is what is used to document and verify that the
>>> client-only build actually works.
>>>
>>> I also think it is a worthy goal to maintain.
>>>
>> Maintaing is not enough. It would be also good to test it.
>> 
>> And maybe it might be much simpler to have separate
>> spec file for client only build. Because too many if conditions
>> does not improve readability of spec file. But that's up to
>> others to decide what would be simpler.
>
>The maintenance cost you mention is the only con I can see.
>
>I think that if we decide to support it, client-only support should be part of
>configure machinery. It would enable packagers to simply run
>./configure --disable-server && make install
>and have the client installed. It would make easy to package it for whatever
>distro.
>
I didn't mention anything about spec file only solution
for client only build.

But too many optional features does not improve readability
in spec file.

We have many optional features in upstream sssd spec file.
e.g.
%configure \

   //snip

--disable-static \
--disable-rpath \
%if %{with sssd_user}
--with-sssd-user=sssd \
%endif
%{with_initscript} \
%{?with_syslog} \
%{?with_cifs_utils_plugin_option} \
%{?with_python3_option} \
%{?enable_polkit_rules_option} \
%{?enable_systemtap_opt} \
%{?experimental}

But there are also optional features which
are not coverent in umpstrema spec file
otherwise the spec file would not be maintanable.

e.g. --with-samba

But as I mention in previous mail its up to you
to decide whether client only build should
be handled in upstream spec file or in separate spec file.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] client-only FreeIPA build

[Lukas Slebodnik]

On (22/11/16 11:25), Rob Crittenden wrote:
>Lukas Slebodnik wrote:
>> On (22/11/16 16:29), Petr Spacek wrote:
>>> On 22.11.2016 16:27, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 22.11.2016 16:04, Petr Spacek wrote:
>>>>> Hello,
>>>>>
>>>>> the recent changes with regard to
>>>>> http://www.freeipa.org/page/V4/Integration_Improvements
>>>>> beg a question whether we should invest into supporting client-only 
>>>>> builds in
>>>>> FreeIPA build system.
>
>Note that the Integration efforts don't really apply. The client-only
>install is for doing client enrollment and integration can mean lots of
>things.
>
>>>>>
>>>>> Right now, FreeIPA can be built on all architectures we care about so 
>>>>> there is
>>>>> no incentive to invest into client-only build - this applies to binary/RPM
>>>>> builds.
>>>>
>>>> Client-only build lowers the barrier for porting IPA to new platforms 
>>>> (porting
>>>> only client code is *much* easier than porting the whole thing), so I would
>>>> very much prefer if we kept it.
>>>
>>> Understood.
>>>
>> Agree about portability
>> 
>> But upstream spec file needn't have such relicts.
>> The upstream spec file is pure fedora specific.
>
>The upstream spec is what is used to document and verify that the
>client-only build actually works.
>
>I also think it is a worthy goal to maintain.
>
Maintaing is not enough. It would be also good to test it.

And maybe it might be much simpler to have separate
spec file for client only build. Because too many if conditions
does not improve readability of spec file. But that's up to
others to decide what would be simpler.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] client-only FreeIPA build

[Lukas Slebodnik]

On (22/11/16 16:29), Petr Spacek wrote:
>On 22.11.2016 16:27, Jan Cholasta wrote:
>> Hi,
>> 
>> On 22.11.2016 16:04, Petr Spacek wrote:
>>> Hello,
>>>
>>> the recent changes with regard to
>>> http://www.freeipa.org/page/V4/Integration_Improvements
>>> beg a question whether we should invest into supporting client-only builds 
>>> in
>>> FreeIPA build system.
>>>
>>> Right now, FreeIPA can be built on all architectures we care about so there 
>>> is
>>> no incentive to invest into client-only build - this applies to binary/RPM
>>> builds.
>> 
>> Client-only build lowers the barrier for porting IPA to new platforms 
>> (porting
>> only client code is *much* easier than porting the whole thing), so I would
>> very much prefer if we kept it.
>
>Understood.
>
Agree about portability

But upstream spec file needn't have such relicts.
The upstream spec file is pure fedora specific.

>Wondering out loud: What prevents the "porter" from doing full build and then
>packaging only client bits? Yes, he has to install come of the dependencies
>for the build to pass but still, it is way easier than actually making server
>fully functional.
>
>Petr, are you going to allocate time for this soonish or should I open a
>ticket and forget about it for now?
>

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] client-only FreeIPA build

[Lukas Slebodnik]

On (22/11/16 16:04), Petr Spacek wrote:
>Hello,
>
>the recent changes with regard to
>http://www.freeipa.org/page/V4/Integration_Improvements
>beg a question whether we should invest into supporting client-only builds in
>FreeIPA build system.
>
>Right now, FreeIPA can be built on all architectures we care about so there is
>no incentive to invest into client-only build - this applies to binary/RPM 
>builds.
>
>
>The question is, do we need something special in build system for Integration
>Improvements effort? If not, can we drop the remains of client-only build?
>(They are not functional anyway so we should either drop them or fix them.)
>
What do you mean by "remains of client-only build"?
IIRC you drop this feature in the 1st patch set.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Configuring ipa-otpd error when selinux is enable

[Lukas Slebodnik]

On (08/11/16 10:29), 郑磊 wrote:
>Hello everyone,
>
>I have successfully set up the FreeIPA environment on Ubuntu when selinux is 
>disable. But when selinux is enable, there is a configuring ipa-otpd error 
>occurred. 
>
>The ipaserver-install.log shows following informations:
>2016-11-08T01:55:18Z DEBUG   [1/2]: starting ipa-otpd
>2016-11-08T01:55:18Z DEBUG Starting external process
>2016-11-08T01:55:18Z DEBUG args=/bin/systemctl is-active ipa-otpd.socket
>2016-11-08T01:55:18Z DEBUG Process finished, return code=3
>2016-11-08T01:55:18Z DEBUG stdout=inactive
>
>2016-11-08T01:55:18Z DEBUG stderr=
>2016-11-08T01:55:18Z DEBUG Loading StateFile from 
>'/var/lib/ipa/sysrestore/sysrestore.state'
>2016-11-08T01:55:18Z DEBUG Saving StateFile to 
>'/var/lib/ipa/sysrestore/sysrestore.state'
>2016-11-08T01:55:18Z DEBUG Starting external process
>2016-11-08T01:55:18Z DEBUG args=/bin/systemctl restart ipa-otpd.socket
>2016-11-08T01:55:18Z DEBUG Process finished, return code=1
>2016-11-08T01:55:18Z DEBUG stdout=
>2016-11-08T01:55:18Z DEBUG stderr=Job for ipa-otpd.socket failed. See 
>"systemctl status ipa-otpd.socket" and "journalctl -xe" for details.
>
>2016-11-08T01:55:18Z DEBUG Traceback (most recent call last):
>  File "/usr/lib/python2.7/dist-packages/ipaserver/install/service.py", line 
> 447, in start_creation
>run_step(full_msg, method)
>  File "/usr/lib/python2.7/dist-packages/ipaserver/install/service.py", line 
> 437, in run_step
>method()
>  File "/usr/lib/python2.7/dist-packages/ipaserver/install/service.py", line 
> 585, in __start
>self.restart()
>  File "/usr/lib/python2.7/dist-packages/ipaserver/install/service.py", line 
> 347, in restart
>self.service.restart(instance_name, capture_output=capture_output, 
> wait=wait)
>  File "/usr/lib/python2.7/dist-packages/ipaplatform/base/services.py", line 
> 301, in restart
>skip_output=not capture_output)
>  File "/usr/lib/python2.7/dist-packages/ipapython/ipautil.py", line 479, in 
> run
>raise CalledProcessError(p.returncode, arg_string, str(output))
>CalledProcessError: Command '/bin/systemctl restart ipa-otpd.socket' returned 
>non-zero exit status 1
>
>2016-11-08T01:55:18Z DEBUG   [error] CalledProcessError: Command 
>'/bin/systemctl restart ipa-otpd.socket' returned non-zero exit status 1
>2016-11-08T01:55:18Z DEBUG   File 
>"/usr/lib/python2.7/dist-packages/ipapython/admintool.py", line 171, in execute
>return_value = self.run()
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/cli.py", line 318, 
> in run
>cfgr.run()
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 310, 
> in run
>self.execute()
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 332, 
> in execute
>for nothing in self._executor():
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 372, 
> in __runner
>self._handle_exception(exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 394, 
> in _handle_exception
>six.reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 362, 
> in __runner
>step()
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 359, 
> in 
>step = lambda: next(self.__gen)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/util.py", line 81, 
> in run_generator_with_yield_from
>six.reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/util.py", line 59, 
> in run_generator_with_yield_from
>value = gen.send(prev_value)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 586, 
> in _configure
>next(executor)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 372, 
> in __runner
>self._handle_exception(exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 449, 
> in _handle_exception
>self.__parent._handle_exception(exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 394, 
> in _handle_exception
>six.reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 446, 
> in _handle_exception
>super(ComponentBase, self)._handle_exception(exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 394, 
> in _handle_exception
>six.reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 362, 
> in __runner
>step()
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/core.py", line 359, 
> in 
>step = lambda: next(self.__gen)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/util.py", line 81, 
> in run_generator_with_yield_from
>six.reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/ipapython/install/util.py", line 59, 
> in run_generator_with_yield_from
>value = gen.send(prev_value)
>  File 

Re: [Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

[Lukas Slebodnik]

On (15/08/16 15:30), Petr Spacek wrote:
>On 15.8.2016 15:07, Fraser Tweedale wrote:
>> On Mon, Aug 15, 2016 at 07:48:22AM +0200, Jan Cholasta wrote:
>>> On 12.8.2016 18:57, Petr Spacek wrote:
 On 12.8.2016 11:33, Jan Cholasta wrote:
> On 4.8.2016 18:18, Petr Vobornik wrote:
>> On 07/22/2016 07:13 AM, Fraser Tweedale wrote:
>>> On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote:
 Hi,

 On 14.7.2016 13:44, Fraser Tweedale wrote:
> Hi all,
>
> The attached patch includes SANs in cert-show output.  If you have
> certs with esoteric altnames (especially any that are more than just
> ASN.1 string types), please test with those certs.
>
> https://fedorahosted.org/freeipa/ticket/6022

 I think it would be better to have a separate attribute for each 
 supported
 SAN type rather than cramming everything into subject_alt_name. That 
 way if
 you care only about a single specific type you won't have to go 
 through all
 the values and parse them. Also it would allow you to use param types
 appropriate to the SAN types (DNSNameParam for DNS names, Principal for
 principal names, etc.)

 Nitpick: please don't mix moving existing stuff and adding new stuff 
 in a
 single patch.

>>> Updated patches attached.
>>>
>>> Patches 0092..0094 are refactors and bugfixes.
>>> Patch 0090-2 is the main feature (depends on 0092..0094).
>>>
>>> Thanks,
>>> Fraser
>>>
>>
>> bump for review
>
> Patch 0092: ACK
>
> Patch 0093: ACK
>
> Patch 0094: ACK
>
> Patch 0090:
>
> 1) Generic otherNames (san_other) do not work correctly. The OID is not
> included in the value and names with complex type other than 
> KerberosPrincipal
> are not parsed correctly. The value should include the OID and DER blob 
> of the
> name.
>
> 2) With --all, san_other should be included in the result for all 
> otherNames,
> even the known ones, to provide (limited) forward compatibility.
>
> 3) Do we have to support *all* the name types? I mean we could, for the 
> sake
> of completeness, but it might be easier to just keep the few ones we 
> actually
> care about (email, DNS name, principal name, UPN and directory name in 
> your
> patch 0095).

 As far as I remember this reasoning usually comes back to bite us into 
 butt.

 - "Implement only this subset, nobody else needs the other the of ...
 (whatever, e.g. DNS record type)."
 - "Yes my lord."

 Two months later:

 - "We need to support for XYZ. It was (already) late yesterday!"

 :-)
>>>
>>> Care to give a concrete example of when this actually happened? Because IIRC
>>> this happened once or twice, not "usually".
>
>I do not have list at hand, sorry. It is just my feeling.
>
>
>>> Anyway, I'm fine with whatever, my point was that additional effort needs to
>>> be put in to support "everything" and even then, we wouldn't actually
>>> support everything (two months later: "we need to support extension XYZ!").
>>>
>> Sure, but in this case it was minimal additional effort to support
>> even the esoteric name types - it is only for display after all; if
>> an uncommon altname is (somehow) there we can display it.
>
>That is the main point. The cost is minimal so why not to do it?
>
maybe because there would not be anyone who would write a test?
And feature without tests is bad feature.

Petr, are you volunteer?
If no then please stop bikeshading.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] ipa-kdb: Allow to build with samba 4.5

[Lukas Slebodnik]

On (09/08/16 17:08), Alexander Bokovoy wrote:
>On Tue, 09 Aug 2016, Lukas Slebodnik wrote:
>> On (09/08/16 14:59), Alexander Bokovoy wrote:
>> > On Fri, 05 Aug 2016, Lukas Slebodnik wrote:
>> > > ehlo,
>> > > 
>> > > attached patches fix a build of freeipa on fedora 25 and fedora rawhide.
>> > > IMHO, this change in krb5pac.h is an ABI change and samba guys should
>> > > also bump a SONAME to related (private?) libraries. I could not see it;
>> > > but maybe I overlooked it.
>> > It an interesting question which you might raise upstream. krb5pac.h is
>> > auto-generated from krb5pac.idl, the same happens for all IDL-based
>> > definitions. They are not versioned, though.
>> > 
>> I can ask but I am not sure which library is connected to the
>> header file "gen_ndr/krb5pac.h". If it is a internal library then
>> ABI change is not guaranted but ipa might have problems without requires.
>We have dependency to libndr-krb5pac.so.0(NDR_KRB5PAC_0.0.1)(64bit)
>
>The problem here is that we could get some heads up with ABI tracker
>(http://abi-laboratory.pro/tracker/) but Samba is not there. I can ask
>Andrey P. about adding it, though.
>
>Fedora's Taskotron has dist.abicheck but any attempt to get results for
>Samba builds caused server error for me.
>
I almost sent a mail to samba-devel about ABI change.
But I realized that it's not an ABI change because
the position of "struct dom_sid2 *" and "struct samr_RidWithAttributeArray"
was not changed. They were just wrapped into "struct
PAC_DOMAIN_GROUP_MEMBERSHIP"

It was just an API change.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] ipa-kdb: Allow to build with samba 4.5

[Lukas Slebodnik]

On (09/08/16 14:59), Alexander Bokovoy wrote:
>On Fri, 05 Aug 2016, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> attached patches fix a build of freeipa on fedora 25 and fedora rawhide.
>> IMHO, this change in krb5pac.h is an ABI change and samba guys should
>> also bump a SONAME to related (private?) libraries. I could not see it;
>> but maybe I overlooked it.
>It an interesting question which you might raise upstream. krb5pac.h is
>auto-generated from krb5pac.idl, the same happens for all IDL-based
>definitions. They are not versioned, though.
>
I can ask but I am not sure which library is connected to the
header file "gen_ndr/krb5pac.h". If it is a internal library then
ABI change is not guaranted but ipa might have problems without requires.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] ipa_pwd_extop: Fix warning declaration shadows previous

[Lukas Slebodnik]

On (08/08/16 13:30), thierry bordaz wrote:
>
>
>On 08/05/2016 02:16 PM, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> attached patches fixes few compiler warnings in ipa-extop.
>> Sorry for not following naming convention for patches.
>> But I do not remeber my numer and you will use github/pagure
>> anyway.
>> 
>> LS
>> 
>> 
>Hi Lukas,
>
>0001-ipa_pwd_extop-Fix-warning-decalration-shadows-previo.patch looks ok but
>there is a leak in the remaining code.
>In fact bind_sdn and target_sdn need to be freed (slapi_sdn_free())
>before the end of the 'if (dn)' statement.
>Do you want to fix it in your patch of should we use an other patch ?
>
>
>0002-ipa-pwd-extop-Fix-warning-assignment-discards-const-.patch is ok. Ack
>
If I it is not introduced by this patch then it will be better
to prepare another patch. "git blame" would be confusing.

Thank you for review.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0213] support multiple uid values in slapi-nis users map

[Lukas Slebodnik]

On (08/08/16 11:35), Alexander Bokovoy wrote:
>On Mon, 08 Aug 2016, Martin Basti wrote:
>> 
>> 
>> On 08.08.2016 09:34, Alexander Bokovoy wrote:
>> > When SSSD resolves AD users on behalf of slapi-nis, it can accept any
>> > user identifier, including user principal name (UPN) which may be
>> > different than the canonical user name which SSSD returns.
>> > 
>> > As result, the entry created by slapi-nis will be using canonical user
>> > name but the filter for search will refer to the original (aliased)
>> > name. The search will not match the newly created entry.
>> > 
>> > The issue is fixed  in slapi-nis-0.56.1 by returning two values for
>> > 'uid' attribute: the canonical one and the aliased one. This way the
>> > search will match.
>> > 
>> > Standard LDAP schema allows multiple values for 'uid' attribute. We
>> > actually use the same trick for 'cn' attribute in the groups map
>> > already.
>> > 
>> > https://fedorahosted.org/freeipa/ticket/6138
>> > 
>> > 
>> > 
>> > 
>> Hello,
>> 
>> should we bump requires to slapi-nis-0.56.1 in freeipa.spec?
>No, this is not required. In Fedora we'll submit a combined update --
>I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide already
>but did not submit a Bodhi request.
>
How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
  dnf update freeipa-server.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] ipa-kdb: Allow to build with samba 4.5

[Lukas Slebodnik]

ehlo,

attached patches fix a build of freeipa on fedora 25 and fedora rawhide.
IMHO, this change in krb5pac.h is an ABI change and samba guys should
also bump a SONAME to related (private?) libraries. I could not see it;
but maybe I overlooked it.

LS
>From 02db5adc82c36592f8aef5fd4d5e2f2e27f15b11 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 5 Aug 2016 08:29:27 +0200
Subject: [PATCH 1/2] ipa-kdb: Allow to build with samba 4.5

daemons/ipa-kdb/ipa_kdb_mspac.c: In function 'filter_logon_info':
daemons/ipa-kdb/ipa_kdb_mspac.c:1536:19: error: 'struct PAC_LOGON_INFO'
  has no member named 'res_group_dom_sid'
 if (info->info->res_group_dom_sid != NULL &&
   ^~
daemons/ipa-kdb/ipa_kdb_mspac.c:1537:19: error: 'struct PAC_LOGON_INFO'
  has no member named 'res_groups'; did you mean 'resource_groups'?
 info->info->res_groups.count != 0) {
   ^~
mv -f .deps/ipa_kdb_delegation.Tpo .deps/ipa_kdb_delegation.Plo
Makefile:806: recipe for target 'ipa_kdb_mspac.lo' failed
make[3]: *** [ipa_kdb_mspac.lo] Error 1
make[3]: *** Waiting for unfinished jobs

Related change in samba
https://github.com/samba-team/samba/commit/4406cf792a599724f55777a45efb6367a9bd92b2

Resolves:
https://fedorahosted.org/freeipa/ticket/6173
---
 daemons/configure.ac| 12 
 daemons/ipa-kdb/ipa_kdb_mspac.c |  9 +
 2 files changed, 21 insertions(+)

diff --git a/daemons/configure.ac b/daemons/configure.ac
index 
94d66d813728fe4e32f9e3c0eef920d8e2395d8f..5c5a1046397aa97ba18cafc1b81dc2a6fb2dfd34
 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -170,6 +170,18 @@ PKG_CHECK_MODULES([SAMBAUTIL], [samba-util])
 SAMBA40EXTRA_LIBPATH="-L`$PKG_CONFIG --variable=libdir samba-util`/samba 
-Wl,-rpath=`$PKG_CONFIG --variable=libdir samba-util`/samba"
 AC_SUBST(SAMBA40EXTRA_LIBPATH)
 
+bck_cflags="$CFLAGS"
+CFLAGS="$NDRPAC_CFLAGS"
+AC_CHECK_MEMBER(
+[struct PAC_DOMAIN_GROUP_MEMBERSHIP.domain_sid],
+[AC_DEFINE([HAVE_STRUCT_PAC_DOMAIN_GROUP_MEMBERSHIP], [1],
+   [struct PAC_DOMAIN_GROUP_MEMBERSHIP is available.])],
+[AC_MSG_NOTICE([struct PAC_DOMAIN_GROUP_MEMBERSHIP is not available])],
+ [[#include 
+   #include ]])
+
+CFLAGS="$bck_cflags"
+
 LIBPDB_NAME=""
 AC_CHECK_LIB([samba-passdb],
  [make_pdb_method],
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 
80e7055fd6cd7b962eeffbccc675a73d73700793..65cc03565dc06d1052c6acd0c0d6ee7265b37b36
 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -20,6 +20,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
+
 #include "ipa_kdb.h"
 #include "ipa_mspac.h"
 #include 
@@ -1533,10 +1535,17 @@ krb5_error_code filter_logon_info(krb5_context context,
 
 /* According to MS-KILE, ResourceGroups must be zero, so check
  * that it is the case here */
+#ifdef HAVE_STRUCT_PAC_DOMAIN_GROUP_MEMBERSHIP
+if (info->info->resource_groups.domain_sid != NULL &&
+info->info->resource_groups.groups.count != 0) {
+return EINVAL;
+}
+#else
 if (info->info->res_group_dom_sid != NULL &&
 info->info->res_groups.count != 0) {
 return EINVAL;
 }
+#endif
 
 return 0;
 }
-- 
2.9.2

>From 7d064bc2dda88552f597c1e8dfa2cf176a89ac77 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 5 Aug 2016 08:34:23 +0200
Subject: [PATCH 2/2] ipa-kdb: Fix unit test after packaging changes in krb5

Resolves:
https://fedorahosted.org/freeipa/ticket/6173
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
135e9c980011c6c2730c6c29a3c22098e48270d5..7b5bb906ea541da10e0a9f5f9970f5937728ee11
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -108,6 +108,8 @@ BuildRequires:  python-netifaces >= 0.10.4
 # Build dependencies for unit tests
 BuildRequires:  libcmocka-devel
 BuildRequires:  nss_wrapper
+# Required by ipa_kdb_tests
+BuildRequires:  %{_libdir}/krb5/plugins/kdb/db2.so
 
 %if 0%{?with_python3}
 BuildRequires:  python3-devel
-- 
2.9.2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Broken IPA installations on F24

[Lukas Slebodnik]

On (03/08/16 14:17), Martin Basti wrote:
>Hello all,
>
>
>update resteasy-*-3.0.17 from updates-testing prevents IPA (PKI CA) to be
>installed on f24,
>
>ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to configure CA
>instance: Command '/usr/sbin/pkispawn -s CA -f /tmp/tmpEQulGP' returned
>non-zero exit status 1
>ipa.ipaserver.install.cainstance.CAInstance: CRITICAL See the installation
>logs and the following files/directories for more information:
>ipa.ipaserver.install.cainstance.CAInstance: CRITICAL /var/log/pki/pki-tomcat
>  [error] RuntimeError: CA configuration failed.
>ipa.ipapython.install.cli.install_tool(Server): ERRORCA configuration
>failed.
>ipa.ipapython.install.cli.install_tool(Server): ERRORThe
>ipa-server-install command failed. See /var/log/ipaserver-install.log for
>more information
>
>Workaround:
>
># dnf downgrade  resteasy-atom-provider resteasy-client resteasy-core
>resteasy-jackson-provider resteasy-jaxb-provider --allowerasing
>
Fraser,
Is a bug in resteasy or dogtag?

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0023 Bug in the ipapwd plugin

[Lukas Slebodnik]

On (13/07/16 16:50), thierry bordaz wrote:
>https://fedorahosted.org/freeipa/ticket/6030

>>From 4efedc5e674db92f9f7c160429df543422ed8afb Mon Sep 17 00:00:00 2001
>From: Thierry Bordaz 
>Date: Wed, 13 Jul 2016 15:34:20 +0200
>Subject: [PATCH] Ticket 6030 Bug in the ipapwd plugin
>
>ipapwd_encrypt_encode_key allocates 'kset' on the heap but
>with num_keys and keys not being initialized.
>Then ipa_krb5_generate_key_data initializes them with the
>generated keys.
>If ipa_krb5_generate_key_data fails (here EINVAL meaning no
>principal->realm.data), num_keys and keys are left uninitialized.
>Upon failure, ipapwd_keyset_free is called to free 'kset'
>that contains random num_keys and keys.
>
>allocates kset with calloc so that kset->num_keys==0 and
>kset->keys==NULL
>
>https://fedorahosted.org/freeipa/ticket/6030
>---
> daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c 
>b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>index 5ca155d..46bf79a 100644
>--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>@@ -148,7 +148,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct 
>ipapwd_krbcfg *krbcfg,
> pwd.length = strlen(data->password);
> }
> 
>-kset = malloc(sizeof(struct ipapwd_keyset));
>+kset = calloc(sizeof(struct ipapwd_keyset));
I though that calloc need two arguments

man malloc says:
   void *malloc(size_t size);
   void *calloc(size_t nmemb, size_t size);

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] Fix minor typos

[Lukas Slebodnik]

On (05/07/16 18:21), Florence Blanc-Renaud wrote:
>On 07/04/2016 08:30 PM, Yuri Chornoivan wrote:
>> Hi,
>> 
>> A fresh portion of typo fixes.
>> 
>> Thanks in advance for reviewing.
>> 
>> Best regards,
>> Yuri
>> 
>> 
>Hi Yuri,
>
>thanks for your patch!
>
>The patch introduces new pep8/pep257 complaints that you can detect using
>"git diff HEAD~$NUMBER_OF_PATCHES -U0 | pep8 --diff" (mainly due to line
>length).
>The code was not be compliant before your patch but it is a good practice to
>fix the existing issue if you modify the same line.
>
>You can find more information regarding coding style in the wiki page
>Contribute/Code [1]
>
>Apart from this, I have the following comment:
>- DOM\name is one of the Active Directory formats used to specify a domain
>and a name. Your fix should rather escape the \ so that it is kept inside the
>docstring.
>
Maybe it would be better to user user(username) instead of name
e.g. 'DOM\username', or 'username@domain'

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains

[Lukas Slebodnik]

On (01/07/16 11:13), Lenka Doudova wrote:
>And, of course, a patch file :)
>
>
>On 07/01/2016 11:09 AM, Lenka Doudova wrote:
>> Hi all,
>> 
>> here's patch with basic test suite for support of UPN.
>> 
>> Note: it needs to be applied on top of my patch 0025.2 (or later, if
>> there's will be more fixes to that patch).
>> 
>> 
>> Lenka
>> 
>

>From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001
>From: Lenka Doudova 
>Date: Fri, 1 Jul 2016 11:00:57 +0200
>Subject: [PATCH] Tests: Support of UPN for trusted domains
>
>Basic set of tests to verify support of UPN functionality.
>
>Test cases:
>- establish trust
>- verify the trust recognizes UPN
>- verify AD user with UPN can be resolved
>- verify AD user with UPN can authenticate
>- remove trust
>
>https://fedorahosted.org/freeipa/ticket/5354
>---
> ipatests/test_integration/test_trust.py | 32 
> 1 file changed, 32 insertions(+)
>
>diff --git a/ipatests/test_integration/test_trust.py 
>b/ipatests/test_integration/test_trust.py
>index 
>d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84
> 100644
>--- a/ipatests/test_integration/test_trust.py
>+++ b/ipatests/test_integration/test_trust.py
>@@ -388,3 +388,35 @@ class TestExternalTrustWithRootDomain(ADTrustBase):
> 
> tasks.remove_trust_with_ad(self.master, self.ad_domain)
> tasks.clear_sssd_cache(self.master)
>+
>+
>+class TestTrustWithUPN(ADTrustBase):
>+"""
>+Test support of UPN for trusted domains
>+"""
>+def test_upn_in_nonposix_trust(self):
>+""" Check that UPN is listed as trust attribute """
>+result = self.master.run_command(['ipa', 'trust-show', self.ad_domain,
>+  '--all', '--raw'])
>+
>+assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text
>+
>+def test_upn_user_resolution_in_nonposix_trust(self):
>+""" Check that user with UPN can be resolved """
>+upnuser = 'upnu...@upnsuffix.com'
>+result = self.master.run_command(['getent', 'passwd', upnuser])
Is there a special reason for not using pwd.getpwnam() ?

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Lukas Slebodnik]

On (28/06/16 14:50), Alexander Bokovoy wrote:
>On Tue, 28 Jun 2016, Milan Kubík wrote:
>> > > It's real packaging bug and have to be fixed.
>> > Right.
>> > 
>> > > The same files are owned by two packages even though one depens
>> > > on other.
>> > > Milan, please fiel a fedora bug.
>> > > 
>> > > [root@5946ca9bf02b /]# rpm -q pki-server pki-base
>> > > pki-server-10.3.3-1.fc24.noarch
>> > > pki-base-10.3.3-1.fc24.noarch
>> > > 
>> > > [root@5946ca9bf02b /]# rpm -ql pki-base | grep pkiparser.py
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo
>> > > 
>> > > [root@5946ca9bf02b /]# rpm -ql pki-server | grep pkiparser.py
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo
>> > This is not a real bug:
>> > https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
>> > 
>> > 
>> > In most cases, it should not be necessary for multiple packages to
>> > contain identical copies of the same file. However, if it is necessary,
>> > multiple packages may contain identical copies of the same file, as long
>> > as the following requirements are met:
>> > 
>> >   The packages sharing ownership of the identical files are built from
>> > a single SRPM.
>> > OR
>> > 
>> >   The packages sharing ownership of the identical files are not in a
>> > dependency chain (e.g. if package A requires package B, they should not
>> > both contain identical files, either A or B must own the common files,
>> > but not both.) 
>> > --
>> > 
>> > The bug here is that they come from different repositories and thus from
>> > the dnf/yum point of view are built from different source packages.
>> > 
>> > > 
>> > > [root@5946ca9bf02b /]# rpm -q --requires pki-server | grep pki
>> > > pki-base = 10.3.3-1.fc24
>> > > pki-base-java = 10.3.3-1.fc24
>> > > pki-tools = 10.3.3-1.fc24
>> > > 
>> > > LS
>> > 
>> The bug is exactly the violation of the second clause. pki-server
>> requires pki-base while both own the files.
>Right -- what I wanted to point is that the conflict you get from dnf is
>not due to violating this policy, it is due to the files ownership
>coming from subpackages built from different source packages. Package
>policy violations are not always (not in this case) enforced by
>dnf/yum/rpm.
If there was not a violation of packaging guidelines then it would not be a
problem if pki-base is from one repository and pki-server from other.

It has to work because they are built from the same source.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Lukas Slebodnik]

On (28/06/16 10:57), Alexander Bokovoy wrote:
>On Tue, 28 Jun 2016, Petr Vobornik wrote:
>> On 06/27/2016 08:11 PM, Lukas Slebodnik wrote:
>> > On (27/06/16 17:55), Milan Kubík wrote:
>> > > Hi all,
>> > > 
>> > > the pki packages that are currently in the COPR repo [1] are broken. 
>> > > There is
>> > > a conflict between pki-server and pki-base:
>> > > 
>> > > Error: Transaction check error:
>> > >  file 
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
>> > > from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
>> > > package pki-base-10.3.3-1.fc24.noarch
>> > >  file 
>> > > /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
>> > > from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
>> > > package pki-base-10.3.3-1.fc24.noarch
>> > > 
>> > > [1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/
>> > > 
>> > I can see the same with pki-core in fedora 24 updates-testing.
>> > File a fedora bug.
>> > 
>> > LS
>> > 
>> 
>> Right, even though I can't reproduce, the package in freeipa-master copr
>> should be the same as the one in updates testing. It was built from the
>> same srpm:
>> https://kojipkgs.fedoraproject.org//packages/pki-core/10.3.3/1.fc24/src/pki-core-10.3.3-1.fc24.src.rpm
>One particular issue could be that you have pki-server installed from
>one source and pki-base considered from a different one. For yum and dnf
>there is a difference where the package comes from and all subpackages
>of the same source package should be coming from the same repository to
>avoid conflicts like this because after install the package keeps its
>source repo mark.
It is not a "particular issue".
It's real packaging bug and have to be fixed.

The same files are owned by two packages even though one depens on other.
Milan, please fiel a fedora bug.

[root@5946ca9bf02b /]# rpm -q pki-server pki-base
pki-server-10.3.3-1.fc24.noarch
pki-base-10.3.3-1.fc24.noarch

[root@5946ca9bf02b /]# rpm -ql pki-base | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

[root@5946ca9bf02b /]# rpm -ql pki-server | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

[root@5946ca9bf02b /]# rpm -q --requires pki-server | grep pki
pki-base = 10.3.3-1.fc24
pki-base-java = 10.3.3-1.fc24
pki-tools = 10.3.3-1.fc24

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Lukas Slebodnik]

On (27/06/16 17:55), Milan Kubík wrote:
>Hi all,
>
>the pki packages that are currently in the COPR repo [1] are broken. There is
>a conflict between pki-server and pki-base:
>
>Error: Transaction check error:
>  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
> from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
> package pki-base-10.3.3-1.fc24.noarch
>  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
> from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
> package pki-base-10.3.3-1.fc24.noarch
>
>[1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/
>
I can see the same with pki-core in fedora 24 updates-testing.
File a fedora bug.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (24/06/16 21:09), Alexander Bokovoy wrote:
>On Fri, 24 Jun 2016, Lukas Slebodnik wrote:
>> > > > > ah sorry, since 1.14.0 is not release yet we use 1.13.9x to track the
>> > > > > alpha and beta releases and still have incrementing version numbers. 
>> > > > > So,
>> > > > > it might be better to use '>= 1.13.90' in the spec file instead of
>> > > > > '1.14.0'.
>> > > > +1, At this point '>= 1.13.90' should be safe.
>> > > -1
>> > > I vote for official release.
>> > > I cannot see a reason why this patch should be pushed immediately.
>> > > 1.13.90 is just a sssd convention for alpha release and it can be 
>> > > confusing for
>> > > others whyt there isn't tarball for 1.13.90
>> > It would allow git master to be built against sssd git master without
>> > additional problems. It is consistency here that I'm after.
>> > 
>> It allows it even now and without this patch.
>> I'm sorry I miss a logic here.
>No,
That's not true.

You wrote: "It would allow git master to be built"
against sssd git master"
^^
One more time. This patch will not change that
because you can build freeipa git master
against "sssd git master" even now.

It's not my problem that freeipa requires git master of other project.
But it does not mean that you need to officialy requires
some weird version "1.13.90". It is really confusing.

>it does not prevent you from running with the code that does not
>have needed support. 1.13.4 has no extdom certificate request support
>so you would need to make sure you are actually installing the correct
>SSSD packages manually while changing the version to 1.13.90 would make
>clear we demand a specific functionality.
Building freeipa and installing are two different things.
If you need to install freeipa git master then you need to use
extra copr anyway because:
A) you cannot install freeipa master in rawhide because there isn't
   pki-core-10.3.3-1.fc25
b) you cannot install freeipa master on fedora 24 due lots of missing
   dependencies (including libsss_nss_idmap-1.14.0)

If one would like to install freeipa git master rpms without copr
then he/she will not able to do it on fedora 24 without new libsss_nss_idmap
because dnf will not be able to find dependency
  "libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.2.0)(64bit)"

>It is a very small thing, of
>course, but helpful to those who have to deal with rebases/updates of
>their distribution packages and have not been following freeipa-devel@
>list in detail. At the very least the inability to find 1.13.90 in a
>regular place would cause question being asked.
>
It's helpful but confusing. That's the reason why we should avoid using
1.13.90. I doubt that anyone will try to use alpha version of sssd or freeipa
on other distributions (debian, ubuntu) especially if they do not work reliably
on fedora. So there's no reason for a rush.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (24/06/16 20:43), Alexander Bokovoy wrote:
>On Fri, 24 Jun 2016, Lukas Slebodnik wrote:
>> On (24/06/16 20:00), Alexander Bokovoy wrote:
>> > On Fri, 24 Jun 2016, Sumit Bose wrote:
>> > > On Fri, Jun 24, 2016 at 05:53:27PM +0200, Martin Basti wrote:
>> > > >
>> > > >
>> > > > On 24.06.2016 15:09, Martin Basti wrote:
>> > > > >
>> > > > >
>> > > > > On 24.06.2016 14:59, Sumit Bose wrote:
>> > > > > > On Fri, Jun 24, 2016 at 02:00:24PM +0200, Martin Basti wrote:
>> > > > > > >
>> > > > > > > On 22.06.2016 23:20, Lukas Slebodnik wrote:
>> > > > > > > > On (22/06/16 11:57), Martin Basti wrote:
>> > > > > > > > > On 09.06.2016 21:02, Martin Basti wrote:
>> > > > > > > > > > On 09.06.2016 14:45, Martin Basti wrote:
>> > > > > > > > > > > On 09.06.2016 14:42, Martin Basti wrote:
>> > > > > > > > > > > > On 09.06.2016 14:38, Lukas Slebodnik wrote:
>> > > > > > > > > > > > > On (09/06/16 14:29), Martin Basti wrote:
>> > > > > > > > > > > > > > On 09.06.2016 14:22, Alexander Bokovoy wrote:
>> > > > > > > > > > > > > > > On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>> > > > > > > > > > > > > > > > On Fri, May 20, 2016 at 09:23:46PM +0200, 
>> > > > > > > > > > > > > > > > Sumit Bose wrote:
>> > > > > > > > > > > > > > > > > Hi,
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > this patch allows the extom plugin to lookup
>> > > > > > > > > > > > > > > > > users by certificate which
>> > > > > > > > > > > > > > > > > is needed in the case where a IPA client
>> > > > > > > > > > > > > > > > > wants to lookup an AD user who
>> > > > > > > > > > > > > > > > > has the certificate stored in AD. To make
>> > > > > > > > > > > > > > > > > this work the related patches
>> > > > > > > > > > > > > > > > > I just send to sssd-devel are needed as well.
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > Currently the patches miss the change in the
>> > > > > > > > > > > > > > > > > required version of SSSD.
>> > > > > > > > > > > > > > > > > since the SSSD patches are not committed. But
>> > > > > > > > > > > > > > > > > the patches are needed to
>> > > > > > > > > > > > > > > > > fully test the SSSD patches. I will send a
>> > > > > > > > > > > > > > > > > new version with the needed
>> > > > > > > > > > > > > > > > > changes to the minimal SSSD version when the 
>> > > > > > > > > > > > > > > > > SSSD patches are
>> > > > > > > > > > > > > > > > > committed.
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > bye,
>> > > > > > > > > > > > > > > > > Sumit
>> > > > > > > > > > > > > > > > The patch works fine (tested
>> > > > > > > > > > > > > > > > together with the
>> > > > > > > > > > > > > > > > corresponding SSSD
>> > > > > > > > > > > > > > > > patches), so ACK from me. The code also looks
>> > > > > > > > > > > > > > > > good to me, but I'm not
>> > > > > > > > > > > > > > > > sure if reviewing an IPA patc

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (24/06/16 20:00), Alexander Bokovoy wrote:
>On Fri, 24 Jun 2016, Sumit Bose wrote:
>> On Fri, Jun 24, 2016 at 05:53:27PM +0200, Martin Basti wrote:
>> > 
>> > 
>> > On 24.06.2016 15:09, Martin Basti wrote:
>> > >
>> > >
>> > > On 24.06.2016 14:59, Sumit Bose wrote:
>> > > > On Fri, Jun 24, 2016 at 02:00:24PM +0200, Martin Basti wrote:
>> > > > >
>> > > > > On 22.06.2016 23:20, Lukas Slebodnik wrote:
>> > > > > > On (22/06/16 11:57), Martin Basti wrote:
>> > > > > > > On 09.06.2016 21:02, Martin Basti wrote:
>> > > > > > > > On 09.06.2016 14:45, Martin Basti wrote:
>> > > > > > > > > On 09.06.2016 14:42, Martin Basti wrote:
>> > > > > > > > > > On 09.06.2016 14:38, Lukas Slebodnik wrote:
>> > > > > > > > > > > On (09/06/16 14:29), Martin Basti wrote:
>> > > > > > > > > > > > On 09.06.2016 14:22, Alexander Bokovoy wrote:
>> > > > > > > > > > > > > On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>> > > > > > > > > > > > > > On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit 
>> > > > > > > > > > > > > > Bose wrote:
>> > > > > > > > > > > > > > > Hi,
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > this patch allows the extom plugin to lookup
>> > > > > > > > > > > > > > > users by certificate which
>> > > > > > > > > > > > > > > is needed in the case where a IPA client
>> > > > > > > > > > > > > > > wants to lookup an AD user who
>> > > > > > > > > > > > > > > has the certificate stored in AD. To make
>> > > > > > > > > > > > > > > this work the related patches
>> > > > > > > > > > > > > > > I just send to sssd-devel are needed as well.
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Currently the patches miss the change in the
>> > > > > > > > > > > > > > > required version of SSSD.
>> > > > > > > > > > > > > > > since the SSSD patches are not committed. But
>> > > > > > > > > > > > > > > the patches are needed to
>> > > > > > > > > > > > > > > fully test the SSSD patches. I will send a
>> > > > > > > > > > > > > > > new version with the needed
>> > > > > > > > > > > > > > > changes to the minimal SSSD version when the 
>> > > > > > > > > > > > > > > SSSD patches are
>> > > > > > > > > > > > > > > committed.
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > bye,
>> > > > > > > > > > > > > > > Sumit
>> > > > > > > > > > > > > > The patch works fine (tested
>> > > > > > > > > > > > > > together with the
>> > > > > > > > > > > > > > corresponding SSSD
>> > > > > > > > > > > > > > patches), so ACK from me. The code also looks
>> > > > > > > > > > > > > > good to me, but I'm not
>> > > > > > > > > > > > > > sure if reviewing an IPA patch requires something
>> > > > > > > > > > > > > > more (CI? Coverity?)
>> > > > > > > > > > > > > ACK from me as well, I forgot to send email about it,
>> > > > > > > > > > > > > though I reviewed
>> > > > > > > > > > > > > this patch a week ago.
>> > > > > > > > > > > > >
>> > > > > > > > > > > > Pushed to master: 
>>

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (24/06/16 17:53), Martin Basti wrote:
>
>
>On 24.06.2016 15:09, Martin Basti wrote:
>> 
>> 
>> On 24.06.2016 14:59, Sumit Bose wrote:
>> > On Fri, Jun 24, 2016 at 02:00:24PM +0200, Martin Basti wrote:
>> > > 
>> > > On 22.06.2016 23:20, Lukas Slebodnik wrote:
>> > > > On (22/06/16 11:57), Martin Basti wrote:
>> > > > > On 09.06.2016 21:02, Martin Basti wrote:
>> > > > > > On 09.06.2016 14:45, Martin Basti wrote:
>> > > > > > > On 09.06.2016 14:42, Martin Basti wrote:
>> > > > > > > > On 09.06.2016 14:38, Lukas Slebodnik wrote:
>> > > > > > > > > On (09/06/16 14:29), Martin Basti wrote:
>> > > > > > > > > > On 09.06.2016 14:22, Alexander Bokovoy wrote:
>> > > > > > > > > > > On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>> > > > > > > > > > > > On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit Bose 
>> > > > > > > > > > > > wrote:
>> > > > > > > > > > > > > Hi,
>> > > > > > > > > > > > > 
>> > > > > > > > > > > > > this patch allows the extom plugin to lookup
>> > > > > > > > > > > > > users by certificate which
>> > > > > > > > > > > > > is needed in the case where a IPA client
>> > > > > > > > > > > > > wants to lookup an AD user who
>> > > > > > > > > > > > > has the certificate stored in AD. To make
>> > > > > > > > > > > > > this work the related patches
>> > > > > > > > > > > > > I just send to sssd-devel are needed as well.
>> > > > > > > > > > > > > 
>> > > > > > > > > > > > > Currently the patches miss the change in the
>> > > > > > > > > > > > > required version of SSSD.
>> > > > > > > > > > > > > since the SSSD patches are not committed. But
>> > > > > > > > > > > > > the patches are needed to
>> > > > > > > > > > > > > fully test the SSSD patches. I will send a
>> > > > > > > > > > > > > new version with the needed
>> > > > > > > > > > > > > changes to the minimal SSSD version when the SSSD 
>> > > > > > > > > > > > > patches are
>> > > > > > > > > > > > > committed.
>> > > > > > > > > > > > > 
>> > > > > > > > > > > > > bye,
>> > > > > > > > > > > > > Sumit
>> > > > > > > > > > > > The patch works fine (tested
>> > > > > > > > > > > > together with the
>> > > > > > > > > > > > corresponding SSSD
>> > > > > > > > > > > > patches), so ACK from me. The code also looks
>> > > > > > > > > > > > good to me, but I'm not
>> > > > > > > > > > > > sure if reviewing an IPA patch requires something
>> > > > > > > > > > > > more (CI? Coverity?)
>> > > > > > > > > > > ACK from me as well, I forgot to send email about it,
>> > > > > > > > > > > though I reviewed
>> > > > > > > > > > > this patch a week ago.
>> > > > > > > > > > > 
>> > > > > > > > > > Pushed to master: aa734da49440c5d12c0f8d4566505adaeef254e8
>> > > > > > > > > > 
>> > > > > > > > > It's very likey that this commit will break build of
>> > > > > > > > > freeipa-master. I didn't try.
>> > > > > > > > > 
>> > > > > > > > > Because it uses new function sss_nss_getnamebycert
>> > > > > > > > > from the library libsss_nss_idmap which is not in fedora.
>> > > > > > > > > It was pushed to sssd master just today.
>> > > > >

Re: [Freeipa-devel] [PATCH] 498 Update Contributors.txt

[Lukas Slebodnik]

On (23/06/16 15:22), Martin Kosek wrote:
>Update .mailmap to fix wrong commit author and re-generate
>the Developer contributor list.
>
>-- 
>Martin Kosek 
>Manager, Software Engineering - Identity Management Team
>Red Hat, Inc.

>From 4271bdb36d111b90da3daf3f4312ec40d7db590f Mon Sep 17 00:00:00 2001
>From: Martin Kosek 
>Date: Thu, 23 Jun 2016 15:19:59 +0200
>Subject: [PATCH] Update Contributors.txt
>
>Update .mailmap to fix wrong commit author and re-generate
>the Developer contributor list.
>---
> .mailmap | 1 +
> Contributors.txt | 1 +
> 2 files changed, 2 insertions(+)
>
>diff --git a/.mailmap b/.mailmap
>index 
>422a0089cc6f7de4aaed6a446a5d090bcee3fee8..4fe0587a4550b9ca4b89501a55d96540df4c10cf
> 100644
>--- a/.mailmap
>+++ b/.mailmap
>@@ -51,6 +51,7 @@ Sumit Bose   
>
> Thierry Bordaz  
> Thierry Bordaz     
> 
> Thierry Bordaz     
> 
>+Thierry Bordaz     
>
I know that it's autogenrated.
But does Thierry need to be there 4 times?

It is not a NACK

LS

> Tomáš Babej 
> Tomáš Babej 
> William Jon McCann 
>diff --git a/Contributors.txt b/Contributors.txt
>index 
>71be27da5ab415deb11589d5bf82d684b2d85f9a..a003a3edb5c1f291b94402f6784b70fa7bcb1298
> 100644
>--- a/Contributors.txt
>+++ b/Contributors.txt
>@@ -10,6 +10,7 @@ Developers:
>   Tomáš Babej
>   Martin Babinsky
>   Kyle Baker
>+  Jan Barta
>   Martin Bašti
>   Sylvain Baubeau
>   Florence Blanc-Renaud
>-- 
>2.5.5
>

>-- 
>Manage your subscription for the Freeipa-devel mailing list:
>https://www.redhat.com/mailman/listinfo/freeipa-devel
>Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (22/06/16 11:57), Martin Basti wrote:
>
>
>On 09.06.2016 21:02, Martin Basti wrote:
>> 
>> 
>> On 09.06.2016 14:45, Martin Basti wrote:
>> > 
>> > 
>> > On 09.06.2016 14:42, Martin Basti wrote:
>> > > 
>> > > 
>> > > On 09.06.2016 14:38, Lukas Slebodnik wrote:
>> > > > On (09/06/16 14:29), Martin Basti wrote:
>> > > > > On 09.06.2016 14:22, Alexander Bokovoy wrote:
>> > > > > > On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>> > > > > > > On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit Bose wrote:
>> > > > > > > > Hi,
>> > > > > > > > 
>> > > > > > > > this patch allows the extom plugin to lookup
>> > > > > > > > users by certificate which
>> > > > > > > > is needed in the case where a IPA client
>> > > > > > > > wants to lookup an AD user who
>> > > > > > > > has the certificate stored in AD. To make
>> > > > > > > > this work the related patches
>> > > > > > > > I just send to sssd-devel are needed as well.
>> > > > > > > > 
>> > > > > > > > Currently the patches miss the change in the
>> > > > > > > > required version of SSSD.
>> > > > > > > > since the SSSD patches are not committed. But
>> > > > > > > > the patches are needed to
>> > > > > > > > fully test the SSSD patches. I will send a
>> > > > > > > > new version with the needed
>> > > > > > > > changes to the minimal SSSD version when the SSSD patches are
>> > > > > > > > committed.
>> > > > > > > > 
>> > > > > > > > bye,
>> > > > > > > > Sumit
>> > > > > > > The patch works fine (tested together with the corresponding SSSD
>> > > > > > > patches), so ACK from me. The code also looks
>> > > > > > > good to me, but I'm not
>> > > > > > > sure if reviewing an IPA patch requires something
>> > > > > > > more (CI? Coverity?)
>> > > > > > ACK from me as well, I forgot to send email about it,
>> > > > > > though I reviewed
>> > > > > > this patch a week ago.
>> > > > > > 
>> > > > > Pushed to master: aa734da49440c5d12c0f8d4566505adaeef254e8
>> > > > > 
>> > > > It's very likey that this commit will break build of
>> > > > freeipa-master. I didn't try.
>> > > > 
>> > > > Because it uses new function sss_nss_getnamebycert
>> > > > from the library libsss_nss_idmap which is not in fedora.
>> > > > It was pushed to sssd master just today.
>> > > > 
>> > > > LS
>> > > 
>> > > If this is true, can you/somebody provide the SRPM of SSSD with
>> > > the required functionality please? We may need to add it to
>> > > @freeipa/freeipa-master copr and bump required version of SSSD.
>> > > 
>> > > Martin^2
>> > > 
>> > 
>> > Yes, you were right, master build is broken.
>> > Martin^2
>> > 
>> 
>> SSSD master build has been added to @freeipa/freeipa-master copr as a
>> workaround (to unblock automatic testing an developers)
>> 
>> Please bump version in specfile accordingly (I don't know in which
>> version of SSSD will be required function)
>> 
>> Martin^2
>> 
>Bumping SSSD version in requires and buildrequires
>Patch attached

>From f2b394085157954768bc93a73b854778c65bfdcd Mon Sep 17 00:00:00 2001
>From: Martin Basti <mba...@redhat.com>
>Date: Wed, 22 Jun 2016 10:49:39 +0200
>Subject: [PATCH] Bump SSSD requires
>
>https://fedorahosted.org/freeipa/ticket/4955
>---
> freeipa.spec.in | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>0d5c745d5306cd7141c573454bd1c1e6a78c7e7f..befc7af9ee2ceefa41b1b999df4bdb1c6607bea8
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
> BuildRequires:  python-qrcode-core >= 5.0.0
> BuildRequires:  python-dns >= 1.11.1
> BuildRequires:  libsss_idmap-devel
>-BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
>+BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
> BuildRequires:  java-headless
> BuildRequires:  rhino
> BuildRequires:  libverto-devel
>@@ -327,7 +327,7 @@ Requires: pam_krb5
> Requires: curl
> Requires: libcurl >= 7.21.7-2
> Requires: xmlrpc-c >= 1.27.4
>-Requires: sssd >= 1.13.3-5
>+Requires: sssd >= 1.14.0
NACK

A) It's not explained in commit message why you need to bump Requires for sssd.
   IIRC, you need just new libsss_nss_idmap-devel.
B) You forgot add detection for newer version of libsss_nss_idmap at configure
   time
Hint: * daemons/configure.ac
  * 
https://autotools.io/pkgconfig/pkg_check_modules.html#pkgconfig.pkg_check_modules.specification

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0133] Require 389-ds-base >= 1.3.5.6

[Lukas Slebodnik]

On (16/06/16 12:00), Petr Spacek wrote:
>Hello,
>
>Require 389-ds-base >= 1.3.5.6
>
>Old DS handles LDAP filters incorrectly and breaks bind-dyndb-ldap.
>See https://www.redhat.com/archives/freeipa-devel/2016-June/msg00477.html
>
>https://fedorahosted.org/freeipa/ticket/2008
>
>-- 
>Petr^2 Spacek

>From 6cadda4044cf2ea85c84e04937455ab7726207e1 Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Thu, 16 Jun 2016 11:58:56 +0200
>Subject: [PATCH] Require 389-ds-base >= 1.3.5.6
>
>Old DS handles LDAP filters incorrectly and breaks bind-dyndb-ldap.
>See https://www.redhat.com/archives/freeipa-devel/2016-June/msg00477.html
>
>https://fedorahosted.org/freeipa/ticket/2008
>---
> freeipa.spec.in | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>d0f6888b47dbc6bcb7dcaf271d71900d67f97a2b..0d5c745d5306cd7141c573454bd1c1e6a78c7e7f
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -42,7 +42,7 @@ Source0:freeipa-%{version}.tar.gz
> BuildRoot:  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> 
> %if ! %{ONLY_CLIENT}
>-BuildRequires:  389-ds-base-devel >= 1.3.5
>+BuildRequires:  389-ds-base-devel >= 1.3.5.6
I know that patch was pushed and it fixed your problem.
but I am little bit curious why did you need to change
version in BuildRequires?

If I understand correctly FreeIPA complies well with 1.3.5.
The only problem was at runtime.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0492] Translations: update ipa-4-3 translations

[Lukas Slebodnik]

On (09/06/16 12:32), Martin Basti wrote:
>
>
>On 07.06.2016 12:51, Martin Babinsky wrote:
>> On 06/01/2016 05:10 PM, Martin Basti wrote:
>> > Patch attached.
>> > 
>> ACK
>> 
>Pushed to ipa-4-3: 22fcf65cd1b674b21496b677818a8c75adcd70a6
>
I am not sure but it's very likely that this patch broke build
in ipa-4-3 branch.

//snip
 /usr/bin/install -c -m 644 Int32.h GetKeytabControl.h GKNewKeys.h 
GKCurrentKeys.h GKReply.h KrbKey.h TypeValuePair.h 
'/home/user/workspace/freeipa/rpmbuild/BUILDROOT/freeipa-4.3.1-20160611121012Zjenkins7git262054a.fc24.x86_64.'
make[4]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1/asn1c'
make[3]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1/asn1c'
make[3]: Entering directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1'
make[4]: Entering directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1'
make[4]: Nothing to be done for 'install-exec-am'.
make[4]: Nothing to be done for 'install-data-am'.
make[4]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1'
make[3]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1'
make[2]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/asn1'
cd install/po && make install || exit 1;
make[2]: Entering directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/install/po'
Merging ipa.pot into bn_IN.po
..
 done.
Creating bn_IN.mo
Merging ipa.pot into de.po
..
 done.
Creating de.mo
de.po:1056: 'msgid' and 'msgstr' entries do not both begin with '\n'
de.po:12101: 'msgid' and 'msgstr' entries do not both begin with '\n'
/usr/bin/msgfmt: found 4 fatal errors
Makefile:88: recipe for target 'de.mo' failed
make[2]: *** [de.mo] Error 1
make[2]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1/install/po'
Makefile:115: recipe for target 'client-install' failed
make[1]: *** [client-install] Error 1
make[1]: Leaving directory 
'/home/user/workspace/freeipa/rpmbuild/BUILD/freeipa-4.3.1'
error: Bad exit status from /var/tmp/rpm-tmp.YUcPEp (%install)


RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.YUcPEp (%install)
Makefile:258: recipe for target 'rpms' failed
make: *** [rpms] Error 1
Build step 'Execute shell' marked build as failure

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0203 adtrust: remove ipanttrustpartner parameter

[Lukas Slebodnik]

On (10/06/16 11:01), Martin Kosek wrote:
>On 06/10/2016 10:01 AM, Martin Basti wrote:
>> 
>> 
>> On 09.06.2016 21:45, Alexander Bokovoy wrote:
>>> On Thu, 09 Jun 2016, Martin Basti wrote:


 On 09.06.2016 17:56, Martin Babinsky wrote:
> On 06/06/2016 01:37 PM, Alexander Bokovoy wrote:
>> On Mon, 06 Jun 2016, Jan Cholasta wrote:
>>> On 6.6.2016 13:22, Martin Basti wrote:


 On 06.06.2016 13:14, Alexander Bokovoy wrote:
> On Mon, 06 Jun 2016, Martin Basti wrote:
>>
>>
>> On 06.06.2016 12:36, Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> MS-ADTS spec requires that TrustPartner field should be equal to the
>>> commonName (cn) of the trust. We used it a bit wrongly to express
>>> trust relationship between parent and child domains. In fact, we
>>> have parent-child relationship recorded in the DN (child domains
>>> are part of the parent domain's container).
>>>
>>> Remove the argument that was never used externally but only
>>> supplied by
>>> trust-specific code inside the IPA framework.
>>>
>>> Part of https://fedorahosted.org/freeipa/ticket/5354
>>>
>>>
>>>
>>
>> Hello, how is handled backward compatibility here, you just removes
>> the option from API, without any additional logic for older clients.
> This is not used by the external clients at all. It is part of 
> internal
> logic of the code in trust.py+com.redhat.trust.fetch-domains which
> always talk to the same server they are running on.
>
> @register()
> class trustdomain_add(LDAPCreate):
>  __doc__ = _('Allow access from the trusted domain')
>  NO_CLI = True
>
>

 Yes sorry, not old IPA clients, but it was part of API, shown in API
 browser, and since this was in API, it is set to stone. So If you think
 that it is safe to be removed and nobody can hit this, I'm okay for
 removing that option. Maybe we should at least wrote it to release 
 notes
 (I'll let Honza to express his feelings as API versioning/compatibility
 sensei)
>>>
>>> IMHO it is safe to remove.
>>>

 And you forgot to increment api version in VERSION file
>> Updated patch attached, with a VERSION change.
>>
>>
>>
> ACK
>

 Is there any ticket for this?
>>> As I wrote in the commit message and in the email,
>>> it is part of https://fedorahosted.org/freeipa/ticket/5354
>>>
>> Sorry I misread that ticket in the commit message, because ipatool was unable
>> to parse it from commit message
>> 
>> Pushed to master: 185806432d6dfccc5cdd73815471ce60a575b073
>
>I see no link to this ticket in the commit message in
>https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=185806432d6dfccc5cdd73815471ce60a575b073
>Did you push old version of this patch?
>
>In general, I would suggest using the patch format from
>http://www.freeipa.org/page/Contribute/Patch_Format
>It makes automation easier...
>
And it would be much easier for author with .git-commit-template
@see
https://git.fedorahosted.org/cgit/sssd.git/commit/?id=3d9edb4c510028def2df41aa7b0ce705b197e6fc

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

[Lukas Slebodnik]

On (09/06/16 14:29), Martin Basti wrote:
>On 09.06.2016 14:22, Alexander Bokovoy wrote:
>> On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>> > On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit Bose wrote:
>> > > Hi,
>> > > 
>> > > this patch allows the extom plugin to lookup users by certificate which
>> > > is needed in the case where a IPA client wants to lookup an AD user who
>> > > has the certificate stored in AD. To make this work the related patches
>> > > I just send to sssd-devel are needed as well.
>> > > 
>> > > Currently the patches miss the change in the required version of SSSD.
>> > > since the SSSD patches are not committed. But the patches are needed to
>> > > fully test the SSSD patches. I will send a new version with the needed
>> > > changes to the minimal SSSD version when the SSSD patches are
>> > > committed.
>> > > 
>> > > bye,
>> > > Sumit
>> > 
>> > The patch works fine (tested together with the corresponding SSSD
>> > patches), so ACK from me. The code also looks good to me, but I'm not
>> > sure if reviewing an IPA patch requires something more (CI? Coverity?)
>> ACK from me as well, I forgot to send email about it, though I reviewed
>> this patch a week ago.
>> 
>Pushed to master: aa734da49440c5d12c0f8d4566505adaeef254e8
>
It's very likey that this commit will break build of
freeipa-master. I didn't try.

Because it uses new function sss_nss_getnamebycert
from the library libsss_nss_idmap which is not in fedora.
It was pushed to sssd master just today.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] git commit message

[Lukas Slebodnik]

On (08/06/16 14:09), Petr Vobornik wrote:
>On 06/08/2016 10:07 AM, Petr Spacek wrote:
>> On 7.6.2016 15:11, Stanislav Laznicka wrote:
>>> Hello,
>>>
>>> Thank you for your patch. As the thin-client patches were pushed in the
>>> meantime, the patch won't apply. Could you please send a rebased version?
>>>
>>> Also, I have a few comments to the patch:
>>>
>>> 1) I think that the commit message should be rather a brief conclusion to 
>>> the
>>> changes made in the commit. This could help for faster orientation in the
>>> changes that were made to a certain part of code should you be searching 
>>> for a
>>> bug introduced by a commit. Should some more info be required, it can be 
>>> added
>>> to the ticket. Could you therefore shorten the commit message?
>> 
>> (My personal opinion, no golden standard.)
>> 
>> Honestly I disagree with Standa. Yes, the commit message seems to be a bit
>> long but *tickets* are not the best place to put *technical* information 
>> into.
>> 
>> Tickets are planning tool but keep in mind that Trac may/will vanish one day
>> and all we will have will be (Git?) repo.
>
>+1
>
>The commit message is very good and honestly I'd like to see more of
>such commit messages.
>
Regarding to commit message.

I like recommendation about git commit message from Openstack wiki
https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages
We can at least inspire.

IMHO longer commit message is always better.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][patch-0037]Fixes of dnssec tests

[Lukas Slebodnik]

On (06/05/16 11:14), Oleg Fayans wrote:
>On 05/06/2016 09:48 AM, Martin Basti wrote:
>> On 06.05.2016 09:36, Oleg Fayans wrote:
>>> Tests are finally stable:
>>>
>>> = test session starts
>>> ==
>>> platform linux2 -- Python 2.7.11 -- py-1.4.30 -- pytest-2.7.3
>>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
>>> plugins: multihost, sourceorder
>>> collected 8 items
>>>
>>> test_integration/test_dnssec.py 
>>>
>>> = 8 passed in 5561.48 seconds
>>> ==
>>>
>>>
>>>
>>>
>>>
>> PATCH 38 LGTM
>> 
>> PATCH 37 IIRC I refused to accept workaround for this issue when you
>> send this (almost the same) patch for first time, are you sure that we
>> want to hide real issues in tests, to just have green color there?
>> 
>
>The underlying issue is 7 months old. Latest update in the issue from
>Peter Spacek is: "I do not have time to investigate this issue now",
So the solution should be to escalate this issue and ensure that
Petr's manager reserve enough time for him to fix the bug.

The bug is in FreeIPA and not in test and therefore tests should not be
changed.

If you want to add workaround then please add workaround(s) to the freeipa
related part of code.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Improving bug reporting

[Lukas Slebodnik]

On (04/05/16 13:22), Rob Crittenden wrote:
>Lukas Slebodnik wrote:
>> On (04/05/16 12:56), Alexander Bokovoy wrote:
>> I'm sorry but it was a TL;DR mail without any useful information
>> to the topic.
>> 
>> The topic is "Improving bug reporting". I do not care much
>> how downstreams handle bug reports.
>> 
>> I like David proposal with template. But I do not like
>> proposal for debian; because there isn't an upstream version.
>> therefore I proposed to add recommendation to test with upstream version
>> of FreeIPA (fedora)
>> 
>> We should be very kind to users of other distributions but it happen
>> to me that my direct reports to upstream were closed because
>> fedora had patched version of packages. I did't like this way
>> but I understant why it was closed in upstream.
>> 
>> I did not propose to directly close tickets reported for non-upstream 
>> versions.
>> I proposed to add an recommendation to the template to test with upstream
>> version.
>> 
>> BTW We also suggest such way also in sssd. We have some users who had 
>> problems
>> with sssd due to buggy version of sssd in downstram (el7.1). We fixed many 
>> bugs
>> in upstream but they were not fixed in downstream. Therefore we started
>> to build upstream versions of sssd to older distributions[1].
>> We *SAVED* a lot of time due to this recommendation. This is a best practice
>> which we use also for reports from ubuntu 14.04. There is buggy version of
>> sssd-1.11.5.1 and it does not worth to spend a time with investigation unless
>> bug is confirmed with latest upstream version. Fortunately, Timo has latest
>> upstream version of sssd in ppa. If there wasn't a ppa I would prepare
>> it myself.
>
>I don't think sssd is something to measure against in this case. IPA has so
>many more moving parts it has taken quite a lot longer to get near the same
>point as sssd. Debian support has almost literally been a one-man operation.
>
This is yet another reason to test with upstream version of FreeIPA
(or with version which is more tested) before reporing a bug.

There are projects which test on more platforms(debian, centos, fedora)
in upstream (cockpit, sssd ...). It would be good if FreeIP get to this state
but we are not there yet. Therefore if we want to have a reasonable
bug report we shoudl recommend to test with better supported version.

sssd was just an example how it is helpful to test with
latest upstream version before reporting a bug.
The same applies to any project.

>IPA as a server didn't work in any real way until Timo got 4.3.1 built
>because without replication it's an interesting but non-production exercise.
>So IMHO any existing server builds on old platforms are not supportable. The
>same is probably true for the client packages which had all sorts of gotchas.
>
>Timo has done an terrific job getting his patches upstream, quite a few of
>which have been specifically to make IPA more distro agnostic.
>
>So honestly, I think a new line should be drawn using 4.3.1 as the starting
>point and provide just best-effort support for older releases. So workarounds
>only and probably no patches unless you can show it also affects the latest,
>and if Timo wants to backport then that's up to him.
>
FreeIPA 4.3 branch is not branch with long term support from upstrem point of
view. I would wait for 4.4.

>As for the template, part of the problem with the trac tickets is it is often
>developers filing bugs against things they see and I know I've filed a ton of
>awful, no details bugs thinking I'd get to it "real soon" and then not. A
>template might help at least remind what is necessary, but that is just as
>easily ignorable as the almost identical template in bugzilla.
>
>So I'd say it's up to the first round of triage to push back on poorly filed
>tickets. Maybe give Petr a ban hammer to close any incomplete tickets when
>they come in. I think devs would get the idea pretty quickly. He could be
>gentler with user-reported issues.
>
+1

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Improving bug reporting

[Lukas Slebodnik]

On (04/05/16 12:56), Alexander Bokovoy wrote:
>On Wed, 04 May 2016, Lukas Slebodnik wrote:
>> On (04/05/16 11:05), Alexander Bokovoy wrote:
>> > On Tue, 03 May 2016, Robbie Harwood wrote:
>> > > Lukas Slebodnik <lsleb...@redhat.com> writes:
>> > > 
>> > > > On (03/05/16 12:29), Robbie Harwood wrote:
>> > > > > David Kupka <dku...@redhat.com> writes:
>> > > > >
>> > > > > > --8<- trac-ticket-template-proposal 
>> > > > > > --->8--
>> > > > > > Related SW versions:
>> > > > > > On server:
>> > > > > > {{{
>> > > > > > $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server
>> > > > >
>> > > > > I think this is a good idea.  However, we are on Debian/family as
>> > > > > well now, and I think we want to accept bugs that come from these
>> > > > > users as well.
>> > > >
>> > > > FreeIPA is heavily patched on debian and has quite old version there
>> > > > 4.0.5.
>> > > >
>> > > > The better would be recommend to reproduce with upstream version
>> > > > (fedora/CentOS).
>> > > 
>> > > (FreeIPA 4.1.4 is available on Debian, but your point still stands.)
>> > > 
>> > > In summary: I don't like that upstream is conflated with fedora/CentOS.
>> > > Of course I understand that this was done to ease development and not
>> > > out of malice.  But longer term I would like Debian/Ubuntu FreeIPA to be
>> > > less of an afterthought because I believe we can attract users to our
>> > > product.  I believe this to be especially true with working
>> > > freeipa-client on those distros, which we now have and I am very happy
>> > > about.
>> > I think you miss context here. There was huge amount of work done in
>> > last couple months to get FreeIPA 4.3 running on Ubuntu and Debian. It
>> > made to Ubuntu 16.04. It didn't make to Debian proper yet for a single
>> > reason: FreeIPA tarball ships with a minified JS code for parts of the
>> > web UI framework which is against some of Debian policies and therefore
>> > FreeIPA is blocked from entering Debian.
>> > 
>> I think you misses context here.
>:) No, I'm not.
>
>> The porpose of reporting bugs to upstream is to file bugs to upstream 
>> version.
>> If the downstream version is patched with non-upstream patches
>> then you need to find out wheter bug is in upstream or downstream
>> caused by non-upstream patches. And it should be task of reporter
>> to ensure that bug is in upstream.
>It is business as usual -- people would file bug where they want, not
>where you want them to file regardless how you propose them to deal with
>it. So far, we have by far many more requests/bugs about Debian/Ubuntu
>integration than Fedora on the mailing list exactly because
>Debian/Ubuntu versions people tend to run with are known to contain
>incomplete support. So for these it does not matter where they are
>filed because downstreams aren't going to fix the bugs in, say, Ubuntu
>12.04 or 14.04 apart from what Timo provides with existing PPAs.
>
>Reporting proper information is good and I think our upstream page that
>David proposed should have subsections per distro flavor (and links to
>SSSD troubleshooting guides too) but don't put much hope in them, people
>who were not able to find out existing PPAs or solutions on the list,
>will not be searching for FreeIPA wiki page to know how to report bugs
>either.
>
>> As I mentioned in previous mail.
>> If freeipa get to the state that debian has pure upstream version
>> then we can recommend to report bugs + exact version with dpkg-query
>> 
>> The purpose of David's mail was to simplify job for developers
>> and developers life will not be easier if developer need to
>> find out wheter bug is in upstream or it's caused by downstream patches.
>It is easy: if it is a bug on Fedora/RHEL/CentOS -- it is bug for Fedora
>and RHEL maintainers to review and decide to forward or fix, if needed.
>
>If the bug on Debian/Ubuntu and reported there -- it is bug for
>Debian/Ubuntu maintainers to decide to forward or fix, if needed.
>
>If these two groups of people are overlapping with FreeIPA/SSSD
>upstreams, there is no problem as they would know how to handle the
>situation.
>
>If bugs were filed FreeIPA/SSSD upstream and reported issues are at a
>specific downstream, 

Re: [Freeipa-devel] Improving bug reporting

[Lukas Slebodnik]

On (04/05/16 11:05), Alexander Bokovoy wrote:
>On Tue, 03 May 2016, Robbie Harwood wrote:
>> Lukas Slebodnik <lsleb...@redhat.com> writes:
>> 
>> > On (03/05/16 12:29), Robbie Harwood wrote:
>> > > David Kupka <dku...@redhat.com> writes:
>> > > 
>> > > > --8<- trac-ticket-template-proposal --->8--
>> > > > Related SW versions:
>> > > > On server:
>> > > > {{{
>> > > > $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server
>> > > 
>> > > I think this is a good idea.  However, we are on Debian/family as
>> > > well now, and I think we want to accept bugs that come from these
>> > > users as well.
>> > 
>> > FreeIPA is heavily patched on debian and has quite old version there
>> > 4.0.5.
>> > 
>> > The better would be recommend to reproduce with upstream version
>> > (fedora/CentOS).
>> 
>> (FreeIPA 4.1.4 is available on Debian, but your point still stands.)
>> 
>> In summary: I don't like that upstream is conflated with fedora/CentOS.
>> Of course I understand that this was done to ease development and not
>> out of malice.  But longer term I would like Debian/Ubuntu FreeIPA to be
>> less of an afterthought because I believe we can attract users to our
>> product.  I believe this to be especially true with working
>> freeipa-client on those distros, which we now have and I am very happy
>> about.
>I think you miss context here. There was huge amount of work done in
>last couple months to get FreeIPA 4.3 running on Ubuntu and Debian. It
>made to Ubuntu 16.04. It didn't make to Debian proper yet for a single
>reason: FreeIPA tarball ships with a minified JS code for parts of the
>web UI framework which is against some of Debian policies and therefore
>FreeIPA is blocked from entering Debian.
>
I think you misses context here.

The porpose of reporting bugs to upstream is to file bugs to upstream version.
If the downstream version is patched with non-upstream patches
then you need to find out wheter bug is in upstream or downstream
caused by non-upstream patches. And it should be task of reporter
to ensure that bug is in upstream.

ATM the easiest way is to test with fedora.

>There are people like Timo Aaltonen who work on the Debian/Ubuntu
>support but they cannot do everything on their own. JS code issue is
>unique to Debian so ideally someone would need to contribute fixes that
>are right from Debian point of view but nobody did it so far. You are
>welcome.
>
I contributed in past :-)
but into C related code.

As I mentioned in previous mail.
If freeipa get to the state that debian has pure upstream version
then we can recommend to report bugs + exact version with dpkg-query

The purpose of David's mail was to simplify job for developers
and developers life will not be easier if developer need to
find out wheter bug is in upstream or it's caused by downstream patches.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Improving bug reporting

[Lukas Slebodnik]

On (03/05/16 18:48), Robbie Harwood wrote:
>Lukas Slebodnik <lsleb...@redhat.com> writes:
>
>> On (03/05/16 12:29), Robbie Harwood wrote:
>>>David Kupka <dku...@redhat.com> writes:
>>>
>>>> --8<- trac-ticket-template-proposal --->8--
>>>> Related SW versions:
>>>> On server:
>>>> {{{
>>>> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
>>>
>>> I think this is a good idea.  However, we are on Debian/family as
>>> well now, and I think we want to accept bugs that come from these
>>> users as well.
>>
>> FreeIPA is heavily patched on debian and has quite old version there
>> 4.0.5.
>>
>> The better would be recommend to reproduce with upstream version
>> (fedora/CentOS).
>
>(FreeIPA 4.1.4 is available on Debian, but your point still stands.)
>
4.1.4 is only in experimental.
and sid(ustable) has only 4.0.5

Neither of these versions has long term support from upstream point of view.
And try to look into patches
http://anonscm.debian.org/cgit/pkg-freeipa/freeipa.git/tree/debian/patches

>In summary: I don't like that upstream is conflated with fedora/CentOS.
>Of course I understand that this was done to ease development and not
>out of malice.  But longer term I would like Debian/Ubuntu FreeIPA to be
>less of an afterthought because I believe we can attract users to our
>product.  I believe this to be especially true with working
>freeipa-client on those distros, which we now have and I am very happy
>about.
>
If freeipa get to the state that there will not be any non-upstream
patches in distibutions then we will not consider Fedora as upstream.
It might easily happen that non-upstream patch caused a bug.

BTW sssd is already in such state. Therefore we needn't care
about distribution; we just need to know the version of sssd.

Fell free to send patches if you want to have debian as 1st class citizen for
freeipa-server.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Improving bug reporting

[Lukas Slebodnik]

On (03/05/16 12:29), Robbie Harwood wrote:
>David Kupka  writes:
>
>> --8<- trac-ticket-template-proposal --->8--
>> Related SW versions:
>> On server:
>> {{{
>> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
>> certmonger
>> }}}
>> On client:
>> {{{
>> $ rpm -q freeipa-client krb5-workstation certmonger
>> }}}
>
>I think this is a good idea.  However, we are on Debian/family as well
>now, and I think we want to accept bugs that come from these users as
>well.  So, in the interest of completeness, I believe the corresponding
>invocations are:
>
>> $ dpkg-query -W freeipa-server pki-base 389-ds-base bind9 samba \
>> krb5-kdc krb5-admin-server krb5-kpropd
>
>Note the split on the krb5 packaging; depending on what piece(s) you're
>checking for in the RPM krb5-server, this list is probably reducible.
>
>> $ dpkg-query -W freeipa-client krb5-user certmonger
>
>To give an example of what output of these looks like, here's a run of
>the server command on a machine that's missing some packages:
>
>> rharwood@thriss:~$ dpkg-query -W freeipa-server pki-base 389-ds-base \
>> bind9 samba krb5-kdc krb5-admin-server krb5-kpropd
>> bind9
>> krb5-admin-server   1.13.2+dfsg-5
>> krb5-kdc1.13.2+dfsg-5
>> samba   2:4.3.8+dfsg-1
>> dpkg-query: no packages found matching freeipa-server
>> dpkg-query: no packages found matching pki-base
>> dpkg-query: no packages found matching 389-ds-base
>> dpkg-query: no packages found matching krb5-kpropd
>> rharwood@thriss:~$ 
>
>i.e., it has bind9, krb5-admin-server, krb5-kdc, and samba, but is
>missing the rest of the requested packages.
>

FreeIPA is heavily patched on debian and has quite old version there
4.0.5.

The better would be recommend to reproduce with upstream version
(fedora/CentOS).

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0469] make: fail when API.txt or ACI.txt differ from code

[Lukas Slebodnik]

On (29/04/16 16:32), Martin Basti wrote:
>https://fedorahosted.org/freeipa/ticket/5865
>
>Patch attached.

>From 511f9bb1645a707ee83571123df9548731cc9387 Mon Sep 17 00:00:00 2001
>From: Martin Basti 
>Date: Fri, 29 Apr 2016 16:28:28 +0200
>Subject: [PATCH] make: fail when ACI.txt or API.txt differs from values in
> source code
>
>This regression was caused by commit 6acaf73b0c6f7301d5a5d4292a4f9926cc370867 
>before this commit make rpms failed when API.txt did not match api
>
>https://fedorahosted.org/freeipa/ticket/5865
>---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Makefile b/Makefile
>index 
>82e6936a7162ff6e0359521d5c1299ff8ee55220..9cc4d40d191cc0ab27a5c150535e49be89b23b72
> 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -188,7 +188,7 @@ version-update: release-update
>   fi
> 
>   if [ "$(SKIP_API_VERSION_CHECK)" != "yes" ]; then \
>-  ./makeapi --validate; \
>+  ./makeapi --validate && \
>   ./makeaci --validate; \
>   fi
ACK

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (25/04/16 09:59), Jan Cholasta wrote:
>On 25.4.2016 09:34, Petr Spacek wrote:
>> On 25.4.2016 09:29, Lukas Slebodnik wrote:
>> > On (25/04/16 07:23), Jan Cholasta wrote:
>> > > Hi,
>> > > 
>> > > On 22.4.2016 13:29, Petr Spacek wrote:
>> > > > Hello,
>> > > > 
>> > > > Makefile: add sed to BuildRequires
>> > > > 
>> > > > It was requried since forever but we did not explicitly mention it.
>> > > 
>> > > IIRC sed is part of the minimum build environemnt and as such should not 
>> > > be
>> > > explicitly required in the spec file. I personally don't care, but this 
>> > > is
>> > > the likely reason why it wan't there from the beginning.
>> > > 
>> > +1
>> > 
>> > It is part of group "@buildsys-build".
>> > and fedora packaging guidelines does not recommend to list
>> > packages from this group in BuildRequires.
>> 
>> I consider this piece of Fedora guidelines brain-dead as "explicit is better
>> than implicit". Anyway, feel free to NACK it so the status of the patch is
>> clear and this thread can die. I do not insist on it.
>
>I can't find it in the guidelines anymore, so LGTM.
>
It seems that it was changed since I read it last time.

There is vague description of which packages should be there.
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2
   It is important that your package list all necessary build dependencies
   using the BuildRequires?:
   tag. You may assume that enough of an environment exists for RPM to function
   and execute basic shell scripts, but you should not assume any other packages
   are present as RPM dependencies and anything brought into the buildroot
   by the build system may change over time.

But utility fedora-review still complains if you list packages from group
"@buildsys-build"

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (22/04/16 13:29), Petr Spacek wrote:
>Hello,
>
>Makefile: add sed to BuildRequires
>
>It was requried since forever but we did not explicitly mention it.
>
>Makefile: replace perl with sed
>
>Perl was missing in BuildRequires anyway and it is used only on one place,
>all other places are using sed.
>
>-- 
>Petr^2 Spacek

>From 3c7a3c87d62358407d856119e384c70040acec1e Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Fri, 22 Apr 2016 10:40:11 +0200
>Subject: [PATCH] Makefile: replace perl with sed
>
>Perl was missing in BuildRequires anyway and it is used only on one place,
>all other places are using sed.
>---
> Makefile | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
ACK

>From 2deaef91ab71c0e78b2142c2102cfe65f0e4ed96 Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Fri, 22 Apr 2016 10:40:37 +0200
>Subject: [PATCH] Makefile: add sed to BuildRequires
>
>It was requried since forever but we did not explicitly mention it.
>---
> freeipa.spec.in | 1 +
> 1 file changed, 1 insertion(+)
>
NACK

As it was mentioned elsewhere in thread this patch is not needed.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (25/04/16 07:23), Jan Cholasta wrote:
>Hi,
>
>On 22.4.2016 13:29, Petr Spacek wrote:
>> Hello,
>> 
>> Makefile: add sed to BuildRequires
>> 
>> It was requried since forever but we did not explicitly mention it.
>
>IIRC sed is part of the minimum build environemnt and as such should not be
>explicitly required in the spec file. I personally don't care, but this is
>the likely reason why it wan't there from the beginning.
>
+1

It is part of group "@buildsys-build".
and fedora packaging guidelines does not recommend to list
packages from this group in BuildRequires.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 957 ipa-client-install: fix typo in nslcd service name

[Lukas Slebodnik]

On (22/04/16 08:09), Martin Basti wrote:
>
>
>On 22.04.2016 05:02, Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
>> > On (21/04/16 16:42), Rob Crittenden wrote:
>> > > Lukas Slebodnik wrote:
>> > > > On (21/04/16 19:25), Petr Vobornik wrote:
>> > > > > related but does not implement
>> > > > > https://fedorahosted.org/freeipa/ticket/5806
>> > > > > -- 
>> > > > > Petr Vobornik
>> > > > 
>> > > > >  From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17
>> > > > > 00:00:00 2001
>> > > > > From: Petr Vobornik <pvobo...@redhat.com>
>> > > > > Date: Thu, 21 Apr 2016 19:23:31 +0200
>> > > > > Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>> > > > > 
>> > > > > related but does not implement
>> > > > > https://fedorahosted.org/freeipa/ticket/5806
>> > > > > ---
>> > > > > client/ipa-client-install | 2 +-
>> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > > > > 
>> > > > > diff --git a/client/ipa-client-install b/client/ipa-client-install
>> > > > > index 
>> > > > > c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
>> > > > > 100755
>> > > > > --- a/client/ipa-client-install
>> > > > > +++ b/client/ipa-client-install
>> > > > > @@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
>> > > > >   nscd.service_name)
>> > > > > 
>> > > > >   nslcd = services.knownservices.nslcd
>> > > > > -if nscd.is_installed():
>> > > > > +if nslcd.is_installed():
>> > > > >   save_state(nslcd)
>> > > > 
>> > > > I thought that milestone "Future Releases" has lower priority
>> > > > then "FreeIPA 4.4 Backlog"
>> > > > 
>> > > > Therefore I would prefer to close ticket #5806 and implement
>> > > > following one
>> > > > https://fedorahosted.org/freeipa/ticket/5557#comment:2
>> > > 
>> > > I don't understand what you are suggesting. Tickets aren't
>> > > swapped like this
>> > > and certainly non-related bugs aren't closed for another.
>> > > 
>> > > This patch just fixes an obvious one-liner as agreed upon in
>> > > triage. The rest
>> > > will be potentially addressed later.
>> > > 
>> > > ACK on the patch.
>+1
>> > > 
>> > I cannot see a reason to fix oneliner.
>> > This code is not tested and should be removed (#5557)
>> 
>> I fail to see the controversy here. These are two completely and
>> absolutely unrelated bugs.
>> 
>> > Or someone should write integration test.
>> > 
>> > I'm sorry but conditional-NACK (missing integration test)
>> 
>> I've been out of the project a while, so perhaps my knowledge is stale,
>> but AFAIU upstream QE writes the integration tests, not the developers.
>> This was at least true when I was a daily contributor.
>>
You were used to old style development model (waterfall).
And management mentioned that we should be more agile.
All agile methodologies assume that also developers write tests.

>> At least at one time QE did testing using --no-sssd. Whether that is
It should have been a long time ago.
Modern freeipa client (4.4) requires sssd and many features
will not work with nslcd. Trust, views ...

>> still the case I don't know but I do remember fixing bugs around it. And
>> this particular bug might not be immediately apparent unless a specific
>> configuration was present. I noticed the bug while reading code, not in
>> production.
>>
>> rob
>> 
>Patch do exactly the same as was agreed on devel meeting. I don't see reason
>why we shouldn't fix oneliner.
>
Because it's not tested and there might be much more things which
does not work. It should be either tested or removed.

Ticket #5557 was triaged before #5806. (4 monts ago vs 10 days ago).
The purpose of ticket "#5557" was to have a single line change in
freeipa spec file. But after a triage it was decided that this part of code
will be deprecated and later removed.

I know that this patch was a oneliner but I do not see a reason why
freeipa developers should waste time with fixing unested code.
It's obvious it could not work since 2013-11-07 and nonody has complained yet.
It's another reason to remove this code.

Another reason is that freeipa-client does not have a dependency
on nss-pam-ldapd.

BTW I moved ticket #5557 to reconsider it one more time.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 957 ipa-client-install: fix typo in nslcd service name

[Lukas Slebodnik]

On (21/04/16 16:42), Rob Crittenden wrote:
>Lukas Slebodnik wrote:
>> On (21/04/16 19:25), Petr Vobornik wrote:
>> > related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>> > --
>> > Petr Vobornik
>> 
>> > From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
>> > From: Petr Vobornik <pvobo...@redhat.com>
>> > Date: Thu, 21 Apr 2016 19:23:31 +0200
>> > Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>> > 
>> > related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>> > ---
>> > client/ipa-client-install | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/client/ipa-client-install b/client/ipa-client-install
>> > index 
>> > c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
>> >  100755
>> > --- a/client/ipa-client-install
>> > +++ b/client/ipa-client-install
>> > @@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
>> >  nscd.service_name)
>> > 
>> >  nslcd = services.knownservices.nslcd
>> > -if nscd.is_installed():
>> > +if nslcd.is_installed():
>> >  save_state(nslcd)
>> 
>> I thought that milestone "Future Releases" has lower priority
>> then "FreeIPA 4.4 Backlog"
>> 
>> Therefore I would prefer to close ticket #5806 and implement following one
>> https://fedorahosted.org/freeipa/ticket/5557#comment:2
>
>I don't understand what you are suggesting. Tickets aren't swapped like this
>and certainly non-related bugs aren't closed for another.
>
>This patch just fixes an obvious one-liner as agreed upon in triage. The rest
>will be potentially addressed later.
>
>ACK on the patch.
>
I cannot see a reason to fix oneliner.
This code is not tested and should be removed (#5557)

Or someone should write integration test.

I'm sorry but conditional-NACK (missing integration test)

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 957 ipa-client-install: fix typo in nslcd service name

[Lukas Slebodnik]

On (21/04/16 19:25), Petr Vobornik wrote:
>related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>-- 
>Petr Vobornik

>From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
>From: Petr Vobornik 
>Date: Thu, 21 Apr 2016 19:23:31 +0200
>Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>
>related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>---
> client/ipa-client-install | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/client/ipa-client-install b/client/ipa-client-install
>index 
>c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
> 100755
>--- a/client/ipa-client-install
>+++ b/client/ipa-client-install
>@@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
> nscd.service_name)
> 
> nslcd = services.knownservices.nslcd
>-if nscd.is_installed():
>+if nslcd.is_installed():
> save_state(nslcd)

I thought that milestone "Future Releases" has lower priority
then "FreeIPA 4.4 Backlog"

Therefore I would prefer to close ticket #5806 and implement following one
https://fedorahosted.org/freeipa/ticket/5557#comment:2

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Instructions to build ipa under RHEL

[Lukas Slebodnik]

On (29/03/16 10:16), Oleg Fayans wrote:
>Hi team,
>
>Is there any kind of $subj available? Like, which repos to enable, etc.
>I'm raising the topic because I was unable to install a number of
>build-time dependencies to build the official 4.3.1 packages under
>RHEL-7.2 (I need freeipa-4.3.1 srpms to build ipa-tests package):
>
>awk '/BuildRequires/ {print $2}' freeipa.spec.in  | xargs yum install -y
>| grep "No package"
It's naive approach for installation of build dependencies
which ignore rpm conditions "%if (0%{?rhel} >= 7)"

Please you right tools in future yum-builddep.

>No package 389-ds-base-devel available.
>No package svrcore-devel available.
>No package samba-devel available.
>No package libwbclient-devel available.
>No package libtalloc-devel available.
>No package libtevent-devel available.
>No package xmlrpc-c-devel available.
>No package python-gssapi available.
>No package pylint available.
>No package python-polib available.
>No package libsss_idmap-devel available.
>No package libsss_nss_idmap-devel available.
>No package libunistring-devel available.
>No package python-lesscpy available.
>No package python-pytest-multihost available.
>No package python-pytest-sourceorder available.
>No package python-jwcrypto available.
>No package custodia available.
>No package libini_config-devel available.
Most of devel packages are in optional repositories.

>No package python3-devel available.
python3 is not available in rhel7 and never will be.

>No package libcmocka-devel available.
>No package nss_wrapper available.
>
These dependencies are optional and required only
on fedora.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup

[Lukas Slebodnik]

On (21/03/16 12:30), Martin Basti wrote:
>On 21.03.2016 10:33, Christian Heimes wrote:
>>On 2016-03-21 10:29, Petr Spacek wrote:
>>>On 20.3.2016 21:56, Martin Basti wrote:
Patches attached.
>>>I do not really like
>>>freeipa-mbasti-0442-pylint-remove-bare-except
>>>because it replaces most of
>>>
>>>try: ... except:
>>>
>>>with
>>>
>>>try: ... except Exception:
>>>
>>>
>>>which AFAIK does not add any value. It would be better to replace Exception
>>>with more specific exception so the code raises an error instead of 
>>>continuing
>>>when something really unexpected happens.
>>It adds some value. A bare except also excepts signals like
>>KeyboardInterrupt and SystemExit. except Exception doesn't block these
>>exceptions.
>>
>>But yes, more specific exceptions are better.
>>
>>Christian
>>
>>
>>
>>
>'except Exception' is another pylint check :D
>
>I replaced bare except with a particular exception in cases where it was
>clear. For other occurrences of bare except it covers too much Exception
>types, so catch Exception is more sensible, or I need crystal ball to detect
>what kind of exceptions can be raised there.
>
Agree.

It can be changed to more specific exceptions type of Exception in future.
This change is less risky.

pylint passed on fedora {23, 24, rawhide}

ACK

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan

[Lukas Slebodnik]

On (17/03/16 16:00), Oleg Fayans wrote:
>Hi Lukas,
>
>On 03/17/2016 11:28 AM, Lukas Slebodnik wrote:
>> On (10/03/16 23:09), Oleg Fayans wrote:
>>> Hi Martin,
>>>
>>>
>>>
>>> On 03/08/2016 08:18 PM, Martin Basti wrote:
>>>>
>>>>
>>>> On 08.03.2016 18:24, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 08.03.2016 12:38, Oleg Fayans wrote:
>>>>>> The patches were rebased against the current master
>>>>>>
>>>>>> On 03/04/2016 05:33 PM, Martin Basti wrote:
>>>>>>> * old messages have been removed *
>>>>>>>>>>> 1)
>>>>>>>>>>> this method is unused please remove it
>>>>>>>>>>>
>>>>>>>>>>>   def test_kra_install_master(self):
>>>>>>>> Well, in fact it is used twice: in both domain levels, so I'd better
>>>>>>>> keep it:
>>>>>>>>
>>>>>>>> -bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
>>>>>>>> --collect-only
>>>>>>>> 
>>>>>>>>
>>>>>>>>
>>>>>>>> test session starts
>>>>>>>> =
>>>>>>>>
>>>>>>>>
>>>>>>>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
>>>>>>>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
>>>>>>>> pytest.ini
>>>>>>>> plugins: sourceorder, multihost
>>>>>>>> collected 8 items
>>>>>>>> 
>>>>>>>> 
>>>>>>>>   
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>   
>>>>>>>> 
>>>>>>>> 
>>>>>>>>   
>>>>>>>> 
>>>>>>>> 
>>>>>>>>   
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>   
>>>>>>>> 
>>>>>>> aah my bad, I forgot that pytest executes it when it begins with test_*
>>>>>>> even in parent class
>>>>>>>>>>> 2)
>>>>>>>>>>> Why are these there? I do not see any usage
>>>>>>>>>>>
>>>>>>>>>>> from env_config import get_global_config
>>>>>>>>>>> config = get_global_config()
>>>>>>>> Removed
>>>>>>>>
>>>>>>>>>>> 3) nitpick
>>>>>>>>>>> +num_clients = 0
>>>>>>>>>>> this is set by default
>>>>>>>> Removed
>>>>>>>>
>>>>>>>>>>> otherwise LGTM
>>>>>>>>>>>
>>>>>>>>>>> Results of testing tomorrow.
>>>>>>>>>>>
>>>>>>>>>>> Martin^2
>>>>>>>>>>>
>>>>>>>>>> I applied all patches including workarounds, but test failed.
>>>>>>>>>>
>>>>>>>>>> ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
>>>>>>>>>> ['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
>>>>>>>>>> '--setup-ca', '--ip-address', '192.168.144.102',
>>>>>>>>>> '/root/ipatests/replica-info.gpg']
>>>>>>>>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] The host
>>>>>>>>>> replica1.ipa.test already exists on the master s

Re: [Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan

[Lukas Slebodnik]

On (10/03/16 23:09), Oleg Fayans wrote:
>Hi Martin,
>
>
>
>On 03/08/2016 08:18 PM, Martin Basti wrote:
>> 
>> 
>> On 08.03.2016 18:24, Martin Basti wrote:
>>>
>>>
>>> On 08.03.2016 12:38, Oleg Fayans wrote:
 The patches were rebased against the current master

 On 03/04/2016 05:33 PM, Martin Basti wrote:
> * old messages have been removed *
> 1)
> this method is unused please remove it
>
>   def test_kra_install_master(self):
>> Well, in fact it is used twice: in both domain levels, so I'd better
>> keep it:
>>
>> -bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
>> --collect-only
>> 
>>
>>
>> test session starts
>> =
>>
>>
>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
>> pytest.ini
>> plugins: sourceorder, multihost
>> collected 8 items
>> 
>> 
>>   
>> 
>> 
>> 
>> 
>>   
>> 
>> 
>>   
>> 
>> 
>>   
>> 
>> 
>> 
>>   
>> 
> aah my bad, I forgot that pytest executes it when it begins with test_*
> even in parent class
> 2)
> Why are these there? I do not see any usage
>
> from env_config import get_global_config
> config = get_global_config()
>> Removed
>>
> 3) nitpick
> +num_clients = 0
> this is set by default
>> Removed
>>
> otherwise LGTM
>
> Results of testing tomorrow.
>
> Martin^2
>
 I applied all patches including workarounds, but test failed.

 ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0




 [ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
 ['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
 '--setup-ca', '--ip-address', '192.168.144.102',
 '/root/ipatests/replica-info.gpg']
 [ipa.ipatests.test_integration.host.Host.replica1.cmd51] The host
 replica1.ipa.test already exists on the master server.
 [ipa.ipatests.test_integration.host.Host.replica1.cmd51] You should
 remove it before proceeding:
 [ipa.ipatests.test_integration.host.Host.replica1.cmd51] % ipa
 host-del replica1.ipa.test
 [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
 ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
 ipa-replica-install command failed. See
 /var/log/ipareplica-install.log for more information
 [ipa.ipatests.test_integration.host.Host.replica1.cmd51] Exit
 code: 3
 FAILED
>> this is exactly the error that happens when a workaround for 5627
>> is not
>> applied. I have re-run the tests with all the patches and everything
>> passed. Could you please double-check, whether patch 0027 was applied
>> correctly?
>>
>> bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
>> --pdb
>> 
>>
>>
>> test session starts
>> =
>>
>>
>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
>> pytest.ini
>> plugins: sourceorder, multihost
>> collected 8 items
>>
>> test_integration/test_replica_promotion.py 
>>
>> 
>>
>>
>> 8 passed in 7561.93 seconds
>> =
>>
>>
>>
> I will
>
>>> And it needs ticket, otherwise it will not be in 4-3 branch.
>> https://fedorahosted.org/freeipa/ticket/5723
>>> NACK
>>>
>>> 1)
>>> ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>>>
>>>
>>> [ipa.ipatests.test_integration.host.Host.replica2.ParamikoTransport]
>>> RUN ['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
>>> 'Secret123', '--setup-ca', '--ip-address', '192.168.200.103', '-r',
>>> 'IPA.TEST']
>>> [ipa.ipatests.test_integration.host.Host.replica2.cmd65] RUN
>>> ['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
>>> '--setup-ca', '--ip-address', '192.168.200.103', '-r', 'IPA.TEST']
>>> [ipa.ipatests.test_integration.host.Host.replica2.cmd65] IPA client is
>>> already configured on this 

Re: [Freeipa-devel] [PATCH 0088-0095] Add --forward-policy option into installers

[Lukas Slebodnik]

On (10/03/16 22:14), Petr Spacek wrote:
>Hello,
>
>I forgot to send a patches before I leave, so here it is:
>
>Auto-detect default value for --forward-policy option in installers
>
>See
>https://fedorahosted.org/freeipa/ticket/5710
>commit messages, and design page
>https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/AutomaticEmptyZones
>
>
>I did not have time to test it thoroughly but it LGTM :-D
>
>Please note that this is first part, it does not solve upgrade (yet) and
>warnings in forwardzone-* interface.
>
>This can be solved in another patch set, this can be pushed if it passes 
>review.
>
ENOPATH

:-)

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] BUILD: Remove detection of libcheck

[Lukas Slebodnik]

ehlo,

simple patch is attached.

LS
>From 2b34cdbb3b36dcf95746fdf3d843f66989b0f1c0 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Thu, 10 Mar 2016 10:26:52 +0100
Subject: [PATCH] BUILD: Remove detection of libcheck

The unit test framework check has not been used in freeipa for long time
(if ever) but there was still conditional check for this framework.
It just produced confusing warning:
Without the 'CHECK' library, you will be unable
to run all tests in the 'make check' suite
---
 BUILD.txt|  2 +-
 daemons/configure.ac | 11 ---
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/BUILD.txt b/BUILD.txt
index d948495..b5660aa 100644
--- a/BUILD.txt
+++ b/BUILD.txt
@@ -23,7 +23,7 @@ autoconf automake m4 libtool gettext python-devel python-ldap 
\
 python-setuptools python-nss python-netaddr python-gssapi \
 python-rhsm pyOpenSSL pylint python-polib libipa_hbac-python python-memcached \
 sssd python-lxml python-pyasn1 python-qrcode-core python-dns m2crypto \
-check libsss_idmap-devel libsss_nss_idmap-devel java-headless rhino \
+libsss_idmap-devel libsss_nss_idmap-devel java-headless rhino \
 libverto-devel systemd libunistring-devel python-lesscpy python-yubico \
 python-backports-ssl_match_hostname softhsm-devel openssl-devel \
 p11-kit-devel pki-base python-pytest-multihost python-pytest-sourceorder
diff --git a/daemons/configure.ac b/daemons/configure.ac
index 2a1f6aa..2906def 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -208,17 +208,6 @@ dnl 
---
 PKG_CHECK_MODULES([LIBVERTO], [libverto])
 
 dnl ---
-dnl - Check for check unit test framework http://check.sourceforge.net/
-dnl ---
-PKG_CHECK_MODULES([CHECK], [check >= 0.9.5], [have_check=1], [have_check=])
-if test x$have_check = x; then
-AC_MSG_WARN([Without the 'CHECK' library, you will be unable to run all 
tests in the 'make check' suite])
-else
-AC_CHECK_HEADERS([check.h],,AC_MSG_ERROR([Could not find CHECK headers]))
-fi
-AM_CONDITIONAL([HAVE_CHECK], [test x$have_check != x])
-
-dnl ---
 dnl - Check for cmocka unit test framework http://cmocka.cryptomilk.org/
 dnl ---
 PKG_CHECK_EXISTS(cmocka,
-- 
2.7.2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (09/03/16 18:59), Martin Basti wrote:
>On 09.03.2016 18:16, Alexander Bokovoy wrote:
>>On Wed, 09 Mar 2016, Lukas Slebodnik wrote:
>>>On (09/03/16 13:33), Alexander Bokovoy wrote:
>>>>On Wed, 09 Mar 2016, Lukas Slebodnik wrote:
>>>>>On (03/02/16 14:30), Lukas Slebodnik wrote:
>>>>>>On (29/01/16 19:59), Alexander Bokovoy wrote:
>>>>>>>On Fri, 29 Jan 2016, Lukas Slebodnik wrote:
>>>>>>>>On (29/01/16 12:12), Lukas Slebodnik wrote:
>>>>>>>>>ehlo,
>>>>>>>>>
>>>>>>>>>attached patch shoudl fix build on fedora-24.
>>>>>>>>>It blocks static analysis scan.
>>>>>>>>>
>>>>>>>>>Even though it unblock build on fedora-24
>>>>>>>>>the solution is not ideal. It's possible that some changes
>>>>>>>>>need to be done in samba side as well.
>>>>>>>>>(missing prototypes for trim_string, smb_xstrdup
>>>>>>>>>
>>>>>>>>>LS
>>>>>>>>
>>>>>>>>BTW there is also another issue in IPA-SAM.
>>>>>>>>The value of macro LDAP_PAGE_SIZE has changed
>>>>>>>>and therefore there is a warning.
>>>>>>>>
>>>>>>>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>>>>>>>#define LDAP_PAGE_SIZE 1024
>>>>>>>>^
>>>>>>>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>>>>>>>  from ipa_sam.c:31:
>>>>>>>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the
>>>>>>>>location of the previous definition
>>>>>>>>#define LDAP_PAGE_SIZE 1000
>>>>>>>This is something we should fix. I'll look at it once in Brno.
>>>>>>Here is a related change in samba
>>>>>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>>>>>
>>>>>>
>>>>>Please review attached patch.
>>>>>
>>>>>LS
>>>>
>>>>>From 770577899357a812475d06d1da74254e6f83205d Mon Sep 17 00:00:00 2001
>>>>>From: Lukas Slebodnik <lsleb...@redhat.com>
>>>>>Date: Wed, 9 Mar 2016 10:16:58 +0100
>>>>>Subject: [PATCH] ipa-sam: Change value of LDAP_PAGE_SIZE
>>>>>
>>>>>The value of LDAP_PAGE_SIZE was changed in samba-4.4
>>>>>and samba commit message says: "This matches Windows' Active Directory
>>>>>maximum page size."
>>>>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>>>>
>>>>>
>>>>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>>>>#define LDAP_PAGE_SIZE 1024
>>>>>
>>>>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>>>>   from ipa_sam.c:31:
>>>>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of
>>>>>the previous definition
>>>>>#define LDAP_PAGE_SIZE 1000
>>>>>---
>>>>>daemons/ipa-sam/ipa_sam.c | 3 ++-
>>>>>1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>>diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
>>>>>index 
>>>>>9216e63587995ef719015e34f96f48262eaf171f..dba7ba2c803ae384bedaed9ae874a6a01232abfb
>>>>>100644
>>>>>--- a/daemons/ipa-sam/ipa_sam.c
>>>>>+++ b/daemons/ipa-sam/ipa_sam.c
>>>>>@@ -111,7 +111,8 @@ char *escape_ldap_string(TALLOC_CTX *mem_ctx,
>>>>>const char *s); /* available in li
>>>>>bool secrets_store(const char *key, const void *data, size_t size);
>>>>>/* available in libpdb.so */
>>>>>void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct
>>>>>unixid *unix_id); /* available in libsmbconf.so */
>>>>>
>>>>>-#define LDAP_PAGE_SIZE 1024
>>>>>+#undef  LDAP_PAGE_SIZE
>>>>>+#define LDAP_PAGE_SIZE 1000
>>>>>#define LDAP_OBJ_SAMBASAMACCOUNT "ipaNTUserAttrs"
>>>>>#define LDAP_OBJ_TRUSTED_DOMAIN "ipaNTTrustedDomain"
>>>>>#define LDAP_OBJ_ID_OBJECT "ipaIDobject"
>>>>>-- 
>>>>>2.7.2
>>>>>
>>>>ACK but I wonder if we should be using the one defined by smbldap.h?
>>>>
>>>I checked header file on CentOS 7 and and it will work there as well.
>>>Updated patch is attached.
>>ACK.
>>
>Pushed to master: 0906cc28b8387a62945d2531dd19bef60f731364
>
BTW if you wnat to get rid of warning on fedora 24 (freeipa-4.3)
then it might be pushed there as well.

But feel free to create ticket for it yourself.
I'm happy with fixed warning on master

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] FreeIPA COPR repos for 4.2.4 and 4.3.1 release candidates

[Lukas Slebodnik]

On (09/03/16 15:22), Jan Pazdziora wrote:
>On Wed, Mar 09, 2016 at 02:57:07PM +0100, Petr Vobornik wrote:
>> 
>> I didn't realize that pvoborni/freeipa is still used by somebody and not a
>> personal testing repo - it was created at a time when @freeipa group didn't
>> exist.
>> 
>> For testing, please use
>> @freeipa/freeipa-4-3  for stable 4.3 release
>
>Thank you, that works. The only minor drawback is that
>pvoborni/freeipa-4-3 used to be announced as official repo while this
>one says
>
>   This *will* be the official COPR repository
>
>suggesting it is not yet considered official.
>
>> In a near future
>> @freeipa/freeipa-4-3-centos7  will contain CentOS build of 4.3
>
>Should I cast some preference, it'd be nice to have the CentOS builds
>in @freeipa/freeipa-4-3 as well. The copr repo can have multiple
>releases (there already are Fedora 23, 24, and rawhide) and adding
>CentOS there instead to separate location could increase visibility
>and decrease confusion.
>
Yes, it can have multiple release but freeipa-4.3 requires
many dependencies which are not in CentOS 7
I would prefer if these dependencies were not in @freeipa/freeipa-4-3
but in separate copr repos which can be maintained by domain experts.
e.g. we have our sssd repo
https://copr.fedorainfracloud.org/coprs/g/sssd/sssd-1-13/

And it's simpler to describe details of externla repo on front page of
@freeipa/freeipa-4-3-centos7

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (09/03/16 13:33), Alexander Bokovoy wrote:
>On Wed, 09 Mar 2016, Lukas Slebodnik wrote:
>>On (03/02/16 14:30), Lukas Slebodnik wrote:
>>>On (29/01/16 19:59), Alexander Bokovoy wrote:
>>>>On Fri, 29 Jan 2016, Lukas Slebodnik wrote:
>>>>>On (29/01/16 12:12), Lukas Slebodnik wrote:
>>>>>>ehlo,
>>>>>>
>>>>>>attached patch shoudl fix build on fedora-24.
>>>>>>It blocks static analysis scan.
>>>>>>
>>>>>>Even though it unblock build on fedora-24
>>>>>>the solution is not ideal. It's possible that some changes
>>>>>>need to be done in samba side as well.
>>>>>>(missing prototypes for trim_string, smb_xstrdup
>>>>>>
>>>>>>LS
>>>>>
>>>>>BTW there is also another issue in IPA-SAM.
>>>>>The value of macro LDAP_PAGE_SIZE has changed
>>>>>and therefore there is a warning.
>>>>>
>>>>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>>>>#define LDAP_PAGE_SIZE 1024
>>>>>^
>>>>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>>>>   from ipa_sam.c:31:
>>>>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
>>>>>previous definition
>>>>>#define LDAP_PAGE_SIZE 1000
>>>>This is something we should fix. I'll look at it once in Brno.
>>>Here is a related change in samba
>>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>>
>>Please review attached patch.
>>
>>LS
>
>>From 770577899357a812475d06d1da74254e6f83205d Mon Sep 17 00:00:00 2001
>>From: Lukas Slebodnik <lsleb...@redhat.com>
>>Date: Wed, 9 Mar 2016 10:16:58 +0100
>>Subject: [PATCH] ipa-sam: Change value of LDAP_PAGE_SIZE
>>
>>The value of LDAP_PAGE_SIZE was changed in samba-4.4
>>and samba commit message says: "This matches Windows' Active Directory
>>maximum page size."
>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>
>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>#define LDAP_PAGE_SIZE 1024
>>
>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>from ipa_sam.c:31:
>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
>>previous definition
>>#define LDAP_PAGE_SIZE 1000
>>---
>>daemons/ipa-sam/ipa_sam.c | 3 ++-
>>1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
>>index 
>>9216e63587995ef719015e34f96f48262eaf171f..dba7ba2c803ae384bedaed9ae874a6a01232abfb
>> 100644
>>--- a/daemons/ipa-sam/ipa_sam.c
>>+++ b/daemons/ipa-sam/ipa_sam.c
>>@@ -111,7 +111,8 @@ char *escape_ldap_string(TALLOC_CTX *mem_ctx, const char 
>>*s); /* available in li
>>bool secrets_store(const char *key, const void *data, size_t size); /* 
>>available in libpdb.so */
>>void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid 
>>*unix_id); /* available in libsmbconf.so */
>>
>>-#define LDAP_PAGE_SIZE 1024
>>+#undef  LDAP_PAGE_SIZE
>>+#define LDAP_PAGE_SIZE 1000
>>#define LDAP_OBJ_SAMBASAMACCOUNT "ipaNTUserAttrs"
>>#define LDAP_OBJ_TRUSTED_DOMAIN "ipaNTTrustedDomain"
>>#define LDAP_OBJ_ID_OBJECT "ipaIDobject"
>>-- 
>>2.7.2
>>
>ACK but I wonder if we should be using the one defined by smbldap.h?
>
I checked header file on CentOS 7 and and it will work there as well.
Updated patch is attached.

LS
>From e80014f6c1d4a4cc19f135a797c3f0823ad388c1 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Wed, 9 Mar 2016 10:16:58 +0100
Subject: [PATCH] ipa-sam: Do not redefine LDAP_PAGE_SIZE

The value of LDAP_PAGE_SIZE was changed in samba-4.4
and it caused warning because it's already defined
in samba header files

ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
 #define LDAP_PAGE_SIZE 1024

In file included from /usr/include/samba-4.0/smbldap.h:24:0,
 from ipa_sam.c:31:
/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
previous definition
 #define LDAP_PAGE_SIZE 1000
---
 daemons/ipa-sam/ipa_sam.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
9216e63587995ef719015e34f96f48262eaf171f..4c1fda5f82b43f69929613f9938410b32cff31e7
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -111,7 +111,6 @@ char *escape_ldap_string(TALLOC_CTX *mem_ctx, const char 
*s); /* available in li
 bool secrets_store(const char *key, const void *data, size_t size); /* 
available in libpdb.so */
 void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid 
*unix_id); /* available in libsmbconf.so */
 
-#define LDAP_PAGE_SIZE 1024
 #define LDAP_OBJ_SAMBASAMACCOUNT "ipaNTUserAttrs"
 #define LDAP_OBJ_TRUSTED_DOMAIN "ipaNTTrustedDomain"
 #define LDAP_OBJ_ID_OBJECT "ipaIDobject"
-- 
2.7.2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (09/03/16 13:33), Alexander Bokovoy wrote:
>On Wed, 09 Mar 2016, Lukas Slebodnik wrote:
>>On (03/02/16 14:30), Lukas Slebodnik wrote:
>>>On (29/01/16 19:59), Alexander Bokovoy wrote:
>>>>On Fri, 29 Jan 2016, Lukas Slebodnik wrote:
>>>>>On (29/01/16 12:12), Lukas Slebodnik wrote:
>>>>>>ehlo,
>>>>>>
>>>>>>attached patch shoudl fix build on fedora-24.
>>>>>>It blocks static analysis scan.
>>>>>>
>>>>>>Even though it unblock build on fedora-24
>>>>>>the solution is not ideal. It's possible that some changes
>>>>>>need to be done in samba side as well.
>>>>>>(missing prototypes for trim_string, smb_xstrdup
>>>>>>
>>>>>>LS
>>>>>
>>>>>BTW there is also another issue in IPA-SAM.
>>>>>The value of macro LDAP_PAGE_SIZE has changed
>>>>>and therefore there is a warning.
>>>>>
>>>>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>>>>#define LDAP_PAGE_SIZE 1024
>>>>>^
>>>>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>>>>   from ipa_sam.c:31:
>>>>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
>>>>>previous definition
>>>>>#define LDAP_PAGE_SIZE 1000
>>>>This is something we should fix. I'll look at it once in Brno.
>>>Here is a related change in samba
>>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>>
>>Please review attached patch.
>>
>>LS
>
>>From 770577899357a812475d06d1da74254e6f83205d Mon Sep 17 00:00:00 2001
>>From: Lukas Slebodnik <lsleb...@redhat.com>
>>Date: Wed, 9 Mar 2016 10:16:58 +0100
>>Subject: [PATCH] ipa-sam: Change value of LDAP_PAGE_SIZE
>>
>>The value of LDAP_PAGE_SIZE was changed in samba-4.4
>>and samba commit message says: "This matches Windows' Active Directory
>>maximum page size."
>>https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a
>>
>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>#define LDAP_PAGE_SIZE 1024
>>
>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>from ipa_sam.c:31:
>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
>>previous definition
>>#define LDAP_PAGE_SIZE 1000
>>---
>>daemons/ipa-sam/ipa_sam.c | 3 ++-
>>1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
>>index 
>>9216e63587995ef719015e34f96f48262eaf171f..dba7ba2c803ae384bedaed9ae874a6a01232abfb
>> 100644
>>--- a/daemons/ipa-sam/ipa_sam.c
>>+++ b/daemons/ipa-sam/ipa_sam.c
>>@@ -111,7 +111,8 @@ char *escape_ldap_string(TALLOC_CTX *mem_ctx, const char 
>>*s); /* available in li
>>bool secrets_store(const char *key, const void *data, size_t size); /* 
>>available in libpdb.so */
>>void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid 
>>*unix_id); /* available in libsmbconf.so */
>>
>>-#define LDAP_PAGE_SIZE 1024
>>+#undef  LDAP_PAGE_SIZE
>>+#define LDAP_PAGE_SIZE 1000
>>#define LDAP_OBJ_SAMBASAMACCOUNT "ipaNTUserAttrs"
>>#define LDAP_OBJ_TRUSTED_DOMAIN "ipaNTTrustedDomain"
>>#define LDAP_OBJ_ID_OBJECT "ipaIDobject"
>>-- 
>>2.7.2
>>
>ACK but I wonder if we should be using the one defined by smbldap.h?
>
It might be better. I will test it and send patch soon.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] French translation for FreeIPA

[Lukas Slebodnik]

On (07/03/16 12:20), Martin Kosek wrote:
>On 03/07/2016 11:48 AM, Jérôme Fenal wrote:
>> 2016-02-29 18:45 GMT+01:00 Jérôme Fenal :
>> 
>>> Hi all,
>>>
>>> Just a quick note to let you that I completed the translation of what
>>> was available to translate on Zanata.
>>>
>>> Can you please check it passes the QA, that the strings available on
>>> Zanata are the latest ones, and that it can flow back into RHEL7?
>>>
>> 
>> ​Hello there,
>> 
>> No news good news, or everybody is swamped in BZs? :-)​
>
>Hi Jérôme,
>
>Thanks for the translation! The new strings should get to FreeIPA 4.3.1, right
>Tomas?
FreeIPA 4.2.x will be released sooner :-)
Do you plan to include new translation there?

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] FreeIPA COPR repos for 4.2.4 and 4.3.1 release candidates

[Lukas Slebodnik]

On (04/03/16 17:33), Petr Vobornik wrote:
>Hello all,
>
>COPR repostories for testing of upcoming 4.3.1 and 4.2.4 releases were
>created in new @freeipa group:
>* @freeipa/freeipa-4-2-rc for f23
>* @freeipa/freeipa-4-3-rc for f23, f24, rawhide
>
>@freeipa/freeipa-4-2-rc is undergoing pre-release testing. It also means that
>there is push freeze in ipa-4-2 branch.
>
>4.3.1 is not finished yet, but the COPR repository already contains initial
>package matching state of ipa-4-3 branch from today.
>
>[1] https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-4-2-rc/
I can see three test failing with freeipa-4.2
test_dnssec.py
  -- it' s known bug in freeipa
test_vault.py
  -- there are failures due to problem with installation of replica
test_caless.py
  -- TypeError: install() takes exactly 2 arguments (1 given)
  -- It should be already fixed in master
 e5189ef6e23e4691f6c74541da5bc1a0b0f2e73f
 3507bcd3dfe2b0f1e7fae6f219a925ec6904ab47

BTW I'm  lazy to file bugs in track. Feel free to file it yourselft if you need
to backport/fix tests.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] host-del & client uninstall: additional discussion related to DNS needed

[Lukas Slebodnik]

On (04/03/16 16:48), Petr Spacek wrote:
>On 4.3.2016 15:05, Rob Crittenden wrote:
>> Petr Spacek wrote:
>>> On 3.3.2016 18:15, Martin Basti wrote:


 On 03.03.2016 17:36, Petr Vobornik wrote:
> On 03/03/2016 03:52 PM, Martin Basti wrote:
>> Hello all,
>>
>> related tickets:
>> https://fedorahosted.org/freeipa/ticket/5676
>> https://fedorahosted.org/freeipa/ticket/5675
>> https://fedorahosted.org/freeipa/ticket/5715
>>
>> I'm trying to implement both tickets, but I don't like the way we
>> decided on devel meeting anymore.
>>
>> https://fedorahosted.org/freeipa/ticket/5676#comment:1
>>
>> 1)
>> ipa host-del --updatedns
>>
>> I propose to only delete A,  and related PTR records (SSHFP records
>> explained later). The record are somehow managed by IPA
>>
>> I don't like the idea of having an extra option to specify record types
>> that should be removed or a flag that will remove DNS entry completely.
>> IMO that is duplication of dnsrecord-mod/del functionality, host-del
>> should not be used for managing DNS. If somebody wants better
>> granularity, the one should use 'dnsrecord-mod zone rec --type-rec=' or
>> 'dnsrecord-del --del-all'
>
> AFAIK the proposal on devel meeting was:
>
> --update-dns will delete A, , SSHFP
> --update-dns=all will delete the whole DNS record LDAP entry
>
> there was also a proposal for granularity, e.g., --update-dns=a,.
 Yes this looks for me like doing an alias for dnsrecord-del command

>
> Then it was agreed that --update-dns won't search for SRV records (not
> mentioned here, so OK).
>
> PTR records weren't discussed or decision was not recorded.
 When we remove A/, then we should remove PTR as well
>
> The proposal above keeps backwards compatibility though it may not be
> possible to do with current framework. Or do we have support for 
> multivalued
> enum with default value(s) which acts as a flag?
 It needs big hacks in framework, to support is as Flag for old client and 
 Enum
 for new clients
>
> If the new option type is too complicated to introduce, then I would 
> prefer
> to keep current option(flag) with behavior matching proposal for
> --update-dns or --update-dns=all.
 To use "--update-dns will delete A, , SSHFP" only was proposed by me 
 here.

>
> Definitely big +1 on not introducing a new option.
>
> No need to over-engineer it.
>
> Not sure about PTR records.
>
>>
>> Note: due backward compatibility --updatedns cannot be migrated to ENUM,
>> new option needed
>
>>
>> 2)
>> SSHFP records and host-del (https://fedorahosted.org/freeipa/ticket/5715)
>>
>> host-del removes SSH keys from LDAP, thus there is no reason to keep
>> SSHFP record in DNS, thus SSHFP records should be removed always (even
>> without --updatedns option)
>
> ACK
>
>>
>> 3)
>> ipa-client-install --uninstall
>>
>> SSHFP record are always added via nsupdate to DNS, IMO during client
>> uninstall all SSHFP record related to client should be removed via
>> nsupdate too.
>
> IMHO not necessary will be solved either by #5676 and/or #5715(currently
> uninstall indirectly calls ipa-host-disable)
 However host-disable does not do nsupdate, so it will work only for IPA 
 DNS.
 So if nsupdate set SSHPF on non-IPA server, we do not have reverse 
 operation
 in uninstall for that.

>
>>
>> 4)
>> https://fedorahosted.org/freeipa/ticket/5676
>>
>> ipa-client-install --uninstall --delete-host#suggestions how to name
>> option for removing host entry for ldap welcome
>>
>> Should this option call 'host-del' or 'host-del --updatedns'?
>>
>> I would like to avoid additional DNS related option to be added to
>> ipa-client-install
>>
>> Also do we really want to implement this ticket? What is the gain there?
>
> The devel discussions which is recorded in
> https://fedorahosted.org/freeipa/ticket/5676#comment:1
>
> Suggests to change default behavior in ipa-client-install --uninstall so
> that it will call:
>
> `ipa host-del --update-dns` instead of `ipa-join --unenroll`. So it will
> also do #3.
>
> Further proposal in #5676 is to introduce a new option(--keephost ??) to
> keep the host records, i.e., the old behavior.
>
> But comment:
> """
> simo: maybe keeping backward compatibility is more important, discuss 
> later
> if --remove option would be better
> """
> suggest that further discussion is needed

 I agree with backward compatibility here. A current user may be very 
 surprised
 that all DNS records of the host disappear.
>>>
>>> The general problem 

Re: [Freeipa-devel] [bind-dyndb-ldap] [PATCH] SPEC: Add missing build dependency

[Lukas Slebodnik]

On (01/03/16 14:35), Petr Spacek wrote:
>On 1.3.2016 12:06, Lukas Slebodnik wrote:
>> On (25/02/16 15:57), Petr Spacek wrote:
>>> On 19.2.2016 13:55, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> Fix build with GCC 4.9+.
>>>>
>>>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>>>> attributes. This removes most of asserts() in the plugin.
>>>> GCC 6 adds warnings for these cases.
>>>>
>>>> We are disabling the unwanted condition pruning by adding
>>>> -fno-delete-null-pointer-checks argument.
>>>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>>>>
>>>> Additionally we silence warnings to prevent build failures when -Werror
>>>> is used.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>>>
>>> Updated version is attached. It contains less autotools magic because it
>>> enables attribute nonnull only under Clang static analyzer and Coverity - 
>>> as a
>>> result we do not have to silence GCC warnings from -Wnonnull.
>>>
>>> Please review so I can fix build in Fedora 24.
>>>
Don't forget to fix rawhide and 24 they are already separate branches.

BTW you have missing build dependency in upstream spec file
and fedora spec files has unnecessary export of clags
export CFLAGS="`isc-config.sh --cflags dns` $RPM_OPT_FLAGS"

LS
>From 3b7c0a4edfdb5f7020696c20229216203d78e3d1 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Thu, 3 Mar 2016 07:45:45 +
Subject: [PATCH 1/2] SPEC: Add missing build dependency

---
 contrib/bind-dyndb-ldap.spec | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index 97adc5e..92283f4 100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -14,6 +14,7 @@ BuildRoot:  
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildRequires:  bind-devel >= 32:9.9.0, bind-lite-devel >= 32:9.9.0
 BuildRequires:  krb5-devel
 BuildRequires:  openldap-devel
+BuildRequires:  libuuid-devel
 BuildRequires:  automake, autoconf, libtool
 
 Requires:   bind >= 32:9.9.0
-- 
2.7.2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0390] Fix build with GCC 4.9+

[Lukas Slebodnik]

On (01/03/16 14:35), Petr Spacek wrote:
>On 1.3.2016 12:06, Lukas Slebodnik wrote:
>> On (25/02/16 15:57), Petr Spacek wrote:
>>> On 19.2.2016 13:55, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> Fix build with GCC 4.9+.
>>>>
>>>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>>>> attributes. This removes most of asserts() in the plugin.
>>>> GCC 6 adds warnings for these cases.
>>>>
>>>> We are disabling the unwanted condition pruning by adding
>>>> -fno-delete-null-pointer-checks argument.
>>>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>>>>
>>>> Additionally we silence warnings to prevent build failures when -Werror
>>>> is used.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>>>
>>> Updated version is attached. It contains less autotools magic because it
>>> enables attribute nonnull only under Clang static analyzer and Coverity - 
>>> as a
>>> result we do not have to silence GCC warnings from -Wnonnull.
>>>
>>> Please review so I can fix build in Fedora 24.
>>>
>>> Thank you.
>>>
>>> -- 
>>> Petr^2 Spacek
>> 
>>>From 4732fe9f4e525c44b46e7ed0734ccaec94fba49e Mon Sep 17 00:00:00 2001
>>> From: Petr Spacek <pspa...@redhat.com>
>>> Date: Fri, 19 Feb 2016 13:39:27 +0100
>>> Subject: [PATCH] Fix build with GCC 4.9+.
>>>
>>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>>> attributes. This removes most of asserts() in the plugin.
>>> GCC 6 adds warnings for these cases.
>>>
>>> We are disabling the unwanted condition pruning by adding
>>> -fno-delete-null-pointer-checks argument.
>>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>>>
>>> Additionally we enable nonnull attribute only when the build is running 
>>> under
>>> Clang static analyzer or Coverity.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>>> ---
>>> configure.ac | 13 +
>>> src/util.h   |  8 ++--
>>> 2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 
>>> a06708b1a5ee64bb64c80272c10ed1a35670c8d0..a0123ac0a62b5acd5238f028d8c42e83af4060db
>>>  100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -39,6 +39,19 @@ AC_TRY_COMPILE([
>>> [CFLAGS="$SAVED_CFLAGS"
>>>  AC_MSG_RESULT([no])])
>>>
>>> +# Check if build chain supports -fno-delete-null-pointer-checks
>>> +# this flag avoids too agressive optimizations which would remove some 
>>> asserts
>>> +# BIND 9 did the same in its commit 
>>> 603a78708343f063b44affb882ef93bb19a5142a
>>> +AC_MSG_CHECKING([for -fno-delete-null-pointer-checks compiler flag])
>>> +SAVED_CFLAGS="$CFLAGS"
>>> +CFLAGS="$CFLAGS -fno-delete-null-pointer-checks"
>>> +AC_TRY_COMPILE([
>>> +   extern int fdef(void);
>>> +],[],
>>> +[AC_MSG_RESULT([yes])],
>>> +[CFLAGS="$SAVED_CFLAGS"
>>> + AC_MSG_RESULT([no])])
>>> +
>> NACK.
>> 
>> It failes with clang.
>> 
>> configure:12982: checking for -fno-delete-null-pointer-checks compiler flag
>> configure:12999: clang -c -O2 -g -pipe -Wall -Werror=format-security 
>> -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong 
>> --param=ssp-buffer-size=4 -grecord-gcc-switches 
>> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic 
>> -fvisibility=hidden -fno-delete-null-pointer-checks  conftest.c >&5
>> clang-3.8: warning: optimization flag '-fno-delete-null-pointer-checks' is 
>> not supported
>> clang-3.8: warning: argument unused during compilation: 
>> '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1'
>> configure:12999: $? = 0
>> configure:13000: result: yes
>> 
>> Reproducer:
>> autoreconf -if && CC=clang ./configure && make
>
>Thanks! I was testing this only with Clang static analyzer ...
>
>Here is updated patch.
>
>-- 
>Petr^2 Spacek

>From 6b2ac51fe4ff75c9f59499cbaa4306f70db46425 Mon Sep 17 00:00:00 2001
>From: Petr Spacek <pspa...@redhat.com>
>Date: Fri, 19 Feb 2016 13:39:27 +0100
>Subject: [PATCH] Fix build with GCC 4.9+.
>
>GCC 4.9+ is too aggressive when optimizing functions with nonn

Re: [Freeipa-devel] [PATCH 0390] Fix build with GCC 4.9+

[Lukas Slebodnik]

On (25/02/16 15:57), Petr Spacek wrote:
>On 19.2.2016 13:55, Petr Spacek wrote:
>> Hello,
>> 
>> Fix build with GCC 4.9+.
>> 
>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>> attributes. This removes most of asserts() in the plugin.
>> GCC 6 adds warnings for these cases.
>> 
>> We are disabling the unwanted condition pruning by adding
>> -fno-delete-null-pointer-checks argument.
>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>> 
>> Additionally we silence warnings to prevent build failures when -Werror
>> is used.
>> 
>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>
>Updated version is attached. It contains less autotools magic because it
>enables attribute nonnull only under Clang static analyzer and Coverity - as a
>result we do not have to silence GCC warnings from -Wnonnull.
>
>Please review so I can fix build in Fedora 24.
>
>Thank you.
>
>-- 
>Petr^2 Spacek

>From 4732fe9f4e525c44b46e7ed0734ccaec94fba49e Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Fri, 19 Feb 2016 13:39:27 +0100
>Subject: [PATCH] Fix build with GCC 4.9+.
>
>GCC 4.9+ is too aggressive when optimizing functions with nonnull
>attributes. This removes most of asserts() in the plugin.
>GCC 6 adds warnings for these cases.
>
>We are disabling the unwanted condition pruning by adding
>-fno-delete-null-pointer-checks argument.
>BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>
>Additionally we enable nonnull attribute only when the build is running under
>Clang static analyzer or Coverity.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>---
> configure.ac | 13 +
> src/util.h   |  8 ++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
>diff --git a/configure.ac b/configure.ac
>index 
>a06708b1a5ee64bb64c80272c10ed1a35670c8d0..a0123ac0a62b5acd5238f028d8c42e83af4060db
> 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -39,6 +39,19 @@ AC_TRY_COMPILE([
> [CFLAGS="$SAVED_CFLAGS"
>  AC_MSG_RESULT([no])])
> 
>+# Check if build chain supports -fno-delete-null-pointer-checks
>+# this flag avoids too agressive optimizations which would remove some asserts
>+# BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a
>+AC_MSG_CHECKING([for -fno-delete-null-pointer-checks compiler flag])
>+SAVED_CFLAGS="$CFLAGS"
>+CFLAGS="$CFLAGS -fno-delete-null-pointer-checks"
>+AC_TRY_COMPILE([
>+  extern int fdef(void);
>+],[],
>+[AC_MSG_RESULT([yes])],
>+[CFLAGS="$SAVED_CFLAGS"
>+ AC_MSG_RESULT([no])])
>+
NACK.

It failes with clang.

configure:12982: checking for -fno-delete-null-pointer-checks compiler flag
configure:12999: clang -c -O2 -g -pipe -Wall -Werror=format-security 
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong 
--param=ssp-buffer-size=4 -grecord-gcc-switches 
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic 
-fvisibility=hidden -fno-delete-null-pointer-checks  conftest.c >&5
clang-3.8: warning: optimization flag '-fno-delete-null-pointer-checks' is not 
supported
clang-3.8: warning: argument unused during compilation: 
'-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1'
configure:12999: $? = 0
configure:13000: result: yes

Reproducer:
autoreconf -if && CC=clang ./configure && make

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 200] slapi-nis: update configuration to allow external members

[Lukas Slebodnik]

On (26/02/16 12:37), Tomas Babej wrote:
>
>
>On 02/26/2016 07:30 AM, Jan Cholasta wrote:
>> On 22.2.2016 19:56, Tomas Babej wrote:
>>>
>>>
>>> On 02/22/2016 06:14 PM, Alexander Bokovoy wrote:
 On Mon, 22 Feb 2016, Tomas Babej wrote:
>
>
> On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
>> Hi,
>>
>> attached patch should update compat tree configuration if it exist to
>> follow slapi-nis 0.55 which has support for external members of IPA
>> groups.
>>
>> However, the real work is done in SSSD. These patches are not
>> upstreamed
>> yet. We'll need to bump SSSD dependency in future once they come to
>> distros.
>>
>>
>>
>
> This looks good.
>
> However, the new update file needs to be added to Makefile.am.
> Additionally, patch adds a whitespace error.
 Updated patch is attached.

>>>
>>> ACK.
>>>
>>> This should not be pushed until the dependency for SSSD can be bumped.
>> 
>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-d872920f74
>> 
>
>Attaching the required spec change.
>
>Tomas

>From dae8b8fd0b23bf25ccf75b275deaa5c599faa27b Mon Sep 17 00:00:00 2001
>From: Tomas Babej 
>Date: Fri, 26 Feb 2016 12:35:09 +0100
>Subject: [PATCH] spec: Bump required sssd version to 1.13.3-5
>
>Required as part of: https://fedorahosted.org/freeipa/ticket/4403
 ^
There isn't mentioned sssd related ticket in slapi-nis bug
It would be good to add also sssd related ticket to commit message
https://fedorahosted.org/sssd/ticket/2522

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 953 advise: configure TLS in redhat_nss_pam_ldapd and redhat_nss_ldap plugins

[Lukas Slebodnik]

On (25/02/16 18:01), Petr Vobornik wrote:
>I did not add --enableldapstarttls to config_redhat_nss_ldap because I'm not
>sure if it is present on el5 (IMO it is not).
>
I can confirm it doesn't have such option
[root@host /]# authconfig --help | grep -A1 "tls\|ssl"
  --enableldaptls, --enableldapssl
enable use of TLS with LDAP
  --disableldaptls, --disableldapssl
disable use of TLS with LDAP
[root@host /]# cat /etc/issue
CentOS release 5.11 (Final)
Kernel \r on an \m

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0420] Set BuildRequires to pylint 1.4

[Lukas Slebodnik]

On (24/02/16 11:06), Petr Vobornik wrote:
>On 02/24/2016 09:50 AM, Lukas Slebodnik wrote:
>>On (23/02/16 14:23), Rob Crittenden wrote:
>>>Lukas Slebodnik wrote:
>>>>On (23/02/16 17:09), Martin Basti wrote:
>>>>>We cannot guarantee that versions older than 1.4 will work with freeipa 
>>>>>code.
>>>>>
>>>>>Patch attached.
>>>>
>>>>>From a59e72a0b87231c0f2e0d737057550dd532feed7 Mon Sep 17 00:00:00 2001
>>>>>From: Martin Basti <mba...@redhat.com>
>>>>>Date: Tue, 23 Feb 2016 16:58:07 +0100
>>>>>Subject: [PATCH] Set BuildRequires to pylint >= 1.4
>>>>>
>>>>>We can guarantee that only pylint 1.4 and newer will work
>>>>>
>>>>>https://fedorahosted.org/freeipa/ticket/5615
>>>>>---
>>>>>freeipa.spec.in | 2 +-
>>>>>1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>>diff --git a/freeipa.spec.in b/freeipa.spec.in
>>>>>index 
>>>>>54a11bfc8cced643c19c29c5ada70bacf7540395..219c5ca2f13eaac14746ec4689ba611bbc6fc377
>>>>> 100644
>>>>>--- a/freeipa.spec.in
>>>>>+++ b/freeipa.spec.in
>>>>>@@ -76,7 +76,7 @@ BuildRequires:  python-netaddr
>>>>>BuildRequires:  python-gssapi >= 1.1.2
>>>>>BuildRequires:  python-rhsm
>>>>>BuildRequires:  pyOpenSSL
>>>>>-BuildRequires:  pylint >= 1.0
>>>>>+BuildRequires:  pylint >= 1.4
>>>>
>>>>I can build rpms even withour pylint and pylint is not executed
>>>>anywhere in spec file. (in other words, my patch was rejected)
>>>>Why does it need to be in BuildRequires?
>>>
>>>pylint is part of the in-tree build process (make rpms). It is not
>>>executed when building upstream packages.
>>>
>>It's not buildrequires becuase I can rebuild src.rpm
>>without it. It should not be there or it should be optional
>>to do not break developer workflow.
>>
>>e.g. "%bcond_with extra_dependencies_for_pylint"
>>
>>The upstream spec files is close to the fedora spec file
>>and pylint is istalled there even though it's not used.
>>
>>Another use case is coverity scan.
>>
>>LS
>>
>
>Basically I agree with Lukas but this patch is the way how we do build
>upstream right now.
>
>The proposed change would required more refactoring of build process and is
>out of scope of this ticket. Feel free to open a ticket for it. It should not
>block this patch.
I have a different opinion here.

The ticket #5615 says: "Prepare IPA for pylint 1.5.2"
It does not say anything about minimal requirement on pylint 1.4
even though all current version of fedora has pylint 1.4.

Older version of pylint (1.3.1) is available only in fedora 20.
Which is out of support for more than a year.
Patch is neither required for fedora nor for ticket #5615

However there is only pylint-1.3.1 in epel7
And it would cause issues there.

Summary: This patch does not solve anything.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0420] Set BuildRequires to pylint 1.4

[Lukas Slebodnik]

On (23/02/16 14:23), Rob Crittenden wrote:
>Lukas Slebodnik wrote:
>> On (23/02/16 17:09), Martin Basti wrote:
>>> We cannot guarantee that versions older than 1.4 will work with freeipa 
>>> code.
>>>
>>> Patch attached.
>> 
>>>From a59e72a0b87231c0f2e0d737057550dd532feed7 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti <mba...@redhat.com>
>>> Date: Tue, 23 Feb 2016 16:58:07 +0100
>>> Subject: [PATCH] Set BuildRequires to pylint >= 1.4
>>>
>>> We can guarantee that only pylint 1.4 and newer will work
>>>
>>> https://fedorahosted.org/freeipa/ticket/5615
>>> ---
>>> freeipa.spec.in | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>>> index 
>>> 54a11bfc8cced643c19c29c5ada70bacf7540395..219c5ca2f13eaac14746ec4689ba611bbc6fc377
>>>  100644
>>> --- a/freeipa.spec.in
>>> +++ b/freeipa.spec.in
>>> @@ -76,7 +76,7 @@ BuildRequires:  python-netaddr
>>> BuildRequires:  python-gssapi >= 1.1.2
>>> BuildRequires:  python-rhsm
>>> BuildRequires:  pyOpenSSL
>>> -BuildRequires:  pylint >= 1.0
>>> +BuildRequires:  pylint >= 1.4
>> 
>> I can build rpms even withour pylint and pylint is not executed
>> anywhere in spec file. (in other words, my patch was rejected)
>> Why does it need to be in BuildRequires?
>
>pylint is part of the in-tree build process (make rpms). It is not
>executed when building upstream packages.
>
It's not buildrequires becuase I can rebuild src.rpm
without it. It should not be there or it should be optional
to do not break developer workflow.

e.g. "%bcond_with extra_dependencies_for_pylint"

The upstream spec files is close to the fedora spec file
and pylint is istalled there even though it's not used.

Another use case is coverity scan.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0420] Set BuildRequires to pylint 1.4

[Lukas Slebodnik]

On (23/02/16 17:09), Martin Basti wrote:
>We cannot guarantee that versions older than 1.4 will work with freeipa code.
>
>Patch attached.

>From a59e72a0b87231c0f2e0d737057550dd532feed7 Mon Sep 17 00:00:00 2001
>From: Martin Basti 
>Date: Tue, 23 Feb 2016 16:58:07 +0100
>Subject: [PATCH] Set BuildRequires to pylint >= 1.4
>
>We can guarantee that only pylint 1.4 and newer will work
>
>https://fedorahosted.org/freeipa/ticket/5615
>---
> freeipa.spec.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>54a11bfc8cced643c19c29c5ada70bacf7540395..219c5ca2f13eaac14746ec4689ba611bbc6fc377
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -76,7 +76,7 @@ BuildRequires:  python-netaddr
> BuildRequires:  python-gssapi >= 1.1.2
> BuildRequires:  python-rhsm
> BuildRequires:  pyOpenSSL
>-BuildRequires:  pylint >= 1.0
>+BuildRequires:  pylint >= 1.4

I can build rpms even withour pylint and pylint is not executed
anywhere in spec file. (in other words, my patch was rejected)
Why does it need to be in BuildRequires?

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 200] slapi-nis: update configuration to allow external members

[Lukas Slebodnik]

On (22/02/16 12:48), Alexander Bokovoy wrote:
>Hi,
>
>attached patch should update compat tree configuration if it exist to
>follow slapi-nis 0.55 which has support for external members of IPA
>groups.
>
>However, the real work is done in SSSD. These patches are not upstreamed
>yet. We'll need to bump SSSD dependency in future once they come to
>distros.
>
File a fedora BZ and I can backport patches
after successful review in sssd/upstream.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [patch 0033] spec file: update the python-polib dependency name to python2-polib

[Lukas Slebodnik]

On (15/02/16 17:00), Petr Vobornik wrote:
>On 02/15/2016 04:37 PM, Milan Kubík wrote:
>>Reflect the updated name of the package.
>>
>
>Seems to me as a packaging bug in python-polib. It should use python_provide
>macro to handle the transition.
There is not a bug in python-polib

sh# rpm -q python2-polib
python2-polib-1.0.7-2.fc23.noarch

sh# rpm -q --provides python2-polib
python-polib = 1.0.7-2.fc23
python2-polib = 1.0.7-2.fc23

However it is a change in behaviour in dnf/yum.
You can see more details in BZ1291850 or better BZ1096506.

This a readon why "dnf builddep" will try to remove package.
(it's not downgrade from dnf point of view)

sh# dnf builddep freeipa.spec
Last metadata expiration check performed 0:17:37 ago on Mon Feb 15 16:19:14
2016.
Package python-setuptools-18.0.1-2.fc23.noarch is already installed, skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Error: installed package python2-polib-1.0.7-2.fc23.noarch obsoletes
python-polib < 1.0.7-2.fc23 provided by python-polib-1.0.3-6.fc23.noarch
(try to add '--allowerasing' to command line to replace conflicting packages)


You might try to file a dnf BZ but mine 1291850 was two tiles closed as not a
but and then closed as a duplicate of another bug.

IMHO the simplest solution would to push the patch with better explanation
in's a workaround.

LSommit message becuase it's a workaround.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (29/01/16 19:59), Alexander Bokovoy wrote:
>On Fri, 29 Jan 2016, Lukas Slebodnik wrote:
>>On (29/01/16 12:12), Lukas Slebodnik wrote:
>>>ehlo,
>>>
>>>attached patch shoudl fix build on fedora-24.
>>>It blocks static analysis scan.
>>>
>>>Even though it unblock build on fedora-24
>>>the solution is not ideal. It's possible that some changes
>>>need to be done in samba side as well.
>>>(missing prototypes for trim_string, smb_xstrdup
>>>
>>>LS
>>
>>BTW there is also another issue in IPA-SAM.
>>The value of macro LDAP_PAGE_SIZE has changed
>>and therefore there is a warning.
>>
>>ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
>>#define LDAP_PAGE_SIZE 1024
>>^
>>In file included from /usr/include/samba-4.0/smbldap.h:24:0,
>>from ipa_sam.c:31:
>>/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
>>previous definition
>>#define LDAP_PAGE_SIZE 1000
>This is something we should fix. I'll look at it once in Brno.
Here is a related change in samba
https://github.com/samba-team/samba/commit/8c2609f3186d40afb5954737dc174ce190cd368a

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] CONFIGURE: Replace obsolete macros

[Lukas Slebodnik]

ehlo,

The AC_PROG_LIBTOOL macro is obsoleted by since libtool-2.0
which is already in rhel6+

https://fedorahosted.org/FedoraReview/wiki/AutoTools

simple patch is attached

LS
>From 079fbfa32cb7c70d76828d96f1db3ed05e7e10c0 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Tue, 2 Feb 2016 09:09:03 +0100
Subject: [PATCH] CONFIGURE: Replace obsolete macros

The AC_PROG_LIBTOOL macro is obsoleted by since libtool-2.0
which is already in rhel6+

https://fedorahosted.org/FedoraReview/wiki/AutoTools
---
 asn1/configure.ac| 2 +-
 client/configure.ac  | 3 +--
 daemons/configure.ac | 2 +-
 install/configure.ac | 1 -
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/asn1/configure.ac b/asn1/configure.ac
index 
c3e398ea20d4112ed8308ba5eb822ab22d256436..c0209142821482b1e76c8eac3d9179224847ff38
 100644
--- a/asn1/configure.ac
+++ b/asn1/configure.ac
@@ -6,7 +6,7 @@ AC_INIT([ipa-server],
 
 AC_CONFIG_HEADERS([config.h])
 AC_PROG_CC_C99
-AC_PROG_LIBTOOL
+LT_INIT
 
 AM_INIT_AUTOMAKE([foreign])
 
diff --git a/client/configure.ac b/client/configure.ac
index 
8e3a71f7b4c08f4c608606bd8f3ef846daeb9a4c..58f23afa71e9d377afd52c26b6e4c1dfb7b404a6
 100644
--- a/client/configure.ac
+++ b/client/configure.ac
@@ -3,8 +3,7 @@ m4_include(version.m4)
 AC_INIT([ipa-client],
 IPA_VERSION,
 [https://hosted.fedoraproject.org/projects/freeipa/newticket])
-LT_INIT()
-AC_PROG_LIBTOOL
+LT_INIT
 
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_SUBDIRS([../asn1])
diff --git a/daemons/configure.ac b/daemons/configure.ac
index 
f2eebee51a0b5fd0648c835fd1f44667c5e49068..2a1f6aa8a24188c9b5f2d12fc25dbab079c2247c
 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -14,7 +14,7 @@ AM_MAINTAINER_MODE
 AC_PROG_CC_C99
 AC_STDC_HEADERS
 AC_DISABLE_STATIC
-AC_PROG_LIBTOOL
+LT_INIT
 
 AC_HEADER_STDC
 
diff --git a/install/configure.ac b/install/configure.ac
index 
cf19758a1e24c0682db954bf910120dbd31a05fa..b5f77bf8c737a437fe78ec5bfdc95599fce99760
 100644
--- a/install/configure.ac
+++ b/install/configure.ac
@@ -13,7 +13,6 @@ AM_MAINTAINER_MODE
 #AC_PROG_CC
 #AC_STDC_HEADERS
 #AC_DISABLE_STATIC
-#AC_PROG_LIBTOOL
 
 #AC_HEADER_STDC
 
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (29/01/16 12:12), Lukas Slebodnik wrote:
>ehlo,
>
>attached patch shoudl fix build on fedora-24.
>It blocks static analysis scan.
>
>Even though it unblock build on fedora-24
>the solution is not ideal. It's possible that some changes
>need to be done in samba side as well.
>(missing prototypes for trim_string, smb_xstrdup
>
>LS

>>From f9057ca98557094a4db84ac072ee9efd02a4ff79 Mon Sep 17 00:00:00 2001
>From: Lukas Slebodnik <lsleb...@redhat.com>
>Date: Fri, 29 Jan 2016 10:40:18 +0100
>Subject: [PATCH 1/3] IPA-SAM: Fix build with samba 4.4
>
>samba_util.h is not shipped with samba-4.4
>and it was indirectly included by "ndr.h"
>
>Some functions have prototypes in different header file
>"util/talloc_stack.h" and other does not have declarations
>in other header file. But they are still part of libsamba-util.so
>
>sh$ objdump -T /usr/lib64/libsamba-util.so.0.0.1 | grep -E "trim_s|xstrdup"
>00022200 gDF .text  001f  SAMBA_UTIL_0.0.1 smb_xstrdup
>000223b0 gDF .text  019d  SAMBA_UTIL_0.0.1 trim_string
>
>ipa_sam.c: In function 'ldapsam_uid_to_sid':
>ipa_sam.c:836:24: warning: implicit declaration of function 'talloc_stackframe'
>  [-Wimplicit-function-declaration]
>  TALLOC_CTX *tmp_ctx = talloc_stackframe();
>^
>ipa_sam.c: In function 'pdb_init_ipasam':
>ipa_sam.c:4493:2: warning: implicit declaration of function 'trim_string'
>  [-Wimplicit-function-declaration]
>  trim_string( uri, "\"", "\"" );
>  ^
>ipa_sam.c:4580:26: warning: implicit declaration of function 'smb_xstrdup'
>   [-Wimplicit-function-declaration]
>  ldap_state->domain_dn = smb_xstrdup(dn);
>  ^
>---
> daemons/ipa-sam/ipa_sam.c | 6 ++
> 1 file changed, 6 insertions(+)
>
>diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
>index 
>7274d600b532f101e8a614a47eea7632ed70..871775b0a19e9c273652ff7a0b497d86bb866aa6
> 100644
>--- a/daemons/ipa-sam/ipa_sam.c
>+++ b/daemons/ipa-sam/ipa_sam.c
>@@ -19,6 +19,12 @@
> #include 
> #include 
> #include 
>+#include 
>+
>+#ifndef _SAMBA_UTIL_H_
>+bool trim_string(char *s, const char *front, const char *back);
>+char *smb_xstrdup(const char *s);
>+#endif
Bump,
it would be good to unblock static analysis scans.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

ehlo,

attached patch shoudl fix build on fedora-24.
It blocks static analysis scan.

Even though it unblock build on fedora-24
the solution is not ideal. It's possible that some changes
need to be done in samba side as well.
(missing prototypes for trim_string, smb_xstrdup

LS
>From f9057ca98557094a4db84ac072ee9efd02a4ff79 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 10:40:18 +0100
Subject: [PATCH 1/3] IPA-SAM: Fix build with samba 4.4

samba_util.h is not shipped with samba-4.4
and it was indirectly included by "ndr.h"

Some functions have prototypes in different header file
"util/talloc_stack.h" and other does not have declarations
in other header file. But they are still part of libsamba-util.so

sh$ objdump -T /usr/lib64/libsamba-util.so.0.0.1 | grep -E "trim_s|xstrdup"
00022200 gDF .text  001f  SAMBA_UTIL_0.0.1 smb_xstrdup
000223b0 gDF .text  019d  SAMBA_UTIL_0.0.1 trim_string

ipa_sam.c: In function 'ldapsam_uid_to_sid':
ipa_sam.c:836:24: warning: implicit declaration of function 'talloc_stackframe'
  [-Wimplicit-function-declaration]
  TALLOC_CTX *tmp_ctx = talloc_stackframe();
^
ipa_sam.c: In function 'pdb_init_ipasam':
ipa_sam.c:4493:2: warning: implicit declaration of function 'trim_string'
  [-Wimplicit-function-declaration]
  trim_string( uri, "\"", "\"" );
  ^
ipa_sam.c:4580:26: warning: implicit declaration of function 'smb_xstrdup'
   [-Wimplicit-function-declaration]
  ldap_state->domain_dn = smb_xstrdup(dn);
  ^
---
 daemons/ipa-sam/ipa_sam.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
7274d600b532f101e8a614a47eea7632ed70..871775b0a19e9c273652ff7a0b497d86bb866aa6
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -19,6 +19,12 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifndef _SAMBA_UTIL_H_
+bool trim_string(char *s, const char *front, const char *back);
+char *smb_xstrdup(const char *s);
+#endif
 
 #include 
 #include 
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] extdom: Remove unused macro

[Lukas Slebodnik]

ehlo,

Last usage of the macro SSSD_SYSDB_SID_STR was removed
in the commit 0ee8fe11aea9811c724182def3f50960d5dd87b3

LS
>From 8f7818fb1efc1a224ddf7360d8c7bf6bd36da852 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 13:08:07 +0100
Subject: [PATCH 4/4] extdom: Remove unused macro

Last usage of the macre SSSD_SYSDB_SID_STR was removed
in the commit 0ee8fe11aea9811c724182def3f50960d5dd87b3
---
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
f5905c78e5f6eb635fcd0acf0afeda3bdb3b9baa..445624f3986944d3469214fe9c7b34ce7d9b36d1
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -489,8 +489,6 @@ int pack_ber_sid(const char *sid, struct berval **berval)
 return LDAP_SUCCESS;
 }
 
-#define SSSD_SYSDB_SID_STR "objectSIDString"
-
 int pack_ber_user(struct ipa_extdom_ctx *ctx,
   enum response_types response_type,
   const char *domain_name, const char *user_name,
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 2/3] ASN1: Fix warning Wpointer-to-int-cast

[Lukas Slebodnik]

On (29/01/16 13:06), Martin Babinsky wrote:
>On 01/29/2016 12:48 PM, Lukas Slebodnik wrote:
>>On (29/01/16 12:17), Martin Kosek wrote:
>>>On 01/29/2016 12:15 PM, Lukas Slebodnik wrote:
>>>>ehlo,
>>>>
>>>>the first patch is very simple and it just suppress warning.
>>>>The second patch is either bug or dead code. I fixed it as a bug.
>>>>I'm not sure how to test 2nd patch.
>>>>
>>>>LS
>>>
>>>Thanks. But isn't this the code generated by asn1 tool? Maybe it would be
>>>better to fix the tool, or maybe regenerate it with a newer version of the
>>>tool? Otherwise the improvements will get lost.
>>>
>>As you wish.
>>
>>Updated patch is attached.
>>
>>LS
>>
>>
>>
>Hi Lukas,
>
>I have done testing with the skeleton code generated by asn1c 9.27 in the
>past. While it did fix some of the issues reported by the static analyzer, it
>also introduced plenty of new ones that were not there before.
>
What kind of issues?

Were problems in genrated code or in tepmplate?

Which tests did you use?

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (29/01/16 12:12), Lukas Slebodnik wrote:
>ehlo,
>
>attached patch shoudl fix build on fedora-24.
>It blocks static analysis scan.
>
>Even though it unblock build on fedora-24
>the solution is not ideal. It's possible that some changes
>need to be done in samba side as well.
>(missing prototypes for trim_string, smb_xstrdup
>
>LS

BTW there is also another issue in IPA-SAM.
The value of macro LDAP_PAGE_SIZE has changed
and therefore there is a warning.

ipa_sam.c:114:0: warning: "LDAP_PAGE_SIZE" redefined
 #define LDAP_PAGE_SIZE 1024
 ^
In file included from /usr/include/samba-4.0/smbldap.h:24:0,
 from ipa_sam.c:31:
/usr/include/samba-4.0/smb_ldap.h:81:0: note: this is the location of the 
previous definition
 #define LDAP_PAGE_SIZE 1000

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 2/3] ASN1: Fix warning Wpointer-to-int-cast

[Lukas Slebodnik]

On (29/01/16 12:17), Martin Kosek wrote:
>On 01/29/2016 12:15 PM, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> the first patch is very simple and it just suppress warning.
>> The second patch is either bug or dead code. I fixed it as a bug.
>> I'm not sure how to test 2nd patch.
>> 
>> LS
>
>Thanks. But isn't this the code generated by asn1 tool? Maybe it would be
>better to fix the tool, or maybe regenerate it with a newer version of the
>tool? Otherwise the improvements will get lost.
>
As you wish.

Updated patch is attached.

LS
>From 47ca1fddb778c9aa68fb82f70458364a77d66aea Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 12:44:19 +0100
Subject: [PATCH 2/2] ASN1: Regenerate code with new tool asn1c-0.9.27

asn1c-0.9.27 generates code without compiler warning
and fixes dead-code/bug

constr_SET_OF.c: In function 'SET_OF_decode_uper':
constr_SET_OF.c:907:18: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
 (int)nelems, (int)ct ? ct->effective_bits : -1);
  ^
constr_SET_OF.c:924:5: warning: statement with no effect [-Wunused-value]
 rv.code == RC_FAIL;
 ^
---
 asn1/asn1c/BIT_STRING.c   |   9 +-
 asn1/asn1c/GKCurrentKeys.c|   6 +-
 asn1/asn1c/GKCurrentKeys.h|   3 +-
 asn1/asn1c/GKNewKeys.c|  12 +-
 asn1/asn1c/GKNewKeys.h|   3 +-
 asn1/asn1c/GKReply.c  |   8 +-
 asn1/asn1c/GKReply.h  |   3 +-
 asn1/asn1c/GetKeytabControl.c |  10 +-
 asn1/asn1c/GetKeytabControl.h |   3 +-
 asn1/asn1c/INTEGER.c  | 466 +-
 asn1/asn1c/INTEGER.h  |  17 ++
 asn1/asn1c/Int32.c|   7 +-
 asn1/asn1c/Int32.h|   3 +-
 asn1/asn1c/KrbKey.c   |  10 +-
 asn1/asn1c/KrbKey.h   |   3 +-
 asn1/asn1c/NativeEnumerated.c |  11 +-
 asn1/asn1c/NativeInteger.c|  32 ++-
 asn1/asn1c/OCTET_STRING.c | 449 +++-
 asn1/asn1c/OCTET_STRING.h |   8 +-
 asn1/asn1c/TypeValuePair.c|   8 +-
 asn1/asn1c/TypeValuePair.h|   3 +-
 asn1/asn1c/asn_codecs.h   |   4 +-
 asn1/asn1c/asn_codecs_prim.c  |  43 ++--
 asn1/asn1c/asn_internal.h |  27 ++-
 asn1/asn1c/asn_system.h   |  35 +++-
 asn1/asn1c/ber_decoder.c  |   2 +-
 asn1/asn1c/ber_decoder.h  |   1 +
 asn1/asn1c/constr_CHOICE.c|  70 ---
 asn1/asn1c/constr_SEQUENCE.c  | 253 +++
 asn1/asn1c/constr_SET_OF.c|  17 +-
 asn1/asn1c/der_encoder.h  |   1 +
 asn1/asn1c/per_decoder.c  |  38 
 asn1/asn1c/per_decoder.h  |  14 +-
 asn1/asn1c/per_encoder.c  | 136 
 asn1/asn1c/per_encoder.h  |  24 ++-
 asn1/asn1c/per_opentype.c | 378 ++
 asn1/asn1c/per_opentype.h |  22 ++
 asn1/asn1c/per_support.c  | 207 +--
 asn1/asn1c/per_support.h  |  38 +++-
 asn1/asn1c/xer_decoder.c  |  14 +-
 asn1/asn1c/xer_decoder.h  |   7 +-
 41 files changed, 1931 insertions(+), 474 deletions(-)
 create mode 100644 asn1/asn1c/per_opentype.c
 create mode 100644 asn1/asn1c/per_opentype.h

diff --git a/asn1/asn1c/BIT_STRING.c b/asn1/asn1c/BIT_STRING.c
index 
6469d4fd2c8782048e228c19e67950f3b2dc1305..9b9827127b7ba438676630efef975e0e80ac45aa
 100644
--- a/asn1/asn1c/BIT_STRING.c
+++ b/asn1/asn1c/BIT_STRING.c
@@ -15,7 +15,7 @@ static ber_tlv_tag_t asn_DEF_BIT_STRING_tags[] = {
 static asn_OCTET_STRING_specifics_t asn_DEF_BIT_STRING_specs = {
sizeof(BIT_STRING_t),
offsetof(BIT_STRING_t, _asn_ctx),
-   1,  /* Special indicator that this is a BIT STRING type */
+   ASN_OSUBV_BIT
 };
 asn_TYPE_descriptor_t asn_DEF_BIT_STRING = {
"BIT STRING",
@@ -50,14 +50,15 @@ BIT_STRING_constraint(asn_TYPE_descriptor_t *td, const void 
*sptr,
const BIT_STRING_t *st = (const BIT_STRING_t *)sptr;
 
if(st && st->buf) {
-   if(st->size == 1 && st->bits_unused) {
-   _ASN_CTFAIL(app_key, td,
+   if((st->size == 0 && st->bits_unused)
+   || st->bits_unused < 0 || st->bits_unused > 7) {
+   _ASN_CTFAIL(app_key, td, sptr,
"%s: invalid padding byte (%s:%d)",
td->name, __FILE__, __LINE__);
return -1;
}
} else {
-   _ASN_CTFAIL(app_key, td,
+   _ASN_CTFAIL(app_key, td, sptr,
"%s: value not given (%s:%d)",
td->name, __FILE__, __LINE__);
return -1;
diff --git a/asn1/asn1c/GKCurrentKeys.c b/asn1/asn1c/GKCurrentKeys.c
index 
abcc53130d64259d0f2947b04412b4b769bb4360..bc7c1aacabdad433abeab83ddb43ba405aaff14b
 100644
--- a/asn1/asn1c/GKCurrentKeys.c
+++ b/asn1/asn1c/GKCurre

[Freeipa-devel] [PATCH 2/3] ASN1: Fix warning Wpointer-to-int-cast

[Lukas Slebodnik]

ehlo,

the first patch is very simple and it just suppress warning.
The second patch is either bug or dead code. I fixed it as a bug.
I'm not sure how to test 2nd patch.

LS
>From 49b9cd2f037f52d5095acc0d05ee1684ebb6a121 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 11:52:08 +0100
Subject: [PATCH 2/3] ASN1: Fix warning Wpointer-to-int-cast

The cast operator "()" has higher priority then ternary
operator "?:" and therefore the first argument of ternary operator
(pointer) was cast to "int" and then interpreted as boolean value.

I think we wanted to cast value of ternary operator to type int.
However it's not necesary becuase both of branches has type int.
---
 asn1/asn1c/constr_SET_OF.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/asn1/asn1c/constr_SET_OF.c b/asn1/asn1c/constr_SET_OF.c
index 
09f27db53dada6869accd8dbb3aba0f56b49e00b..312cfc2e91d3cc710bc26a12322828c00e400b9b
 100644
--- a/asn1/asn1c/constr_SET_OF.c
+++ b/asn1/asn1c/constr_SET_OF.c
@@ -904,7 +904,7 @@ SET_OF_decode_uper(asn_codec_ctx_t *opt_codec_ctx, 
asn_TYPE_descriptor_t *td,
nelems = uper_get_length(pd,
ct ? ct->effective_bits : -1, );
ASN_DEBUG("Got to decode %d elements (eff %d)",
-   (int)nelems, (int)ct ? ct->effective_bits : -1);
+   (int)nelems, ct ? ct->effective_bits : -1);
if(nelems < 0) _ASN_DECODE_STARVED;
}
 
-- 
2.5.0

>From fae6c79b65be4b21859f703dbcb7cfbb20286070 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 11:56:23 +0100
Subject: [PATCH 3/3] ASN1: Fix warning Wunused-value

There was used operator "equal to" but there should be
an assignment.
---
 asn1/asn1c/constr_SET_OF.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/asn1/asn1c/constr_SET_OF.c b/asn1/asn1c/constr_SET_OF.c
index 
312cfc2e91d3cc710bc26a12322828c00e400b9b..0d5efa4f83c189887822f4012fd2a500faf04ae4
 100644
--- a/asn1/asn1c/constr_SET_OF.c
+++ b/asn1/asn1c/constr_SET_OF.c
@@ -921,7 +921,7 @@ SET_OF_decode_uper(asn_codec_ctx_t *opt_codec_ctx, 
asn_TYPE_descriptor_t *td,
ASN_DEBUG("Failed to add element into %s",
td->name);
/* Fall through */
-   rv.code == RC_FAIL;
+   rv.code = RC_FAIL;
} else {
ASN_DEBUG("Failed decoding %s of %s (SET OF)",
elm->type->name, td->name);
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] IPA-SAM: Fix build with samba 4.4

[Lukas Slebodnik]

On (29/01/16 12:12), Lukas Slebodnik wrote:
>ehlo,
>
>attached patch shoudl fix build on fedora-24.
>It blocks static analysis scan.
>
>Even though it unblock build on fedora-24
>the solution is not ideal. It's possible that some changes
>need to be done in samba side as well.
>(missing prototypes for trim_string, smb_xstrdup
>
>LS

>>From f9057ca98557094a4db84ac072ee9efd02a4ff79 Mon Sep 17 00:00:00 2001
>From: Lukas Slebodnik <lsleb...@redhat.com>
>Date: Fri, 29 Jan 2016 10:40:18 +0100
>Subject: [PATCH 1/3] IPA-SAM: Fix build with samba 4.4
>
>samba_util.h is not shipped with samba-4.4
>and it was indirectly included by "ndr.h"
>
I checked the samba 4.4 release notes and they intentionaly
removed samba_util.h

@see
https://github.com/samba-team/samba/blob/master/WHATSNEW.txt#L191

REMOVED FEATURES


Public headers
--

Several public headers are not installed any longer. They are made for internal
use only. More public headers will very likely be removed in future releases.

The following headers are not installed any longer:
dlinklist.h, gen_ndr/epmapper.h, gen_ndr/mgmt.h, gen_ndr/ndr_atsvc_c.h,
gen_ndr/ndr_epmapper_c.h, gen_ndr/ndr_epmapper.h, gen_ndr/ndr_mgmt_c.h,
gen_ndr/ndr_mgmt.h,gensec.h, ldap_errors.h, ldap_message.h, ldap_ndr.h,
ldap-util.h, pytalloc.h, read_smb.h, registry.h, roles.h, samba_util.h,

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] import rpm causes failure during IPA caless install

[Lukas Slebodnik]

On (08/01/16 14:14), Jan Cholasta wrote:
>On 8.1.2016 14:09, Martin Basti wrote:
>>
>>
>>On 08.01.2016 14:00, Martin Kosek wrote:
>>>On 01/08/2016 01:45 PM, Martin Basti wrote:
Hello all,

fix for ticket https://fedorahosted.org/freeipa/ticket/5535
requires to import rpm module

This import somehow breaks nsslib in IPA
https://fedorahosted.org/freeipa/ticket/5572


We have 2 ways how to fix it:

1) move import rpm to body of methods (attached patch)
We are not sure how stable is this solution.

2) use solution with rpmdevtools proposed here:
https://www.redhat.com/archives/freeipa-devel/2016-January/msg00092.html
This should be rock stable but it needs many dependencies (rpm-python
too, perl)

The second way looks safer, so I would like to reimplement it, do you
all agree
or do you have better idea?
Feedback welcome, please ASAP.

Martin^2
>>>Since it's Friday, I invested 15 minutes to practice my C skills and
>>>use the
>>>python-cffi library to call rpm rpmvercmp library call directly
>>>(attached):
>>>
>>>$ python rpm.py 4.2.0-15.el7 4.2.0-15.el7_2.3
>>>4.2.0-15.el7 < 4.2.0-15.el7_2.3
>>>
>>>This would not introduce any additional dependency besides rpm-devel,
>>>right? :-)
>
>Not rpm-devel, but rpm-libs (you should dlopen "librpm.so.3").
>
CentOS 7 has librpm.so.3
but fedora 23+ has librpm.so.7

So if it is possible it will be good to avoid using specific vertsion.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] import rpm causes failure during IPA caless install

[Lukas Slebodnik]

On (08/01/16 14:18), Martin Babinsky wrote:
>On 01/08/2016 02:14 PM, Jan Cholasta wrote:
>>On 8.1.2016 14:09, Martin Basti wrote:
>>>
>>>
>>>On 08.01.2016 14:00, Martin Kosek wrote:
On 01/08/2016 01:45 PM, Martin Basti wrote:
>Hello all,
>
>fix for ticket https://fedorahosted.org/freeipa/ticket/5535
>requires to import rpm module
>
>This import somehow breaks nsslib in IPA
>https://fedorahosted.org/freeipa/ticket/5572
>
>
>We have 2 ways how to fix it:
>
>1) move import rpm to body of methods (attached patch)
>We are not sure how stable is this solution.
>
>2) use solution with rpmdevtools proposed here:
>https://www.redhat.com/archives/freeipa-devel/2016-January/msg00092.html
>
>This should be rock stable but it needs many dependencies (rpm-python
>too, perl)
>
>The second way looks safer, so I would like to reimplement it, do you
>all agree
>or do you have better idea?
>Feedback welcome, please ASAP.
>
>Martin^2
Since it's Friday, I invested 15 minutes to practice my C skills and
use the
python-cffi library to call rpm rpmvercmp library call directly
(attached):

$ python rpm.py 4.2.0-15.el7 4.2.0-15.el7_2.3
4.2.0-15.el7 < 4.2.0-15.el7_2.3

This would not introduce any additional dependency besides rpm-devel,
right? :-)
>>
>>Not rpm-devel, but rpm-libs (you should dlopen "librpm.so.3").
>>
>>>I'm afraid that this can cause the same issue as import rpm, because the
>>>nsslib is used from C library
>>
>>I would be surprised if NSS was used in this particular function.
>>
>
>If it would work then we could compare version like in this quickly hacked
>and untested patch.
>
>-- 
>Martin^3 Babinsky

>From 32ebf02d38b7174816f81579aab6ebbe26e80f64 Mon Sep 17 00:00:00 2001
>From: Martin Babinsky 
>Date: Fri, 8 Jan 2016 14:17:14 +0100
>Subject: [PATCH] use rpmvercmp via python-cffi for version comparison
>
>---
> ipaplatform/redhat/tasks.py | 45 ++---
> 1 file changed, 14 insertions(+), 31 deletions(-)
>
>diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
>index a0b4060..66ca3ad 100644
>--- a/ipaplatform/redhat/tasks.py
>+++ b/ipaplatform/redhat/tasks.py
>@@ -36,7 +36,7 @@ from subprocess import CalledProcessError
> from nss.error import NSPRError
> from pyasn1.error import PyAsn1Error
> from six.moves import urllib
>-import rpm
>+from cffi import FFI
> 
> from ipapython.ipa_log_manager import root_logger, log_mgr
> from ipapython import ipautil
>@@ -49,33 +49,16 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
> from ipaplatform.base.tasks import BaseTaskNamespace
> 
> 
>-# copied from rpmUtils/miscutils.py
>-def stringToVersion(verstring):
>-if verstring in [None, '']:
>-return (None, None, None)
>-i = verstring.find(':')
>-if i != -1:
>-try:
>-epoch = str(long(verstring[:i]))
>-except ValueError:
>-# look, garbage in the epoch field, how fun, kill it
>-epoch = '0' # this is our fallback, deal
>-else:
>-epoch = '0'
>-j = verstring.find('-')
>-if j != -1:
>-if verstring[i + 1:j] == '':
>-version = None
>-else:
>-version = verstring[i + 1:j]
>-release = verstring[j + 1:]
>-else:
>-if verstring[i + 1:] == '':
>-version = None
>-else:
>-version = verstring[i + 1:]
>-release = None
>-return (epoch, version, release)
>+def compare_rpm_versions(ver1, ver2):
>+ffi = FFI()
>+ffi.cdef("""
>+int rpmvercmp (const char *a, const char *b);
>+""")
>+C = ffi.dlopen("rpmlib.so.3")
rpmlib.so.3 does not exist.

It is is librpm.so.3
But as I mentioned in different mail. The soname was bumped in fedora 23.

So please use only 'ffi.dlopen("rpm")'

It would be also good to write unit test for this function.
So you can detect issues with cffi is any.


a = '4.2.0-15.el7'
b = '4.2.0-15.el7_2.3'
c = '4.2.0-16.el7'
compare_rpm_versions(a, b)
compare_rpm_versions(b, b)
compare_rpm_versions(c, b)

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0122-0123] reimplementation of package version comparison code

[Lukas Slebodnik]

On (08/01/16 16:59), Martin Babinsky wrote:
>Patch 0122 reimplements version checking and fixes
>https://fedorahosted.org/freeipa/ticket/5572
>
>Patch 0123 contains unit test for version checking code.
>
>Thanks to Martin^1 for the idea of using CFFI for calling rpm C-API directly.
>
>-- 
>Martin^3 Babinsky

>From c7a5d8970d100d071597b4e1d7cef8a27b8cd485 Mon Sep 17 00:00:00 2001
>From: Martin Babinsky 
>Date: Fri, 8 Jan 2016 15:54:00 +0100
>Subject: [PATCH 2/3] tests for package version comparison
>
>These tests will ensure that our package version handling code can correctly
>decide when to upgrade IPA master.
>
>https://fedorahosted.org/freeipa/ticket/5572
>---
> ipatests/test_ipaserver/test_version_comparsion.py | 47 ++
> 1 file changed, 47 insertions(+)
> create mode 100644 ipatests/test_ipaserver/test_version_comparsion.py
>
>diff --git a/ipatests/test_ipaserver/test_version_comparsion.py 
>b/ipatests/test_ipaserver/test_version_comparsion.py
>new file mode 100644
>index 
>..51d069b23ba389ffce39e948cdbc7a1faaa84563
>--- /dev/null
>+++ b/ipatests/test_ipaserver/test_version_comparsion.py
>@@ -0,0 +1,47 @@
>+#
>+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
>+#
>+
>+"""
>+tests for correct RPM version comparison
>+"""
>+
>+from ipaplatform.tasks import tasks
>+import pytest
>+
>+version_strings = [
>+("3.0.el6", "3.0.0.el6", "older"),
>+("3.0.0.el6", "3.0.0.el6_8.2", "older"),
>+("3.0.0-42.el6", "3.0.0.el6", "newer"),
>+("3.0.0-1", "3.0.0-42", "older"),
>+("3.0.0-42.el6", "3.3.3.fc20", "older"),
>+("4.2.0-15.el7", "4.2.0-15.el7_2.3", "older"),
>+("4.2.0-15.el7_2.3", "4.2.0-15.el7_2.3", "equal"),
>+("4.2.0-1.fc23", "4.2.1.fc23", "older"),
>+("4.2.3-alpha.fc23", "4.2.3-2.fc23", "older"),  # numeric version 
>elements have
>+# precedence over letters
>+("4.3.90.201601080923GIT55aeea7-0.fc23", "4.3.0-1.fc23", "newer")
>+]
>+
>+
>+@pytest.fixture(params=version_strings)
>+def versions(request):
>+return request.param
>+
>+class TestVersionComparsion(object):
>+
>+def test_versions(self, versions):
>+version_string1, version_string2, expected_comparison = versions
>+
>+ver1 = tasks.parse_ipa_version(version_string1)
>+ver2 = tasks.parse_ipa_version(version_string2)
>+
>+if expected_comparison == "newer":
>+assert ver1 > ver2
>+elif expected_comparison == "older":
>+assert ver1 < ver2
>+elif expected_comparison == "equal":
>+assert ver1 == ver2
>+else:
>+raise TypeError(
>+"Unexpected comparison string: {}", expected_comparison)
>-- 
>2.5.0
>

>From 9677e1a3ca2f5837f1b779780127adf27efa81df Mon Sep 17 00:00:00 2001
>From: Martin Babinsky 
>Date: Fri, 8 Jan 2016 14:17:14 +0100
>Subject: [PATCH 1/3] use FFI call to rpmvercmp function for version comparison
>
>Stop using rpm-python to compare package versions since the implicit NSS
>initialization upon  the module import breaks NSS handling in IPA code. Call
>rpm-libs C-API function via CFFI instead.
>
>Big thanks to Martin Kosek  for sharing the code snippet
>that spurred this patch.
>
>https://fedorahosted.org/freeipa/ticket/5572
>---
> freeipa.spec.in |  2 +-
> ipaplatform/redhat/tasks.py | 59 +
> 2 files changed, 29 insertions(+), 32 deletions(-)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>7e956538d0f6c24bab636579303e0c7b5eeec199..7f31d41d16b2a26b404c277595f0994a21123f80
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -214,7 +214,7 @@ Requires: python-pyasn1
> Requires: dbus-python
> Requires: python-dns >= 1.11.1
> Requires: python-kdcproxy >= 0.3
>-Requires: rpm-python
>+Requires: rpm-devel
/usr/lib64/librpm.so.7 is provided by package rpm-libs


sh$ rpm -qf /usr/lib64/librpm.so.7
rpm-libs-4.13.0-0.rc1.7.fc23.x86_64

It's not very common to depend on devel packages.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0122-0123] reimplementation of package version comparison code

[Lukas Slebodnik]

On (08/01/16 17:56), Martin Babinsky wrote:
>On 01/08/2016 05:23 PM, Lukas Slebodnik wrote:
>>On (08/01/16 16:59), Martin Babinsky wrote:
>>>Patch 0122 reimplements version checking and fixes
>>>https://fedorahosted.org/freeipa/ticket/5572
>>>
>>>Patch 0123 contains unit test for version checking code.
>>>
>>>Thanks to Martin^1 for the idea of using CFFI for calling rpm C-API directly.
>>>
>>>--
>>>Martin^3 Babinsky
>>
>>>From c7a5d8970d100d071597b4e1d7cef8a27b8cd485 Mon Sep 17 00:00:00 2001
>>>From: Martin Babinsky <mbabi...@redhat.com>
>>>Date: Fri, 8 Jan 2016 15:54:00 +0100
>>>Subject: [PATCH 2/3] tests for package version comparison
>>>
>>>These tests will ensure that our package version handling code can correctly
>>>decide when to upgrade IPA master.
>>>
>>>https://fedorahosted.org/freeipa/ticket/5572
>>>---
>>>ipatests/test_ipaserver/test_version_comparsion.py | 47 
>>>++
>>>1 file changed, 47 insertions(+)
>>>create mode 100644 ipatests/test_ipaserver/test_version_comparsion.py
>>>
>>>diff --git a/ipatests/test_ipaserver/test_version_comparsion.py 
>>>b/ipatests/test_ipaserver/test_version_comparsion.py
>>>new file mode 100644
>>>index 
>>>..51d069b23ba389ffce39e948cdbc7a1faaa84563
>>>--- /dev/null
>>>+++ b/ipatests/test_ipaserver/test_version_comparsion.py
>>>@@ -0,0 +1,47 @@
>>>+#
>>>+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
>>>+#
>>>+
>>>+"""
>>>+tests for correct RPM version comparison
>>>+"""
>>>+
>>>+from ipaplatform.tasks import tasks
>>>+import pytest
>>>+
>>>+version_strings = [
>>>+("3.0.el6", "3.0.0.el6", "older"),
>>>+("3.0.0.el6", "3.0.0.el6_8.2", "older"),
>>>+("3.0.0-42.el6", "3.0.0.el6", "newer"),
>>>+("3.0.0-1", "3.0.0-42", "older"),
>>>+("3.0.0-42.el6", "3.3.3.fc20", "older"),
>>>+("4.2.0-15.el7", "4.2.0-15.el7_2.3", "older"),
>>>+("4.2.0-15.el7_2.3", "4.2.0-15.el7_2.3", "equal"),
>>>+("4.2.0-1.fc23", "4.2.1.fc23", "older"),
>>>+("4.2.3-alpha.fc23", "4.2.3-2.fc23", "older"),  # numeric version 
>>>elements have
>>>+# precedence over letters
>>>+("4.3.90.201601080923GIT55aeea7-0.fc23", "4.3.0-1.fc23", "newer")
>>>+]
>>>+
>>>+
>>>+@pytest.fixture(params=version_strings)
>>>+def versions(request):
>>>+return request.param
>>>+
>>>+class TestVersionComparsion(object):
>>>+
>>>+def test_versions(self, versions):
>>>+version_string1, version_string2, expected_comparison = versions
>>>+
>>>+ver1 = tasks.parse_ipa_version(version_string1)
>>>+ver2 = tasks.parse_ipa_version(version_string2)
>>>+
>>>+if expected_comparison == "newer":
>>>+assert ver1 > ver2
>>>+elif expected_comparison == "older":
>>>+assert ver1 < ver2
>>>+elif expected_comparison == "equal":
>>>+assert ver1 == ver2
>>>+else:
>>>+raise TypeError(
>>>+"Unexpected comparison string: {}", expected_comparison)
>>>--
>>>2.5.0
>>>
>>
>>>From 9677e1a3ca2f5837f1b779780127adf27efa81df Mon Sep 17 00:00:00 2001
>>>From: Martin Babinsky <mbabi...@redhat.com>
>>>Date: Fri, 8 Jan 2016 14:17:14 +0100
>>>Subject: [PATCH 1/3] use FFI call to rpmvercmp function for version 
>>>comparison
>>>
>>>Stop using rpm-python to compare package versions since the implicit NSS
>>>initialization upon  the module import breaks NSS handling in IPA code. Call
>>>rpm-libs C-API function via CFFI instead.
>>>
>>>Big thanks to Martin Kosek <mko...@redhat.com> for sharing the code snippet
>>>that spurred this patch.
>>>
>>>https://fedorahosted.org/freeipa/ticket/5572
>>>---
>&

Re: [Freeipa-devel] 4.3 on rawhide build task fail

[Lukas Slebodnik]

On (22/12/15 16:31), Petr Vobornik wrote:
>Build of 4.3 on Fedora rawhide failed at the end on rpmdiff check. Builds for
>all arches were successful and also works in COPR.
> 0 free 1 open 4 done 0 failed
>12284450 build (rawhide, /freeipa:b2442d51ba3f2a5f907f72e9bd90c5889bd89c0e):
>open (buildppcle-07.phx2.fedoraproject.org) -> FAILED: BuildError: mismatch
>when analyzing python3-ipatests-4.3.0-1.fc24.noarch.rpm, rpmdiff output was:
>error: cannot open Packages index using db5 - Permission denied (13)
>error: cannot open Packages database in /var/lib/rpm
>error: cannot open Packages database in /var/lib/rpm
>removed REQUIRES python3-ipalib(armv7hl-32) = 4.3.0-1.fc24
>added REQUIRES python3-ipalib(x86-64) = 4.3.0-1.fc24
>0 free 0 open 4 done 1 failed
I think that log file is crystal clear.

The noarch package "python3-ipatests-4.3.0-1.fc24.noarch.rpm"
requires packages with strict architecture.

sh$ wget 
https://kojipkgs.fedoraproject.org//work/tasks/4513/12284513/python3-ipatests-4.3.0-1.fc24.noarch.rpm

sh $rpm -qp --requires python3-ipatests-4.3.0-1.fc24.noarch.rpm
/usr/bin/python3
freeipa-client-common = 4.3.0-1.fc24
python(abi) = 3.5
python3-coverage
python3-ipalib(x86-64) = 4.3.0-1.fc24
python3-nose
python3-polib
python3-pytest >= 2.6
python3-pytest-multihost >= 0.5
python3-pytest-sourceorder
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1
tar
xz

noarch pacakges are build for each architecture: armv7hl-32, x86-64, i686
But the same package should be built on each platform.

In your case requires, provides are different. This is a reason
why rpmdiff failed for some noarch packages.

Attached are two patches which fix issues with build in koji.
The 1st patch removes usage of %{_isa} in noarch packages.

The second one violates python packaging guidelines
http://fedoraproject.org/wiki/Packaging:Python#Reviewer_checklist
But there seems to be bug (in rpmbuild???) because "rpm --eval" does not
generate provides with architecture.

sh$ wget 
https://kojipkgs.fedoraproject.org//work/tasks/4513/12284513/python2-ipatests-4.3.0-1.fc24.noarch.rpm

sh$ rpm -qp --provides python2-ipatests-4.3.0-1.fc24.noarch.rpm
freeipa-tests(x86-64) = 4.3.0-1.fc24
ipa-tests(x86-64) = 4.3.0
python-ipatests = 4.3.0-1.fc24
python-ipatests(x86-64) = 4.3.0-1.fc24
python2-ipatests = 4.3.0-1.fc24

sh$ rpm --eval "%{?python_provide:%python_provide python2-ipatests}"
Provides: python-ipatests = %{version}-%{release}
Obsoletes: python-ipatests < %{version}-%{release}

So better workaround could be to replace macro "%python_provide"
with manually generated "Provides" and "Obsoletes"
It's up to you and discussion with python experts :-)

LS
>From 0674e1e6aae2423c050be520b9c1b13f8feeb3d8 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 4 Jan 2016 19:02:24 +0100
Subject: [PATCH 1/2] Remove _isa from requires and provides

---
 freeipa.spec | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/freeipa.spec b/freeipa.spec
index 
9c32876a0faa45dbe6aac49551264c0366777b03..a1de4dc5dd2442899c6a36cb48a732fd49ad7909
 100644
--- a/freeipa.spec
+++ b/freeipa.spec
@@ -365,7 +365,7 @@ BuildArch: noarch
 %{?python_provide:%python_provide python2-ipaclient}
 Requires: %{name}-client-common = %{version}-%{release}
 Requires: %{name}-common = %{version}-%{release}
-Requires: python2-ipalib%{?_isa} = %{version}-%{release}
+Requires: python2-ipalib = %{version}-%{release}
 Requires: python-dns >= 1.11.1
 
 %description -n python2-ipaclient
@@ -402,7 +402,7 @@ Summary: IPA administrative tools
 Group: System Environment/Base
 BuildArch: noarch
 Requires: %{name}-client-common = %{version}-%{release}
-Requires: python2-ipalib%{?_isa} = %{version}-%{release}
+Requires: python2-ipalib = %{version}-%{release}
 Requires: python-ldap
 
 Provides: %{alt_name}-admintools = %{version}
@@ -425,7 +425,7 @@ BuildArch: noarch
 Obsoletes: %{name}-python < 4.2.91
 Provides: %{name}-python = %{version}-%{release}
 Requires: %{name}-common = %{version}-%{release}
-Requires: python2-ipalib%{?_isa} = %{version}-%{release}
+Requires: python2-ipalib = %{version}-%{release}
 
 Provides: %{alt_name}-python-compat = %{version}
 Conflicts: %{alt_name}-python-compat
@@ -561,10 +561,10 @@ If you are using IPA, you need to install this package.
 Summary: IPA tests and test tools
 BuildArch: noarch
 Obsoletes: %{name}-tests < 4.2.91
-Provides: %{name}-tests%{?_isa} = %{version}-%{release}
+Provides: %{name}-tests = %{version}-%{release}
 %{?python_provide:%python_provide python2-ipatests}
 Requires: %{name}-client-common = %{version}-%{release}
-Requires: python2-ipalib%{?_isa} = %{version}-%{release}
+Requires: python2-ipalib = %{version}-%{release}
 Requires: tar
 Requires: xz
 Requires: python-

Re: [Freeipa-devel] 4.3 on rawhide build task fail

[Lukas Slebodnik]

On (05/01/16 10:37), Lukas Slebodnik wrote:
>On (22/12/15 16:31), Petr Vobornik wrote:
>>Build of 4.3 on Fedora rawhide failed at the end on rpmdiff check. Builds for
>>all arches were successful and also works in COPR.
>> 0 free 1 open 4 done 0 failed
>>12284450 build (rawhide, /freeipa:b2442d51ba3f2a5f907f72e9bd90c5889bd89c0e):
>>open (buildppcle-07.phx2.fedoraproject.org) -> FAILED: BuildError: mismatch
>>when analyzing python3-ipatests-4.3.0-1.fc24.noarch.rpm, rpmdiff output was:
>>error: cannot open Packages index using db5 - Permission denied (13)
>>error: cannot open Packages database in /var/lib/rpm
>>error: cannot open Packages database in /var/lib/rpm
>>removed REQUIRES python3-ipalib(armv7hl-32) = 4.3.0-1.fc24
>>added REQUIRES python3-ipalib(x86-64) = 4.3.0-1.fc24
>>0 free 0 open 4 done 1 failed
>I think that log file is crystal clear.
>
>The noarch package "python3-ipatests-4.3.0-1.fc24.noarch.rpm"
>requires packages with strict architecture.
>
>sh$ wget 
>https://kojipkgs.fedoraproject.org//work/tasks/4513/12284513/python3-ipatests-4.3.0-1.fc24.noarch.rpm
>
>sh $rpm -qp --requires python3-ipatests-4.3.0-1.fc24.noarch.rpm
>/usr/bin/python3
>freeipa-client-common = 4.3.0-1.fc24
>python(abi) = 3.5
>python3-coverage
>python3-ipalib(x86-64) = 4.3.0-1.fc24
>python3-nose
>python3-polib
>python3-pytest >= 2.6
>python3-pytest-multihost >= 0.5
>python3-pytest-sourceorder
>rpmlib(CompressedFileNames) <= 3.0.4-1
>rpmlib(FileDigests) <= 4.6.0-1
>rpmlib(PayloadFilesHavePrefix) <= 4.0-1
>rpmlib(PayloadIsXz) <= 5.2-1
>tar
>xz
>
>noarch pacakges are build for each architecture: armv7hl-32, x86-64, i686
>But the same package should be built on each platform.
>
>In your case requires, provides are different. This is a reason
>why rpmdiff failed for some noarch packages.
>
>Attached are two patches which fix issues with build in koji.
>The 1st patch removes usage of %{_isa} in noarch packages.
>
>The second one violates python packaging guidelines
>http://fedoraproject.org/wiki/Packaging:Python#Reviewer_checklist
>But there seems to be bug (in rpmbuild???) because "rpm --eval" does not
>generate provides with architecture.
>
>sh$ wget 
>https://kojipkgs.fedoraproject.org//work/tasks/4513/12284513/python2-ipatests-4.3.0-1.fc24.noarch.rpm
>
>sh$ rpm -qp --provides python2-ipatests-4.3.0-1.fc24.noarch.rpm
>freeipa-tests(x86-64) = 4.3.0-1.fc24
>ipa-tests(x86-64) = 4.3.0
>python-ipatests = 4.3.0-1.fc24
>python-ipatests(x86-64) = 4.3.0-1.fc24
>python2-ipatests = 4.3.0-1.fc24
>
>sh$ rpm --eval "%{?python_provide:%python_provide python2-ipatests}"
>Provides: python-ipatests = %{version}-%{release}
>Obsoletes: python-ipatests < %{version}-%{release}
>
>So better workaround could be to replace macro "%python_provide"
>with manually generated "Provides" and "Obsoletes"
>It's up to you and discussion with python experts :-)
>
>LS

>>From 0674e1e6aae2423c050be520b9c1b13f8feeb3d8 Mon Sep 17 00:00:00 2001
>From: Lukas Slebodnik <lsleb...@redhat.com>
>Date: Mon, 4 Jan 2016 19:02:24 +0100
>Subject: [PATCH 1/2] Remove _isa from requires and provides
>
And here is a link to koji build with the patches
http://koji.fedoraproject.org/koji/taskinfo?taskID=12405513

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 027] Require Dogtag 10.2.6-13 to fix KRA uninstall

[Lukas Slebodnik]

On (05/01/16 12:24), Christian Heimes wrote:
>The combination of a bug in Dogtag's sslget command and a new feature
>in mod_nss causes an incomplete uninstallation of KRA. The bug has been
>fixed in Dogtag 10.2.6-13.
>
and it ins in fedora 23 stable for a week
https://bodhi.fedoraproject.org/updates/FEDORA-2015-c7dd78ac78

LS

>https://fedorahosted.org/freeipa/ticket/5469
>https://fedorahosted.org/pki/ticket/1704
>
>Signed-off-by: Christian Heimes 

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH-0019][Tests]Enabled --auto-reverse by default for master installation

[Lukas Slebodnik]

On (17/12/15 13:53), Oleg Fayans wrote:
>
>-- 
>Oleg Fayans
>Quality Engineer
>FreeIPA team
>RedHat.

>From ed4630140386c1043e36733eb42ec402cc276bee Mon Sep 17 00:00:00 2001
>From: Oleg Fayans 
>Date: Thu, 17 Dec 2015 13:50:19 +0100
>Subject: [PATCH] Enabled automatic creation of reverse zone during master
> installation
>
The commit message does not contain ticket.
It is also not explained why this change was done.
Could you update commit message with verbose explanation?

Proper commit message might prevent removal of this change in future
based on "git blame"

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Testing FreeIPA 4.3 for GA

[Lukas Slebodnik]

On (15/12/15 08:32), Lukas Slebodnik wrote:
>On (15/12/15 01:05), Petr Vobornik wrote:
>>Blocking patches for FreeIPA 4.3 were pushed, ipa-4-3 branch was created.
>>Master branch is ready for 4.4 development.
>>
>>A build is available for testing in my pvoborni/freeipa-4-3 COPR repo [1]
>>until the official 4.3 repo is created. The repo contains only this build.
>>The build is not pure upstream ipa-4-3 branch but rather a build which will
>>go to Fedora rawhide and official 4.3 copr repo - Fedora downstream spec with
>>the SELinux workaround applied [2][3].
>>
>>Known issues:
>>* https://fedorahosted.org/freeipa/ticket/5469 - requires fix in PKI
>>* https://bugzilla.redhat.com/show_bug.cgi?id=1289930 - SELinux Policy update
>>needed for connection check
>>
>>I have started drafting release page [4].
>>
>It looks like there is a bug in dnf, because it cannot install
>freeipa-tests package; which was renamed to python2-ipatests.
>
>[root@3a8359f9e1cd ~]# rpm -q freeipa-server
>freeipa-server-4.3.0-1.fc23.x86_64
>
>
>[root@3a8359f9e1cd ~]# dnf install -y --best freeipa-tests
>Last metadata expiration check performed 0:09:23 ago on Tue Dec 15 07:16:26
>2015.
>Error: package freeipa-tests-4.2.3-1.1.fc23.x86_64 requires freeipa-client =
>4.2.3-1.1.fc23, but none of the providers can be installed.
>package freeipa-tests-4.2.2-1.fc23.x86_64 requires freeipa-python =
>4.2.2-1.fc23, but none of the providers can be installed
>(try to add '--allowerasing' to command line to replace conflicting packages)
>
>
>[root@3a8359f9e1cd ~]# rpm -qp --provides
>./python2-ipatests-4.3.0-1.fc23.noarch.rpm
>freeipa-tests(x86-64) = 4.3.0-1.fc23
>ipa-tests(x86-64) = 4.3.0
>python-ipatests = 4.3.0-1.fc23
>python2-ipatests = 4.3.0-1.fc23
>
>
>FYI: It works with yum-deprecated.
>
dnf bug https://bugzilla.redhat.com/show_bug.cgi?id=1291850

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Testing FreeIPA 4.3 for GA

[Lukas Slebodnik]

On (15/12/15 01:05), Petr Vobornik wrote:
>Blocking patches for FreeIPA 4.3 were pushed, ipa-4-3 branch was created.
>Master branch is ready for 4.4 development.
>
>A build is available for testing in my pvoborni/freeipa-4-3 COPR repo [1]
>until the official 4.3 repo is created. The repo contains only this build.
>The build is not pure upstream ipa-4-3 branch but rather a build which will
>go to Fedora rawhide and official 4.3 copr repo - Fedora downstream spec with
>the SELinux workaround applied [2][3].
>
>Known issues:
>* https://fedorahosted.org/freeipa/ticket/5469 - requires fix in PKI
>* https://bugzilla.redhat.com/show_bug.cgi?id=1289930 - SELinux Policy update
>needed for connection check
>
I would suggest to do small change in packaging.

sh$ rpm -qp --requires ./freeipa-client-4.3.0-1.fc23.x86_64.rpm | grep pam
pam_krb5

pam_krb5 is not used by default. I will suggest to use weak dependencies here
If you do not want to remove it completely.
http://rpm.org/wiki/PackagerDocs/Dependencies#Weakdependencies


LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Testing FreeIPA 4.3 for GA

[Lukas Slebodnik]

On (15/12/15 01:05), Petr Vobornik wrote:
>Blocking patches for FreeIPA 4.3 were pushed, ipa-4-3 branch was created.
>Master branch is ready for 4.4 development.
>
>A build is available for testing in my pvoborni/freeipa-4-3 COPR repo [1]
>until the official 4.3 repo is created. The repo contains only this build.
>The build is not pure upstream ipa-4-3 branch but rather a build which will
>go to Fedora rawhide and official 4.3 copr repo - Fedora downstream spec with
>the SELinux workaround applied [2][3].
>
>Known issues:
>* https://fedorahosted.org/freeipa/ticket/5469 - requires fix in PKI
>* https://bugzilla.redhat.com/show_bug.cgi?id=1289930 - SELinux Policy update
>needed for connection check
>
>I have started drafting release page [4].
>
It looks like there is a bug in dnf, because it cannot install
freeipa-tests package; which was renamed to python2-ipatests.

[root@3a8359f9e1cd ~]# rpm -q freeipa-server
freeipa-server-4.3.0-1.fc23.x86_64


[root@3a8359f9e1cd ~]# dnf install -y --best freeipa-tests
Last metadata expiration check performed 0:09:23 ago on Tue Dec 15 07:16:26
2015.
Error: package freeipa-tests-4.2.3-1.1.fc23.x86_64 requires freeipa-client =
4.2.3-1.1.fc23, but none of the providers can be installed.
package freeipa-tests-4.2.2-1.fc23.x86_64 requires freeipa-python =
4.2.2-1.fc23, but none of the providers can be installed
(try to add '--allowerasing' to command line to replace conflicting packages)


[root@3a8359f9e1cd ~]# rpm -qp --provides
./python2-ipatests-4.3.0-1.fc23.noarch.rpm
freeipa-tests(x86-64) = 4.3.0-1.fc23
ipa-tests(x86-64) = 4.3.0
python-ipatests = 4.3.0-1.fc23
python2-ipatests = 4.3.0-1.fc23


FYI: It works with yum-deprecated.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0069] Add 'review' target for make

[Lukas Slebodnik]

On (10/12/15 18:04), Petr Spacek wrote:
>On 9.12.2015 15:30, Petr Spacek wrote:
>> Hello,
>> 
>> this patch automates some of sanity checks proposed by Petr Vobornik.
>> 
>> 'make review' should be used in root of clean Git tree which has patches 
>> under
>> review applied.
>>
+1 for automatisation
-1 for name.

Your script does not review[1] anything. Code review(peer review) is a job
for humans. What do you think about alternative
names: "review-helper", "sanity-check" ...

>> Magic in review.sh attempts to detect nearest remote branch which can be used
>> as diff base for review. Please see review.sh for further details.
>
>And here is the patch! :-)
>
>-- 
>Petr^2 Spacek

[1] https://en.wikipedia.org/wiki/Code_review

>From 52393bf2bb0ad74dbe37e496b1fd41a6ab22bd90 Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Tue, 8 Dec 2015 12:06:33 +0100
>Subject: [PATCH] Add 'review' target for make. It automates following tasks:
>
>- check if ACI.txt and API.txt are up-to-date
>- check if VERSION was changed if API was changed
>- pep8 --diff does not produce new errors when ran on diff from origin/branch
>- make lint does not produce errors
>---
> Makefile  |  4 
> review.sh | 64 +++
> 2 files changed, 68 insertions(+)
> create mode 100755 review.sh
>
>diff --git a/Makefile b/Makefile
>index 
>d2c37f1597a011af2bd9ef1a4f7ce87ac59620a3..b85b3a5d6362150dcebf16745414c3d416c9547f
> 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -79,6 +79,10 @@ check: bootstrap-autogen server tests
>   (cd $$subdir && $(MAKE) check) || exit 1; \
>   done
> 
>+# works only in Git tree
>+review: version-update
>+  ./review.sh
>+
> bootstrap-autogen: version-update client-autogen
>   @echo "Building IPA $(IPA_VERSION)"
>   cd asn1; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr 
> --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
>diff --git a/review.sh b/review.sh
>new file mode 100755
>index 
>..3a5114fbe88d3aba8e926e049500f5ac5f41a83e
>--- /dev/null
>+++ b/review.sh
>@@ -0,0 +1,64 @@
>+#!/bin/bash
>+is_tree_clean() {
>+  test "$(git status --porcelain "$@")" == ""
>+  return $?
>+}
>+
>+log_error() {
>+  echo "ERROR: ${1}, continuing ..."
>+  errs=(${errs[@]} "ERROR: ${1}\n")
>+}
>+
>+set -o errexit -o nounset #-o xtrace
>+
>+LOGFILE="$(mktemp --suff=.log)"
>+exec > >(tee -i ${LOGFILE})
>+exec 2>&1
>+
>+echo -n "make lint is running ... "
>+make --silent lint || log_error "make lint failed"
>+
>+# Go backwards in history until you find a remote branch from which current
>+# branch was created. This will be used as base for git diff.
>+PATCHCNT=0
>+BASEBRANCH=""
If base branch means the remote branch which was used for cloning the cerrent
git HEAD than you can simplify it a littl bit.
git rev-parse --symbolic-full-name @{upstream}

>+CURRBRANCH="$(git branch --remote --contains)"
>+while [ "${BASEBRANCH}" == "" ]
>+do
>+  BASEBRANCH="$(git branch --remote --contains "HEAD~${PATCHCNT}" | grep 
>-v "^. ${CURRBRANCH}$" || :)"
>+  PATCHCNT="$(expr "${PATCHCNT}" + 1)"
Then for patch count you can use
PATCHCNT=$(git log --oneline $BASEBRANCH..HEAD | wc -l)

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison

[Lukas Slebodnik]

On (09/12/15 19:22), Martin Basti wrote:
>https://fedorahosted.org/freeipa/ticket/5535
>
>Patch attached.

>From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00 2001
>From: Martin Basti 
>Date: Wed, 9 Dec 2015 18:53:35 +0100
>Subject: [PATCH] Fix version comparison
>
>Use RPM library to compare vendor versions of IPA for redhat platform
>
>https://fedorahosted.org/freeipa/ticket/5535
>---
> freeipa.spec.in |  2 ++
> ipaplatform/redhat/tasks.py | 19 +++
> 2 files changed, 21 insertions(+)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir}
> Requires: gzip
> Requires: python-gssapi >= 1.1.0
> Requires: custodia
>+Requires: rpm-python
>+Requires: rpmdevtools
Could you explain why do you need the 2nd package?
It does not contains any python modules
and I cannot see usage of any binary in this patch

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0115] fix error message assertion in negative forced client reenrollment tests

[Lukas Slebodnik]

On (08/12/15 18:02), Martin Babinsky wrote:
>This patch fixes the assertions in negative test cases of
>'test_forced_client_reenrollment' CI test suite.
>
>On ipa-4-2 it fixes https://fedorahosted.org/freeipa/ticket/5511 and makes
>all 8 tests in this suite green, shiny and happy again.
>
>It also fixes negative test cases on master branch, but the positive cases
>are broken there due to https://fedorahosted.org/freeipa/ticket/5528
>
>-- 
>Martin^3 Babinsky

>From eb152f6996a8b653d8676ade826e806898fdf556 Mon Sep 17 00:00:00 2001
>From: Martin Babinsky 
>Date: Tue, 8 Dec 2015 17:00:11 +0100
>Subject: [PATCH] fix error message assertion in negative forced client
> reenrollment tests
>
>https://fedorahosted.org/freeipa/ticket/5511
>
>---
> ipatests/test_integration/test_forced_client_reenrollment.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py 
>b/ipatests/test_integration/test_forced_client_reenrollment.py
>index 
>e1edff9b7f2d535a341d1e8ca55917012943818e..df0e90526c5c8dd78d3af9d7ddb7c9bdbf5a2268
> 100644
>--- a/ipatests/test_integration/test_forced_client_reenrollment.py
>+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
>@@ -198,7 +198,7 @@ class TestForcedClientReenrollment(IntegrationTest):
> assert 'IPA Server: %s' % server.hostname in result.stderr_text
> 
> if expect_fail:
>-err_msg = 'Kerberos authentication failed using keytab'
>+err_msg = "Kerberos authentication failed: "
Test passed for freeipa-4.2:
* fedora 23 with krb5-1.14
* rhel7.2 with krb5-1.13

ACK

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for sudo rules validating

[Lukas Slebodnik]

On (10/12/15 11:40), Tomas Babej wrote:
>On 12/10/2015 09:05 AM, Lukas Slebodnik wrote:
>> On (08/12/15 14:47), Tomas Babej wrote:
>>>
>>>
>>> On 12/03/2015 04:33 PM, Tomas Babej wrote:
>>>>
>>>>
>>>> On 12/03/2015 04:26 PM, Aleš Mareček wrote:
>>>>> Hello,
>>>>>
>>>>> ACK for code
>>>>> NACK for the placing "get_client_ip_with_hostmask" function to 
>>>>> test_sudo.py (this function should be in some more general file)
>>>>>
>>>>
>>>> What place would you propose? The task.py is not a good place, as this
>>>> is not really a task.
>>>>
>>>> Nevertheless, I'd rather have it moved when an use case outside
>>>> test_sudo.py actually arises. Right now it would lead to unnecessary
>>>> cluttering.
>>>>
>>>> Tomas
>>>>
>>>
>>> I re-discovered ipatests.test_integration.util (two years after I
>>> created it :D) - which seemed ideal for this function.
>>>
>>> Updated patch attached.
>>>
>>> Tomas
>> 
>>>From 33552d6078d75ee99f9ec19ae143df5a61ba84a4 Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej <tba...@redhat.com>
>>> Date: Wed, 2 Dec 2015 15:25:49 +0100
>>> Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
>>> hostmask
>>>
>>> IPA sudo tests worked under the assumption that the clients
>>> that are executing the sudo commands have their IPs assigned
>>> within 255.255.255.0 hostmask.
>>>
>>> Removes this (invalid) assumption and adds a
>>> dynamic detection of the hostmask of the IPA client.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5501
>>> ---
>>> ipatests/test_integration/test_sudo.py | 33 
>>> +++--
>>> ipatests/test_integration/util.py  | 16 
>>> 2 files changed, 43 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/ipatests/test_integration/util.py 
>>> b/ipatests/test_integration/util.py
>>> index 
>>> 1a1bb3fcc923c9f2721f0a4c1cb7a1ba2ccc2dd8..187f39e80e84af0eb4938fb19ac3d3c7c2280ed9
>>>  100644
>>> --- a/ipatests/test_integration/util.py
>>> +++ b/ipatests/test_integration/util.py
>>> @@ -58,3 +58,19 @@ def run_repeatedly(host, command, assert_zero_rc=True, 
>>> test=None,
>>>  .format(cmd=' '.join(command),
>>>  times=timeout / time_step,
>>>  timeout=timeout))
>>> +
>>> +
>>> +def get_host_ip_with_hostmask(host):
>>> +"""
>>> +Detects the IP of the host including the hostmask.
>>> +
>>> +Returns None if the IP could not be detected.
>>> +"""
>>> +
>>> +ip = host.ip
>>> +result = host.run_command(['ip', 'addr'])
>>> +full_ip_regex = r'(?P%s/\d{1,2}) ' % re.escape(ip)
>>> +match = re.search(full_ip_regex, result.stdout_text)
>> ./make-lint 
>> * Module ipatests.test_integration.util
>> ipatests/test_integration/util.py:72: [E0602(undefined-variable), 
>> get_host_ip_with_hostmask] Undefined variable 're')
>> ipatests/test_integration/util.py:73: [E0602(undefined-variable), 
>> get_host_ip_with_hostmask] Undefined variable 're')
>> ===
>> Errors were found during the static code check.
>> If you are certain that any of the reported errors are false positives, 
>> please
>> mark them in the source code according to the pylint documentation.
>> ===
>> Makefile:124: recipe for target 'lint' failed
>> 
>> LS
>> 
>
>Nothing can break when moving a function, right? Missing import fixed.
>
>Tomas

>From c176ff1ab9ea1c56dc0c5d44bc490d925fad1497 Mon Sep 17 00:00:00 2001
>From: Tomas Babej <tba...@redhat.com>
>Date: Wed, 2 Dec 2015 15:25:49 +0100
>Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
> hostmask
>
>IPA sudo tests worked under the assumption that the clients
>that are executing the sudo commands have their IPs assigned
>within 255.255.255.0 hostmask.
>
>Removes this (invalid) assumption and adds a
>dynamic detection of the hostmask of the IPA client.
>
>https://fedorahosted.org/freeipa/ticket/5501
>---
> ipatests/test_integration/test_sudo.py | 32 ++--
> ipatests/test_integration/util.py  | 17 +
> 2 files changed, 43 insertions(+), 6 deletions(-)
>
Thank you very much.

ACK

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code