Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,15 @@ endif

CFLAGS += -include config.h

# Only include build rules when not running configuration targets
ifeq ($(filter $(check_goal),config defconfig),)
include mk/common.mk
# Only skip build rules when running ONLY config/defconfig (no other targets)
ifneq ($(filter-out config defconfig,$(check_goal)),)
# Has targets other than config/defconfig
include mk/common.mk
else ifeq ($(check_goal),)
# Empty goals means building 'all'
include mk/common.mk
endif
# Otherwise, only config/defconfig targets - skip mk/common.mk

KCONFIGLIB := tools/kconfig/kconfiglib.py
$(KCONFIGLIB):
Expand Down
10 changes: 5 additions & 5 deletions configs/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ config CONFIGURED
bool
default y

# Dependency detection using Kbuild toolchain functions
# Dependency detection using Kconfiglib shell function
config HAVE_SDL2
def_bool $(success,pkg-config --exists sdl2)
def_bool $(shell,pkg-config --exists sdl2 && echo y)
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 16, 2025

Choose a reason for hiding this comment

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

If pkg-config fails, $(shell,…) expands to an empty string, so this def_bool line becomes syntactically invalid and breaks Kconfig parsing. Please emit an explicit "n" on failure to keep the default well-formed.

Prompt for AI agents
Address the following comment on configs/Kconfig at line 9:

<comment>If pkg-config fails, $(shell,…) expands to an empty string, so this def_bool line becomes syntactically invalid and breaks Kconfig parsing. Please emit an explicit &quot;n&quot; on failure to keep the default well-formed.</comment>

<file context>
@@ -4,18 +4,18 @@ config CONFIGURED
+# Dependency detection using Kconfiglib shell function
 config HAVE_SDL2
-    def_bool $(success,pkg-config --exists sdl2)
+    def_bool $(shell,pkg-config --exists sdl2 &amp;&amp; echo y)
 
 config HAVE_PIXMAN
</file context>
Suggested change
def_bool $(shell,pkg-config --exists sdl2 && echo y)
def_bool $(shell,pkg-config --exists sdl2 && echo y || echo n)
Fix with Cubic


config HAVE_PIXMAN
def_bool $(success,pkg-config --exists pixman-1)
def_bool $(shell,pkg-config --exists pixman-1 && echo y)
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 16, 2025

Choose a reason for hiding this comment

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

The $(shell,…) call returns nothing when pixman-1 is unavailable, leaving def_bool without a value and triggering a parse error. Please ensure the command echoes "n" on failure.

Prompt for AI agents
Address the following comment on configs/Kconfig at line 12:

<comment>The $(shell,…) call returns nothing when pixman-1 is unavailable, leaving def_bool without a value and triggering a parse error. Please ensure the command echoes &quot;n&quot; on failure.</comment>

<file context>
@@ -4,18 +4,18 @@ config CONFIGURED
 
 config HAVE_PIXMAN
-    def_bool $(success,pkg-config --exists pixman-1)
+    def_bool $(shell,pkg-config --exists pixman-1 &amp;&amp; echo y)
 
 config HAVE_LIBPNG
</file context>
Suggested change
def_bool $(shell,pkg-config --exists pixman-1 && echo y)
def_bool $(shell,pkg-config --exists pixman-1 && echo y || echo n)
Fix with Cubic


config HAVE_LIBPNG
def_bool $(success,pkg-config --exists libpng)
def_bool $(shell,pkg-config --exists libpng && echo y)
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 16, 2025

Choose a reason for hiding this comment

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

When libpng is not installed, this $(shell,…) expansion is empty, leaving def_bool malformed and causing Kconfig parsing to fail. Echo an explicit "n" on failure to restore correct behaviour.

Prompt for AI agents
Address the following comment on configs/Kconfig at line 15:

<comment>When libpng is not installed, this $(shell,…) expansion is empty, leaving def_bool malformed and causing Kconfig parsing to fail. Echo an explicit &quot;n&quot; on failure to restore correct behaviour.</comment>

<file context>
@@ -4,18 +4,18 @@ config CONFIGURED
 
 config HAVE_LIBPNG
-    def_bool $(success,pkg-config --exists libpng)
+    def_bool $(shell,pkg-config --exists libpng &amp;&amp; echo y)
 
 config HAVE_LIBJPEG
</file context>
Suggested change
def_bool $(shell,pkg-config --exists libpng && echo y)
def_bool $(shell,pkg-config --exists libpng && echo y || echo n)
Fix with Cubic


config HAVE_LIBJPEG
def_bool $(success,pkg-config --exists libjpeg)
def_bool $(shell,pkg-config --exists libjpeg && echo y)
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 16, 2025

Choose a reason for hiding this comment

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

On dependency failure this $(shell,…) call expands to nothing, leaving def_bool without a value and causing a parse error. Echo "n" on failure to keep the default valid.

Prompt for AI agents
Address the following comment on configs/Kconfig at line 18:

<comment>On dependency failure this $(shell,…) call expands to nothing, leaving def_bool without a value and causing a parse error. Echo &quot;n&quot; on failure to keep the default valid.</comment>

<file context>
@@ -4,18 +4,18 @@ config CONFIGURED
 
 config HAVE_LIBJPEG
-    def_bool $(success,pkg-config --exists libjpeg)
+    def_bool $(shell,pkg-config --exists libjpeg &amp;&amp; echo y)
 
 choice
</file context>
Suggested change
def_bool $(shell,pkg-config --exists libjpeg && echo y)
def_bool $(shell,pkg-config --exists libjpeg && echo y || echo n)
Fix with Cubic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above is not necessary since our Kconfiglib ( https://github.com/sysprog21/Kconfiglib ) can deal properly.

Copy link

Choose a reason for hiding this comment

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

Thanks for the feedback! I've saved this as a new learning to improve future reviews.


choice
prompt "Backend Selection"
Expand Down
9 changes: 5 additions & 4 deletions mk/common.mk
Original file line number Diff line number Diff line change
Expand Up @@ -681,10 +681,11 @@ __FORCE:
# Only include dependencies when building known targets
build-goals := all clean $(target-builds)
ifneq ($(MAKECMDGOALS),)
# MAKECMDGOALS is not empty, check if it's a known target
ifneq ($(filter $(MAKECMDGOALS),$(build-goals)),)
# Known target, include dependencies (except for clean)
ifneq "$(MAKECMDGOALS)" "clean"
# MAKECMDGOALS is not empty, check if ALL goals are known
# (i.e., no unknown goals remain after filtering out known ones)
ifeq ($(filter-out $(build-goals),$(MAKECMDGOALS)),)
# All goals are known, include dependencies (except for clean-only builds)
ifeq ($(filter clean,$(MAKECMDGOALS)),)
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 16, 2025

Choose a reason for hiding this comment

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

This condition now skips dependency includes whenever a clean goal is present, so make clean all loses its dependency files. Please only treat clean-only invocations as the skip case.

Prompt for AI agents
Address the following comment on mk/common.mk at line 688:

<comment>This condition now skips dependency includes whenever a clean goal is present, so `make clean all` loses its dependency files. Please only treat clean-only invocations as the skip case.</comment>

<file context>
@@ -681,10 +681,11 @@ __FORCE:
+    # (i.e., no unknown goals remain after filtering out known ones)
+    ifeq ($(filter-out $(build-goals),$(MAKECMDGOALS)),)
+        # All goals are known, include dependencies (except for clean-only builds)
+        ifeq ($(filter clean,$(MAKECMDGOALS)),)
             -include $(target-depends)
         endif
</file context>
Suggested change
ifeq ($(filter clean,$(MAKECMDGOALS)),)
ifneq ($(filter-out clean,$(MAKECMDGOALS)),)
Fix with Cubic

-include $(target-depends)
endif
endif
Expand Down