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

[tools/depends] Bump swig 4.2.0 #24552

Merged
merged 5 commits into from Jan 29, 2024
Merged

[tools/depends] Bump swig 4.2.0 #24552

merged 5 commits into from Jan 29, 2024

Conversation

fuzzard
Copy link
Contributor

@fuzzard fuzzard commented Jan 21, 2024

Description

Bump swig to 4.2.0 and fix building with swig 4.2.0

Motivation and context

Fixes #24385
More information can be seen in that issue regarding the end result of the swig compilation

How has this been tested?

@heitbaum

What is the effect on users?

N/A

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@fuzzard fuzzard added Type: Fix non-breaking change which fixes an issue Component: Depends v21 Omega labels Jan 21, 2024
@fuzzard fuzzard added this to the Omega 21.0 Beta 3 milestone Jan 21, 2024
@sundermann
Copy link
Contributor

Compiles on my machine for webOS. Maybe the webOS build failure was just a fluke

@sundermann
Copy link
Contributor

Jenkins build this please

@sundermann
Copy link
Contributor

Apparently, the issue seems to arise from this part in Toolchain-Native.cmake that appends the target sysroot:

if(NOT "/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot" STREQUAL "")
  list(APPEND CMAKE_FIND_ROOT_PATH /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi /home/stephan.linux/arm-webos-linux-gn>
  set(CMAKE_LIBRARY_PATH "${CMAKE_LIBRARY_PATH}:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/usr/lib/arm-webos-linux-gnueabi:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/lib/arm-webos-lin>
endif()

If this part is present the build will i.e. resolve bzip2 from the target sysroot:

-- Found BZip2: /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr/lib/libbz2.so (found version "1.0.8")

Causing a lot of redefinition warnings:

In file included from /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr/include/ctype.h:27,
                 from /home/stephan.linux/xbmc/tools/depends/native/pcre2/aarch64-linux-native/src/pcre2_internal.h:56,
                 from /home/stephan.linux/xbmc/tools/depends/native/pcre2/aarch64-linux-native/src/pcre2_tables.c:51:
/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr/include/features.h:326: warning: "__STDC_ISO_10646__" redefined
  326 | #define __STDC_ISO_10646__              200009L

With that part commented in Toolchain-Native.cmake does not resolve the wrong libraries and compiles without warnings:

-- Could NOT find BZip2 (missing: BZIP2_LIBRARIES BZIP2_INCLUDE_DIR)

It's also possible that the root cause for this is that use_host='arm-webos-linux-gnueabi' which seems wrong

@sundermann
Copy link
Contributor

sundermann commented Jan 27, 2024

The inclusion of bzip was just an example for why it was failing. There's more libraries being pulled in (at least libreadline for pcre2). But there may be more.

Why are target sysroot even included in CMAKE_LIBRARY_PATH and CMAKE_FIND_ROOT_PATH ? The native stuff builds fine for me with that commented

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 27, 2024

On other platforms, they do not point to target locations. they are host locations. The comment is incorrect. So im guessing the webos stuff isnt differentiating somewhere?

# where is the target environment
set(CMAKE_FIND_ROOT_PATH /Users/Shared/xbmc-depends/arm-darwin23.1.0-native)
set(CMAKE_LIBRARY_PATH /Users/Shared/xbmc-depends/arm-darwin23.1.0-native/lib)

Target toolchain is

set(CMAKE_FIND_ROOT_PATH /Users/Shared/xbmc-depends/macosx14.2_arm64-target-debug)
set(CMAKE_LIBRARY_PATH /Users/Shared/xbmc-depends/macosx14.2_arm64-target-debug/lib)

@sundermann
Copy link
Contributor

Parts from the webOS cmake config:

Native:

# where is the target environment
set(CMAKE_FIND_ROOT_PATH /home/stephan.linux/kodi-deps/aarch64-linux-gnu-native)
set(CMAKE_LIBRARY_PATH /home/stephan.linux/kodi-deps/aarch64-linux-gnu-native/lib)
if(NOT "/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot" STREQUAL "")
  list(APPEND CMAKE_FIND_ROOT_PATH /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/libc /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/lib/arm-webos-linux-gnueabi/sysroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/usr /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/sysroot/usr)
  set(CMAKE_LIBRARY_PATH "${CMAKE_LIBRARY_PATH}:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/usr/lib/arm-webos-linux-gnueabi:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/lib/arm-webos-linux-gnueabi")
endif()

Target:

# where is the target environment
set(CMAKE_FIND_ROOT_PATH /home/stephan.linux/kodi-deps/arm-webos-linux-gnueabi-release)
set(CMAKE_LIBRARY_PATH /home/stephan.linux/kodi-deps/arm-webos-linux-gnueabi-release/lib)
if(NOT "" STREQUAL "")
  list(APPEND CMAKE_FIND_ROOT_PATH  /usr)
endif()
# Currently this is only set to reject android by default
if(NOT "/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot" STREQUAL "")
  list(APPEND CMAKE_FIND_ROOT_PATH /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/libc /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/lib/arm-webos-linux-gnueabi/sysroot /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/usr /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/sysroot/usr)
  # Explicitly set this as last. This potentially is /usr which can then cause linux
  # cross compilation to search paths that are not relevant to target arch (eg host libs)
  # x86/x86_64 jenkins CI jobs require /usr for libs like iconv
  list(APPEND CMAKE_FIND_ROOT_PATH /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot)
  set(CMAKE_LIBRARY_PATH "${CMAKE_LIBRARY_PATH}:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/usr/lib/arm-webos-linux-gnueabi:/home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/lib/arm-webos-linux-gnueabi")
endif()

It's not the first two lines that are the problem. The problem is inside the if condition where stuff is appended to CMAKE_FIND_ROOT_PATH and CMAKE_LIBRARY_PATH.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 27, 2024

The if condition is a catch all for every platform that uses it. What path explicitly is a target lib being found in the native list?

I cant build webos locally on a mac (at least last time i tried), so really have no way to test/look at this.

@sundermann
Copy link
Contributor

-- Found BZip2: /home/stephan.linux/arm-webos-linux-gnueabi_sdk-buildroot/arm-webos-linux-gnueabi/sysroot/usr/lib/libbz2.so (found version "1.0.8")

Basically anything inside $TOOLCHAIN/arm-webos-linux-gnueabi should be considered target and should not be included in the native toolchain file. So any paths resolving a (sub)path of @use_toolchain@/@use_host@ are problematic.

I'm building for webOS on a mac inside a aarch64 ubuntu-22.04 vm btw.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 27, 2024

Can you PR whatever change you think is relevant and we'll see what breaks/fallout i guess. Whatever the change ends up being isnt exactly relevant to this PR, and should be resolved regardless i imagine.

fuzzard added 3 commits January 28, 2024 15:55
Bison 3.5 is a dependency of swig 4.2.0 cmake builds. macos ships bison 2.3.
so add as a general native dep
pcre2 is a dependency of swig 4.2.0+
swig 4.2.0 introduced a change that adds a constructor to the generated AddonModuleXbmcaddon.i.cpp
file. This causes failures such as

build/swig/AddonModuleXbmcaddon.i.cpp: In function 'PyObject* PythonBindings::xbmcaddon_XBMCAddon_xbmcaddon_Settings_New(PyTypeObject*, PyObject*, PyObject*)':
build/swig/AddonModuleXbmcaddon.i.cpp:1751:52: error: no matching function for call to 'XBMCAddon::xbmcaddon::Settings::Settings()'
 1751 |     apiResult = new XBMCAddon::xbmcaddon::Settings();
      |                                                    ^
In file included from ../xbmc/interfaces/legacy/Addon.h:14,
                 from build/swig/AddonModuleXbmcaddon.i.cpp:30:
../xbmc/interfaces/legacy/Settings.h:58:3: note: candidate: 'XBMCAddon::xbmcaddon::Settings::Settings(std::shared_ptr<CSettingsBase>)'
   58 |   Settings(std::shared_ptr<CSettingsBase> settings);
      |   ^~~~~~~~
../xbmc/interfaces/legacy/Settings.h:58:3: note:   candidate expects 1 argument, 0 provided

If we disable the contructor for Settings, we get the same generated output as swig <=4.1.1
@fuzzard fuzzard merged commit cabba8d into xbmc:master Jan 29, 2024
2 checks passed
@fuzzard fuzzard deleted the depends_swig branch January 29, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Depends Type: Fix non-breaking change which fixes an issue v21 Omega
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile with swig 4.2.0 fails
2 participants