Fix : gh #256 : Use -O0 not -o0 to set optimisation level for TARGET=linux - #258
Merged
Merged
Conversation
-o0 was replaced with -O0 to correctly set optimisation level. This issue only happens with build target LINUX Fixes #256
|
All contributors have signed the CLA ✍️ ✅ |
Ulrond
added
bug
There was a problem hiding this comment.
Pull request overview
Fixes a build-flag typo in the TARGET=linux Makefile branch where -o0 (output file) was incorrectly used instead of -O0 (optimization level 0), which could cause downstream builds (notably the ut-control sub-make invocation) to emit an unintended output file named 0.
Changes:
- Replace
-o0with-O0in the LinuxCOMPILERdefinition. - Replace
-o0with-O0in the LinuxUT_CONTROL_COMPILERdefinition (used when invoking the ut-control sub-make).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jul 27, 2026
Fix : gh #130 : Use -O0 not -o0 to set optimisation level for TARGET=linux
rdkcentral/ut-control#131
Open
Adds check_makefile_flags to the tests suite, alongside the existing
check_macros static gate.
Two checks:
scan walks the build files for a lowercase -o glued to an
optimisation level (-o0, -os, -og). gcc accepts these
silently and a later -o $@ on the same command line wins,
so nothing ever fails - which is why #256 went unnoticed.
expect asks make printenv what COMPILER and UT_CONTROL_COMPILER
actually expand to and asserts -ggdb -O0 -Wall are present,
catching the flags being dropped as well as mistyped.
Runs before the framework build so a flag typo is reported in
seconds rather than after a full compile. Expectations apply to
TARGET=linux only; arm takes its compiler from the SDK.
Verified by reintroducing -o0: both checks fail, exit 1.
Refs #256
Ulrond
force-pushed
the
feature/gh256-fix-lowercase-o-optimisation-flag
branch
from
July 27, 2026 20:54
23e5268 to
8646879
Compare
Contributor
|
Release scripts are all ok: |
Contributor
|
Also tests look fine: |
Contributor
@looopTools , please follow this, for checks to pass. |
Contributor
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
|
@kanjoe24 sorry I had not noticed the missing CLA signing. I have done so now. |
…lowercase-o-optimisation-flag
The .PHONY declaration added alongside the compiler-flag check duplicated the one already present earlier in the file, and neither listed every recipe-only target. With 'framework', 'run', 'rebuild' and 'cleanall' missing, a stray file or directory of the same name made GNU make treat the target as up to date and skip the recipe entirely. Collapse both declarations into one covering all ten targets.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/check_makefile_flags.py:185
- If the provided
rootpath does not exist or is not a directory,os.walk()will scan 0 files and the check will exit successfully. This can mask misconfiguration when the script is invoked from CI or from a different working directory.
root = os.path.abspath(args.root)
scanned, findings = scan_tree(root)
tests/check_makefile_flags.py:79
scan_file()returnsNoneon read errors, andscan_tree()just skips that file. That means the guard can incorrectly pass if a build file becomes unreadable (e.g., permissions or transient FS issues). For a regression guard, it should fail closed on read errors.
This issue also appears on line 183 of the same file.
except OSError as error:
print(f"{RED}Error: cannot read '{path}': {error}{RESET}")
return None
Contributor
Author
|
@kanjoe24 sorry for the churn — re-review needed. Your approval was auto-dismissed by the ruleset ( Since your approval there are two changes:
The |
github-project-automation
Bot
moved this from Todo
to Approved For Merge
in UT-Core Roadmap
github-project-automation
Bot
moved this from Approved For Merge
to Resolved
in UT-Core Roadmap
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #256
Applies the patch supplied by @looopTools in the issue, with authorship preserved via
git am.-o0is not an optimisation flag — lowercase-ois "write output to this file", so-o0asks for a file named0. TheTARGET=linuxbranch of the Makefile used it in two places; both are now-O0.TARGET=armis untouched — it takes$(CC)/$(CXX)from the SDK and never carried the flag.Note for reviewers: where the stray
0file actually comes fromEvery compile/link rule in ut-core appends its own
-o $@after$(COMPILER)(Makefile lines 152, 167, 172, 191, 198), and GCC lets the last-owin —gcc -ggdb -o0 -Wall -c t.c -o t.oexits 0 and producest.o, no file named0. So ut-core's own objects were named correctly; what was silently lost is the intended-O0(benign in practice, as-O0is gcc's default).The object-naming symptom in the issue shows up downstream:
UT_CONTROL_COMPILERis handed to the ut-control sub-make asCC="gcc -ggdb -o0 -Wall", and any command there that supplies no-oof its own writes a file called0. The sibling typo in ut-control itself is rdkcentral/ut-control#130.Verification
make printenv TARGET=linuxnow reportsCOMPILER [ gcc -ggdb -O0 -Wall].grep -rn '\-o0' --include=Makefile --include=*.mk --include=*.sh .returns no hits.make TARGET=linux VARIANT=CPPundertests/: full success,ut-testlinks.make TARGET=linuxundertests/: all objects compile andut-testlinks. The run then stops on the pre-existingcheck_macrosgate (30/33 macros documented inut_cunit.h), which is source-content based and unaffected by compiler flags.0anywhere in the tree after either clean build.Second commit: a regression guard in the tests suite
The typo itself is two characters, but it survived because no functional test could ever have caught it — gcc accepted
-o0, the later-o $@won, and-O0is gcc's default, so every build stayed green and every binary stayed correct. Only a static check on the build files can see it.tests/check_makefile_flags.pyis wired into the suite ascheck_makefile_flags, alongside the existingcheck_macrosgate, and runs before the framework build so a flag typo costs seconds rather than a full compile.Two checks:
scanMakefile/*.mk/*.shfor a lowercase-oglued to an optimisation level —-o0,-os,-og,-ofast. Comments are stripped before matching, so prose describing the typo does not trip it. Third-party trees (framework/,build/,cpp_libs/) are skipped.expectmake printenvwhatCOMPILERandUT_CONTROL_COMPILERactually expand to and asserts-ggdb -O0 -Wallare present. This catches the flags being dropped or overridden, not just mistyped.Expectations apply to
TARGET=linuxonly —armtakes its compiler from the SDK.Verified by reintroducing
-o0on top of the fix:Both checks fire,
makeexits 1. On the fixed tree: 25 build files scanned, 0 findings, 2/2 expectations met — confirmed forTARGET=linux,VARIANT=CPP, andTARGET=arm(scan only).