Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spec file: clean up BuildRequires #159

Closed
wants to merge 7 commits into from
Closed

spec file: clean up BuildRequires #159

wants to merge 7 commits into from

Conversation

HonzaCholasta
Copy link
Contributor

@HonzaCholasta HonzaCholasta commented Oct 12, 2016

Add missing cyrus-sasl-devel, python-cffi, python-custodia, python-nose,
python-paste, python-sssdconfig and systemd-python BuildRequires.

Remove unused custodia, java-headless, m4, policycoreutils,
python-kdcproxy, python-rhsm, pyOpenSSL and systemd-units BuildRequires.

Correct versioned BuildRequires and provide explanatory comments.

spec file: do not include BuildRequires for lint by default

Lint is never executed from rpmbuild, so the BuildRequires for lint are
purely informational.

Include them only if %with_lint RPM macro is specified.

pylint: enable the import-error check

Check for import errors with pylint to make sure new python package
dependencies are not overlooked.

ipaserver: remove ipalib import from setup.py

Instead of importing ipalib to get IPA version string, create setup.py from
a template and have the version string automatically filled in.

This makes it possible to build the ipaserver package without having to
have ipalib dependencies installed.

makeapi, makeaci: do not fail on missing imports

Add import hook to makeapi and makeaci which makes them ignore import
errors in modules in our source tree and instead print a warning.

This makes it possible to build IPA without having to have most of our
runtime dependencies installed.

client: remove unused libcurl build dependency

pwpolicy: do not run klist on import

On pwpolicy module import, "klist -V" is run to determine if the installed
krb5 version supports account lockout (>= 1.8).

Remove the check, as we require a krb5 version which does support account
lockout (1.12).

@pspacek
Copy link
Contributor

pspacek commented Oct 13, 2016

Interestingly, the CI failed with following errors:

************* Module ipatests.test_xmlrpc.test_automount_plugin

ipatests/test_xmlrpc/test_automount_plugin.py:34: [E0401(import-error), ] Unable to import 'nose.tools')

************* Module ipatests.test_xmlrpc.test_hbactest_plugin

ipatests/test_xmlrpc/test_hbactest_plugin.py:27: [E0401(import-error), ] Unable to import 'nose.tools')

************* Module ipatests.test_xmlrpc.test_pwpolicy_plugin

ipatests/test_xmlrpc/test_pwpolicy_plugin.py:24: [E0401(import-error), ] Unable to import 'nose.tools')

************* Module ipatests.test_xmlrpc.test_external_members

ipatests/test_xmlrpc/test_external_members.py:24: [E0401(import-error), ] Unable to import 'nose')

************* Module lite-server

lite-server.py:36: [E0401(import-error), ] Unable to import 'paste')

lite-server.py:37: [E0401(import-error), ] Unable to import 'paste.gzipper')

lite-server.py:38: [E0401(import-error), ] Unable to import 'paste.urlmap')

************* Module ipa-ods-exporter

daemons/dnssec/ipa-ods-exporter:29: [E0401(import-error), ] Unable to import 'systemd.daemon')

daemons/dnssec/ipa-ods-exporter:30: [E0401(import-error), ] Unable to import 'systemd.journal')

Weren't you to eager in pruning BuildRequires? :-)

@pspacek pspacek self-assigned this Oct 13, 2016
@MartinBasti
Copy link
Contributor

Please update BUILD.txt with how to run pylint with build, probably freeipa.org should be updated as well

@HonzaCholasta
Copy link
Contributor Author

@pspacek, @mbasti-rh, fixed.

@pspacek
Copy link
Contributor

pspacek commented Oct 18, 2016

  • python2-dateutil and python2-sss-murmur packages are missing in with_lint condition.
  • the message from import error was confusing to me:

ipaserver/plugins/dogtag.py:244: No module named backports_abc
ipaserver/plugins/dogtag.py:244: No module named backports_abc

In person, we were talking about some changes in the message. I would try something like this:

argv[0]: ipaserver/plugins/dogtag.py:244: No module named backports_abc. Ignoring missing module. It might be required only for lint or only conditionally.

@stlaz stlaz self-assigned this Oct 19, 2016
@stlaz
Copy link
Contributor

stlaz commented Oct 19, 2016

For some reason, after running sudo dnf builddep freeipa.spec, which is successful, if I run the same command again, if fails:

[login@vm freeipa-git]$ sudo dnf builddep --spec freeipa.spec
Last metadata expiration check: 0:23:03 ago on Wed Oct 19 13:53:25 2016.
Failed to open: 'freeipa.spec', not a valid spec file.
Error: Some packages could not be found.

Adding -v or -d 10 options did not provide any more useful output about this error. This may possibly be a dnf bug.

edit: This was done on minimal systems with "Development Tools" group installed. The very same .spec file works on other systems, though.

@martbab
Copy link
Contributor

martbab commented Oct 19, 2016

On F24 you need to explicitly install python-srpm-macros due to broken package dependencies

dnf install -y python-srpm-macros

otherwise dnf/rpm is unable to expand python-specific macros in the spec and fails.

@stlaz stlaz added the ack Pull Request approved, can be merged label Oct 19, 2016
@stlaz
Copy link
Contributor

stlaz commented Oct 19, 2016

@martbab Thanks, that worked. However, first set of patches was not yet ACKed in #171.

@stlaz stlaz removed the ack Pull Request approved, can be merged label Oct 19, 2016
@pspacek
Copy link
Contributor

pspacek commented Oct 20, 2016

Version rebased on top of current master (without PR 171) is available from https://github.com/pspacek/freeipa/tree/pr159-rebase .

@pspacek
Copy link
Contributor

pspacek commented Oct 20, 2016

Commit 6256e80 breaks pylint because it is not compatible with ipaplatform implementation introduced in 8f98fa1. I do not care which part will be changed but we need to fix this first. (Either change ipaplatform not to confuse pylint or improve pylint plugin.)

@pspacek
Copy link
Contributor

pspacek commented Oct 20, 2016

This pull request was (again) rebased on top of PR#171. PR#171 changes ipaplatform handling to symlinks so the issue caused by __path__ trick should go away. I.e. the rebased version is functional only with PR#171.

The code is again available from https://github.com/pspacek/freeipa/tree/pr159-rebase

@pspacek
Copy link
Contributor

pspacek commented Oct 20, 2016

The rebased PR have passed build + all XMLRPC tests in Jenkins:
job/build_refactoring-build-f24build_refactoring/18/

The test included everything up to e33e00b

Jan Cholasta added 7 commits October 24, 2016 13:51
Add missing cyrus-sasl-devel, python-cffi, python-custodia,
python-dateutil, python-nose, python-paste, python-sss-murmur,
python-sssdconfig and systemd-python BuildRequires.

Remove unused custodia, java-headless, m4, policycoreutils,
python-kdcproxy, python-rhsm, pyOpenSSL and systemd-units BuildRequires.

Correct versioned BuildRequires and provide explanatory comments.

https://fedorahosted.org/freeipa/ticket/6418
Lint is never executed from rpmbuild, so the BuildRequires for lint are
purely informational.

Include them only if %with_lint RPM macro is specified.

Update .travis.yml accordingly.

https://fedorahosted.org/freeipa/ticket/6418
Check for import errors with pylint to make sure new python package
dependencies are not overlooked.

https://fedorahosted.org/freeipa/ticket/6418
Instead of importing ipalib to get IPA version string, create setup.py from
a template and have the version string automatically filled in.

This makes it possible to build the ipaserver package without having to
have ipalib dependencies installed.

https://fedorahosted.org/freeipa/ticket/6418
Add import hook to makeapi and makeaci which makes them ignore import
errors in modules in our source tree and instead print a warning.

This makes it possible to build IPA without having to have most of our
runtime dependencies installed.

https://fedorahosted.org/freeipa/ticket/6418
The configure script checks for libcurl, but it is never actually used
anywhere.

https://fedorahosted.org/freeipa/ticket/6418
On pwpolicy module import, "klist -V" is run to determine if the installed
krb5 version supports account lockout (>= 1.8).

Remove the check, as we require a krb5 version which does support account
lockout (1.12).

https://fedorahosted.org/freeipa/ticket/6418
@pspacek pspacek added the ack Pull Request approved, can be merged label Oct 24, 2016
@ghost ghost added the pushed Pull Request has already been pushed label Oct 24, 2016
@ghost ghost unassigned pspacek Oct 24, 2016
@ghost ghost closed this Oct 24, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
5 participants