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

REF: Split off static libraries and binary artifacts into separate outputs #32

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

Conversation

carterbox
Copy link
Member

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.

This PR splits the artifacts into smaller packages according to their type according to the "build everything, then split" method. The compilation is performed in the top-level package and installed to a staging folder instead of the $PREFIX. Then for each output, an install script decides which files are moved to the prefix using pattern matching.

Note that in this recipe, I have placed the libjpeg and libjpegturbo artifacts into separate outputs because they have different SO names. However, the number of outputs could be reduced by one if the static libraries were shipped together instead of separately.

Why? I'm slowly working throught my dependency tree and trying to eliminate static libraries from my end-user environments. Our channel also has a CFEP against static libaries being shipped in the default output of a recipe.

@conda-forge-webservices
Copy link
Contributor

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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Jinja2 variable definitions are suggested to take a {%<one space>set<one space><variable name><one space>=<one space><expression><one space>%} form. See lines [1, 3]

For recipe:

  • It looks like the '_libturbojpeg_api' output doesn't have any tests.

@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) and found it was in an excellent condition.

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

For recipe:

  • It looks like the '_libturbojpeg_api' output doesn't have any tests.

recipe/meta.yaml Outdated Show resolved Hide resolved
Comment on lines +27 to +28
# Binary package; this is the exported name from the pre-multi-output recipe
- name: libjpeg-turbo
Copy link
Member Author

@carterbox carterbox Mar 1, 2024

Choose a reason for hiding this comment

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

Because this output contains or depends on all binaries, named-libraries, manpages, and examples; no downstream packages should break. However, downstream recipes will need to migrate themselves to libjpeg-turbo-dev the next time they are built.

@carterbox carterbox marked this pull request as draft March 1, 2024 02:39
@hmaarrfk
Copy link
Contributor

hmaarrfk commented Mar 1, 2024

why not just remove outright the static libraries. are we really interested in maintaining static workflows?

@carterbox carterbox marked this pull request as ready for review March 1, 2024 17:38
@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/meta.yaml) and found it was in an excellent condition.

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

For recipe/meta.yaml:

  • It looks like the '_libturbojpeg_api' output doesn't have any tests.

@carterbox carterbox marked this pull request as draft July 31, 2024 03:35
@carterbox
Copy link
Member Author

This PR now relies on the conda-build v24.7 package which was recently released on the defaults channel, but has not yet been released on conda-forge.

@carterbox carterbox closed this Aug 15, 2024
@carterbox carterbox reopened this Aug 15, 2024
@carterbox
Copy link
Member Author

@conda-forge-admin, please rerender

@hmaarrfk
Copy link
Contributor

is there a need for the static library? it may have just been included by accident 6 years ago.

@carterbox
Copy link
Member Author

is there a need for the static library? it may have just been included by accident 6 years ago.

There is no static library subpackage in the latest revision of this PR. I removed it after your previous request to remove it from the recipe.

@hmaarrfk
Copy link
Contributor

ok maybe the title needs to be updated.

Are you still interested in doing so much work on this recipe? Would it be acceptable to just set STATIC=0 and move on? or do you want to remove any other files?

I find that maybe the documentation files don't really belong here.

It seems the world has moved on to online documentation.

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.

2 participants