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

Travis - use F28 for testing #1785

Closed
wants to merge 1 commit into from
Closed

Conversation

stlaz
Copy link
Contributor

@stlaz stlaz commented Apr 6, 2018

python2 pylint fails on Fedora 28 with errors about relative imports from ipapplatform that seem to be false-positives. Use only python3 pylint for Travis.

The Fedora 28 test-runner container in this commit is only to show that the tests pass, I'll update the Fedora 28 container in DockerHub FreeIPA repo once we agree on this PR.

@stlaz
Copy link
Contributor Author

stlaz commented Apr 6, 2018

Note that on Rawhide, FreeIPA does not build due to (samba?) C API changes: https://travis-ci.org/stlaz/freeipa/builds/362337027

@tiran
Copy link
Member

tiran commented Apr 6, 2018

python2 pylint is passing for me on F28. If you run into relative import issues, then simply add from __future__ import absolute_import to the failing files.

@stlaz
Copy link
Contributor Author

stlaz commented Apr 6, 2018

The issues are with so many files that I just did not find adding the import reasonable: https://travis-ci.org/stlaz/freeipa/jobs/362492712

@tiran
Copy link
Member

tiran commented Apr 6, 2018

For maximum compatibility with Python 3, all files should have the future import. Pylint is pointing out an actual issue. I raises the issue, when we started to migrate to Python 3. Back then, the rest of the time felt it wasn't necessary.

@stlaz
Copy link
Contributor Author

stlaz commented Apr 6, 2018

I see. I'll add the imports, then.

@slaykovsky slaykovsky added the re-run Trigger a new run of PR-CI label Apr 6, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 6, 2018
@slaykovsky slaykovsky added the re-run Trigger a new run of PR-CI label Apr 6, 2018
@freeipa-pr-ci freeipa-pr-ci removed re-run Trigger a new run of PR-CI labels Apr 6, 2018
@abbra
Copy link
Contributor

abbra commented Apr 9, 2018

The failure in https://travis-ci.org/stlaz/freeipa/jobs/362337028 is not due to Samba changes. Instead, pylint insists our executable is a python module:

internal error with sending report for module ['install/oddjob/com.redhat.idm.trust-fetch-domains']
'module' object has no attribute 'location'
AttributeError: 'module' object has no attribute 'location'

and then there is another set of errors from astroid:

************* Module ipaserver.dcerpc
ipaserver/dcerpc.py:1: [F0002(astroid-error), ] <type 'exceptions.AttributeError'>: 'module' object has no attribute 'location')
************* Module ipaserver.plugins.trust
ipaserver/plugins/trust.py:839: [E1101(no-member), trust_add.validate_options] Instance of 'TrustDomainJoins' has no 'configured' member)
ipaserver/plugins/trust.py:938: [E1101(no-member), trust_add.validate_range] Instance of 'TrustDomainJoins' has no 'remote_domain' member)
ipaserver/plugins/trust.py:1047: [E1101(no-member), trust_add.execute_ad] Instance of 'TrustDomainJoins' has no 'remote_domain' member)
ipaserver/plugins/trust.py:1067: [E1101(no-member), trust_add.execute_ad] Instance of 'TrustDomainJoins' has no 'remote_domain' member)

these may be valid ones, may be induced by some changes in the pylint infra.

@stlaz
Copy link
Contributor Author

stlaz commented Apr 9, 2018

If you however look in https://travis-ci.org/stlaz/freeipa/jobs/362337032, you'll see the build is failing because of an incomplete type dereference (and a couple of other warnings):

  CC       ipa_sam.lo
ipa_sam.c: In function ‘_smbldap_get_ldap’:
ipa_sam.c:351:14: error: dereferencing pointer to incomplete type ‘struct smbldap_state’
  return state->ldap_struct;
              ^~
ipa_sam.c: In function ‘_smbldap_get_paged_results’:
ipa_sam.c:362:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
ipa_sam.c: In function ‘_smbldap_get_ldap’:
ipa_sam.c:354:1: warning: control reaches end of non-void function [-Wreturn-type]
 }

@abbra
Copy link
Contributor

abbra commented Apr 9, 2018

@stlaz there is something wrong with the build environment:

checking for pdb_enum_upn_suffixes in -lpdb... no
configure: WARNING: libpdb does not have pdb_enum_upn_suffixes, no support for realm domains in ipasam
checking for smbldap_get_ldap in -lsmbldap... no
configure: WARNING: libsmbldap is not opaque, not using smbldap_get_ldap
checking for smbldap_set_bind_callback in -lsmbldap... no
configure: WARNING: libsmbldap is not opaque, not using smbldap_set_bind_callback

The package it tried to use is samba-4.8.0-7.fc29 and that package has all required symbols. Unfortunately, I don't see config.log from travis build to understand why it failed to detect them.

@freeipa-pr-ci freeipa-pr-ci added the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 10, 2018
@stlaz stlaz removed the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 10, 2018
@stlaz
Copy link
Contributor Author

stlaz commented Apr 10, 2018

Pushed latest update with absolute imports.

@stlaz stlaz added the needs review Pull Request is waiting for a review label Apr 10, 2018
@freeipa-pr-ci freeipa-pr-ci added the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 10, 2018
@stlaz stlaz removed the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 11, 2018
@freeipa-pr-ci freeipa-pr-ci added the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 17, 2018
@stlaz stlaz removed the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Apr 19, 2018
Add absolute_import from __future__ so that pylint
does not fail and to achieve python3 behavior in
python2.
@stlaz
Copy link
Contributor Author

stlaz commented Apr 19, 2018

Either ACK or NACK in 24hrs please.

@abbra
Copy link
Contributor

abbra commented Apr 19, 2018

LGTM.

@abbra abbra added ack Pull Request approved, can be merged and removed needs review Pull Request is waiting for a review labels Apr 19, 2018
@stlaz stlaz added the pushed Pull Request has already been pushed label Apr 20, 2018
@stlaz
Copy link
Contributor Author

stlaz commented Apr 20, 2018

master:

  • b5bdd07 Add absolute_import future imports

@stlaz stlaz closed this Apr 20, 2018
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