Skip to content

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.

@isuruf isuruf changed the base branch from python-debug to main November 15, 2022 01:06
@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

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.

@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

@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
Contributor

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
Contributor

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
Contributor

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

Development

Successfully merging this pull request may close these issues.

@conda-forge-admin, please re-render Are debug builds supported? Including Python debug builds Debug builds? Python debug

9 participants