conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
944 stars 1.71k forks source link

[question] Emscripten ports and `conan-center-index` #7427

Open jgsogo opened 2 years ago

jgsogo commented 2 years ago

I'm playing with emscripten these days and I'm not sure how to deal with some of the libraries that are also provided as ports https://github.com/emscripten-ports (probably it's also my poor experience using emscripten).

Currently emscripten 2.0.30 (https://github.com/conan-io/conan-center-index/pull/6163) provides the following ports:

$> emcc --show-ports
    ogg (USE_OGG=1; zlib license)
    SDL2_gfx (zlib license)
    SDL2_image (USE_SDL_IMAGE=2; zlib license)
    giflib (USE_GIFLIB=1; MIT license)
    SDL2_net (zlib license)
    icu (USE_ICU=1; Unicode License)
    SDL2_mixer (USE_SDL_MIXER=2; zlib license)
    libjpeg (USE_LIBJPEG=1; BSD license)
    vorbis (USE_VORBIS=1; zlib license)
    SDL2 (USE_SDL=2; zlib license)
    bzip2 (USE_BZIP2=1; BSD license)
    regal (USE_REGAL=1; Regal license)
    harfbuzz (USE_HARFBUZZ=1; MIT license)
    zlib (USE_ZLIB=1; zlib license)
    SDL2_ttf (USE_SDL_TTF=2; zlib license)
    libmodplug (USE_MODPLUG=1; public domain)
    freetype (USE_FREETYPE=1; freetype license)
    cocos2d
    Boost headers v1.70.0 (USE_BOOST_HEADERS=1; Boost license)
    libpng (USE_LIBPNG=1; zlib license)
    bullet (USE_BULLET=1; zlib license)
    mpg123 (USE_MPG123=1; zlib license)

Some of these libraries are already Conan packages and they work if cross-compiled to emscripten, so I'm not sure what is the advantage of using the ports over regular libraries (I haven't check the diff, so maybe without the ports some functions might break in runtime). Also, ports can be outdated, for example, zlib is still using 1.2.8.

From the Conan perspective we could follow different paths:

SSE4 commented 2 years ago

I personally not a fan of fragmentation, where you need to obtain sources from different places in order to get ports for different platforms.

ideally, all patches should go upstream. if it's impossible, I'd say we need to copy their patches. the reason is that we're already apply some patches, which are vital in many cases, but if you will try to use emscripten ports as is, you will lost them. include patches that fix cmake targets and so on. as a result, we may get a very different behavior for emscripten ports.

to my knowledge, most of libraries should port flawlessly. at least ones providing just algorithms. problematic libraries are mostly ones who use various system APIs (graphics, audio, etc).

jgsogo commented 2 years ago

So you are in this team, right? ▶️ apply patches from the port and build the Conan package as usual

I'm mostly there too, but I would like to find a way to keep those patches updated so people using the port and people using the Conan package can expect the same behavior.

madebr commented 2 years ago

Things are not so simple to patch and/or maintain when including different targets. e.g. the Nintendo Switch

devkitpro provides ports for some common game libraries. See https://github.com/devkitPro/pacman-packages/tree/master/switch

The patches for e.g. SDL2 are non-trivial and conflict with other platforms.

If we want to support this, then it should be possible to conditionally add patches. e.g. for sdl

conandata.yml

patches:
  2.0.14:
    all:
      - patch_file: "patches/2.0.14-0001-fix-this.patch"
        base_path: "source_subfolder"
      - patch_file: "patches/2.0.14-0002-fix-that.patch"
        base_path: "source_subfolder"
    Emscripten:
      - patch_file: "patches/Emscripten/2.0.14-0001-fix.patch"
        base_path: "source_subfolder"
    "Nintendo Switch":
      - patch_file: "patches/Nintendo Switch/SDL2-2.0.14.patch"
        base_path: "source_subfolder"

conanfile.py

def _patch_sources(self):
    for patch in self.conan_data.get("patches", {}).get(self.version, {}).get("all", []):
        tools.patch(**patch)
    for patch in self.conan_data.get("patches", {}).get(self.version, {}).get(str(self.settings.os), []):
        tools.patch(**patch)

Looking at the Switch case, it will need special handling anyways in the recipe (=extra requirements + system libraries)

To keep this maintainable, I would add a requirement to include comments/commands on how to update the patches. For the sdl case, this could be:

wget "https://raw.githubusercontent.com/devkitPro/pacman-packages/master/switch/SDL2/SDL2-2.0.14.patch"

or add a helper script that will automatically fetch the newest patches:

update_patches.sh

#!/bin/sh
set -e
wget "https://raw.githubusercontent.com/devkitPro/pacman-packages/master/switch/SDL2/SDL2-2.0.14.patch" -O "patches/Nintendo Switch/SDL2-2.0.14.patch"

conan v2.0 will have helper function to automatically apply patches, so I don't know how that will work.

jgsogo commented 2 years ago

IMHO, it would also be very pretentious to try to centralize those patches in ConanCenter. We know that fragmentation is bad, but probably a single source of truth where everyone contributes is a dream (and unmaintainable in the long term).

I envision a workflow where the CI will retrieve the patches from those remotes (some script in this repo), put them in place and run the conan create afterward. Those patches will be uploaded together with the recipe to the remote, so reproducibility is guaranteed, everyone consuming from ConanCenter remote will get them and will be able to build from sources.

No external dependencies, and the patches are maintained by someone else, centralized in a repo where people are dedicated to that effort.

SSE4 commented 2 years ago

create an empty target with just the corresponding link flag (how to identify the versions!!!)

using pre-built libraries from emsdk is definitely not an option IMO for instance, if you link with this pre-built boost:

this will be confusing and misleading, as well it may take a significant time to consumers to debug why options they have passed were silently ignored.

if we cannot afford centralizing patches in our repo, we may create alternative packages for Emscripten, like:

madebr commented 2 years ago

This alias feature request would also be useful to create aliases for different ports.

e.g. for sdl:

class SdlConan(ConanFile):
    name = "sdl"
    def set_alias(self):
        if self.settings.os == "Emscripten":
            self.alias = f"emscripten-sdl/{self.version}"
        elif self.settings.os == "Nintendo Switch":
            self.alias = f"nx-sdl/{self.version}"
        else:
            # move current sdl recipe to libsdl
            self.alias = f"libsdl/{self.version}"
KerstinKeller commented 2 years ago

@madebr I think there is really no need in this case to have an alias. You can have regular requires self.requires(f"emscripten-sdl/{self.version}". You mainly need aliases when you want to reference the same package, but a different version of that package.