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

Created the GitHub Actions step Upload to PyPi #535

Merged
merged 3 commits into from
Apr 2, 2020

Conversation

evandrocoan
Copy link
Contributor

@evandrocoan evandrocoan commented Apr 2, 2020

Runs only on tagged releases of anki:

  1. https://anki.tenderapp.com/discussions/ankidesktop/40025-anki-in-python-package-index

Requires a GitHub Actions Secret to be set with the name TWINE_PASSWORD where the contents are a pypi token, i.e., TWINE_PASSWORD: pypi-12k3j12lj3hj1kj2bç31.... Notice the token contents have to start with pypi-, e.g., pypi-12k3j12lj3hj1kj2bç31....

  1. http://github.com/ankitects/anki/settings/secrets
  2. http://pyfound.blogspot.com/2019/07/pypi-now-supports-uploading-via-api.html

As there are 3 different projects:

  1. https://pypi.org/project/aqt/
  2. https://pypi.org/project/anki/
  3. https://pypi.org/project/ankirspy/

The token TWINE_PASSWORD will have to have access to all repositories on the account. An alternative to this would be to duplicate the code 3 times, each one of them, uploading one repository at a time with a different TWINE_PASSWORD token. On this case, 3 pypi tokens would have to be created, one for each repository.

  1. TWINE_PASSWORD_AQT
  2. TWINE_PASSWORD_ANKI
  3. TWINE_PASSWORD_ANKIRSPY
      - name: Upload aqt to PyPi Linux/Mac OS
        if: matrix.BUILD_TYPE == 'build' && startsWith(github.ref, 'refs/tags/')
        env:
          TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD_AQT }}
          TWINE_USERNAME: __token__
        run: |
          python -m pip install twine
          twine upload --non-interactive --skip-existing --verbose dist/aqt*

      - name: Upload Anki to PyPi Linux/Mac OS
        if: matrix.BUILD_TYPE == 'build' && startsWith(github.ref, 'refs/tags/')
        env:
          TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD_ANKI }}
          TWINE_USERNAME: __token__
        run: |
          python -m pip install twine
          twine upload --non-interactive --skip-existing --verbose dist/anki-*

      - name: Upload ankirspy to PyPi Linux/Mac OS
        if: matrix.BUILD_TYPE == 'build' && startsWith(github.ref, 'refs/tags/')
        env:
          TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD_ANKIRSPY }}
          TWINE_USERNAME: __token__
        run: |
          python -m pip install twine
          twine upload --non-interactive --skip-existing --verbose dist/ankirspy-*

These are the testing package/repositories I created on PyPi to test if this is working. I will delete them after this is merged:

  1. https://pypi.org/project/anki2/
  2. https://pypi.org/project/ankirspy2/
  3. https://pypi.org/project/aqt2/

ping @agentydragon

@evandrocoan evandrocoan force-pushed the upload_wheels_to_pypi branch from cb015f2 to 069cc11 Compare April 2, 2020 05:57
@agentydragon
Copy link
Contributor

Thanks! This will be really great to have. One use case I can think of (besides mine, which is loading complex html and css into my card templates) is that now Anki addon authors might have an easier time running automated tests for them against a variety of Anki releases.

@@ -1,3 +1,5 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

Hasnt utf8 been the default for a long time now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

edition = "2018"
authors = ["Ankitects Pty Ltd and contributors"]
authors = ["Ankitects Pty Ltd and contributors <[email protected]>"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this mail address here intended? I'd expect this to rather point to Anki support page

Copy link
Contributor Author

@evandrocoan evandrocoan Apr 2, 2020

Choose a reason for hiding this comment

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

I do not know any anki support email and pypi is rejecting the wheel if there is no email.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dae, is there an email for support?

edition = "2018"
authors = ["Ankitects Pty Ltd and contributors"]
authors = ["Ankitects Pty Ltd and contributors <[email protected]>"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Member

@dae dae left a comment

Choose a reason for hiding this comment

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

Thanks Evandro, on the whole it looks good; I've made some comments below.

pylib/setup.py Outdated
version = fh.read().strip()
minimum_python_version = (3, 7)

if sys.version_info < minimum_python_version:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand the point of this, as we care about install-time checks, and that should already be covered by python_requires below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not see that. I Removed it.

.build/vernum: ../meta/version
sed -i.bak 's/.*automatically updated.*/ version="$(VER)", # automatically updated/' setup.py
rm setup.py.bak
@touch $@
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a change that was required for the sdist work you were doing, and should not be needed at the moment?

Copy link
Contributor Author

@evandrocoan evandrocoan Apr 2, 2020

Choose a reason for hiding this comment

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

You are right. This is only required if you try to build anki as sdist instead of bdist_wheel. But having this allow me to use sdist too and cargo already requires this make rule to update its version. So it is just something you have to do anyway (for cargo.toml).

@evandrocoan evandrocoan force-pushed the upload_wheels_to_pypi branch from 70e7d81 to 28bb567 Compare April 2, 2020 15:31
@evandrocoan evandrocoan closed this Apr 2, 2020
@evandrocoan evandrocoan reopened this Apr 2, 2020
@evandrocoan evandrocoan force-pushed the upload_wheels_to_pypi branch from 34837ec to 18f4a4d Compare April 2, 2020 17:45
# Conflicts:
#	.github/workflows/checks.yml
@evandrocoan evandrocoan force-pushed the upload_wheels_to_pypi branch from 18f4a4d to 58ba764 Compare April 2, 2020 17:47
@dae
Copy link
Member

dae commented Apr 2, 2020

Thanks Evandro! I'd prefer to point people to the support site - I wonder if pypi will accept that.

@dae dae merged commit 342bb64 into ankitects:master Apr 2, 2020
@evandrocoan evandrocoan deleted the upload_wheels_to_pypi branch April 3, 2020 00:17
@evandrocoan
Copy link
Contributor Author

evandrocoan commented Apr 3, 2020

I tried uploading it to pypi using the change you did on the commit ee71e5d and pypi rejected the upload/wheel: 'Ankitects Pty Ltd and contributors <https://help.ankiweb.net>' is an invalid value for Author-email

F:\anki>twine upload "dist\ankirspy2-2.1.31-cp38-none-win_amd64.whl" Uploading ankirspy2-2.1.31-cp38-none-win_amd64.whl ███████████████████████████████████████████| 3.00M/3.00M [00:10<00:00, 291kB/s] NOTE: Try --verbose to see response content. HTTPError: 400 Client Error: 'Ankitects Pty Ltd and contributors <https://help.ankiweb.net>' is an invalid value for Author-email. Error: Use a valid email address See https://packaging.python.org/specifications/core-metadata for url: https://upload.pypi.org/legacy/

Using "Ankitects Pty Ltd and contributors <[email protected]>" works: https://pypi.org/project/ankirspy2/2.1.31/

image

This problem happens because of the authors tag on .toml file is being parsed by maturin as author.email and this is blindly being passed forward to python as author.email, while python has the option to use only authors. I opened an issue on maturin about this: PyO3/maturin#289

@dae
Copy link
Member

dae commented Apr 3, 2020

Thanks for looking into it. Would you mind trying this branch and see if it fixes it for you when no @ is included? If it solves the problem, I can send through a PR.

https://github.com/ankitects/maturin/tree/optional_email

After checking it out, you should be able to run 'cargo install --path .' to install the binary to ~/.cargo/bin, and you can then copy it to the pyenv\bin or scripts folder

@evandrocoan
Copy link
Contributor Author

evandrocoan commented Apr 3, 2020

It worked: https://pypi.org/project/ankirspy2/

image

I opened a pull request: PyO3/maturin#290

@dae
Copy link
Member

dae commented Apr 20, 2020

Password is set up, so this should be good to go when 2.1.24 is released. Thanks for your work on this Evandro!

@agentydragon
Copy link
Contributor

agentydragon commented Apr 20, 2020

Importing the anki2 Python package (https://pypi.org/project/anki2/) does not work for me. After sudo pip3 install anki2, I get:

$ python3
Python 3.7.5 (default, Nov 20 2019, 09:21:52) 
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import anki2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.7/dist-packages/anki2/__init__.py", line 6, in <module>
    from anki.buildinfo import version
ModuleNotFoundError: No module named 'anki'
>>> 

I guess maybe it would work with if Anki were instead released in https://pypi.org/project/anki/?

agentydragon added a commit to agentydragon/anki-utils that referenced this pull request Apr 20, 2020
@evandrocoan
Copy link
Contributor Author

evandrocoan commented Apr 20, 2020

That module was not supposed to work because I had to rename anki to anki2 (and others) in order to test the pypi upload. Then, the only thing which should be working on it would be uploading it to pypi.

If I decided to make my anki2 and other uploads to pypi to work, I would have to do more work as rename all imports as from anki.buildinfo import version to from anki2.buildinfo import version and etc.

@agentydragon
Copy link
Contributor

agentydragon commented Apr 20, 2020

Oh, OK. Will @dae 's upload once he does a next release overwrite the anki module?

If so, might be better to remove the anki2 one from PyPI to avoid confusion.

@evandrocoan
Copy link
Contributor Author

I just deleted the anki2 and others. And his upload should work, but his upload will be these pypi repositories:

  1. https://pypi.org/project/aqt/
  2. https://pypi.org/project/anki/
  3. https://pypi.org/project/ankirspy/

But you do not have to wait for his upload, you can install them right now, These are the same Python wheels which will be uploaded when the first release of anki is launched. Just download the right of according to your OS.

https://github.com/ankitects/anki/actions/runs/82453775

If these wheels are not working for you right now, then they will not work when anki is finally released to pypi. Can you confirm the wheel you need for your OS is working right now?

image

@agentydragon
Copy link
Contributor

I intend to use the wheels from a Bazel workspace via https://github.com/bazelbuild/rules_python, and I don't know how to make it consume wheels the rules install themselves from a requirements.txt. I think I'll wait for the release.

@dae
Copy link
Member

dae commented Apr 27, 2020

I've tagged 2.1.24 in preparation for a release, and the wheels appear to have built correctly. Two things I noticed:

  • They're currently only built for Python 3.7, so Python 3.8 users won't be able to install it
  • aqt should probably have a dependency on the same version of anki, and anki should probably have a dependency on the same version of ankirspy

@evandrocoan
Copy link
Contributor Author

They're currently only built for Python 3.7, so Python 3.8 users won't be able to install it

Accordingly to the pypi pages:

  1. https://pypi.org/project/aqt/#files
  2. https://pypi.org/project/anki/#files
  3. https://pypi.org/project/ankirspy/#files

The aqt and anki wheels should be available for any python 3. With the exception of the wheels for ankirspy which are only for python 3.7.

  1. If beyond distributing the wheel for python 3.7, I could add a pull request to distribute the ankispy also as a source wheel, then, someone using python 3.8 or superior can install it (as long as they have a rust compiler installed). pypi should prefer to install the wheels if they match the user Python version, but fall back to the source distribution if no binary wheels are available for the users' Python.
  2. I could also open a pull request adding Python 3.8 to the build matrix, making anki compiling Python 3.8 wheels, beyond the Python 3.7 wheels of ankirspy.

aqt should probably have a dependency on the same version of anki, and anki should probably have a dependency on the same version of ankirspy

I could open a pull request adding this restriction.

@dae, can you delete the versions 0.0.1 and 0.0.2 of aqt, anki and ankirspy? https://pypi.org/project/ankirspy/#history

You can delete it by accessing the management page:

  1. https://pypi.org/manage/project/ankirspy/releases/
  2. https://pypi.org/manage/project/anki/releases/
  3. https://pypi.org/manage/project/aqt/releases/

image

I just installed anki from pypi on my python 3.8 and it worked for ankirspy, but it should not because it installed that version 0.0.1:

C:\Users\Professional>pip install ankirspy
Collecting ankirspy
  Downloading ankirspy-0.0.1.tar.gz (939 bytes)
Building wheels for collected packages: ankirspy
  Building wheel for ankirspy (setup.py) ... done
  Created wheel for ankirspy: filename=ankirspy-0.0.1-py3-none-any.whl size=1098 sha256=66a72cf3d33453c8bfcccc2babc5fc04643cfa52eb48b5a296ddb19b4cacca73
  Stored in directory: c:\users\professional\appdata\local\pip\cache\wheels\44\0b\b0\bbbbdb245dcf6eba03388af893e2bc0726e9173df041e2ce5a
Successfully built ankirspy
Installing collected packages: ankirspy
Successfully installed ankirspy-0.0.1

C:\Users\Professional>pip install aqt
Collecting aqt
  Downloading aqt-2.1.24%2B359b9f5c-py3-none-any.whl (3.5 MB)
     |████████████████████████████████| 3.5 MB 1.1 MB/s
Requirement already satisfied: send2trash in f:\python\lib\site-packages (from aqt) (1.5.0)
Requirement already satisfied: requests in f:\python\lib\site-packages (from aqt) (2.23.0)
Requirement already satisfied: beautifulsoup4 in f:\python\lib\site-packages (from aqt) (4.8.2)
Requirement already satisfied: psutil; sys_platform == "win32" in f:\python\lib\site-packages (from aqt) (5.7.0)
Requirement already satisfied: pywin32; sys_platform == "win32" in f:\python\lib\site-packages (from aqt) (227)
Requirement already satisfied: pyqt5>=5.9 in f:\python\lib\site-packages (from aqt) (5.14.1)
Requirement already satisfied: jsonschema in f:\python\lib\site-packages (from aqt) (3.2.0)
Requirement already satisfied: markdown in f:\python\lib\site-packages (from aqt) (2.6.11)
Requirement already satisfied: pyaudio in f:\python\lib\site-packages (from aqt) (0.2.11)
Requirement already satisfied: idna<3,>=2.5 in f:\python\lib\site-packages (from requests->aqt) (2.8)
Requirement already satisfied: urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1 in f:\python\lib\site-packages (from requests->aqt) (1.25.8)
Requirement already satisfied: certifi>=2017.4.17 in f:\python\lib\site-packages (from requests->aqt) (2019.11.28)
Requirement already satisfied: chardet<4,>=3.0.2 in f:\python\lib\site-packages (from requests->aqt) (3.0.4)
Requirement already satisfied: soupsieve>=1.2 in f:\python\lib\site-packages (from beautifulsoup4->aqt) (2.0)
Requirement already satisfied: PyQt5-sip<13,>=12.7 in f:\python\lib\site-packages (from pyqt5>=5.9->aqt) (12.7.1)
Requirement already satisfied: pyrsistent>=0.14.0 in f:\python\lib\site-packages (from jsonschema->aqt) (0.16.0)
Requirement already satisfied: attrs>=17.4.0 in f:\python\lib\site-packages (from jsonschema->aqt) (18.1.0)
Requirement already satisfied: six>=1.11.0 in f:\python\lib\site-packages (from jsonschema->aqt) (1.14.0)
Requirement already satisfied: setuptools in f:\python\lib\site-packages (from jsonschema->aqt) (45.2.0.post20200210)
Installing collected packages: aqt
Successfully installed aqt-2.1.24

C:\Users\Professional>pip install anki
Collecting anki
  Downloading anki-2.1.24%2B359b9f5c-py3-none-any.whl (156 kB)
     |████████████████████████████████| 156 kB 1.1 MB/s
Requirement already satisfied: psutil; sys_platform == "win32" in f:\python\lib\site-packages (from anki) (5.7.0)
Requirement already satisfied: decorator in f:\python\lib\site-packages (from anki) (4.4.2)
Requirement already satisfied: requests in f:\python\lib\site-packages (from anki) (2.23.0)
Requirement already satisfied: protobuf in f:\python\lib\site-packages (from anki) (3.11.3)
Requirement already satisfied: beautifulsoup4 in f:\python\lib\site-packages (from anki) (4.8.2)
Requirement already satisfied: idna<3,>=2.5 in f:\python\lib\site-packages (from requests->anki) (2.8)
Requirement already satisfied: urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1 in f:\python\lib\site-packages (from requests->anki) (1.25.8)
Requirement already satisfied: chardet<4,>=3.0.2 in f:\python\lib\site-packages (from requests->anki) (3.0.4)
Requirement already satisfied: certifi>=2017.4.17 in f:\python\lib\site-packages (from requests->anki) (2019.11.28)
Requirement already satisfied: setuptools in f:\python\lib\site-packages (from protobuf->anki) (45.2.0.post20200210)
Requirement already satisfied: six>=1.9 in f:\python\lib\site-packages (from protobuf->anki) (1.14.0)
Requirement already satisfied: soupsieve>=1.2 in f:\python\lib\site-packages (from beautifulsoup4->anki) (2.0)
Installing collected packages: anki
Successfully installed anki-2.1.24

C:\Users\Professional>

@agentydragon
Copy link
Contributor

agentydragon commented Apr 28, 2020

Unfortunately this doesn't seem to yet work for my setup (which is relatively unusual - http://bazel.io plus Python rules):

 (Traceback (most recent call last):
  File "/usr/lib/python3.8/runpy.py", line 193, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.8/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/__main__.py", line 253, in <module>
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/__main__.py", line 190, in main
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/__main__.py", line 162, in determine_possible_extras
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/__main__.py", line 165, in <dictcomp>
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/rules_python/packaging/whl.py", line 107, in extras
  File "/home/agentydragon/.cache/bazel/_bazel_agentydragon/1e1ce2d608af8b7ce7596d72ecee98a7/external/rules_python/tools/piptool.par/rules_python/packaging/whl.py", line 69, in metadata
  File "/usr/lib/python3.8/zipfile.py", line 1514, in open
    zinfo = self.getinfo(name)
  File "/usr/lib/python3.8/zipfile.py", line 1441, in getinfo
    raise KeyError(
KeyError: "There is no item named 'anki-2.1.24+359b9f5c.dist-info/METADATA' in the archive"
)

Looks like for some reason the Bazel Python rules are looking inside anki-2.1.24+359b9f5c.dist-info/METADATA in the whl, instead of anki-2.1.24.dist-info (which exists). The string 359b9f5c is only present in anki/buildinfo.py, so my guess is that the confusion comes from the actual filename of the whl, which is anki-2.1.24+359b9f5c-py3-none-any.whl. Could we strip the Git commit ID from the wheel filename?

(in case I'm doing something wrong: my requirements.txt is anki==2.1.24.)

But other than that: pip3 install anki works! Thank you for the contribution @evandrocoan.

@evandrocoan
Copy link
Contributor Author

evandrocoan commented Apr 28, 2020

Looks like for some reason the Bazel Python rules are looking inside anki-2.1.24+359b9f5c.dist-info/METADATA in the whl, instead of anki-2.1.24.dist-info (which exists).

You could open an issue asking them to not assume that the directory inside the wheel has the same name as the wheel file, as the file can be renamed as we are doing.

@dae
Copy link
Member

dae commented Apr 30, 2020

Hi guys,

  • Since this will only run on releases, I'm happy to accept a patch that strips the build hash from the filenames in the upload process.
  • I've removed the old 0.0.x versions
  • I'd prefer not to add the complexity of a source distribution to pypi, but am happy to accept a PR that builds separate wheels for Python 3.8 on tagged releases, and I think that's the most user friendly option anyway. Probably better not to run them on normal pushes/PRs - the more steps we add, the more likely things CI is to fail due to transient issues.

@evandrocoan
Copy link
Contributor Author

evandrocoan commented Apr 30, 2020

I've removed the old 0.0.x versions

Thanks!

a patch that strips the build hash from the filenames in the upload process

builds separate wheels for Python 3.8 on tagged releases

I am working on a pull request to do this.

@evandrocoan
Copy link
Contributor Author

evandrocoan commented May 1, 2020

I just finished the implementation with the pull requests:

  1. Improve checks naming #598 - Improve checks naming
  2. Strips the build hash from pypi package releases #601 - Strips the build hash from pypi package releases
  3. Add missing aqt and anki modules dependency requirements #600 Add missing aqt and anki modules dependency requirements

Probably better not to run them on normal pushes/PRs - the more steps we add, the more likely things CI is to fail due to transient issues

Beyond these problems, we also do not have enough space to cache the things and speed up the build process, as the limit is 5GB (#528 - Create actions for Windows and Mac OS) and a single build with Python 3.7/Rust uses about 3.5GB and we cannot use the same cache for Python 3.7 and 3.8. Then, if we also cached Python 3.8, it would take about 7G, making the cache fail most times.

@dae
Copy link
Member

dae commented May 1, 2020

.25 is now on PyPI, including Python 3.8 wheels. Thanks for all your work on this Evandro, I'm sure a lot of people will find them useful.

agentydragon added a commit to agentydragon/anki-utils that referenced this pull request May 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants