feat: Add Support for Rattler-Build Build Backend

[seneschall]

This PR adds support for the rattler-build backend which uses the v1 recipe syntax. In order to enable this, I had to make changes in a lot of places in the code base since rattler-build handles a lot of things differently from conda-build. One important change I made e.g. was to introduce a RecipePath class which stores the build-backend for every recipe alongside the path that points to it. This way the rest of the code can automatically determine which build-backend to use for every given recipe based on whether the recipe folder contains a meta.yaml or a recipe.yaml.

During these changes I also added type hints wherever possible in order to better understand the existing codebase and in order to catch type errors via the linter instead of at runtime. Wherever possible, I also changed string based paths to pathlib Path objects to both improve code readability by clearly marking strings that are supposed to be paths and to make the manipulation of paths safer and more idiomatic.

Also, I deprecated the testonly flag because from my understanding of the existing code, bioconda-utils expects to be able to call conda-build --test on the recipe directory. This definitely doesn't work for rattler-build test, as rattler-build expects to be passed a directory of already built packages, but from what I understand conda-build does the same, as per this excerpt from their help: "RECIPE_PATH argument must be a path to built package file". Also, there don't seem to be any existing test cases where testonly is set to True. If I am mistaken, please feel free to revert this change.

I ran all tests from test/test_utils.py and test/test_recipe.py locally, but I only tested for native builds and not for docker based builds. The parts of the code that handle the docker build process should thus be examined more closely. The tests all passed except for test_multi_build. This one also fails on my machine for the version from the master branch, however. I also added a single test case, test/test_utils.py::test_rattler_recipe.

Parts of the code where I was unsure about something or which should be improved, I have marked with TODO (rb). If you have any questions about my code, don't hesitate to ask.

To summarise:

• Added support for rattler-build
• Add type hints to a lot of the code
• Refactored string based paths to pathlib Path objects for safety and to clarify what parameters functions expect to be passed
• Deprecated the testonly flag, as it seems to be broken as far as I understand and it also isn't covered by any of the existing test cases
• Linting has not yet been implemented for rattler-build recipes. Rattler recipes are simply skipped when linting

[seneschall]

I have done my best to merge with the master branch but as the two were quite divergent it is possible I missed something and caused breaking changes. I will not have time to work on this PR until next week and I didn't get the chance to test the merged code locally yet so please be patient. But if you want you can start reviewing this PR regardless. I will answer you as soon as possible.

[dlaehnemann]

Hey @seneschall , @johanneskoester just pointed me here. So I had a quick glance to get a first idea of what you are up to, and left some random comments. And I figured I'd point to some stuff that I'm aware of, in case you have not seen those.

One is the recent linting changes, that already moved towards better support of multiple outputs per recipe, for which I already looked at the new v1 format and tried to make the linting changes support that new format, while also still supporting the meta.yaml format. Those changes are here, if you want to have a quick look:
#1086

So if you need input on updating the linting setup, let me know. I've also recently reworked this a bit, to hopefully make it easier to add and adapt linting test cases.

Generally, for the linting updates, an important reference for me was the section on multiple output recipes recipes in the rattler-build docs, because that was the aim.

Otherwise, during that work I also noticed that conda-forge already implements rattler-build support, so that might be a good source of inspiration. I only stumbled upon the respective linting changes there, but you can probably find respective changes for the build infrastructure:
https://github.com/conda-forge/conda-smithy/blob/b23bf94817f619c47263009545ded147d3e06844/conda_smithy/linter/lints.py#L13

Also, this uses a dedicated package called rattler-build-conda-compat, so maybe this can also help here?

And finally, a general thought I already had while scanning the code: Wouldn't it make sense to have most changes in recipe.py, and fewer in utils.py? And maybe some structure with a generalized Recipe class, which basically just has methods to decide which subclass to return during parsing, and then something like v0Recipe and v1Recipe (or MetaRecipe and RecipeRecipe, or ...) which implement the details for each type of recipe?

[dlaehnemann]

Can we link to the respective docs here? I always find it useful to be able to look at the actual docs and read around in case I need more context to understand something.

[seneschall]

I got that from the output of conda-build --help. Would it be sufficient to link to the page below then? The actual command is a bit buried in the page:

https://docs.conda.io/projects/conda-build/en/stable/resources/commands/conda-build.html

[seneschall]

Actually this deviates from the output I get from running conda-build --test on version 26.3.0. The output says:

  -t, --test            Test package (assumes package is already built).
                        RECIPE_PATH argument must be a path to built package
                        file.

Whereas the page I linked above says:

       -t, --test
              Test package (assumes package  is  already  built).   RECIPE_DIR
              argument  can  be  either recipe directory, in which case source
              download may be necessary to resolve package version, or path to
              built  package  .tar.bz2 file, in which case no source is neces-
              sary.

So maybe this is a recent change in behaviour and the online docs haven't been updated yet?

[dlaehnemann]

Maybe just mention that this is from the conda-build --help command in your particular version 26.3.0 and the five the quote as you do. That should keep it traceable / reproducible.

[dlaehnemann]

As there are multiple imports from conda_build here now, it might be cleaner to do something like import conda_build as cb (analogous to the rattler_build import below), and then refer to cb.config, cb.metadata and cb.api. This would also make the source of functions clearer while reading the code.

[seneschall]

Sure!

[seneschall]

Hm this doesn't seem to work. If I do this, my linter complains that conda_build doesn't have an attribute called metadata or an attribute called config. Apparently Python won't import submodules with this syntax?

[dlaehnemann]

OK, I just learned something... 😅

Maybe just group them and make the imports somewhat systematic. For example, something like:

import conda_build as cb
import conda_build.config as cb_config
import conda_build.metadata as metadata

[dlaehnemann]

Suggested change

	f"Either meta and rattler must be set but both are None for recipe: {path.path.as_posix()}"
	f"Either meta or rattler must be set, but both are None for recipe: {path.path.as_posix()}"

[dlaehnemann]

If there are multiple packages in a v1 (rattler-build) recipe, shouldn't this return all of them? Just returning the first one seems arbitrary. So maybe self.rattler should just contain a single package at this point, or there should be some more specific handling of the indexing, here? Or am I misinterpreting what is being indexed over, and these are variants, not multiple outputs/packages?

[seneschall]

I did it this way because I assumed the list elements it to be variants which should all have the same name. But you are right the output could also be a list of rendered multi-output recipes (see https://rattler-build.prefix.dev/dev/py-rattler-build/tutorials/multi_output_and_staging/#example-1-multi-output-with-inter-output-dependencies).

So I suppose we should also differentiate between Multi- and Single-Output recipes when returning the package name. Perhaps this could be handled automatically via a unified Recipe class as you suggested above.

My question for that case would be how the Multi-Output recipes should be handled in building the DAG (see function build from graphy.py). If I see this correctly right now the graph.build function just assumes all dependencies from a given recipe are the dependencies of that single package which can be found in that recipe.

[dlaehnemann]

This is probably a left-over from a previous code iteration?

Suggested change

	# elif self.rattler is not None:
	# requirements = self.rattler[0].get("requirements")

[dlaehnemann]

Suggested change

	f"Either meta and rattler must be set but both are None for recipe: {self.path.path.as_posix()}"
	f"Either meta or rattler must be set, but both are None for recipe: {self.path.path.as_posix()}"

[johanneskoester]

Very impressive work, this is a massive change and required a lot of careful code inspection and thinking.

Can you please also add linting support for v1 recipes? I don't want that we develop our own linter for those. Instead, please add a dependency on conda-smithy and use their lintify_meta_yaml (I know, the name suggests v0 recipes, but it actually also supports v1 (rattler) format by setting recipe_version=1) see here.

Further, QC and tests (in part) still fail.

[seneschall]

Okay, so to the broader points mentioned above:

@dlaehnemann

• Concerning why I haven't created a new Recipe class in recipe.py:
  • There are actually three different use-cases for the recipes that each require a different way of handling them.
    • In the most common case we only need a path that points to the recipe and the build system, e.g. when building with docker where we construct a build command. But also this is how the recipes are mostly passed around. This used to be just the path as a string, but I have replaced it with the utils.RecipePath class, so we can retain the information what build system should be used for a given recipe
    • The second case is that we only need to extract the information about the package name and the dependencies from a given recipe. This is used in graph.build for instance, for the creation of the DAG. This was already kept separate before I made my changes because for conda-build recipes this doesn't require a fully rendered recipe (see utils.load_meta_fast). For rattler this also requires a different handling of the rendered recipes because the graph.build function loads the recipes into memory in parallel using utils.parallel_iter and utils.load_meta_and_recipe_fast. Unfortunately, the parallelisation requires all inputs and outputs to be in a class that can be serialised with pickle, which the rendered rattler variants cannot. In order to handle this, I have created the utils.MetaOrRattler class
    • And finally we need the fully rendered version of the recipe. For building rattler recipes natively, I have decided to use py-rattler-build and its classes for this purpose. With conda-build, bioconda-utils always uses the CLI-tool no matter if it's building natively or using Docker, so here we only need the build system and the path to the recipe. Currently the only usage of the fully rendered meta.yaml recipes (which is what the recipe.Recipe class stores) seems to be for specific cases like linting, autobumbing, and updating the pinnings and for build.do_not_consider_for_additional_platforms, for which we don't necessarily need a new class for rattler recipes.
  • Since the recipe module seems to be used only for the Recipe class, I have decided to keep my new classes in the utils module.
• But I have noticed two additional things which I'll still need to implement:
  • Besides linting, I also haven't implemented autobumping for rattler recipes. But as with linting, maybe I can also use external libraries for this
  • Another very important question would be how multi output recipes should be handled when building the DAG. The DAG's nodes are supposed to contain the name of the package, but this won't work for multi output recipes. Maybe we could differentiate between single- and multi-output recipes in graph.build and simply take the package name as the node name in the case of a single-output recipe and the directory name in the case of a multi-output recipe

@johanneskoester

• Yes I can implement linting using those libraries
• As for the failing tests, I will look into that when I'm done with the other changes discussed above

[dlaehnemann]

Thanks for the explanations! I have not looked at the DAG / graph building code and what it does. But I have looked at the multi output recipes extensively. So to your question regarding this:

Each of the outputs has its own package: name: entry, so this should be usable as a unique name for each output.

But I can imagine that the building of the packages themselves gets interesting once they depend on each other like in the Multi-Output with Inter-Output Dependencies example you also linked above. But at least the example also gives clear code on how to handle this.