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_pip_as_python_dependency (and others) ignored when run via conda-build #393

Closed
2 tasks done
mbargull opened this issue Nov 21, 2023 · 15 comments · Fixed by conda/conda#13357
Closed
2 tasks done
Labels
locked [bot] locked due to inactivity type::bug describes erroneous operation, use severity::* to classify the type

Comments

@mbargull
Copy link
Member

Checklist

  • I added a descriptive title
  • I searched open reports and couldn't find a duplicate

What happened?

conda-forge/numpy-feedstock#305 (comment)

For some reason pip (, setuptools, wheel) have been installed into the build environment. I'll investigate why.

This is an issue with conda-libmamba-solver and some legacy code in conda.plan that only conda-build uses which leads to add_pip_as_python_dependency: false being ignored.


The reason for this is

meaning, conda.plan for whatever reason calls solver.solve_for_transaction in a different context (via stack_context_default which uses search_path=()) that ignores the config files.
In conjunction with conda-libmamba-solver not reusing the index that has been created beforehand in the correct context, this means that settings that influence the index, like add_pip_as_python_dependency: false, will be ignored.
(One can workaround this by using settings via environment variables, e.g., CONDA_ADD_PIP_AS_PYTHON_DEPENDENCY=0 conda-build ... since only the search_path but not the env vars are ignored in that conda.plan part.)


Proper solution would be to remove the conda.plan usage from conda-build (hopefully happening soon).
Maybe we could have a workaround in conda-libmamba-solver that calls mamba_utils.init_api_context beforehand and then not calling it again in solve_final_state (sidestepping the conda.plan stack_context_default usage)?
(conda.base.context would still be the incorrect state -- so if that one is also used, that would need a workaround too).

Conda Info

No response

Conda Config

No response

Conda list

No response

Additional Context

No response

@mbargull mbargull added the type::bug describes erroneous operation, use severity::* to classify the type label Nov 21, 2023
@jaimergp
Copy link
Contributor

Ugh. Thanks for the report @mbargull!

To be clear, this is conda-forge setting that env var in their config, right? Not conda-build itself.

How conda-build interfaces with the index and the solver is... peculiar :)

@jaimergp
Copy link
Contributor

In conjunction with conda-libmamba-solver not reusing the index that has been created beforehand in the correct context

I wonder if the solution here is to export this to disk and hope the metadata gets to libmamba intact. That'll incur some IO overhead though. But that would be one more hack adding to the conda-build integrations pile.

@mbargull
Copy link
Member Author

To be clear, this is conda-forge setting that env var in their config, right? Not conda-build itself.

No, no env var involved. This is add_pip_as_python_dependency: false set in .condarc as part of conda-forge's build setup.
(I only mentioned the corresponding env var as a workaround.)

@jaimergp
Copy link
Contributor

Oh yes, sorry. Ok, got it. I'm all in for the conda.plan removal. All the bugs we have received come from that interface in one way or another.

@mbargull
Copy link
Member Author

I'm all in for the conda.plan removal.

My plan (no pun intended...) is to purge Dist first and plan second right after.
I've added a note regarding conda.plan to the Dist removal agenda item for next weeks build tools meeting.

All the bugs we have received come from that interface in one way or another.

Oh, I'm not surprised... virtual hugs incoming :)

mbargull added a commit to mbargull/conda-smithy that referenced this issue Nov 21, 2023
until conda/conda-libmamba-solver#393 is fixed

Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
mbargull added a commit to mbargull/conda-smithy that referenced this issue Nov 21, 2023
until conda/conda-libmamba-solver#393 is fixed

Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
@mbargull
Copy link
Member Author

mbargull commented Nov 22, 2023

I took another look, but couldn't see an obvious/feasible way to workaround this in conda-libmamba-solver.
I think it could be fine if we try to change it in conda.plan itself.
The stack_context_default usage was introduced in
conda/conda@f516488#diff-ba93f394ef9904af9e780c700f6d422d5360e71638f087886a9946c6b0f990c4R444
and there is no indication from that commit if the stack_context_default was actually intentionally used or whether stack_context might've been fine too.

I'd say, let's just change this to stack_context and see how it goes (until we rip that code out for good).
We can let conda-build's test suite run with a patched conda.plan to try it out.

@jaimergp
Copy link
Contributor

There are several pages of commits in March 21, 2019. That's odd, so I took a look at all of that is coming from conda/conda#8435. A tiny PR with 111 changed files, 250 commits and no CI :D

@mbargull
Copy link
Member Author

Yep, those things happened a lot back then in the conda-build code base but also sometimes in conda's (Ilan Schnell was not stoked about that...).
(I actually linked only to the commit since GitHub's UI gives a direct link to the PR -- and looking for the commit in such a PR itself is more challenging (due to commit message and number of commits...).)

@jaimergp
Copy link
Contributor

jaimergp commented Nov 22, 2023

I think this is also affecting the pkg cache. conda-forge sets two paths:

pkgs_dirs:
  - ${FEEDSTOCK_ROOT}/build_artifacts/pkg_cache
  - /opt/conda/pkgs

When conda-build first collects the index, it saves the repodata json to:

DEBUG conda.gateways.repodata:fetch_latest(765): No local cache found for file:///home/conda/feedstock_root/build_artifacts/noarch/repodata.json at /home/conda/feedstock_root/build_artifacts/pkg_cache/cache/f02a6c9a.json

However, when conda-libmamba-solver tries to do the same, it should find the cache already populated. Instead:

DEBUG conda.gateways.repodata:fetch_latest(765): No local cache found for https://conda.anaconda.org/conda-forge/linux-64/repodata.json at /opt/conda/pkgs/cache/497deca9.json

This forces a redownload which might be out of sync at some point.

@jaimergp
Copy link
Contributor

I'd say, let's just change this to stack_context and see how it goes (until we rip that code out for good).
We can let conda-build's test suite run with a patched conda.plan to try it out.

Trying that out in this PR to see if it fixes a weird error too.

@jaimergp
Copy link
Contributor

Opened conda/conda-build#5083

@mbargull
Copy link
Member Author

I think this is also affecting the pkg cache.

Could be, yes. The stackvana and the recently reported conda-forge/ray-packages-feedstock#128 (comment) are both builds that take a very long time.

Generally, all config options that influence the index can be affected.
We possibly only noticed a subset yet because we only set a few in the config files, plus things like channels might get overridden by conda-build anyway (but haven't checked the code paths, TBH).

@jaimergp
Copy link
Contributor

Looks like we don't get the missing keys anymore in the stackvana debugger. It fails for other reasons, though.

@mbargull
Copy link
Member Author

It looks like it only fails with

2023-11-22T20:46:38.5859860Z + python -c 'import lsst'
2023-11-22T20:46:38.5860312Z Traceback (most recent call last):
2023-11-22T20:46:38.5868623Z   File "<string>", line 1, in <module>
2023-11-22T20:46:38.5869792Z ModuleNotFoundError: No module named 'lsst'

which is expected due to the exit 0 in build_stackvana.sh.

@jaimergp
Copy link
Contributor

Ugh, I had missed that exit 0. Thanks! Rerunning :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity type::bug describes erroneous operation, use severity::* to classify the type
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants