URL: https://github.com/SSSD/sssd/pull/720 Author: alexey-tikhonov Title: #720: contrib/ci/deps.sh: added missing dependency Action: opened
PR body: """ Added libnss3-tools to list of debian dependencies: certutil from this package is required by test_pam_responder.py. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/720/head:pr720 git checkout pr720
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
jhrozek commented: """ @sumit-bose do you have an opinion? """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-448606952
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
sumit-bose commented: """ @alexey-tikhonov is right that test_pam_responder.py uses certutil unconditionally.
Adding it as a dependency is ok, however if it is possible with pytest to run the related fixtures only for the NSS build this would be even better. """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-448617911
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
mzidek-rh commented: """
@alexey-tikhonov is right that test_pam_responder.py uses certutil unconditionally.
Adding it as a dependency is ok, however if it is possible with pytest to run the related fixtures only for the NSS build this would be even better.
But we would still need this patch, or not? Maybe I am missing something. Or do you think that we would also expand the list of dependencies conditionally? """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-448642293
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
alexey-tikhonov commented: """
Adding it as a dependency is ok, however if it is possible with pytest to run the related fixtures only for the NSS build this would be even better.
I agree with @mzidek-rh. From my point of view, those are independent issues: (1) full dependencies list for every supported build configuration; (2) unconditional usage of certutil by test.
"""
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-448943099
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
sumit-bose commented: """ I'm sorry, I think should have given more context. SSSD can be build with either NSS or OpenSSL crypto libraries.
The CI build for Debian is using OpenSSL which can be seen in contrib/ci/configure.sh ``` if [[ "$DISTRO_BRANCH" == -redhat-fedora-29* || "$DISTRO_BRANCH" == -redhat-fedora-3* || "$DISTRO_BRANCH" == -debian-* || "$DISTRO_BRANCH" == -redhat-redhatenterprise*-8.*- || "$DISTRO_BRANCH" == -redhat-centos-8.*- ]]; then CONFIGURE_ARG_LIST+=( "--with-crypto=libcrypto" ) fi ```
When writing test_pam_responder.py I was just lazy and added everything needed for either build unconditionally. But for the OpenSSL build the NSS database is not needed at all and create_nssdb_fixture and create_nssdb_no_cert_fixture in test_pam_responder.py do not have to be called.
I hope this explains it better that if the fixtures can be skipped on the OpenSSL build the dependency is not needed anymore because certuilt will then not be called.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-448972374
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
alexey-tikhonov commented: """ Thanks for explanations!
Does it mean that if test_pam_responder.py will be fixed properly then libnss3-dev is not needed as well in Debian dependency list? """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-449014621
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
sumit-bose commented: """
Thanks for explanations!
Does it mean that if test_pam_responder.py will be fixed properly then libnss3-dev is not needed as well in Debian dependency list?
I would say yes and I'd expect that libnspr4-dev can be dropped as well. But it has to be tested. """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-449041310
URL: https://github.com/SSSD/sssd/pull/720 Title: #720: contrib/ci/deps.sh: added missing dependency
alexey-tikhonov commented: """ Created new issue https://pagure.io/SSSD/sssd/issue/3914
Closing this PR. """
See the full comment at https://github.com/SSSD/sssd/pull/720#issuecomment-449365881
URL: https://github.com/SSSD/sssd/pull/720 Author: alexey-tikhonov Title: #720: contrib/ci/deps.sh: added missing dependency Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/720/head:pr720 git checkout pr720
sssd-devel@lists.fedorahosted.org
-
alexey-tikhonov -
jhrozek
-
mzidek-rh
-
sumit-bose