-
Notifications
You must be signed in to change notification settings - Fork 23
Fix build system issues #128
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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) | ||||||
|
|
||||||
| config HAVE_PIXMAN | ||||||
| def_bool $(success,pkg-config --exists pixman-1) | ||||||
| def_bool $(shell,pkg-config --exists pixman-1 && echo y) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||
|
|
||||||
| config HAVE_LIBPNG | ||||||
| def_bool $(success,pkg-config --exists libpng) | ||||||
| def_bool $(shell,pkg-config --exists libpng && echo y) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||
|
|
||||||
| config HAVE_LIBJPEG | ||||||
| def_bool $(success,pkg-config --exists libjpeg) | ||||||
| def_bool $(shell,pkg-config --exists libjpeg && echo y) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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)),) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Prompt for AI agents
Suggested change
|
||||||
| -include $(target-depends) | ||||||
| endif | ||||||
| endif | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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