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

systemd: default withLibBPF to false if isMips64 #194149

Merged
merged 1 commit into from Jan 13, 2023
Merged

systemd: default withLibBPF to false if isMips64 #194149

merged 1 commit into from Jan 13, 2023

Conversation

ghost
Copy link

@ghost ghost commented Oct 3, 2022

Description of changes

libBPF does not compile for mips64 targets using clang (rathern than gcc) because clang lacks the necessary _MIPS_SZPTR compiler builtin. Let's allow the rest of systemd to compile.

Things done
  • Built on platform(s)
    • mips64el-linux
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@ghost ghost self-requested a review as a code owner October 3, 2022 01:54
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Oct 3, 2022
@alyssais
Copy link
Member

alyssais commented Oct 4, 2022

Is there anywhere upstream it would make sense to report this?

@ghost
Copy link
Author

ghost commented Oct 4, 2022

Is there anywhere upstream it would make sense to report this?

That's a good question.

So now I'm thinking that this is actually a nixpkgs bug -- that we're not invoking clang in a way that tells the frontend to make the mips builtins available, even if the backend is emitting mips binaries. Or at least we aren't tricking systemd's build machinery into doing that.

I'm draftifying this while I try to figure that out. Maybe we can leave withLibBPF enabled.

@ghost ghost marked this pull request as draft October 4, 2022 09:12
@ghost ghost mentioned this pull request Jan 13, 2023
13 tasks
@ghost
Copy link
Author

ghost commented Jan 13, 2023

I've rebased this and included everything I know about the situation in the commit message. Realistically I am not likely to have time to fix this in the next few months.

I know that's not ideal, but in the meantime I think that disabling systemd.LibBPF on mips is less bad than having systemd simply be totally broken on mips.

@ghost ghost marked this pull request as ready for review January 13, 2023 10:53
@ghost

This comment was marked as resolved.

libBPF does not compile for mips64 targets using clang (rathern than
gcc) because clang lacks the necessary _MIPS_SZPTR compiler builtin.
Let's allow the rest of systemd to compile.

- The glibc people noticed this problem [way back in
  2011](https://sourceware.org/pipermail/libc-ports/2011-June/001959.html)
  and consider it to be a clang/llvm bug.  I am inclined to agree.

- [clang has the `_MIPS_SZPTR`
  builtin](https://github.com/llvm/clangir/blob/3af9cb5375084541165b4b63d36e3798801c95ab/clang/lib/Basic/Targets/Mips.cpp#L185)
  and seems to have had it since before they switched to git.

This may in fact be a nixpkgs bug -- that we're not invoking clang
in a way that tells the frontend to make the mips builtins
available, even if the backend is emitting mips binaries.  Or at
least we aren't tricking systemd's build machinery into doing that.
@alyssais
Copy link
Member

Pushed a formatting fix, per the comment at the top of the file.

@alyssais alyssais merged commit 6f6b4a1 into NixOS:master Jan 13, 2023
@ghost ghost deleted the pr/systemd/withLibBPF/mips branch January 13, 2023 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: systemd 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants