Merge pull request #10 from FastLED/copilot/warn-on-non-optimal-g3-setting

Author: zackees

src/fbuild/build/flag_builder.py

@@ -222,22 +222,19 @@ def _add_board_extra_flags(self, flags: Dict[str, List[str]]) -> None:
                 if flag.startswith("-D"):
                     flags["common"].append(flag)
 
-    def _warn_debug_flags_in_quick_profile(self) -> None:
-        """Warn if user has debug flags (-g*) in global build_flags during quick profile.
-
-        In quick profile, fbuild defaults to -g0 (no debug info) for fast builds.
-        If the user puts -g/-g1/-g2/-g3 in build_flags, it overrides -g0 and applies
-        to ALL compilation (sketch, core, and libraries), massively inflating .o files
-        and slowing linking.  The user likely intended debug info for their sketch only.
+    def _warn_debug_build_flags(self) -> None:
+        """Warn if user has debug flags (-g*) in global build_flags.
+
+        Flags like -g, -g1, -g2, -g3 in build_flags apply to ALL compilation
+        (sketch, core, and libraries), not just the user's own code.  Compiling
+        the framework with debug info massively inflates object files and slows
+        linking — it is highly unlikely the user intended to debug the framework
+        itself.  Suggest moving the flag to build_src_flags so it only affects
+        sketch files, or replacing it with -g0 to disable debug info entirely.
         """
         if self._debug_flag_warning_shown:
             return
 
-        from fbuild.build.build_profiles import BuildProfile
-
-        if self.context.profile != BuildProfile.QUICK:
-            return
-
         debug_flags = [f for f in self.user_build_flags if f == "-g" or (f.startswith("-g") and f[2:3] in ("0", "1", "2", "3", "g", "d") and not f.startswith("-gnone") and f != "-g0")]
         if not debug_flags:
             return
@@ -249,8 +246,9 @@ def _warn_debug_flags_in_quick_profile(self) -> None:
         flag_str = " ".join(debug_flags)
         log_warning(
             f"build_flags contains '{flag_str}' which applies to ALL files (sketch, core, libraries).\n"
-            f"  In quick profile this overrides the default -g0, inflating object files ~30x and slowing linking.\n"
-            f"  Recommendation: move '{flag_str}' from build_flags to build_src_flags so it only applies to your sketch code."
+            f"  Compiling the framework with debug info inflates object files ~30x and massively slows linking.\n"
+            f"  Recommendation: move '{flag_str}' from build_flags to build_src_flags so it only applies to your sketch code,\n"
+            f"  or replace it with '-g0' in build_flags to disable debug info for all files."
         )
 
     def _add_user_flags(self, flags: Dict[str, List[str]]) -> None:
@@ -265,7 +263,7 @@ def _add_user_flags(self, flags: Dict[str, List[str]]) -> None:
         Args:
             flags: Flags dictionary to update
         """
-        self._warn_debug_flags_in_quick_profile()
+        self._warn_debug_build_flags()
 
         i = 0
         user_flags = self.user_build_flags

tests/unit/build/test_build_src_flags.py

@@ -5,7 +5,7 @@
 1. Are parsed correctly by ini_parser
 2. Flow through BuildContext
 3. Are applied only to sketch compilation (not core or libraries)
-4. Work with the debug flag warning in quick profile
+4. Work with the debug flag warning (fired for all build profiles)
 """
 
 from pathlib import Path
@@ -491,12 +491,12 @@ def mock_compile_source(source_path: Path, output_path=None, extra_flags=None) -
 
 
 # ===========================================================================
-# 5. Debug flag warning in quick profile
+# 5. Debug flag warning (all build profiles)
 # ===========================================================================
 
 
 class TestDebugFlagWarning:
-    """Test that FlagBuilder warns when -g* is in build_flags during quick profile."""
+    """Test that FlagBuilder warns when -g* is in build_flags (any profile)."""
 
     def test_warns_on_g3_in_quick_profile(self) -> None:
         ctx = _make_mock_context(
@@ -546,14 +546,41 @@ def test_no_warn_on_g0(self) -> None:
             fb.build_flags()
             mock_warn.assert_not_called()
 
-    def test_no_warn_in_release_profile(self) -> None:
-        """-g3 in release profile is fine — no warning."""
+    def test_warns_on_g3_in_release_profile(self) -> None:
+        """-g3 in release profile must warn — framework compiled with debug info is always slow."""
         ctx = _make_mock_context(
             user_build_flags=["-g3"],
             user_build_src_flags=[],
             profile=BuildProfile.RELEASE,
         )
         fb = FlagBuilder(ctx)
+        with patch("fbuild.output.log_warning") as mock_warn:
+            fb.build_flags()
+            mock_warn.assert_called_once()
+            msg = mock_warn.call_args[0][0]
+            assert "-g3" in msg
+            assert "build_src_flags" in msg
+
+    def test_warns_on_g_in_release_profile(self) -> None:
+        """-g in release profile must also warn."""
+        ctx = _make_mock_context(
+            user_build_flags=["-g"],
+            user_build_src_flags=[],
+            profile=BuildProfile.RELEASE,
+        )
+        fb = FlagBuilder(ctx)
+        with patch("fbuild.output.log_warning") as mock_warn:
+            fb.build_flags()
+            mock_warn.assert_called_once()
+
+    def test_no_warn_on_g0_in_release_profile(self) -> None:
+        """-g0 in release profile should not warn."""
+        ctx = _make_mock_context(
+            user_build_flags=["-g0"],
+            user_build_src_flags=[],
+            profile=BuildProfile.RELEASE,
+        )
+        fb = FlagBuilder(ctx)
         with patch("fbuild.output.log_warning") as mock_warn:
             fb.build_flags()
             mock_warn.assert_not_called()