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

Build for CPython 3.11 #120

Closed
wants to merge 0 commits into from
Closed

Build for CPython 3.11 #120

wants to merge 0 commits into from

Conversation

altendky
Copy link
Contributor

No description provided.

@dvarrazzo
Copy link
Owner

I wonder why tests didn't run. I've pushed your changes in a branch on our side.

@dvarrazzo
Copy link
Owner

As you can see, Python 3.11 has not been released yet.

@alexprengere
Copy link

In Github Actions, you can use "3.11.0-rc.2" for the "python-version" key, to test the latest RC of Python3.11 (that should be identical to the final release).
You may also upload additional wheels to an existing release, so it is already possible to add 3.11 wheels to the latest release of py-setproctitle.

@altendky
Copy link
Contributor Author

This is what I did to avoid the overhead of tracking pre-releases. It can be simplified to drop pypy support. If you were interested, I could certainly PR such a change here.

https://github.com/pytest-dev/pytest-twisted/blob/88a8c3515b4f8f900f42d5ed414140bdfeef0675/.github/workflows/ci.yml#L148-L161

      - name: Set up ${{ matrix.python.name }}
        if: ${{ job.container == '' }}
        uses: actions/setup-python@v2
        with:
          # This allows the matrix to specify just the major.minor version while still
          # expanding it to get the latest patch version including alpha releases.
          # This avoids the need to update for each new alpha, beta, release candidate,
          # and then finally an actual release version.  actions/setup-python doesn't
          # support this for PyPy presently so we get no help there.
          #
          # CPython -> 3.9.0-alpha - 3.9.X
          # PyPy    -> pypy-3.7
          python-version: ${{ fromJSON(format('["{0}", "{1}"]', format('{0}.0-alpha - {0}.X', matrix.python.action), matrix.python.action))[startsWith(matrix.python.action, 'pypy')] }}
          architecture: x64

@altendky
Copy link
Contributor Author

Also, you only have push configured to trigger the workflow, not pull request activity.

Here's an example setup that builds for pushes to main and v* tags, but otherwise only builds on PRs, including external contributions. I believe that "**" is actually more correct, though I haven't updated pytest-twisted yet.

https://github.com/pytest-dev/pytest-twisted/blob/88a8c3515b4f8f900f42d5ed414140bdfeef0675/.github/workflows/ci.yml#L3-L14

on:
  push:
    branches:
      - main
    tags:
      - v*
  pull_request:
    branches: 
      - "*"
  schedule:
    # Daily at 05:47
    - cron: '47 5 * * *'

@dvarrazzo
Copy link
Owner

Also, you only have push configured to trigger the workflow, not pull request activity.

fixed in master

@altendky
Copy link
Contributor Author

Note that as is now the triggers will result in both push and PR builds when you push code to a PR of your own. Some people are ok with that, I don't like having the double builds. Though note that they are a bit different. The push triggered build is just exactly the commit being built. The PR triggered build is the result of merging the pushed commit into the PR's target branch. It is a bit annoying that there isn't a little more control around these behaviors.

@dvarrazzo
Copy link
Owner

I know, I noticed it a few days ago in the psycopg repository.

image

A bit inconvenient, yes, but running test on contributors' MRs is more important.

@altendky
Copy link
Contributor Author

If you aren't attached to the push trigger for non-master branches, it can be avoided as above.

@amezin
Copy link

amezin commented Nov 16, 2022

Python 3.11 was added to GitHub Actions on Oct 26: https://github.com/actions/python-versions/releases/tag/3.11.0-3328127706

@altendky altendky closed this Nov 16, 2022
@altendky altendky reopened this Nov 16, 2022
@altendky
Copy link
Contributor Author

altendky commented Nov 16, 2022

Huh, looks like 3.11 on macOS maybe changed something about handling a venv virtualenv and the env 'binary' vs. the original. Or I guess maybe it's virtualenv doing the different thing.

https://github.com/dvarrazzo/py-setproctitle/actions/runs/3475932202/jobs/5810683155#step:5:53

____________________________ test_init_getproctitle ____________________________

    @pytest.mark.skipif(
        'sys.platform == "darwin" and os.environ.get("CIBW_TEST_COMMAND")',
        reason="f*cked up binary name",
    )
    def test_init_getproctitle():
        """getproctitle() returns a sensible value at initial call."""
        rv = run_script(
            """
    import setproctitle
    print(setproctitle.getproctitle())
    """,
            args="-u",
        )
>       assert rv == sys.executable + " -u\n"
E       AssertionError: assert '/Library/Fra...S/Python -u\n' == '/Users/runne...n/python -u\n'
E         - /Users/runner/work/py-setproctitle/py-setproctitle/.tox/3.11/bin/python -u
E         + /Library/Frameworks/Python.framework/Versions/3.11/Resources/Python.app/Contents/MacOS/Python -u

I didn't find anything interesting from a quick search through the 3.11 changelog.

@amezin
Copy link

amezin commented Nov 30, 2022

Maybe try to resolve sys.executable if it's a symlink (os.path.realpath), and accept both resolved and unresolved variants?

amezin added a commit to ddterm/gnome-shell-extension-ddterm that referenced this pull request Dec 10, 2022
amezin added a commit to ddterm/gnome-shell-extension-ddterm that referenced this pull request Dec 10, 2022
@altendky
Copy link
Contributor Author

@dvarrazzo, well, it's green...

I held back tox to <4 to avoid ValueError: conflicting factors pypy, 3.8 in pypy-3.8. https://github.com/dvarrazzo/py-setproctitle/actions/runs/3716382749/jobs/6302641135#step:5:39

I extended the @pytest.mark.skipif() for test_init_getproctitle() to include 3.11+ on macOS. It uses neither sys.executable nor the os.path.realpath() of that.

Obviously it would be good to fix these. Are you ok with those being a separate tasks?

@altendky
Copy link
Contributor Author

Working the tox issue at #125.

env:
CIBW_BUILD: ${{matrix.pyver}}-*
CIBW_PRERELEASE_PYTHONS: True
Copy link
Owner

Choose a reason for hiding this comment

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

We don't need a prerelease Python anymore, right?

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've been leaving it in personally, but it can certainly be removed. I figure I'll want it when adding 3.12 support and in the interrim it will most likely have no effect.

Feel free to commit this if you want it removed.

Suggested change
CIBW_PRERELEASE_PYTHONS: True

Copy link
Owner

Choose a reason for hiding this comment

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

It is stated in pretty clear terms that distributing pre-release packages is a bad idea.

There are other occurrences in this changeset: can you please provide a changeset instead of a suggestion? Thank you :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 8e70374.

.github/workflows/test.yml Outdated Show resolved Hide resolved
@dvarrazzo
Copy link
Owner

Squashed and rebased on top of #125.

Packages for Python 3.11 already uploaded on PyPI.

Thank you very much!

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.

4 participants