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

Add sdl3 recipe #29061

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Add sdl3 recipe #29061

wants to merge 39 commits into from

Conversation

ymontmarin
Copy link
Contributor

@ymontmarin ymontmarin commented Feb 7, 2025

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

Copy link
Contributor

github-actions bot commented Feb 7, 2025

Hi! This is the staged-recipes linter and I found some lint.

File-specific lints and/or hints:

  • recipes/sdl3/meta.yaml:
    • lints:
      • The following maintainers have not yet confirmed that they are willing to be listed here: manifoldFR. Please ask them to comment on this PR if they are.

@conda-forge-admin
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 (recipes/sdl3/meta.yaml) and found it was in an excellent condition.

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 11, 2025

@ManifoldFR would you accept to be a maintainer with me ?

@ManifoldFR
Copy link
Contributor

@ManifoldFR would you accept to be a maintainer with me ?

yup !

Copy link
Contributor

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

@ymontmarin
Copy link
Contributor Author

Hi @conda-forge/help-c-cpp I think the recipe for SDL3 is ready to go (probably even ready for ARM) !
The effort in this recipe follow the issue conda-forge/sdl2-feedstock#60 .

However, there is design decision to be taken for SDL:

SDL2 was released in conda-forge in a dedicated recipe: https://github.com/conda-forge/sdl2-feedstock and as pointed
out by @h-vetinari in the issue:
it's a pity that those feedstocks have the "2" in the name and suggest sdl2->sdl renaming.

However sdl name is already taken by: https://github.com/conda-forge/sdl-feedstock which is
actually a compatibility layer between sdl1 and sdl2 as detailed in conda-forge/ffmpeg-feedstock#306.

In addition @traversaro suggest to create a compatibility layer between sdl2 and sdl3 to have for example access to ffmpeg without migration.

With all that in mind, is the good solution to ship this recipe as sdl3 and ship an additional sdl32 for the compatibity ?
Or is it better to update the feedstock sdl with multiple branch to make all the version management there ?

Thank you for your help and I am available to be maintainer on the concerned recipes and to help on the compatibility layer recipe if needed.

@traversaro
Copy link
Contributor

However sdl name is already taken by: https://github.com/conda-forge/sdl-feedstock which is
actually a compatibility layer between sdl1 and sdl2 as detailed in conda-forge/ffmpeg-feedstock#306.

Just for giving the full picture: the feedstock was originally added for packaging sdl 1.2, it was upgraded to contain the sdl12-compat library later.

With all that in mind, is the good solution to ship this recipe as sdl3 and ship an additional sdl32 for the compatibity ? With all that in mind, is the good solution to ship this recipe as sdl3 and ship an additional sdl32 for the compatibity ?

sdl2 and sdl2-compat are not side by side installable (similarly to sdl1.2 and sdl12-compat) so to me the easiest solution is to package them both as the sdl2 package (if you want to select to install one or another, you can use the patch number, that are < 50 for sdl2 and >= 50 for sdl2-compat.

Or is it better to update the feedstock sdl with multiple branch to make all the version management there ?

I am not particularly opposite in just packaging sdl3 as sdl3 conda package, as anyhow something similar was done for all the linux distros I am aware of, and I could not find any package manager that ships a sdl package with version 2.* or 3.* . However, I am not particularly against it either, the only thing I want to make sure is that it would be still possible to install sdl12-compat (currently packaged as sdl in conda-forge), so if you want to start packaging sdl3 under sdl, I would suggest to rename sdl as sdl1, and add a sdl metapackage output to each sdl recipe, with strict version pinning, such as if you install sdl==3.2.5 you are constrained to install sdl3==3.2.5, but you can still install in the same environment a package that depends on sdl1 and a package that depends on sdl2 or sdl3.

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 11, 2025

Ok thank you very much for your insights @traversaro

In that regard, I also believe that shipping SDL as a sdl3 package is the best option to be consistent with other package managers.
It would mean merging this PR!

After that, if I understand correctly, your idea is to update the sdl2-feedstock with a new branch where the recipe has dependency on the sdl3 conda package and build the source code of sdl2-compat package.

I am not familiar with the patch trick you are referring to in order to have one conda package name sdl2 that can be either the SDL2 build, OR the SDL2-compat build withsdl3 conda package dependency.
Could you tell me how to do that ?

@traversaro
Copy link
Contributor

I am not familiar with the patch trick you are refering to in order to have one conda package name sdl2 that can be either the SDL2 build, or SDL2-compat build with SDL3 dependency. Could you tell me how to do that ?

You can build different versions of the same library in the same feedstock by adding a different branch of the feedstock, and then adding the branch to abi_migration_branches, see for example https://github.com/conda-forge/gz-transport-feedstock/blob/7cbc3d3cd67b0c038916c70a4593b567e2efab24/conda-forge.yml#L2 . If we want to maintain both sdl2 and sdl2-compat at the same time, we could do that. At that point, one branch would build sdl2==2.32.50, while another sdl2==2.32.0. However, as upstream actively suggest to migrate to sdl2-compat, we could just package sdl2-compat in sdl2-feedstock and avoid to package new version of actual sdl2 unless it is strictly necessary, to keep things as simple as possible.

@conda-forge-admin
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 (recipes/sdl3/meta.yaml) and found some lint.

Here's what I've got...

For recipes/sdl3/meta.yaml:

  • ❌ In your conda_build_config.yaml, please change the name of MACOSX_DEPLOYMENT_TARGET, to c_stdlib_version!

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13275404280. Examine the logs at this URL for more detail.

@conda-forge-admin
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 (recipes/sdl3/meta.yaml) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

Personally I would be ok with it, but perhaps it could make sense to hear @h-vetinari's opinion on this.

Thanks for all the inputs. It's likely that you understand these libraries better than I do, so I'm happy to take your advice (and some help in maintenance! 😉). From a packaging POV, I agree that it makes sense to keep using versioned outputs (e.g. sdl2, sdl3), especially as it's plausible that these need to be co-installed. However, I would like to avoid multiplying feedstocks for this.

I would much prefer to move everything to https://github.com/conda-forge/sdl-feedstock; for example, we could merge the history from https://github.com/conda-forge/sdl2-feedstock into that feedstock, then turn that into a v2.x branch, and put sdl3 on top (on main). Whether we keep sdl12-compat and/or sdl2-compat in separate feedstocks or also in separate branches within the main sdl-feedstock is secondary to me, but I'd like to stop creating multiple feedstocks for every SDL major version (even if they're rare).

I can help with the git history surgery on this.

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 12, 2025

@h-vetinari thank you for your response ! On my side, the recipe for sdl3 is now fully ready, without any cdt, with all deps recommanded by the upstream, support for gpu, and probably ready for ARM.

If I understand correctly, the strategy is to do the packages setup proposed by @traversaro :

conda packages: sdl1, sdl2, sdl3 where

  • sdl1 actually depends on sdl2 and build SDL12-COMPAT source
  • there is two version of sdl2, one with patch number <50 that build the source of SDL2, and one that depends on sdl3 and build SDL2-COMPAT source
  • sdl3 that build the source SDL3 (following this recipe)

BUT using only one feedstock: https://github.com/conda-forge/sdl-feedstock with multiple branches main, v2.x.<50, v2.x.>=50, v1.2.

So the next steps are:

  • Adding me as maintainer of https://github.com/conda-forge/sdl-feedstock
  • Create the branch v1.2 to release package named sdl1
  • Picking the history of https://github.com/conda-forge/sdl2-feedstock into the repo and create branch v2.x.<50 to release package named sdl2
  • Create a branch v2.x.>=50 with a new recipe that build SDL2-COMPAT to release other versions of package sdl2
  • Put this recipe on the main branch to release package name sdl3
  • All other sdl*-feedstock can be made archive

I can do that if it works for you and ping you if I am stuck.

Thanks again to all!

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 12, 2025

Process starting from here: conda-forge/sdl-feedstock#12

@traversaro
Copy link
Contributor

but I'd like to stop creating multiple feedstocks for every SDL major version (even if they're rare).

Ok for me! Unless someone strongly feel otherwise, can we avoid renaming sdl to sdl1 and just package sdl2-compat in a sdl2 branch instead of having separate branches for sdl2 and sdl2-compat ? Just to reduce the amount of maintenance work, unless it is strictly necessary. In a nutshell my proposal is:

  • v1.2 branch that builds sdl from the sdl12-compat project
  • v2 branch that builds sdl2 from the a v2 release of the sdl project, and eventually will be upgrade to build from sdl2-compat (after sdl v3 is released)
  • main branch that builds sdl3

@traversaro
Copy link
Contributor

I just created the additional branches in https://github.com/conda-forge/sdl-feedstock/branches, @ymontmarin feel free to open PRs against them.

@traversaro
Copy link
Contributor

but I'd like to stop creating multiple feedstocks for every SDL major version (even if they're rare).

I agree, but just to mention a downside in this, is that for sdl12-compat and sdl2-compat we will loose the automatic PRs to bump the version that instead you get if you have separate feedstocks.

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 12, 2025

Ok for me! Unless someone strongly feel otherwise, can we avoid renaming sdl to sdl1 and just package sdl2-compat in a sdl2 branch instead of having separate branches for sdl2 and sdl2-compat ? Just to reduce the amount of maintenance work, unless it is strictly necessary. In a nutshell my proposal is:

  • v1.2 branch that builds sdl from the sdl12-compat project
  • v2 branch that builds sdl2 from the a v2 release of the sdl project, and eventually will be upgrade to build from sdl2-compat (after sdl v3 is released)
  • main branch that builds sdl3

I agree with the even simple solution with only one branch v2 that will be build from source and will become the compat when sdl3recipe will be stable. And no problem for me to keep sdl name.

I am on it ! Thanks for the feedback.

@ymontmarin
Copy link
Contributor Author

ymontmarin commented Feb 13, 2025

Hi @h-vetinari @traversaro,

I think I finished most of the work to package SDL3 and reunite the package sdl, sdl2 and sdl3 on the feedstock: https://github.com/conda-forge/sdl-feedstock !
It was also the occasion to clean the three recipes and avoid the use of CDT's. I fused the list of maintainers for the three packages.

Here are the three PRs on the repository:

Everything seems to work ! Only the validation of the output package for this feedstock seems to be updated.
It looks like the remaining steps are:

  • Edit tokens for output validation in order that sdl-feedstock can upload sdl2 and sdl3 packages.
  • Merge the PR's
  • Mark abi branch
  • Make sdl2-feedstock an archive

Could you help with the token output validation problem ?

@traversaro
Copy link
Contributor

For uploading sdl2 and sdl3 from the sdl-feedstock you need to do a PR like conda-forge/admin-requests#1174 to admin requests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants