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

libmysofa: add package #6076

Merged
merged 31 commits into from
Jan 3, 2025
Merged

libmysofa: add package #6076

merged 31 commits into from
Jan 3, 2025

Conversation

luadebug
Copy link
Contributor

@luadebug luadebug commented Dec 27, 2024

wasm:

In file included from /home/runner/.xmake/cache/packages/2412/l/libmysofa/v1.3.2/source/src/hrtf/reader.c:18:
/home/runner/.xmake/cache/packages/2412/l/libmysofa/v1.3.2/source/src/hrtf/portable_endian.h:117:3: error: platform not supported
  117 | #       error platform not supported
      |         ^

Yet it seems emscripten is kind of behaving like different os, it defines unix, unix, EMSCRIPTEN all time, so currently there is either overwrite logic of portable endian h(to support it since it looks like it wont work over emscripten at all) or either doing something like if is_host("linux") then replace(defined(linux), 1)... Or just limit wasm.
feel free to change & merge.
BTW, I dont know if there is need to rename package name from libmysofa to mysofa... I think it is fine it won't affect over find_package(MySOFA) at all.

@luadebug luadebug marked this pull request as draft December 27, 2024 17:33
@luadebug luadebug changed the title new port libmysofa libmysofa: add package Dec 27, 2024
@luadebug luadebug marked this pull request as ready for review December 28, 2024 07:39
@star-hengxing
Copy link
Contributor

star-hengxing commented Dec 28, 2024

proj points to mysofa-shared.vcxproj everytime.... too troublesome to fixup these flags... My attempt to exclude BUILD_STATIC_LIBS did not go as expected... 😕😕😕

I don't know you mean, you can try

table.insert(configs, "-DBUILD_STATIC_LIBS=" .. (package:config("shared") and "OFF" or "ON"))
table.insert(configs, "-DBUILD_SHARED_LIBS=" .. (package:config("shared") and "ON" or "OFF"))

Is it set_license("BSD-3-Clause")?

Yes, We can reference https://archlinux.org/packages/extra/x86_64/libmysofa


C:\PROGRA~1\MICROS~2\2022\COMMUN~1\VC\Tools\MSVC\1442~1.344\bin\Hostx64\x64\cl.exe  /nologo  -IC:\Users\star\AppData\Local\.xmake\cache\packages\2412\l\libmysofa\v1.3.2\source\build_7ef28615\src -IC:\Users\star\AppData\Local\.xmake\cache\packages\2412\l\libmysofa\v1.3.2\source\windows\third-party\zlib-1.2.11\include -IC:\Users\star\AppData\Local\.xmake\cache\packages\2412\l\libmysofa\v1.3.2\source\src\hrtf /DWIN32 /D_WINDOWS /W3 -Wall /DWIN32 /D_WINDOWS /W3  /Zi /Ob0 /Od /RTC1 -MD -DDEBUG -DVDEBUG /showIncludes /Fosrc\CMakeFiles\mysofa-static.dir\hdf\gcol.c.obj /Fdpdb\ /FS -c C:\Users\star\AppData\Local\.xmake\cache\packages\2412\l\libmysofa\v1.3.2\source\src\hdf\gcol.c

Try to patch cmake to use xrepo zlib for unbundle deps.

@star-hengxing
Copy link
Contributor

star-hengxing commented Dec 28, 2024

We can apply this patch to improve shared library build.
hoene/libmysofa@1f9c8df

@luadebug
Copy link
Contributor Author

luadebug commented Dec 28, 2024

IDK maybe we should limit Wasm here, because

on_install("linux", "android", function (package)
os.cp(path.join(package:scriptdir(), "port", "xmake.lua"), "xmake.lua")
os.cp(path.join(package:scriptdir(), "port", "config.h"), "config.h")
os.cp(path.join(package:scriptdir(), "port", "endian-darwin.h"), "endian-darwin.h")
import("package.tools.xmake").install(package)
end)
is limited to Linux, android & rely over endian-darwin.h just as shown in Wasm error log of this port with Unsupported platform error.
Could be related
https://github.com/linux-sunxi/sunxi-tools/pull/59/files
https://gist.github.com/panzi/6856583?permalink_comment_id=2382595#gistcomment-2382595
https://gist.github.com/panzi/6856583?permalink_comment_id=2372628#gistcomment-2372628

https://github.com/xmake-io/xmake-repo/blob/aa72d472b106727cf9cd823f1ce675e0c4c19ff5/packages/l/libkmod/port/endian-darwin.h is same as https://github.com/hoene/libmysofa/blob/main/src/hrtf/portable_endian.h
Well left to do is limit Wasm & done or either going something https://gitlab.b-data.ch/qgis/qgis/-/blob/final-3_24_2/external/laz-perf/portable_endian.hpp?ref_type=tags#L13-15 which points to https://github.com/hobuinc/laz-perf/blob/2b68700d33bf981d257d6101d96714620c64019c/cpp/lazperf/portable_endian.hpp#L15
It is not chosen by upstream and I'm not sure to go this way or not.

#endif

-#if defined(__linux__) || defined(__CYGWIN__)
+#if defined(__linux__) || defined(__CYGWIN__) || defined(__illumos__)
Copy link
Contributor Author

@luadebug luadebug Dec 28, 2024

Choose a reason for hiding this comment

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

Should it be there || defined(__EMSCRIPTEN__) or its better limit wasm build ? https://github.com/hobuinc/laz-perf/blob/2b68700d33bf981d257d6101d96714620c64019c/cpp/lazperf/portable_endian.hpp#L15
maybe || defined(__GNU__) as well. Though it looks very preferable to just replace existing file with this one.
Well it seems it helped WASM CI for some reason.

@star-hengxing
Copy link
Contributor

star-hengxing commented Dec 31, 2024

If the library documentation says that the platforms (wasm/ios etc...) is supported, but it failed in xrepo ci, we can limit it in on_check.

on_check(function (package)
if package:is_cross() then
if not package:is_plat("windows", "macosx") then
raise("package(igraph) unsupported cross-compilation now. To support it, see https://igraph.org/c/html/latest/igraph-Installation.html#igraph-Installation-cross-compiling")
end
end
end)

@luadebug
Copy link
Contributor Author

luadebug commented Jan 1, 2025

If the library documentation says that the platforms (wasm/ios etc...) is supported, but it failed in xrepo ci, we can limit it in on_check.

on_check(function (package)
if package:is_cross() then
if not package:is_plat("windows", "macosx") then
raise("package(igraph) unsupported cross-compilation now. To support it, see https://igraph.org/c/html/latest/igraph-Installation.html#igraph-Installation-cross-compiling")
end
end
end)

I don't find anything that states out, libmysofa supports wasm/emscripten or not (https://github.com/ColumbiaCEAL/libmysofa-wasm/blob/main/README.md portable endian h not found there). I believe it should pass CI after changing portable endian header, as it is shown in other repo.
Prolly need patch portable endian to support emscripten or borrow it from lazpack repo. Yet I hope I patch it to support wasm properly, hence it repeat lazperf s emscripten support, i believe... %_% At least it helped CI.

@waruqi waruqi merged commit e6a84e8 into xmake-io:dev Jan 3, 2025
67 checks passed
@luadebug luadebug deleted the port branch January 3, 2025 15:20
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.

3 participants