fix: Add useful debug message when files are missing from config#15499
Conversation
| # Define newline for use in error messages and output formatting | ||
| define newline | ||
|
|
||
|
|
There was a problem hiding this comment.
does this intentionally include 2 newlines?
There was a problem hiding this comment.
Yes, make trims the last one off, so to get one newline, you need two.
There was a problem hiding this comment.
Pull request overview
This PR improves error handling when image configuration files reference missing dependencies. Previously, Make would fail with a confusing "no rule to make target" error. Now, users get a clear, actionable error message listing all missing files.
Changes:
- Add a newline helper in
utils.mkfor formatting multi-line error messages - Implement early validation in
imggen.mkto detect missing config dependencies before Make processes them - Modify
get_config_deps.shto userealpath -mso it can handle paths in non-existent directories
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| toolkit/scripts/utils.mk | Adds newline variable definition for formatting error messages |
| toolkit/scripts/imggen.mk | Adds validation logic to check for missing config files and display helpful error messages |
| toolkit/scripts/get_config_deps.sh | Uses realpath -m flag to canonicalize paths even when parent directories don't exist |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Validate that all config dependencies exist before Make tries to process them as prerequisites | ||
| # If we don't do this, Make will error out with a less-than-helpful message about having no rule to make | ||
| # the validation flag (since its a pattern match and if a dependency is missing, it can't match the pattern) |
There was a problem hiding this comment.
Grammar error: 'its' should be "it's" (it is). The contraction is needed here since you're saying "it is a pattern match".
| # the validation flag (since its a pattern match and if a dependency is missing, it can't match the pattern) | |
| # the validation flag (since it's a pattern match and if a dependency is missing, it can't match the pattern) |
Merge Checklist
All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)
*-staticsubpackages, etc.) have had theirReleasetag incremented../cgmanifest.json,./toolkit/scripts/toolchain/cgmanifest.json,.github/workflows/cgmanifest.json)./LICENSES-AND-NOTICES/SPECS/data/licenses.json,./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md,./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)*.signatures.jsonfilessudo make go-tidy-allandsudo make go-test-coveragepassSummary
Improve error messages when image config files reference missing files (PackageLists, scripts, AdditionalFiles, etc.).
Previously, if a referenced file was missing, Make would fail with a confusing message about having no rule to build the validation flag. If the missing file was in a non-existent directory,
realpathwould also print unhelpful errors before the actual failure.This was caused by Make's pattern rule matching behavior. The validation rules is
$(STATUS_FLAGS_DIR)/validate-image-config%.flag:, which matches the variablevalidate-config = $(STATUS_FLAGS_DIR)/validate-image-config-$(config_name).flagThe rule had
config_other_files = $(if $(CONFIG_FILE),$(call shell_real_build_only, $(SCRIPTS_DIR)/get_config_deps.sh $(CONFIG_FILE)),)as a dependency. If one of those calculated files was missing, then make would consider the pattern matched rule un-usable, and would try to find an alternate rule that did have all its dependencies. Unfortunately, we only have one pattern match rule, so we must ensure that all the dependencies we list are either real files, or valid make targets, so make won't skip it.Change Log
imggen.mkto detect missing config dependencies and display a clear error message listing all missing filesrealpath -minget_config_deps.shto handle paths where parent directories don't existprintvar-*targets so users can still debugDoes this affect the toolchain?
NO
Test Methodology
https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1025897&view=results