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

fix AppImageKit compilation by restoring appimage_get_elf_size() #160

Closed
wants to merge 1 commit into from

Conversation

hchunhui
Copy link

Currently AppImageKit can not be compiled with the master branch of libappimage, the build script reports "undefined reference to appimage_get_elf_size()".

the runtime of AppImageKit can only be linked with libappimage_shared, but appimage_get_elf_size() is moved into libappimage since 4f22f90 . Restoring the function will fix the issue.

@hchunhui hchunhui mentioned this pull request Oct 22, 2021
@azubieta
Copy link
Contributor

@hchunhui thanks for the contribution, but I think that this fix should go in the AppImageKit repo instead. As we moved that function in a legacy header so it may be deorecated at some point.

@probonopd @TheAssassin may have some comments on this.

@hchunhui
Copy link
Author

appimage_get_elf_size() can be replace by appimage_get_payload_offset() according to current implementation, but after the replacement we have to link to the whole C++ library instead of libappimage_shared (relatively small, written by C). As the function is used by runtime of AppImageKit, and we may want to keep runtime small, so it may be worth to restore it.

@probonopd
Copy link
Member

Yes, we want runtime.c to be as small and lightweight as possible and no heavyweight C++ dependencies in runtime.c.

Once we had:
https://github.com/AppImage/AppImageKit/blob/d66acbafe8e17f82f19d644b09df45c4e0bcd284/elf.c

@lalten
Copy link

lalten commented Apr 3, 2022

What's the state of this? Without this fixed it looks like AppImageKit can not use any libappimage after 4f22f90 from Jan 2019

lalten added a commit to lalten/AppImageKit that referenced this pull request Apr 3, 2022
The submodule is replaced with CMake FetchContent commands, which fetch the source at configure time (We need to include scripts.cmake from libappimage. ExternalProject_Add only downloads at build time).

Also update libappimage version to latest (otherwise the build fails on g++11) and patch in AppImageCommunity/libappimage#160 (to allow using latest libappimage in AppImageKit.

Also update squashfs-tools version to latest release (otherwise the build fails on g++11)

Also related to AppImage#1165
TheAssassin pushed a commit to lalten/AppImageKit that referenced this pull request Apr 4, 2022
The submodule is replaced with CMake FetchContent commands, which fetch the source at configure time (We need to include scripts.cmake from libappimage. ExternalProject_Add only downloads at build time).

Also update libappimage version to latest (otherwise the build fails on g++11) and patch in AppImageCommunity/libappimage#160 (to allow using latest libappimage in AppImageKit.

Also update squashfs-tools version to latest release (otherwise the build fails on g++11)

Also related to AppImage#1165
lalten added a commit to lalten/AppImageKit that referenced this pull request Aug 18, 2022
The submodule is replaced with CMake FetchContent commands, which fetch the source at configure time (We need to include scripts.cmake from libappimage. ExternalProject_Add only downloads at build time).

Also update libappimage version to latest (otherwise the build fails on g++11) and patch in AppImageCommunity/libappimage#160 (to allow using latest libappimage in AppImageKit.

Also update squashfs-tools version to latest release (otherwise the build fails on g++11)

Also related to AppImage#1165
@TheAssassin
Copy link
Member

I think we should not duplicate code within here, but instead really just move this code to AppImageKit's repository for now. Thanks anyway!

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.

5 participants