Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Desktop][Linux] Add RUNPATH $ORIGIN to flutter_linux_gtk #28525

Merged
merged 3 commits into from
Sep 13, 2021

Conversation

dcharkes
Copy link
Contributor

@dcharkes dcharkes commented Sep 9, 2021

When using dart:ffi's DynamicLibrary.open(...), the RUNPATH of bundle/lib/libflutter_linux_gtk.so is used rather than bundle/flutter_app. libflutter_linux_gtk.so did not have a RUNPATH set, making dlopen only succeed on absolute paths and dynamically linked libraries. Setting RUNPATH enables us to dynamically load shared libraries without dynamically linking them upfront.

With this fix, the ${plugin}_bundled_libraries can work without modifying a flutter app:

${plugin}/linux/CMakeLists.txt

# List of absolute paths to libraries that should be bundled with the plugin
set(mylib_dylib_bundled_libraries
  "${CMAKE_CURRENT_SOURCE_DIR}/../native/lib/linux_x64/libmylib_dylib.so;${CMAKE_CURRENT_SOURCE_DIR}/../native/lib/linux_x64/libmylib_dylib_dependency.so"
  PARENT_SCOPE
)

Before this fix, we also needed to modify a flutter_app to dynamically link the libraries:

${flutter_app}/linux/CMakeLists.txt

# Link bundled libraries from plugins into main executable to enable dlopen
# with only library name.
foreach(plugin ${FLUTTER_PLUGIN_LIST})
  if(${plugin}_bundled_libraries)
    target_link_libraries(
      ${BINARY_NAME}
      PUBLIC
      ${${plugin}_bundled_libraries}
    )
  endif()
endforeach(plugin)

This flutter_app modification is obsolete after this PR.

Tested locally with this patch and flutter --local-engine=host_release --local-engine-src-path=$HOME/flt/engine/src run linux.
Change can be observed with readelf -d flutter_app/build/linux/x64/release/bundle/lib/libflutter_linux_gtk.so | grep RUNPATH.

Relevant issues: flutter/flutter#78819 (comment) and dart-archive/ffi#118.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

/cc @cbracken

It seems reasonable to have a default here, but shouldn't we also override this to $ORIGIN/lib/ in the install step in the template so that just loading the library by name works?

@dcharkes
Copy link
Contributor Author

dcharkes commented Sep 9, 2021

Thanks for the quick review!

It seems reasonable to have a default here, but shouldn't we also override this to $ORIGIN/lib/ in the install step in the template so that just loading the library by name works?

Using $ORIGIN/lib/ doesn't work. libflutter_linux_gtk.so is already in lib/, so looking up things from native code invoked by dart:ffi or DynamicLibrary.open would search in lib/lib/. (Or am I missing something?)

The install step in the app does not modify the rpath in CMake, it simply copies the files:

install(FILES "${FLUTTER_LIBRARY}" DESTINATION "${INSTALL_BUNDLE_LIB_DIR}"
  COMPONENT Runtime)

I'd need to do some more digging on how to teach CMake to change the RUNPATH of a precompiled library (rather than one it is compiling itself).

Edit: I do not see such functionality in https://cmake.org/cmake/help/latest/command/install.html#files.

@stuartmorgan-g
Copy link
Contributor

Using $ORIGIN/lib/ doesn't work. libflutter_linux_gtk.so is already in lib/, so looking up things from native code invoked by dart:ffi or DynamicLibrary.open would search in lib/lib/. (Or am I missing something?)

Isn't $ORIGIN always the executable? I'm having trouble finding much documentation, but it sounds like that's its behavior.

The install step in the app does not modify the rpath in CMake, it simply copies the files

I've swapped this out from when I did all the CMake setup, but IIRC CMake automatically does rpath adjustment during the install phase. IIRC that's part of why I used it to do the bundling.

@dcharkes
Copy link
Contributor Author

dcharkes commented Sep 9, 2021

Isn't $ORIGIN always the executable? I'm having trouble finding much documentation, but it sounds like that's its behavior.

Apparently it is the dynamic library. (At least that is my conclusion after experimenting with this and seeing $ORIGIN/lib fail in the dynamic library. I also could not find a lot of documentation.)

I've swapped this out from when I did all the CMake setup, but IIRC CMake automatically does rpath adjustment during the install phase. IIRC that's part of why I used it to do the bundling.

CMake only does RPATH for non-IMPORTED libraries/executables as far as I can see. And we're building this library in GN/ninja and there we don't have an install step afaik.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Isn't $ORIGIN always the executable? I'm having trouble finding much documentation, but it sounds like that's its behavior.

Apparently it is the dynamic library. (At least that is my conclusion after experimenting with this and seeing $ORIGIN/lib fail in the dynamic library. I also could not find a lot of documentation.)

Interesting. Well, that renders the CMake question moot, so LGTM.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants