Skip to content

Musl build#3114

Draft
MichaelRabek wants to merge 3 commits intolinux-nvme:masterfrom
MichaelRabek:musl-build
Draft

Musl build#3114
MichaelRabek wants to merge 3 commits intolinux-nvme:masterfrom
MichaelRabek:musl-build

Conversation

@MichaelRabek
Copy link

This series of patches contributes to solving the recurring problem with build fails with musl libc. It includes proposed build configuration and a new CI build job.

Things to check before removing draft status:

  • Do all tests pass with the musl build?

libgen.h must be included to silence a compiler error regarding the
basename() function missing when building with musl libc.

The basename() function call in libnvme/src/nvme/linux.c:__nvme_transport_handle_open_direct()
causes a (fatal) compilation warning due to GNU using this declaration of basename:
char *basename (const char *filename)
and POSIX (and also musl) ommiting const. It is thus possible that the const char *devname
would be modified by basename().

The solution to the problem above is to use the internal implementation of basename from glibc
directly and avoid these API compatibility problems.

Signed-off-by: Michal Rábek <mrabek@redhat.com>
Signed-off-by: Michal Rábek <mrabek@redhat.com>
Signed-off-by: Michal Rábek <mrabek@redhat.com>
struct nvme_passthru_cmd dummy = { 0 };
_cleanup_free_ char *path = NULL;
char *name = basename(devname);
/* This used instead of basename() due to behavioral differences between

Choose a reason for hiding this comment

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

Maybe we could define a function nvme_basename() and add it to util.c.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sounds like a good plan to do. FWIW, the windows port is likely to introduce some more wrappers like this anyway.

- name: Mark repo as safe for git
run: git config --global --add safe.directory "$GITHUB_WORKSPACE"
- name: Install musl build dependencies
- run: apt-get install -y --no-install-recommends musl musl-dev musl-tools
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let me add these dependencies to the debian:latest image. The past the apt-get install step failed way to often due to $reason.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

ghcr.io/linux-nvme/debian:latest contains musl musl-dev musl-tools so you can drop the install step here.

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.

3 participants