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

I wanted to let you know that I linted all conda-recipes in your PR (recipes/python-ninja) and found some lint.

Here's what I've got...

For recipes/python-ninja:

• noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

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 (recipes/python-ninja) and found it was in an excellent condition.

Getting an error at https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=513324&view=logs&j=6f142865-96c3-535c-b7ea-873d86b887bd&t=22b0682d-ab9e-55d7-9c79-49f3c3ba4823&l=1749 like [" ERROR (python-ninja,lib/python3.10/site-packages/ninja/data/bin/ninja): .. but ['libstdcxx-ng'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)"]. Does anyone have any suggestions on how to fix this?

@conda-forge/help-python this PR is ready for review!

One concern is that the python-ninja build here seems to build it's own compiled version of ninja. Would it be better to link to the existing ninja package at https://anaconda.org/conda-forge/ninja instead as mentioned in conda-forge/ninja-feedstock#27 (comment)? Or should we just keep python-ninja independent for now and work out how to link the two later?

I think it would be better to dependent on ninja. Consider a project like meson-python which will dependent on ninja-python and meson, where the latter will also depend on ninja.

I think it would be better to dependent on ninja. Consider a project like meson-python which will dependent on ninja-python and meson, where the latter will also depend on ninja.

Agree, but I'm not sure exactly how to make that work 😅 Looking at https://github.com/conda-forge/meson-python-feedstock/blob/main/recipe/meta.yaml, it seems to just depend on ninja right now, so is it building a ninja-python internally to use?

it seems to just depend on ninja right now, so is it building a ninja-python internally to use?

meson-python would use this ninja-python once available, at the moment we fix meson-python to work with the normal ninja.

it seems to just depend on ninja right now, so is it building a ninja-python internally to use?

meson-python would use this ninja-python once available, at the moment we fix meson-python to work with the normal ninja.

RIght, I've just read #19033 and https://github.com/FFY00/meson-python/issues/60 and seem to understand the situation a bit better. Right now the dependency graph looks something like this (correct me if I'm wrong):

graph LR;
    ninja --> meson-python;
    python-ninja-from-PyPI? --> meson-python;
    ninja --> meson;
    meson --> meson-python;
    click ninja "https://github.com/conda-forge/ninja-feedstock" "Open this in a new tab" _blank
    click meson-python "https://github.com/conda-forge/meson-python-feedstock" "Open this in a new tab" _blank
    click meson "https://github.com/conda-forge/meson-feedstock" "Open this in a new tab" _blank

But after this PR, we want to make it look like this:

graph LR;
    ninja --> python-ninja-from-conda-forge;
    python-ninja-from-conda-forge --> meson-python;
    ninja --> meson;
    meson --> meson-python;
    click ninja "https://github.com/conda-forge/ninja-feedstock" "Open this in a new tab" _blank
    click meson-python "https://github.com/conda-forge/meson-python-feedstock" "Open this in a new tab" _blank
    click meson "https://github.com/conda-forge/meson-feedstock" "Open this in a new tab" _blank

Now my question is, how to make this current python-ninja recipe just use ninja from conda-forge? The repo at https://github.com/scikit-build/ninja-python-distributions which I'm currently basing this PR is a bit hard to parse, I can't see where it's trying to pull the non-python ninja binary from during the wheel build process.

@henryiii, you mentioned at conda-forge/ninja-feedstock#27 (comment) that there's a way to make the PyPI-ninja work with an externally compiled Ninja? Could you advise on how?

Right now the dependency graph looks something like this:

There is no python-ninja-from-PyPI in the current setup of meson-python.

I can't see where it's trying to pull the non-python ninja binary from during the wheel build process.

Ninja source code is fetched in CMake at:

https://github.com/scikit-build/ninja-python-distributions/blob/c6b4d914ec72a952d5ab9a6311feae0758667160/CMakeLists.txt#L62-L74

Compilation of the fetched source happens via ExternalProject_add (for non-Linux) or via a custom target (Linux) at:

https://github.com/scikit-build/ninja-python-distributions/blob/c6b4d914ec72a952d5ab9a6311feae0758667160/CMakeLists.txt#L129-L142

Than the ninja binary is inferred heuristically and the install happens at:

https://github.com/scikit-build/ninja-python-distributions/blob/c6b4d914ec72a952d5ab9a6311feae0758667160/CMakeLists.txt#L156

The CMake part however is irrelevant, because this has already been done in the ninja package on conda-forge, instead we only need the Python package without the extra CMake step. The crucial point is

https://github.com/scikit-build/ninja-python-distributions/blob/c6b4d914ec72a952d5ab9a6311feae0758667160/src/ninja/__init__.py#L46-L47

which defines the search for the ninja binary using the the install location of the CMake build. I think this package cannot be added to conda-forge without patching out the CMake build and adjusting the way the ninja binary is found.

Thanks @awvwgk for the detailed pointers! Unfortunately, I'm a little low on bandwidth this week (will be travelling soon and moving house) and might not have time for a few weeks to dig into the Cmake config. Happy for another maintainer to take over and push commits to this PR (or in a separate branch), or we could merge this PR as is, and let people can work on it in the feedstock repo.

Either way, I'd appreciate a few more people being added to the recipe-maintainer list. Personally I'm not that familiar with ninja or compilers, I just use them to do the job 😆

Allowing both cmake and ninja Python packages to target an existing build is planned! Just haven't had a chance to work on it yet. I'll try to work on it next week.

Allowing both cmake and ninja Python packages to target an existing build is planned! Just haven't had a chance to work on it yet. I'll try to work on it next week.

Looks like there's some discussion happening at scikit-build/ninja-python-distributions#127 on this matter. Let me know when things are sorted upstream and I can try to continue work on this conda-forge recipe 😃

Still planned, but very low priority, and making a large changes to the setup.py when I'm planning to get rid of it soon is probably not a great idea. :)

In this case it would seem okay to patch the metadata to remove ninja from the Python dependencies.

My understanding is that ninja-python is an important dep for deepspeed since it enables compiling the ops on-the-fly during runtime. Now I might be wrong and indeed having ninja only might be sufficient. In such as case I am happy to skip the pip check on the deepspeed recipe and replace ninja-python by ninja.

But to do that I would like someone with experience using deepspeed to confirm it's all good.

For discussion related to deepspeed/ninja, please continue at https://github.com/conda-forge/staged-recipes/pull/21708/files#r1068091305 so we don't spam too much that PR.

There should be (almost) no need to every have the ninja-python package. Ninja should be fine, with one exception. Anyone using ninja should try to use the system ninja if the Python package is present - then the ninja package in conda-forge would be fine. The only exception is the Python package is actually a Kitware fork of ninja that provides one extra feature - Jobserver support. If there's a conda package for ninja-python, it should include this forked version with jobserver.

There is the issue that users put ninja in pyproject.toml's build requires, but conda ignores that, and it's the wrong solution; scikit-build-core & meson-python have implemented the correct solution - dynamically adding the dependency via PEP 517's hooks if there is not a sufficient system version.

I'm a bit confused here as to what's going on exactly, but another data point:

The pytype conda package is broken: it depends on ninja, but the conda-forge ninja package does not include the ninja python module:

 % conda list ninja
# packages in environment at /Users/chris/miniconda3/envs/py3:
#
# Name                    Version                   Build  Channel
ninja                     1.11.0               h1b54a9f_0    conda-forge

and yet:

 % pytype pytype_str.py     
Computing dependencies
Analyzing 1 sources with 0 local dependencies
/Users/chris/miniconda3/envs/py3/bin/python: No module named ninja

NOTE: pip install ninja works just fine -- as I understand it, the "ninja" package on PyPi is a Python wrapper, not ninja itself, hence this PR to create a python-ninja package. Which means that it's the pytype recipe that's broken. However, the recipe can't be fixed, because there is no python-ninja package for it to depend on :-(

Given that pip installing works, I'm quite confused as to why we can't have a python-ninja conda package now -- even if it will get refactored soon.

Also: the conda-forge ninja package doesn't install a ninja command either -- not sure what it does ;-)

There should be (almost) no need to every have the ninja-python package.

So is pytype broken? is it using an non-public API or something?

Ninja should be fine,

But this isn't about Ninja itself, it's about other things that use the python package -- apparently: https://github.com/scikit-build/ninja-python-distributions (which looks like the kitware fork, yes)

With one exception. Anyone using ninja should try to use the system ninja if the Python package is present - then the ninja package in conda-forge would be fine.

I'm a bit confused here -- is the "system ninja" in the context of conda, the one that conda installed? The whole distinction between "system" and "local" is fuzzier with conda ...

From pytype's source, it looks like they are correctly looking for a system ninja (I don't see an import ninja in https://github.com/google/pytype/blob/e6984f1bf09fc5d61efc45db68f84f37620e7c02/build_scripts/build_utils.py#L3). I assume it is somewhere, though, since that message above sounds like there's an import ninja happening somewhere. They do have incorrect build requirements at https://github.com/google/pytype/blob/e6984f1bf09fc5d61efc45db68f84f37620e7c02/pyproject.toml#L2 - the pyproject.toml just directly lists ninja, rather than dynamically declaring it via PEP 517. But that shouldn't affect conda, since we don't use those requirements anyway.

Do you know where the import ninja is happening? That's what needs to be fixed.

Also: the conda-forge ninja package doesn't install a ninja command either -- not sure what it does ;-)

It doesn't? So what does it install, then? Given its entire rationale for existing is to install a ninja command...

So is pytype broken? is it using an non-public API or something?

It's broken, yes. It's running python -m ninja instead of ninja. This was broken in google/pytype#642

People are using pip install ninja as a package manager for C++ programs that provide a shell executable and do not provide any python code at all, period. Then because it doesn't provide any python code, they ask to install some python code. I wonder why they didn't just check for both, though...

While that certainly does work, in the sense that it performs an action, it's been stated several times that it's the wrong kind of working.

I think pytype will either need patching to revert that change, or else an upstream fix to try both.

Also: the conda-forge ninja package doesn't install a ninja command either -- not sure what it does ;-)

It doesn't?

My mistake -- it doesn't install a working ninja command:

(junk) chris@Chris-MacBook pytype % conda list ninja
# packages in environment at /Users/chris/miniconda3/envs/junk:
#
# Name                    Version                   Build  Channel
ninja                     1.11.0               h1b54a9f_0    conda-forge
(junk) chris@Chris-MacBook pytype % ninja
ninja: error: loading 'build.ninja': No such file or directory

I've posted an issue on that feedstock:

conda-forge/ninja-feedstock#31

Hmm -- maybe it's not the pytype feedstock (or code) that's broken, it cold be the ninja feedstock is not providing a working ninja, so it's trying to revert to a python call.

Huh? That ninja program works fine. It correctly runs, and attempts to parse the build file. Then it reports a well formatted error that the file it wants doesn't exist.

It's like running python myscript.py and getting a message that there's no myscript.py file in the current directory. Except that ninja tries to load a default filename and doesn't have a REPL.

OK, thanks -- my ignorance -- pytype is apparently broken then.

I'll go close the ticket on the feedstock.

-CHB

conda-forge's pytype is broken-ish because it started needing the python-ninja recently (in the past six months or so, not sure when exactly). If you look in your PR, @PythonCHB, you will see right after your addition and I commented out the pip check because it would fail without this PR here.

conda-forge/pytype-feedstock@main...PythonCHB:pytype-feedstock:main

I contemplated stopping the frequent updates of pytype in conda-forge, but held back because I wasn't sure if people use the pytype package in different ways that they'd not be affected by this missing python-ninja. Happy to enable the pip check and thus prevent future updates if people think that's the best way forward. Let me know.

Also, I recommend you open an issue upstream if this is affecting you. The main maintainer upstream is quite responsive, albeit a bit cautious and methodical in terms updates (e.g., python 3.11 is still not supported: conda-forge/pytype-feedstock#85)

conda-forge's pytype is broken-ish because it started needing the python-ninja recently (in the past six months or so, not sure when exactly). If you look in your PR, @PythonCHB, you will see right after your addition and I commented out the pip check because it would fail without this PR here.

Yes, I saw that, but it's actually checking the wrong thing -- if this was all written built as it should be, then the pytype conda package would depend on the "real" ninja package, and even if pip still thinks it wants the python ninja package.

So my PR actually checks if the pytype command works -- not if the pip dependencies are satisfied. That is, I suspect if pytype is fixed to use a system ninja, it may still show as a pip dependency, as they want "pip install" to "jsut work" for their users.

I wasn't sure if people use the pytype package in different ways that they'd not be affected by this missing python-ninja.

I have no idea -- i'm not a heavy pytype user, I only discovered this because I was checking it out for the first time, but I can tell you that it doesn't work for the very simplest of examples.

Happy to enable the pip check and thus prevent future updates if people think that's the best way forward. Let me know.

I would use my PR instead -- see above. But either way, I don't think dysfunctional pytype packages should be provided :-(

By the way, I went back to a version over a year old, and it was broken then -- are you sure it ever worked?

Also, I recommend you open an issue upstream if this is affecting you.

I've done that, we'll see.

On the other hand, if the pytype maintainers are committed to supporting only the pip-installed ninja, then we should probably supply that in conda-forge. I think they should vendor it if that's what they want, but they need to do what they think is best.

I think it is pretty clear that python-ninja is required for pytype, see python -m ninja (in their codebase) and their requirements (that's what pip check checks btw). I will stop updating pytype-feedstock until we have a resolution.

If you could help determine the last working version of pytype we have on conda-forge, then I will submit a PR to mark all those after it as "broken" to avoid providing a pytype that's broken or half broken. Thank you!

Maybe conda-forge could provide a simple stub package python-ninja with minimal meta data for pip check and a wrapper for python -m ninja such that packages which really want to rely on the PyPI-kind of ninja distribution can do so (see conda-forge/ninja-feedstock#32). I'm not a big fan of this solution, because it just adds more weird complexity to an already messy dependency situation of PyPI vs. other package managers.

As long as some conda-forge packages requires python-ninja, it should be packaged on conda-forge in some way or another.

This is also problematic for deepspeed: conda-forge/deepspeed-feedstock#1

That is not the correct fix. This won't fix other non-conda packaging ecosystems. It should be fixed upstream, then patched here until the fix is released. If there is a python-ninja, it should be a repacking of the actual python ninja package. If someone actually does need the Python package with jobserver support, they should be able to require it. These packages do not, but just have a faulty discovery / usage of ninja only from the package.

Trying to hack around the problem is just going to cause problems later. You can't delete the fake package once it's released.

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on main so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!