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.

Sign up for GitHub

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

Jump to bottom

Allow building on systems using musl C library #19513

Merged
merged 8 commits into from
Oct 23, 2023
Merged

Allow building on systems using musl C library #19513

merged 8 commits into from
Oct 23, 2023

Conversation

marv
Copy link
Contributor

@marv marv commented Oct 20, 2023

Fix a couple of things to allow building on musl based systems:

  • musl doesn't define SIGCLD but only SIGCHLD => ifdef it as done for other signals
  • src/session/session-utils.c uses _PATH_LASTLOG but doesn't include <paths.h> where it's defined (on musl and glibc)
  • musl doesn't provide fts_{open,read,close} itself and needs to link against an external fts library
  • musl doesn't provide argp_{parse,error} itself and needs to link against an external argp library
  • the last 4 patches just fix warnings about including <xyz.h> instead of <sys/xyz.h> so the build on musl systems is warning-free

@marv
common: Don't assume that SIGCLD is always available
5457dff 
The musl C library for example only defines SIGCHLD and not SIGCLD:
```
[...]

  CC       src/common/libcockpit_common_a-cockpitunixsignal.o
src/common/cockpitunixsignal.c:76:17: error: use of undeclared identifier 'SIGCLD'
  { "CLD",      SIGCLD },       /* same as SIGCHLD (mips) */
                ^
```
@marv
session: Include <paths.h> for _PATH_LASTLOG
8c96acc 
Building with the musl libc failed with the following error:
```
src/session/session-utils.c:180:14: error: use of undeclared identifier '_PATH_LASTLOG'
  fd = open (_PATH_LASTLOG, O_RDWR);
             ^
src/session/session-utils.c:183:34: error: use of undeclared identifier '_PATH_LASTLOG'
      warn ("failed to open %s", _PATH_LASTLOG);
                                 ^
src/session/session-utils.c:220:48: error: use of undeclared identifier '_PATH_LASTLOG'
      warn ("failed to pread() %s for uid %u", _PATH_LASTLOG, (unsigned) uid);
                                               ^
src/session/session-utils.c:227:14: error: use of undeclared identifier '_PATH_LASTLOG'
             _PATH_LASTLOG, (unsigned) uid, r, sizeof entry);
             ^
src/session/session-utils.c:261:49: error: use of undeclared identifier '_PATH_LASTLOG'
      warn ("failed to pwrite() %s for uid %u", _PATH_LASTLOG, (unsigned) uid);
                                                ^
src/session/session-utils.c:268:14: error: use of undeclared identifier '_PATH_LASTLOG'
             _PATH_LASTLOG, (unsigned) uid, r, sizeof entry);
             ^
```

glibc and musl declare `_PATH_LASTLOG` in `<paths.h>`, include it to fix this error
@marv
build: Check if we need to link against libfts
361769c 
On systems with musl C library the build of cockpit-bridge otherwise fails with
a linker error:
```
[...]

  CCLD     cockpit-bridge
ld.lld: error: undefined symbol: fts_open
>>> referenced by cockpitcgroupsamples.c:230 (/home/marv/devel/cockpit/src/bridge/cockpitcgroupsamples.c:230)
>>>               libcockpit_metrics_a-cockpitcgroupsamples.o:(notice_cgroups_in_hierarchy) in archive libcockpit-metrics.a

ld.lld: error: undefined symbol: fts_read
>>> referenced by cockpitcgroupsamples.c:233 (/home/marv/devel/cockpit/src/bridge/cockpitcgroupsamples.c:233)
>>>               libcockpit_metrics_a-cockpitcgroupsamples.o:(notice_cgroups_in_hierarchy) in archive libcockpit-metrics.a

ld.lld: error: undefined symbol: fts_close
>>> referenced by cockpitcgroupsamples.c:254 (/home/marv/devel/cockpit/src/bridge/cockpitcgroupsamples.c:254)
>>>               libcockpit_metrics_a-cockpitcgroupsamples.o:(notice_cgroups_in_hierarchy) in archive libcockpit-metrics.a
```
@marv
build: Check if we need to link against libargp
509ad8f 
The build of `cockpit-tls` otherwise fails with a linker error on systems using
the musl C library:
```
[...]

  CCLD     cockpit-tls
ld.lld: error: undefined symbol: argp_parse
>>> referenced by main.c:103 (/home/marv/devel/cockpit/src/tls/main.c:103)
>>>               src/tls/main.o:(main)

ld.lld: error: undefined symbol: argp_error
>>> referenced by main.c:51 (/home/marv/devel/cockpit/src/tls/main.c:51)
>>>               src/tls/main.o:(arg_parse_int)
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
```
@marv
session: Use <signal.h> instead of <sys/signal.h>
8a83f26 
musl warns about the latter being incorrect:
```
[...]

  CC       src/session/session-utils.o
In file included from src/session/session-utils.c:22:
In file included from src/session/session-utils.h:36:
/usr/x86_64-pc-linux-musl/include/sys/signal.h:1:2: warning: redirecting incorrect #include <sys/signal.h> to <signal.h> [-W#warnings]
```

and in glibc `<sys/signal.h>` is also just a wrapper for `<signal.h>`
@marv
tls: Sort includes (NFC)
f7ec4fe
@marv
tls: Include <fcntl.h> instead of <sys/fcntl.h>
5c49d3e 
musl libc warns about it:
```
In file included from src/tls/cockpit-certificate-ensure.c:32:
/usr/x86_64-pc-linux-musl/include/sys/fcntl.h:1:2: warning: redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-W#warnings]
    1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
      |  ^
```
and it's just a wrapper in glibc as well. Use `<fcntl.h>` to be warning-free
when building against musl
@marv
tls: Include <poll.h> instead of <sys/poll.h>
0d0a3b9 
musl libc warns about these:
```
In file included from src/tls/socket-io.c:32:
/usr/x86_64-pc-linux-musl/include/sys/poll.h:1:2: warning: redirecting incorrect #include <sys/poll.h> to <poll.h> [-W#warnings]
    1 | #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
      |  ^
  CC       src/pam-ssh-add/pam_ssh_add_so-pam-ssh-add.o
In file included from src/tls/connection.c:37:
/usr/x86_64-pc-linux-musl/include/sys/poll.h:1:2: warning: redirecting incorrect #include <sys/poll.h> to <poll.h> [-W#warnings]
    1 | #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
      |  ^
```
and glibc's `<sys/poll.h>` also just wraps `<poll.h>`. Use `<poll.h>` to get
rid of the warning on musl based systems
@martinpitt
Copy link
Member

Thanks a lot! We don't run CI on musl, so we can't guarantee that we won't break it again -- but as the C bridge is basically dead and will be dropped from main in less than 6 months, we won't touch it much any more anyway.

Out of interest, why do you even build with --enable-old-bridge? By default it should build the Python bridge now.

@martinpitt martinpitt merged commit 54a27a6 into cockpit-project:main Oct 23, 2023
98 checks passed
@jelly
Copy link
Member

jelly commented Oct 23, 2023

IIRC systemd does not support musl at all and Cockpit basically depends on it, it might not be worthy to chase this. Although for the bridge this is fine ofcourse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinpitt martinpitt martinpitt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marv @martinpitt @jelly