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

Add a debug build of python 3.11 #597

Merged
merged 24 commits into from
Jan 14, 2023
Merged

Add a debug build of python 3.11 #597

merged 24 commits into from
Jan 14, 2023

Conversation

nikhilweee
Copy link
Contributor

@nikhilweee nikhilweee commented Nov 13, 2022

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

There have been multiple conversations around supporting a debug build of python both in this repository (#73) and elsewhere (conda-forge/staged-recipes#4812). There have also been previous attempts at this (#86) but there hasn't been any progress in the last 6 years. This PR is an attempt to revive those efforts.

Fixes #594
Fixes #73
Closes #86
Fixes conda-forge/staged-recipes#1593
Fixes conda-forge/conda-forge.github.io#1017

Closes #598 #599 #600

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@nikhilweee
Copy link
Contributor Author

@conda-forge-admin, please rerender

@nikhilweee nikhilweee mentioned this pull request Nov 13, 2022
5 tasks
@nikhilweee
Copy link
Contributor Author

nikhilweee commented Nov 14, 2022

@conda-forge-admin, please ping team

Hello team, I'm new to building packages for conda-forge but I'm really interested in a debug build of python. I spent the last couple days trying to figure out a way to include a debug build of python. Initially, in #596 I tried adding a separate output called python-debug but that didn't go so well because compiling python-debug would overwrite the build files for python. I couldn't find a way to use separate build folders so I gathered that it's a bad idea to build both packages simultaneously.

In this PR, I basically changed the name of the output to python-debug and made sure that --with-pydebug is being passed on to configure. All goes well until conda runs tests. As you can see in the reference below, conda-build explicitly adds python as a dependency which causes all sorts of clobber warnings in the CI builds. Is there a way to say that python-debug can basically be substituted for python and there's no need to install the latter? Or is it a bad idea to change the name of the package in the first place?

I am starting to run out of ideas and would really appreciate some help. Thanks!

https://github.com/conda/conda-build/blob/cc6c050ee3325e6957dbf60f74dbe79d78d26a8f/conda_build/metadata.py#L2365-L2369

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/python and so here I am doing that.

1 similar comment
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/python and so here I am doing that.

@nikhilweee
Copy link
Contributor Author

Apologies for the double mention, didn't know that the bot will retrigger on edits.

@jakirkham
Copy link
Member

We may want to recheck this Windows ABI incompatibility issue that Ray had flagged ( #73 (comment) )

@nikhilweee
Copy link
Contributor Author

Okay so after the last comment I figured that I shouldn't change the package name because that's how conda figures out not to install another version of python if this debug version is already installed. However, I did change the build string to include dbg. I believe now people can install a debug version using conda install python=*=*dbg* -c conda-forge

@isuruf I didn't intend to merge this into main because that would mean overwriting the non-debug version. I was under the impression that maybe we could maintain {VER}-dbg branches separately and update them less often but I'm open to other ideas. On another note, after almost a week of struggling, the linux builds are now passing and I feel good about my first PR.

@jakirkham
Copy link
Member

Think it would be better to build this with the main branch and just add extra jobs or iterate over builds in a loop (whichever is easiest). If we stick this in a different branch, it will likely fall out of date and become incompatible with the normal python build. Additionally sticking this in main makes it easier to backport this change to older python versions.

@nikhilweee
Copy link
Contributor Author

In that case I should add the flags used to differentiate between the two builds. I had removed it earlier.

@jakirkham
Copy link
Member

We could add a variant to conda_build_config.yaml perhaps like so. Once incorporated into the recipe (and re-rendered), this should generate the additional jobs

@nikhilweee
Copy link
Contributor Author

Thanks @jakirkham, that was very helpful. I added two variants in conda_build_config.yaml. Let's see how the tests go.

@nikhilweee
Copy link
Contributor Author

@conda-forge-admin, please rerender

1 similar comment
@nikhilweee
Copy link
Contributor Author

@conda-forge-admin, please rerender

recipe/meta.yaml Outdated Show resolved Hide resolved
Copy link
Member

@mbargull mbargull left a comment

Choose a reason for hiding this comment

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

We should ensure that the debug builds are only installed if explicitly requested.
Some possibilities:

  1. Push to a debug label so people can install conda-forge/label/debug::python.
    (A separate debug_python label could also be possible if people see the need for it.)
  2. Add a run requirement on some debug/_debug meta-package.
    (The debug/_debug package would then live in another channel/label (or have track_feaures).)
  3. Add some track_features: [debug].

Anaconda had some old 2.7 debug builds an used features: [debug] with an accompanying debug packages that had the corresponding track_features.
Nowadays we don't use features anymore but do still have track_features where needed.
The main cases for track_features are when we want certain variants/packages not installed by default but still have the possibility so seamlessly introduce them even implicitly (e.g. via dependencies).
Since we shouldn't have the need for that implicit behavior and track_features is a non-perfect workaround, I would not go for track_features.
I favor conda-forge/label/debug::python (or the conda-forge/label/debug::debug dependency if people have good reasons for it).

@nikhilweee
Copy link
Contributor Author

@mbargull Can you point me in the right direction on how to push to a separate label? I guess I'll need to pass on the --label argument to conda mambabuild but I can't edit build_steps.sh since that's overwritten by conda-smithy.

@jakirkham
Copy link
Member

Could you please explain why that run_export should be added? Asking as it is my understanding this should be ABI compatibility on UNIX (all that is built now). Also this has been segregated into a different label (where other packages that would build against likely would need to go). That said, not sure that we are going to build packages dependent on the debug build of this package (since we can reuse their existing builds)

@isuruf
Copy link
Member

isuruf commented Nov 22, 2022

See #597 (comment) and the two replies afterwards.

@jakirkham
Copy link
Member

Not seeing the relationship. Am hearing Marcel wants to stick with a label

@isuruf
Copy link
Member

isuruf commented Nov 22, 2022

the suffix for extensions would also be cpython-311d-x86_64-linux-gnu.so and similar

Extensions built with debug cannot be imported in release

@jakirkham
Copy link
Member

Right are we planning to build things with the debug python build? My understanding was no

recipe/meta.yaml Outdated Show resolved Hide resolved
@nikhilweee
Copy link
Contributor Author

@isuruf Please have a look at the unresolved conversations. I need help with figuring out:

  1. why CMake tests fail on debug builds (see relevant CI logs) and
  2. how to add a run_exports for the debug build.

@isuruf
Copy link
Member

isuruf commented Nov 23, 2022

Right are we planning to build things with the debug python build? My understanding was no

No, but it is good to be correct.

@nikhilweee
Copy link
Contributor Author

Hi team, just wondering what are the next steps for this PR?

@nikhilweee
Copy link
Contributor Author

@conda-forge-admin, please rerender

@nikhilweee
Copy link
Contributor Author

@conda-forge-admin, please ping team

Hi team, this PR has had no activity for quite sometime. I wonder what are the next steps?

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/python and so here I am doing that.

@isuruf isuruf merged commit 248b7c4 into conda-forge:main Jan 14, 2023
@hmaarrfk
Copy link

Did this break cross python? We seem to be having trouble with python 3.11 + osx cross python that go away if we pin to _0_cpython

xref: conda-forge/pytorch-cpu-feedstock#158

@hmaarrfk
Copy link

cc: @ngam

@isuruf
Copy link
Member

isuruf commented Jan 15, 2023

What's the error?

@ngam
Copy link
Contributor

ngam commented Jan 15, 2023

It read like missing modules (in the cross-python step at the outset edit: see below). Forcing the 0 build fixed the issue. We will report back with more details soon; hmaarrfk was the one who figured it out!

@ngam
Copy link
Contributor

ngam commented Jan 15, 2023


+ /Users/runner/miniforge3/conda-bld/pytorch-recipe_1673710527742/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/bin/python -m pip install . --no-deps -vv
<string>:5: DeprecationWarning: check_home argument is deprecated and ignored.
Fatal Python error: init_import_site: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
  File "/Users/runner/miniforge3/conda-bld/pytorch-recipe_1673710527742/_build_env/venv/lib/site.py", line 174, in <module>
    sys.meta_path.insert(_index, CrossenvFinder())
                                 ^^^^^^^^^^^^^^^^
  File "/Users/runner/miniforge3/conda-bld/pytorch-recipe_1673710527742/_build_env/venv/lib/site.py", line 119, in __init__
    self.manually_patch_loaded()
  File "/Users/runner/miniforge3/conda-bld/pytorch-recipe_1673710527742/_build_env/venv/lib/site.py", line 167, in manually_patch_loaded
    _patch_module(module, self.PATCHES[name])
  File "/Users/runner/miniforge3/conda-bld/pytorch-recipe_1673710527742/_build_env/venv/lib/site.py", line 55, in _patch_module
    exec(src, module.__dict__, module.__dict__)
  File "<string>", line 35, in <m

actually after the cross-python step. Please refer to this pipeline log for more info: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=641580&view=logs&j=1e869e56-b0a2-5745-eb6f-ceaab3c34dd0&t=a00ae721-76a6-5861-22c5-39610ae19cdd

@isuruf
Copy link
Member

isuruf commented Jan 15, 2023

Should be fixed now. There was an error in rerendering that messed up the labels in anaconda.org.

@hmaarrfk
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants