SDL 2.0.14 #5466

MartinDelille · 2021-05-07T07:23:36Z

Specify library name and version: sdl/2.0.14

I tried to import the sdl library from https://github.com/bincrafters/community/tree/main/recipes/sdl2/

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

MartinDelille · 2021-05-07T07:23:43Z

I still have this error that I don't really know how to handle: https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H032

Can I simply mark the case in need of SystemPackageTool has a ConanInvalidConfiguration ?

SpaceIm · 2021-05-07T09:04:19Z

Few issues:

conandata.yml is missing
add 2.0.14 only for the moment, not 2.0.12
all system_requirements must be abstracted in a CCI recipe

MartinDelille · 2021-05-07T10:06:58Z

conandata.yml is missing

In fact config.yml was missing but it's fixed

add 2.0.14 only for the moment, not 2.0.12

done

all system_requirements must be abstracted in a CCI recipe

Can you be more explicit about this issue I raised here ?

MartinDelille · 2021-05-07T10:16:52Z

My question is partially answered here: #641

Croydon · 2021-05-07T13:04:56Z

I don't think it is the right time already to port this. This recipe could not (really) support all options relying on those system packages: https://github.com/bincrafters/community/blob/4eb2cb217d12d65791d5dec8439d1f23fcc83b1a/recipes/sdl2/all/conanfile.py#L135

MartinDelille · 2021-05-07T15:20:21Z

And what about a partial support for MacOS and Windows ?

SpaceIm · 2021-05-07T17:12:25Z

recipes/sdl2/all/conanfile.py

+        if self.settings.os == "Windows":
+            del self.options.fPIC


I would also remove iconv (maybe only for Visual Studio). In this branch https://github.com/libsdl-org/SDL/blob/889cebb7c20d4195e5d4ac344a2175f2490354cc/CMakeLists.txt#L779, iconv is never tested.

Also on Mac, I'm not sure that libiconv should be provided by conan, since it's a system library.

Finally, this recipe doesn't provide a way to really be sure that iconv is disabled, so sdl:iconv=False is quite fragile.

Also on Mac, I'm not sure that libiconv should be provided by conan, since it's a system library.

In the past, we had several changes back and forth. I think this deserves a general discussion before it is changed again

Croydon · 2021-05-08T12:57:09Z

And what about a partial support for MacOS and Windows ?

Theoretically possible. If you ask me personally, I think it will once again confuse people and pretty much doubles maintenance work.

Co-authored-by: Uilian Ries <uilianries@gmail.com>

jgsogo

IMHO, this recipe is good enough (really good job!) as a starting point to provide SDL. I'm sure it will require some iterations and adjustments, but it is starting to work... and some people will benefit from it.

This reverts commit 70d44e9.

conan-center-bot · 2021-06-15T15:28:59Z

All green in build 16 (b5676d2a717e4f294b524c1a552dedc0907ff3cc):

sdl/2.0.14@:
All packages built successfully! (All logs)

SpaceIm · 2021-06-17T13:06:33Z

Why it is not sdl2? I think it's a very bad idea, all package managers use a different name for sdl (1) and sdl2.

jgsogo · 2021-06-17T13:15:56Z

Hi, @SpaceIm (thanks for putting your other PR in draft). I agree with you, I think it's been a mistake and I would vote to rename this recipe (and deprecate current sdl one).

What do other reviewers think?

prince-chrismc · 2021-06-17T14:22:20Z

No objection

jgsogo · 2021-06-17T14:26:03Z

I've created PR here: #5944

* sdl2 * Rename sdl2 => sdl * Fix * Remove 2.0.12 * Add config.yml * Remove system_requirements() * Linux not supported yet * Remove iconv option for Visual Studio * Don't use iconv from conan for Macos since it is provided by the system * Safe iconv option check * get_safe iconv option * Handle fPIC properly * Fix package_info for Macos / iconv * Move fPIC option handling when shared * Requires libiconv also for Macos * Apply suggestions from code review Co-authored-by: Chris Mc <prince.chrismc@gmail.com> * Update recipes/sdl/all/conanfile.py Co-authored-by: Uilian Ries <uilianries@gmail.com> * Move ConanInvalidConfiguration raise to validate() * Revert "Move ConanInvalidConfiguration raise to validate()" This reverts commit 70d44e9. Co-authored-by: Chris Mc <prince.chrismc@gmail.com> Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com>

MartinDelille added 3 commits May 7, 2021 09:09

sdl2

eac6438

Rename sdl2 => sdl

35ffbab

Fix

bef7bc5

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label May 7, 2021

MartinDelille added 2 commits May 7, 2021 12:02

Remove 2.0.12

730e367

Add config.yml

99cb677

