kmod/patch: simplify patch-hook.o build dependencies - #1473
Closed
georgejguo wants to merge 1 commit into
Closed
Conversation
The build dependencies for patch-hook.o are duplicated. The conditional inclusion of kpatch-patch-hook.c and livepatch-patch-hook.c is already managed in patch-hook.c via #if IS_ENABLED(CONFIG_LIVEPATCH). Signed-off-by: george <guodongtai@kylinos.cn>
Contributor
|
Hi @georgejguo , While it may seem like this is a reasonable optimization, providing the two hook implementation files directly as Make dependencies is important to mark them as such in the build process. GNU Make itself does not automatically include header files in the dependency tree for source files. For example: A - Setup a test directory with skeleton Makefile and dummy source files $ mkdir /tmp/test
$cd /tmp/test
# Create dummy kpatch/livepatch hook implementations
$ touch kpatch-patch-hook.c livepatch-patch-hook.c
# Create a simple patch-hook that always includes livepatch-hook
$ cat << 'EOF' > patch-hook.c
#include "livepatch-hook.c"
EOF
# Skeleton Makefile with full dependencies as per HEAD
$ cat << 'EOF' > Makefile
patch-hook.o: patch-hook.c kpatch-patch-hook.c livepatch-patch-hook.c
touch patch-hook.o
clean:
rm patch-hook.o
EOFB - Verify how the Make dependencies currently work: # Initial "build"
$ make
touch patch-hook.o
# Second make invocation = dependencies are older = no build
$ make
make: 'patch-hook.o' is up to date.
# Third build = patch-hook.c is updated = build
$ touch patch-hook.c && make
touch patch-hook.o
# Fourth build = dependencies are older = no build
$ make
make: 'patch-hook.o' is up to date.
# Fifth build = livepatch-patch-hook.c is updated = build
$ touch livepatch-patch-hook.c && make
touch patch-hook.o
$ make cleanC - See how we lose the fifth build test case when dropping them from the dependency list: # Skeleton Makefile with single dependency
$ cat << 'EOF' > Makefile
patch-hook.o: patch-hook.c
touch patch-hook.o
clean:
rm patch-hook.o
EOF
# Previous test cases match
$ make
touch patch-hook.o
$ make
make: 'patch-hook.o' is up to date.
$ touch patch-hook.c && make
touch patch-hook.o
$ make
make: 'patch-hook.o' is up to date.
# Fifth build = livepatch-patch-hook.c is updated, but no build!
$ touch livepatch-patch-hook.c && make
make: 'patch-hook.o' is up to date. |
|
This PR has been open for 60 days with no activity and no assignee. It will be closed in 7 days unless a comment is added. |
|
This PR was closed because it was inactive for 7 days after being marked stale. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The build dependencies for patch-hook.o are duplicated. The conditional inclusion of kpatch-patch-hook.c and livepatch-patch-hook.c is already managed in patch-hook.c via #if IS_ENABLED(CONFIG_LIVEPATCH).