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

dev-python/passlib: Advertise Argon2 support #22750

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Oct 29, 2021

Adds a new revision: passlib-1.7.4-r1

  • Removed Python 3.7 from PYTHON_COMPAT
  • Updated to EAPI 8
  • Replaced existing IUSE entries for optional runtime dependencies with optfeature messages (see below)
  • Added Argon2 to optfeature messages

Before removing the USE flags, I checked the reverse deps of passlib to make sure none of them would break when removing bcrypt scrypt, and totp:

  • app-admin/ansible-base: Uses passlib (pbkdf2) for tests so it should not be affected by the change
  • dev-python/autobahn: Uses passlib.utils.saslprep so it should not be affected by the change
  • dev-python/flask-security: Implicitly depends on passlib[totp] for multi factor authentication. However, the only package in Portage that uses flask-security is dev-db/pgadmin4. Pgadmin4, in turn, does not seem to use the multi-factor auth feature from flask-security.
  • dev-python/pypiserver: Uses passlib.apache so it should not be affected by the change
  • dev-libs/Ice: Uses passlib.hash with pbkdf2_sha512 so it should not be affected by the change
  • dev-db/pgadmin4: Depends on passlib, but doesn't seem to use it directly.
  • net-mail/mailman: Uses passlib.utils to generate passwords so it should not be affected by the change
  • net-proxy/mitmproxy: Uses passlib.apache so it should not be affected by the change
  • sys-libs/libxcrypt: Allegedly uses passlib's pure Python implementations as a test dependency. Tests still succeed when dev-python/bcrypt and dev-python/scrypt are removed from the system
  • www-apps/radicale/radicale: Has an explicit dependency on passlib[bcrypt]. This has been broken up into a dependency on passlib and another one on bcrypt

Dropped support for Python 3.7.

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. no signoff One or more commits do not indicate GCO sign-off. labels Oct 29, 2021
@seifertm seifertm force-pushed the dev-python/passlib-advertise-argon2-support branch 2 times, most recently from b88b4de to 882dc3f Compare October 29, 2021 09:08
Copy link
Member

@arthurzam arthurzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the work and investigation!

I haven't tested or confirmed those changes, but here are very small changes meanwhile.

# Distributed under the terms of the GNU General Public License v2

EAPI=8
PYTHON_COMPAT=( python3_{8..10} pypy3 )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add new empty line after EAPI=

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

@@ -21,8 +21,9 @@ MY_P="Radicale-${PV}"
RDEPEND="
acct-user/radicale
acct-group/radicale
dev-python/bcrypt[${PYTHON_USEDEP}]
dev-python/defusedxml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it wasn't you fault, but let's fix errors we find :)

Suggested change
dev-python/defusedxml
dev-python/defusedxml[${PYTHON_USEDEP}]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's always a good idea!

I didn't really look much into the radicale ebuilds. I intended to give them some care in a separate change since they are currently umaintained.

@arthurzam
Copy link
Member

Also, for every package whose ebuild you change the *DEPEND variables in this case, please do a revbump (can be using git mv) - for example www-apps/radicale

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-29 09:25 UTC
Newest commit scanned: 882dc3f
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/1f7e8ee9c8/output.html

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the dev-python/passlib-advertise-argon2-support branch from 882dc3f to d825a98 Compare October 29, 2021 11:04
Removed USE dependency on dev-python/passlib[bcrypt].
Bumped revision.

Bug: https://bugs.gentoo.org/820668
Package-Manager: Portage-3.0.28, Repoman-3.0.3
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
…ython/defusedxml

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the dev-python/passlib-advertise-argon2-support branch from d825a98 to cc5c814 Compare October 29, 2021 11:09
@seifertm seifertm changed the title dev-python/passlib: Advertise Argon2 support [please reassign] dev-python/passlib: Advertise Argon2 support Oct 29, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-29 11:15 UTC
Newest commit scanned: d825a98
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/a6631f63f0/output.html

@gentoo-bot gentoo-bot changed the title [please reassign] dev-python/passlib: Advertise Argon2 support dev-python/passlib: Advertise Argon2 support Oct 29, 2021
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @seifertm
Areas affected: ebuilds
Packages affected: dev-python/passlib, www-apps/radicale

dev-python/passlib: @prometheanfire, @gentoo/openstack, @gentoo/python
www-apps/radicale: @gentoo/proxy-maint (maintainer needed)

Linked bugs

Bugs linked: 820668


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. and removed assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. no signoff One or more commits do not indicate GCO sign-off. labels Oct 29, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-29 11:30 UTC
Newest commit scanned: cc5c814
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/38311dc402/output.html

@arthurzam
Copy link
Member

Before removing the USE flags, I checked the reverse deps of passlib to make sure none of them would break when removing bcrypt scrypt, and totp:

`app-admin/ansible-base`: Uses passlib (pbkdf2) for tests so it should not be affected by the change

Based on those lines, and multiple extra docs in this repo, I think bcrypt is still used inside. To be extra sure, lets add bcrypt to test dependencies (and do a mv for a revbump).

`dev-python/autobahn`: Uses `passlib.utils.saslprep` so it should not be affected by the change

`dev-python/flask-security`: Implicitly depends on passlib[totp] for multi factor authentication. However, the only package in Portage that uses flask-security is `dev-db/pgadmin4`. Pgadmin4, in turn, does not seem to use the multi-factor auth feature from flask-security.

`dev-python/pypiserver`: Uses `passlib.apache` so it should not be affected by the change

`dev-libs/Ice`: Uses `passlib.hash` with pbkdf2_sha512 so it should not be affected by the change

`dev-db/pgadmin4`: Depends on passlib, but doesn't seem to use it directly.

`net-mail/mailman`:  Uses `passlib.utils` to generate passwords so it should not be affected by the change

`net-proxy/mitmproxy`: Uses `passlib.apache` so it should not be affected by the change

`sys-libs/libxcrypt`: Allegedly uses passlib's pure Python implementations as a test dependency. Tests still succeed when `dev-python/bcrypt` and `dev-python/scrypt` are removed from the system

`www-apps/radicale/radicale`: Has an explicit dependency on `passlib[bcrypt]`. This has been broken up into a dependency on passlib and another one on bcrypt

I have also scanned those others dependencies, and haven't found other misses. Very good job!

…bcrypt

Bug: https://bugs.gentoo.org/820668
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
…encies with optfeature messages

Bug: https://bugs.gentoo.org/820668
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Closes: https://bugs.gentoo.org/820668
Package-Manager: Portage-3.0.28, Repoman-3.0.3
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the dev-python/passlib-advertise-argon2-support branch from cc5c814 to c52e44a Compare October 29, 2021 13:31
@seifertm
Copy link
Contributor Author

Thanks for the thorough review!

I added bcrypt to the test dependencies of the app-admin/ansible-base ebuilds and bumped the revisions.
The live ebuild wasn't bumped and ansible-base-2.11.3 was copied instead of moved, because v2.11.3 has already been stabilized on some arches.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-29 13:45 UTC
Newest commit scanned: c52e44a
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/7a3c14310c/output.html

@juippis juippis added the hacktoberfest-accepted PR accepted for Hacktoberfest label Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. hacktoberfest-accepted PR accepted for Hacktoberfest maintainer-needed There is at least one affected package with no maintainer. Review it if you can.
Projects
None yet
5 participants