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

build: Find feature headers outside system include dir #43946

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Conversation

camscale
Copy link
Contributor

@camscale camscale commented Jul 9, 2024

Look for PAM and BPF header files that control including features in
teleport binaries more generally instead of assuming they are in the
system include headers or the one location in the buildbox (for libbpf
in /usr/libbpf-1.2.2).

This allows a cross-compiler to be used that sets the standard
environment variables C_INCLUDE_PATH and LIBRARY_PATH to point to
where additional headers and libraries live.

This cleans up the BPF detection in common.mk as it had some
head-scratching failure modes when clang or llvm-strip were not found.
The logic for enabling BPF is clearer now and safer.

Make IS_NATIVE_BUILD into a proper make variable - empty means false,
non-empty means true, and adjust the Makefile where it is used.

@camscale camscale added the no-changelog Indicates that a PR does not require a changelog entry label Jul 9, 2024
Makefile Outdated Show resolved Hide resolved
camscale added a commit that referenced this pull request Jul 9, 2024
@camscale camscale requested a review from jakule July 12, 2024 05:54
@camscale
Copy link
Contributor Author

Jakub has made some changes that conflicts with this that I need to consider how it should integrate. I'll rework this shortly.

common.mk Outdated Show resolved Hide resolved
@camscale
Copy link
Contributor Author

@jakule I've incorporated your libelf pkg-config changes and also started using pkg-config for libbpf too, pinning to the specific version we require. I'm not sure if I got a little silly using false as the fallback for pkg-config when neither it or pkgconf are installed. It seems to work well enough, but is perhaps a little tricky? let me know.

This still conflicts because I need to rebase, but the contents of this PR is what the final version would look like if you just view the full file (common.mk).

common.mk Outdated Show resolved Hide resolved
common.mk Show resolved Hide resolved
Use `pkg-config` / `pkgconf` to find the specific version of `libbpf`
that we want to link to as well as the dependent libraries such as
`libelf`, `libz`, and sometimes `libzstd`. This resolves the variable
dependency on `libzstd` depending on the base OS.

We still take preference of `/lib/libbpf-$(LIBBPF_VER)` if it exists as
it does for the standard buildbox, but that is deprecated and will be
removed when we move to the new buildbox.

If there is no `libelf.pc` config, fall back to hard-coded libraries as
it was previously. Again, this is for use with our existing CentOS 7
buildbox.

This cleans up the BPF detection in `common.mk` as it had some
head-scratching failure modes when clang or llvm-strip were not found.
The logic for enabling BPF is clearer now and safer.
When looking for the PAM header `pam_appl.h` to determine if PAM should
be enabled in teleport, first look in the directories specified in the
`C_INCLUDE_PATH` environment variable before checking the system header
directories. The buildbox-ng has per-architecture include directories
and configures `C_INCLUDE_PATH` to find the correct headers. Fall back
to the existing default directories if not found in `C_INCLUDE_PATH`.
@camscale
Copy link
Contributor Author

I've updated this PR to squash the commits together as that was the easiest way to resolve the conflicts, which I then rebased with --strategy-option=theirs. I've done a local test build with the centos7 buildbox and it is working as expected. I have decided to keep the pkg-config setup for libbpf as it is working as expected and is a little more general.

I'll merge this tomorrow morning my time in case there are any further objections or comments.

@camscale camscale added this pull request to the merge queue Jul 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 18, 2024
@camscale camscale added this pull request to the merge queue Jul 18, 2024
Merged via the queue into master with commit 1093265 Jul 18, 2024
39 checks passed
@camscale camscale deleted the camh/c-includes branch July 18, 2024 12:24
@public-teleport-github-review-bot

@camscale See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants