search

libbpf / bpftool

Automated upstream mirror for bpftool stand-alone build.
Other
411 stars 72 forks source link

mirror: fix building documentation targets #153

Closed viktormalik closed 2 months ago

viktormalik commented 2 months ago

Hi,

I'm trying to start building the bpftool Fedora package from this repo and ran across a strange issue which prevents me from building the documentation.

After some investigation, I found that the issue is in the callable descend command in Makefile.include (which is specific to this repo) which is currently broken due to two reasons:

  1. It is defined only for cases when the output is not suppressed by either V=1 or -s.
  2. The condition for checking the absence of -s is broken and it fails every time there is a variable definition containing the letter 's'.

The descend command is used in documentation targets and due to these errors, these targets fail to build in the following cases:

$ make V=1 doc
make: Nothing to be done for 'doc'.

$ make prefix="/usr" doc    # <- `prefix="/usr"` contains letter 's'
make: Nothing to be done for 'doc'.

This PR fixes the two above issues by (1) defining descend for all cases and (2) fixing the -s check by querying the first word of MAKEFLAGS which is the correct approach for make-4.0 and higher.

qmonnet commented 2 months ago

Hi, thanks a lot for the PR! Please give me a couple days to come back from holiday and take a proper look, I don't trust my review capabilities on the phone 🙂

viktormalik commented 2 months ago

Sure, no problem! Enjoy your holiday :slightly_smiling_face:

viktormalik commented 2 months ago
  • The condition for checking the absence of -s is broken and it fails every time there is a variable definition containing the letter 's'.

I can't reproduce this one. What version of make do you use? My understanding from the doc is that only flags (valid make options) should be passed down with MAKEFLAGS, not random variable definitions. On my side:

$ make --version                  
GNU Make 4.3

For me:

$ make --version
GNU Make 4.4.1

Looking at the release notes of version 4.4, it really seems that this behavior was changed:

* WARNING: Backward-incompatibility!
  Previously only simple (one-letter) options were added to the MAKEFLAGS
  variable that was visible while parsing makefiles.  Now, all options are
  available in MAKEFLAGS.  If you want to check MAKEFLAGS for a one-letter
  option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return
  the set of one-letter options which can be examined via findstring, etc.

so I guess that explains it :slightly_smiling_face:

qmonnet commented 2 months ago

Oh that says it all then, thanks a lot for looking! Perfect, I'll merge now :+1: