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

Generate recipe from project tree using pypa/build #541

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

dholth
Copy link
Contributor

@dholth dholth commented Jul 12, 2024

Description

This is the start of something I've wanted: grayskull pypi . on a checkout to generate a recipe, suitable for committing to the project tree. Like https://github.com/conda/conda/blob/main/recipe/meta.yaml#L5-L7 where source is "../" or the parent directory of the meta.yaml.

pypa/build gets all project metadata in a standard way, including the build-system requires (setuptools, flit, poetry). If these are not already installed, we should add a "conda install " before project.metadata_path(...) or bail and let the user install those. We would also change the pip command used in the build: section so that we never accidentally pip install a build requirement.

Remaining to be done, process all possible metadata out of distribution and into recipe; possibly deal with late "we don't know the package name until after fetch_data" more elegantly since this breaks grayskull's current assumptions.

It's also a little awkward that the recipe output dir is always <given directory>/<project name>. These would be <checkout>/conda.recipe/meta.yaml when included as first-party recipes.

Fix #542

grayskull/strategy/py_build.py Fixed Show fixed Hide fixed
grayskull/strategy/py_build.py Fixed Show fixed Hide fixed
grayskull/strategy/py_build.py Fixed Show fixed Hide fixed
grayskull/strategy/py_build.py Fixed Show fixed Hide fixed
Copy link
Contributor Author

@dholth dholth left a comment

Choose a reason for hiding this comment

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

Thanks for this useful project

f'#% set version = "{config.version}" %}}\n'
)

recipe["package"]["version"] = "{{ version }}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This puts quotes around version: '{{ version }}'; how do we control the YAML more closely with conda-souschef's Recipe?

Copy link
Member

Choose a reason for hiding this comment

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

conda-souschef is just a wrapper around ruamel yaml, you can still manipulate the ruamel yaml object directly.
you can do something like:

recipe["package"]["version"].value = "{{ version }}"

str(r).rsplit(";", 1)[0]
for r in requirements
if not r.marker or r.marker.evaluate()
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do we include the marker as a YAML comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are evaluated in the environment grayskull runs in, and might not be correct for the end user.

Copy link
Member

Choose a reason for hiding this comment

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

there is a property called .comment that you can use for that
a recipe/section object uses this mixin
https://github.com/marcelotrevisani/souschef/blob/92cee9159fb65fa035593f1a8b60a931413ab581/src/souschef/mixins.py#L14

recipe["build"][
"script"
] = "{{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation"
# XXX also --no-index?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice to be careful about not fetching pip requirements, even the build-system requirements (poetry, flit-core, hatchling, ...)

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I think I didn't follow, what do you mean?

for ep in distribution.entry_points
if ep.group == "console_scripts"
]
recipe["test"] = compose_test_section(metadata_dict, [])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this sort of "copy all fields from one format to another" code is destined to be messy...

def origin_is_tree(name: str) -> bool:
"""Return True if it is a directory"""
path = Path(name)
return path.is_dir()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could look for pyproject.toml or setup.py; it should work with either.

Copy link
Member

Choose a reason for hiding this comment

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

that can work, but you also need to look for setup.cfg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the pypa/build method, pyproject.toml is the only necessary file to get the metadata.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, but not all projects uses it :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pypa/build works with setup.py or pyproject.toml. It asks the underlying build system to create a wheel with package.dist-info/METADATA. Then we read METADATA and not pyproject.toml. So with this technique we don't touch setup.py, setup.cfg or pyproject.toml; pypa/build writes METADATA and we read that.

@marcelotrevisani
Copy link
Member

is this still as a draft?

@dholth
Copy link
Contributor Author

dholth commented Jul 29, 2024

I would like feedback but it is still a draft, it's not clean yet.

Copy link
Contributor

@beenje beenje left a comment

Choose a reason for hiding this comment

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

Very happy to see this PR!

I currently use pyproject-build to create a sdist and then run grayskull to generate the recipe from that local sdist.
This will make the process even easier.

One issue I stumbled upon is to find the module name to import (in the test section). It can differ from the distribution name.
I have a workaround when setuptools is used: #551

Do you know if this information can be retrieved from build?

recipe._yaml._yaml_get_pre_comment()[0].value = (
f'#% set name = "{config.name}" %}}\n'
f'#% set version = "{config.version}" %}}\n'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of that, you can use

set_global_jinja_var(recipe, "name", config.name)
set_global_jinja_var(recipe, "version", config.version)


import build
from conda.exceptions import InvalidMatchSpec
from conda.models.match_spec import MatchSpec
Copy link
Contributor

Choose a reason for hiding this comment

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

Having to import conda will make grayskull more difficult to install.
Is it worth adding conda as dependency just for that function?

I haven't checked how requirements are parsed in the current code.


about = {
"summary": metadata["summary"],
"license": metadata["license"],
Copy link
Contributor

Choose a reason for hiding this comment

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

On an example I tested, license was set to the full license text content. The pyproject.toml was using license = {file = "LICENSE"}. In the recipe, we want the SPDX identifier.

@beenje beenje mentioned this pull request Aug 17, 2024
@dholth
Copy link
Contributor Author

dholth commented Aug 17, 2024

@beenje I have been refining this technique here. It makes a wheel and immediately converts it to .conda without bothering with conda-build or a recipe. But it's focused on editable installs and there's no supporting code to manage build dependencies etc. In the future the metadata convert part might be shareable.

pyproject.toml doesn't give your imports in a generic way. You could inspect the contents of a wheel. You could have build-system-specific handling like how flit's name= must match the Python import.

@beenje
Copy link
Contributor

beenje commented Aug 19, 2024

@beenje I have been refining this technique here. It makes a wheel and immediately converts it to .conda without bothering with conda-build or a recipe. But it's focused on editable installs and there's no supporting code to manage build dependencies etc. In the future the metadata convert part might be shareable.

Nice! Another project I have to look into :-)

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.

[FEATURE] Use pypa/build to generate project metadata locally from any build system
3 participants