-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this 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 }}" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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() | ||
] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
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, ...)
There was a problem hiding this comment.
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, []) |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :/
There was a problem hiding this comment.
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.
is this still as a draft? |
I would like feedback but it is still a draft, it's not clean yet. |
There was a problem hiding this 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' | ||
) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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"], |
There was a problem hiding this comment.
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 I have been refining this technique here. It makes a wheel and immediately converts it to 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. |
Nice! Another project I have to look into :-) |
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 intorecipe
; possibly deal with late "we don't know the package name until afterfetch_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