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

Add Package: libandroid-posix-semaphore #8993

Merged
merged 2 commits into from
Feb 11, 2022
Merged

Add Package: libandroid-posix-semaphore #8993

merged 2 commits into from
Feb 11, 2022

Conversation

licy183
Copy link
Member

@licy183 licy183 commented Feb 10, 2022

Bionic libc doesn't support sem_open, sem_close and sem_unlink. This package contains a port of posix named semaphore from musl libc. The mapped file will be created at the temp directory of termux (/data/data/com.termux/files/usr/tmp) rather than tmpfs (/dev/shm).

It provides the NDK <semaphore.h> header. So this commit will update the ndk-sysroot package and termux NDK toolchain. Hope that removing <semaphore.h> header from NDK will not cause the compilation failure of many packages.

See also: #8990

errno = ENAMETOOLONG;
return 0;
}
memcpy(buf, "/data/data/com.termux/files/usr/tmp/sem.", 40);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid hardcode prefix? Preferably use @TERMUX_PREFIX@

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a C source file, not a patch. Seems the shortcut @TERMUX_PREFIX@ can only work on a .patch file.

Copy link
Contributor

Choose a reason for hiding this comment

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

For non-patch files, we have to sed manually to substitute @TERMUX_PREFIX@:

sed -e "s/%VER%/${TERMUX_PKG_VERSION%.*}/g;s/%REL%/${TERMUX_PKG_VERSION}/g" \
-e "s|@TERMUX_PREFIX@|$TERMUX_PREFIX|" \
"$TERMUX_PKG_BUILDER_DIR"/lua.pc.in > lua.pc

This is preferable, but not mandatory I suppose. We have another instance where $TERMUX_PREFIX is hardcoded:

execl("/data/data/com.termux/files/usr/bin/sh", "sh", "-c",

Copy link
Contributor

Choose a reason for hiding this comment

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

Checking on wordexp.c seems like xeffyr was fine with full prefix. Guess this is a go then.

Copy link
Member

Choose a reason for hiding this comment

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

It is preferable avoid hardcoding it, sed'ing it would be better

# Remove <spawn.h> as it's only for future (later than android-27).
# Remove <zlib.h> and <zconf.h> as we build our own zlib.
# Remove unicode headers provided by libicu.
rm usr/include/{sys/{capability,shm,sem},{glob,iconv,spawn,zlib,zconf}}.h
rm usr/include/{sys/{capability,shm,sem},{glob,iconv,semaphore,spawn,zlib,zconf}}.h
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure moving this to a separate package won't break building any other packages, or are we going to shift all those packages to listing this one as a dependency eventually? @Grimler91, you may want to chime in.

Copy link
Member

@Grimler91 Grimler91 Feb 11, 2022

Choose a reason for hiding this comment

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

Can do some test builds and see if anything, or how much, that brakes. Probably more packages need lobandroid-posix-semaphore as dependency

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can add this patch into libandroid-support package rather than split into a sperated package. It would not break other package's dependency.

@Grimler91
Copy link
Member

All/most c++ packages fail with something like

In file included from /home/builder/.termux-build/aspell/src/common/string.hpp:13:
In file included from /home/builder/.termux-build/_cache/android-r23b-api-24-v7/bin/../sysroot/usr/include/c++/v1/algorithm:643:
In file included from /home/builder/.termux-build/_cache/android-r23b-api-24-v7/bin/../sysroot/usr/include/c++/v1/memory:673:
In file included from /home/builder/.termux-build/_cache/android-r23b-api-24-v7/bin/../sysroot/usr/include/c++/v1/atomic:550:
/home/builder/.termux-build/_cache/android-r23b-api-24-v7/bin/../sysroot/usr/include/c++/v1/__threading_support:33:11: fatal error: 'semaphore.h' file not found
# include <semaphore.h>
          ^~~~~~~~~~~~~

So the header file should probably be made available always. We could either replace the ndk sysroot header with the one from this package or split it into a libandroid-posix-semaphore-dev package and make the build system always install that one

@licy183
Copy link
Member Author

licy183 commented Feb 11, 2022

We could either replace the ndk sysroot header with the one from this package or split it into a libandroid-posix-semaphore-dev package and make the build system always install that one

Actually the <semaphore.h> header from this package is copied from ndk sysroot without any modification.

Maybe we can add these sem_* implementations into libandroid-support package rather than split into a sperate package. If some package need to use posix named semaphore, just add libandroid-support as dependency because libandroid-support is explicitly linked.

@Grimler91
Copy link
Member

Actually the <semaphore.h> header from this package is copied from ndk sysroot without any modification.

Oh, yes, let's keep it in the ndk/ndk-sysroot in that case. Some packages fail to build even if the header is present in $PREFIX/include (since there is no guarantee -I$PREFIX/include is passed to compiler). I think we can keep it as a libandroid-posix-semaphore package, and add it as dependency to the packages that make use of it. Header should go into ndk-sysroot as before then though

@licy183
Copy link
Member Author

licy183 commented Feb 11, 2022

Oh, yes, let's keep it in the ndk/ndk-sysroot in that case. Some packages fail to build even if the header is present in $PREFIX/include (since there is no guarantee -I$PREFIX/include is passed to compiler). I think we can keep it as a libandroid-posix-semaphore package, and add it as dependency to the packages that make use of it. Header should go into ndk-sysroot as before then though

OK. I will submit a commit.

@Grimler91
Copy link
Member

Thanks! Semaphore header could probably be moved from ndk-sysroot to libandroid-posix-semaphore to simplify on device builds, but that's something we can fix later.

I added a commit to pass PREFIX as compiler flag instead, to avoid it always being hardcoded

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