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

salt: to 3001.1; python{,3}-pycryptodomex: new package #24136

Closed
wants to merge 2 commits into from
Closed

salt: to 3001.1; python{,3}-pycryptodomex: new package #24136

wants to merge 2 commits into from

Conversation

Goorzhel
Copy link
Contributor

@Goorzhel Goorzhel commented Aug 8, 2020

@Vaelatern

With saltstack/salt#55310 wrapped up, I figured I'd take a stab at updating our salt package. This should also obsolete saltstack/salt#56551.

[ag@server~]# salt-master -V
Salt Version:
           Salt: 3001.1

Dependency Versions:
           cffi: 1.14.0
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: 4.2.0
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 1.0.0
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: Not Installed
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.8.4 (default, Jul 17 2020, 11:12:18)
   python-gnupg: Not Installed
         PyYAML: 5.3
          PyZMQ: 19.0.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2

System Versions:
           dist: void rolling void
         locale: utf-8
        machine: x86_64
        release: 5.7.10_1
         system: Linux
        version: void rolling void

@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 8, 2020

srcpkgs/salt/template:7: do not set pycompile_module, it is autodetected

TIL about xlint. 😅

@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 8, 2020

Hmm:

ag@server $ sudo salt-key -L
Password:
Error: The 'pycryptodomex>=3.9.7' distribution was not found and is required by salt
ag@server $ xbps-query -f salt | grep requires | tee $(tty) | xargs cat
/usr/lib/python3.8/site-packages/salt-3001.1-py3.8.egg-info/requires.txt
Jinja2
msgpack>=0.5,!=0.5.5
PyYAML
MarkupSafe
requests>=1.0.0
distro
pycryptodomex>=3.9.7
pyzmq>=17.0.0

We have pycryptodome in void-packages, but not pycryptodomex (I imagine that's deliberate per comments below, I should Be the Change). For the time being I've papered over this with sudo pip install pycryptodomex.

@fosslinux
Copy link
Contributor

I think the only reason that we don't have pycryptodomex is because no-one ever added it to the repositories. You should make a template for that and add it to this PR (I think), unless there is something I am missing about pycryptodome and pycryptodomex.

@ericonr
Copy link
Member

ericonr commented Aug 9, 2020

I needed it for something once, and they are indeed different packages. I can't find the template for it, unfortunately.

@Goorzhel Goorzhel changed the title salt: upgrade to 3001.1; convert depends to py3 salt: to 3001.1; python{,3}-pycryptodomex: new package Aug 9, 2020
@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 9, 2020

$ salt-master -V | grep cryptodome
   pycryptodome: Not Installed
$ xi python3-pycryptodomex
<...>
$ salt-master -V | grep cryptodome
   pycryptodome: 3.9.8

Not bad, not bad at all.

@fosslinux
Copy link
Contributor

Correct commit naming format would be New package: pycryptodomex-3.9.8

@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 9, 2020

Whoops. Should I format the salt commit accordingly?

@fosslinux
Copy link
Contributor

For salt, normally it would be salt: update to 3001.1, (something else important), or the somethign else important on a new line what you have is currently fine for salt (although if you push again maybe s/upgrade/update). New packages use the New package: package-version` format though.

Here is the documentation on this https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#committing-your-changes

@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 9, 2020

I appreciate the pointers, @fosslinux — commits fixed.

@ericonr
Copy link
Member

ericonr commented Aug 9, 2020

We aren't taking new python2 packages, so you can just build the python3 one without worrying about py2.

@Vaelatern
Copy link
Member

Thanks for the work on this, I'll review and get back on this.

@Vaelatern
Copy link
Member

Looks like python3-pycrptodome already exists.

@Goorzhel
Copy link
Contributor Author

Goorzhel commented Aug 9, 2020

It does, but Salt seems rather particular about having pycryptodomex as opposed to non-x. As far as I can tell, their only difference is that with-x installs to the Python module Cryptodome, and non-x installs to Crypto. @ericonr stated a need for pycryptodomex in the repo, anyway, so I figured I'd fill it.

Hopefully this is my last force-push — I had to iron out some subtle typos.

Salt dropped Python 2 support in version 3001 [1], so all the
dependencies have been converted to `python3-*`.

[1] https://docs.saltstack.com/en/latest/topics/releases/3001.html
@Vaelatern Vaelatern closed this in a9f8de7 Aug 10, 2020
@Vaelatern
Copy link
Member

Thank you!

I'm going to test salt more, but I know this change does make our salt more usable than before!

@Goorzhel
Copy link
Contributor Author

Glad to help!

@Goorzhel Goorzhel deleted the salt branch November 15, 2020 06:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants