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

Convert recipe (on hold) #978

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

Conversation

rouault
Copy link
Contributor

@rouault rouault commented Aug 30, 2024

Cf https://prefix.dev/blog/rattler_build_on_conda_forge

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.
    -->

@rouault
Copy link
Contributor Author

rouault commented Aug 30, 2024

@conda-forge-admin, please rerender

@conda-forge-webservices
Copy link
Contributor

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

I do have some suggestions for making it better though...

For recipe/recipe.yaml:

This is a rattler-build recipe and not yet lintable. We are working on it!

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendeing locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/gdal-feedstock/actions/runs/10639353086.

@rouault
Copy link
Contributor Author

rouault commented Aug 30, 2024

hum conda-smithy doesn't seem happy with the removal of recipe/meta.yaml. I might have missed something in the blog post

@akrherz
Copy link
Contributor

akrherz commented Aug 31, 2024

pushed a suspected change needed

@conda-forge-admin please rerender

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendeing locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/gdal-feedstock/actions/runs/10641099586.

@akrherz
Copy link
Contributor

akrherz commented Aug 31, 2024

@conda-forge-admin, please rerender

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendeing locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/gdal-feedstock/actions/runs/10641163183.

@akrherz
Copy link
Contributor

akrherz commented Aug 31, 2024

I got things a bit further along, but now am unsure what is needed to be done.

 ╭─ Finding outputs from recipe
 │
 ╰─────────────────── (took 0 seconds)
Error:   × Parsing: invalid field `requirements`.
    ╭─[24:1]
 23 │ 
 24 │ requirements:
    · ──────┬─────
    ·       ╰── here
 25 │   build:
    ╰────
  help: multi-output recipes cannot have a top-level requirements field.
        Move `requirements` inside the individual output.

@xylar
Copy link
Contributor

xylar commented Aug 31, 2024

It sounds like we no longer need the redundant requirements at the top of the recipe.yaml. That's a nice change!

@xylar
Copy link
Contributor

xylar commented Aug 31, 2024

@conda-forge-admin, please rerender

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendeing locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/gdal-feedstock/actions/runs/10643835380.

Comment on lines +25 to +28
- package:
name: libgdal-core
- package:
name: libgdal-arrow-parquet
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like something got messed up in the automatic parsing here?

@xylar
Copy link
Contributor

xylar commented Aug 31, 2024

This seems like it's going to be a slog. There are a lot of changes required to get this to render. For example,

    build:
      ignore_run_exports_from:
        - expat

needs to become:

    requirements:
      ignore_run_exports:
        from_package:
          - expat

in a bunch of places. The tests: bit at outside of outputs: needs to go.

rattler-build is also pretty unhappy with:

{{ \".\".join(version | split(\".\")[:2]) }}"

but I couldn't figure out a way to move this to context: that made it happy.

Remove root tests
@xylar
Copy link
Contributor

xylar commented Aug 31, 2024

It seems like we might want to hold off on this until some simpler recipes can point the way. This is an unusually complex feedstock and probably a really good test of the robustness of rattler-build but maybe not a good choice for early adoption.

Also, see:
conda-forge/pyremap-feedstock#25 (comment)
conda-forge/conda-smithy#2047
conda-forge/conda-smithy#2047 (comment)

@rouault rouault marked this pull request as draft August 31, 2024 11:26
@rouault rouault changed the title Convert recipe Convert recipe (on hold) Aug 31, 2024
@wolfv
Copy link
Member

wolfv commented Aug 31, 2024

Thanks both for working on this. Indeed, GDAL will be a good test case and we are more than happy to tackle it, but we should probably indeed follow @xylar's suggestion and tackle a few easier feedstocks first :)

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.

4 participants