Skip to content

Add conda._vendor.toolz fallback for tlz #11709

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

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

kenodegard
Copy link
Contributor

@kenodegard kenodegard commented Aug 9, 2022

Description

Yet another patch to fix the broken canary builds. Adding fallback imports to conda._vendor.toolz for all tlz imports.

This is necessary because of how conda build builds packages:

  1. conda build generates the necessary build environment (which contains the new conda dependencies, e.g. toolz)
  2. conda build generates a build_env_setup.sh script where at the very end, it runs something like:
    ...
    eval "$('path/to/python' -m conda shell.bash hook)"
    conda activate <ENV1>
    conda activate --stack <ENV2>
    
    this occurs in conda_build.build._write_sh_activation_text.
  3. conda build changes directories to the working directory (in this case a shallow clone of our conda repo)
  4. conda build runs the build_env_setup.sh from our working directory so when path/to/python -m conda ... is invoked the CWD (working directory) is added to Python's search path, and hence the conda source code is loaded even though all we are attempting to do is bootstrap the build environment.

The correct fix is to change the path/to/python -m conda ... command to path/to/conda ... but this is a chicken/egg problem, so we need to keep this PR's fix in place until conda-build is updated and released.

Xref: #11698

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@kenodegard kenodegard requested a review from a team as a code owner August 9, 2022 23:44
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Aug 9, 2022
@kenodegard kenodegard mentioned this pull request Aug 9, 2022
7 tasks
@kenodegard kenodegard added build::review trigger a build for this PR and removed build::review trigger a build for this PR labels Aug 10, 2022
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

This issue keeps on giving!

@jezdez jezdez added build::review trigger a build for this PR and removed build::review trigger a build for this PR labels Aug 10, 2022
@dholth
Copy link
Contributor

dholth commented Aug 10, 2022

Could we replace the tlz abbreviation with toolz everywhere? (I notice tlz uses importlib machinery to use cytoolz when available)

@jakirkham
Copy link
Member

Am curious, why not just use tlz as-is?

@dholth
Copy link
Contributor

dholth commented Aug 10, 2022

I would replace all the concat/concatv with the equivalent itertools calls (already a C module) and redirect all import fallbacks through an intermediate "the toolz we ulze" module... seems preferable to use tlz over toolz always, it's a part of toolz.

@kenodegard
Copy link
Contributor Author

@dholth tlz will attempt to import from cytoolz first and falls back to toolz, yes we can replace concat/concatv with native options in Python (itertools.chain.from_iterable and arg unpacking), tracking that work in #11718

@jakirkham would love to but unfortunately, since conda/conda-build haven't been built/packaged with cytoolz/toolz as a dependency in the past, we're running into a chicken/egg problem when building/packaging this new conda (conda-build manages to invoke conda's source code via python -m conda shell.bash hook when creating the build environment)

@kenodegard kenodegard merged commit a6fbbbe into conda:main Aug 10, 2022
@kenodegard kenodegard deleted the tlz-fallback branch August 10, 2022 18:57
@jakirkham
Copy link
Member

Sorry @kenodegard that was a question intended for @dholth

@kenodegard kenodegard mentioned this pull request Aug 11, 2022
3 tasks
@kenodegard kenodegard added the in-progress issue is actively being worked on label Aug 12, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Aug 13, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA in-progress issue is actively being worked on locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants