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

Allow installation of debug version of Python #18

Merged
merged 1 commit into from
Dec 26, 2020

Conversation

YannickJadoul
Copy link
Contributor

It would be useful if the GitHub Action could also easily install and setup a debug version of Python. This PR recognizes a -dbg suffix, and also install the debug version and uses it to setup the virtual environment.

Since there are no tests, I am already using my branch in the CI runs here, and it seems to work: pybind/pybind11#2746

Things to consider:

  • -dbg-dev or -dev-dbg isn't recognized/allowed, but the nightly builds don't seem to have a debug version?
  • Another option would be to add another argument to the action dbg: true or debug: true; this might make more sense if the nightly builds would have a debug build, but if they don't, it might be unexpected that such a combination would not work?
  • The virtual environment is still created in ~/venv-{version}. Should this be ~/venv-{version}-dbg, such that two installations of the same version could coexist as debug and non-debug build? But then -dev doesn't currently allow this either, it seems.

@asottile
Copy link
Member

but the nightly builds don't seem to have a debug version?

They totally do! They're basically the same packages but without docs, libpython symbols, and the news file: https://github.com/deadsnakes/nightly#limitations

Should this be ~/venv-{version}-dbg

Hmmm, this is a good point -- for now it's probably fine to keep them the same -- if someone has a usecase for doing both at the same time then this can be enhanced later

@YannickJadoul
Copy link
Contributor Author

They totally do! They're basically the same packages but without docs, libpython symbols, and the news file: https://github.com/deadsnakes/nightly#limitations

Oh, do you then prefer to have a separate option, to support all 4 combinations (released vs. nightly, debug vs. release), rather than the 3 versions now (release, debug, nightly)? E.g. something like this:

    strategy:
      matrix:
        python-version: [3.9, 3.9-dev]
        debug: [true, false]

      steps:
      - uses: deadsnakes/action@vx.y.z
        with:
          python-version: ${{ matrix.python-version }}
          debug: ${{ matrix.debug }}

@asottile
Copy link
Member

They totally do! They're basically the same packages but without docs, libpython symbols, and the news file: https://github.com/deadsnakes/nightly#limitations

Oh, do you then prefer to have a separate option, to support all 4 combinations (released vs. nightly, debug vs. release), rather than the 3 versions now (release, debug, nightly)? E.g. something like this:

    strategy:
      matrix:
        python-version: [3.9, 3.9-dev]
        debug: [true, false]

      steps:
      - uses: deadsnakes/action@vx.y.z
        with:
          python-version: ${{ matrix.python-version }}
          debug: ${{ matrix.debug }}

Yeah that probably makes sense

@YannickJadoul
Copy link
Contributor Author

Let's see if this works :-) I've pushed an update to pybind/pybind11#2746

@YannickJadoul
Copy link
Contributor Author

Seems so; if I didn't mess up the original use, with debug: false or without debug input, this should work.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit 221de11 into deadsnakes:master Dec 26, 2020
@YannickJadoul YannickJadoul deleted the dbg-versions branch December 26, 2020 22:38
@YannickJadoul
Copy link
Contributor Author

Great! Thanks for the smooth merge & release!

@asottile
Copy link
Member

and thank you for the patch! <3

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.

2 participants