This comment has been minimized.

Sign in to view

Remove system_requirements()

41a6290

This comment has been minimized.

Sign in to view

conan-center-bot added the Unexpected Error label May 7, 2021

Linux not supported yet

ffd90aa

This comment has been minimized.

Sign in to view

conan-center-bot removed the Unexpected Error label May 7, 2021

SpaceIm reviewed May 7, 2021

View reviewed changes

MartinDelille changed the title ~~SDL 2.0.12/2.0.14~~ SDL 2.0.14 May 8, 2021

MartinDelille added 2 commits May 8, 2021 13:33

Remove iconv option for Visual Studio

5fbdbc3

Don't use iconv from conan for Macos since it is provided by the system

649d671

This comment has been minimized.

Sign in to view

Safe iconv option check

82fdc78

This comment has been minimized.

Sign in to view

get_safe iconv option

a02e329

This comment has been minimized.

Sign in to view

conan-center-bot requested review from czoido and jgsogo June 8, 2021 11:58

Update recipes/sdl/all/conanfile.py

a757c69

Co-authored-by: Uilian Ries <uilianries@gmail.com>

jgsogo dismissed prince-chrismc’s stale review via a757c69 June 15, 2021 10:00

jgsogo previously approved these changes Jun 15, 2021

View reviewed changes

jgsogo requested a review from uilianries June 15, 2021 10:01

prince-chrismc previously approved these changes Jun 15, 2021

View reviewed changes

SSE4 previously approved these changes Jun 15, 2021

View reviewed changes

Move ConanInvalidConfiguration raise to validate()

70d44e9

MartinDelille dismissed stale reviews from SSE4, prince-chrismc, and jgsogo via 70d44e9 June 15, 2021 12:01

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jun 15, 2021

Revert "Move ConanInvalidConfiguration raise to validate()"

b5676d2

This reverts commit 70d44e9.

conan-center-bot removed the Failed label Jun 15, 2021

conan-center-bot requested a review from jgsogo June 16, 2021 11:58

SSE4 approved these changes Jun 16, 2021

View reviewed changes

prince-chrismc approved these changes Jun 17, 2021

View reviewed changes

jgsogo approved these changes Jun 17, 2021

View reviewed changes

conan-center-bot merged commit c220b4a into conan-io:master Jun 17, 2021

SpaceIm mentioned this pull request Jun 17, 2021

add sdl_ttf/2.0.15 #5936

Merged

4 tasks

jgsogo mentioned this pull request Jun 17, 2021

[sdl2] Rename from sdl (1/3) #5944

Closed

MartinDelille deleted the sdl branch July 18, 2021 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SDL 2.0.14 #5466

SDL 2.0.14 #5466

MartinDelille commented May 7, 2021 •

edited

Loading

MartinDelille commented May 7, 2021

This comment has been minimized.

SpaceIm commented May 7, 2021 •

edited

Loading

This comment has been minimized.

MartinDelille commented May 7, 2021

This comment has been minimized.

MartinDelille commented May 7, 2021

Croydon commented May 7, 2021

MartinDelille commented May 7, 2021

This comment has been minimized.

SpaceIm May 7, 2021 •

edited

Loading

Croydon May 8, 2021

This comment has been minimized.

Croydon commented May 8, 2021

This comment has been minimized.

This comment has been minimized.

jgsogo left a comment

This comment has been minimized.

conan-center-bot commented Jun 15, 2021

SpaceIm commented Jun 17, 2021

jgsogo commented Jun 17, 2021

prince-chrismc commented Jun 17, 2021

jgsogo commented Jun 17, 2021

SDL 2.0.14 #5466

SDL 2.0.14 #5466

Conversation

MartinDelille commented May 7, 2021 • edited Loading

MartinDelille commented May 7, 2021

This comment has been minimized.

SpaceIm commented May 7, 2021 • edited Loading

This comment has been minimized.

MartinDelille commented May 7, 2021

This comment has been minimized.

MartinDelille commented May 7, 2021

Croydon commented May 7, 2021

MartinDelille commented May 7, 2021

This comment has been minimized.

SpaceIm May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Croydon May 8, 2021

Choose a reason for hiding this comment

This comment has been minimized.

Croydon commented May 8, 2021

This comment has been minimized.

This comment has been minimized.

jgsogo left a comment

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Jun 15, 2021

SpaceIm commented Jun 17, 2021

jgsogo commented Jun 17, 2021

prince-chrismc commented Jun 17, 2021

jgsogo commented Jun 17, 2021

MartinDelille commented May 7, 2021 •

edited

Loading

SpaceIm commented May 7, 2021 •

edited

Loading

SpaceIm May 7, 2021 •

edited

Loading