Check GNUMAKEFLAGS before passing implicit make args

[cottsay]

GNU make also reads flags passed via the GNUMAKEFLAGS environment variable. When we check MAKEFLAGS for flags, we should check GNUMAKEFLAGS as well.

This change is related to #145, but is only the bug fix portion of it.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.80%. Comparing base (ce2b56c) to head (dfdde2b).

Files with missing lines	Patch %	Lines
colcon_cmake/task/cmake/build.py	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #158      +/-   ##
==========================================
+ Coverage   54.74%   54.80%   +0.05%     
==========================================
  Files          10       10              
  Lines         780      781       +1     
  Branches      163      164       +1     
==========================================
+ Hits          427      428       +1     
  Misses        307      307              
  Partials       46       46              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[knmcguire]

Interestingly, this fails for me on windows but succeeds on ubuntu.

I have here a zip folder with some pytests, pixi.toml file that pulls this branch for colcon-cmake, and an example cmake project (claude generated). You can put this in any folder and just do pixi install and then:

pixi run test-all

on windows 11

> pixi run test-all
✨ Pixi task (test-all): pytest test_gnumakeflags.py -v -s                                                                                   
=========================================================== test session starts ============================================================ 
platform win32 -- Python 3.14.3, pytest-9.0.2, pluggy-1.6.0 -- ...\.pixi\envs\default\python.exe         
cachedir: .pytest_cache                                                                                                                      
rootdir: ...                                                                                            
plugins: colcon-core-0.20.1, cov-7.1.0, repeat-0.9.4, rerunfailures-16.1
collected 3 items                                                                                                                           

test_gnumakeflags.py::test_baseline_without_env_vars
=== Test 1: Building WITHOUT GNUMAKEFLAGS ===
Expected: ninja with -j and -l flags

Starting >>> test_gnumakeflags
-- Configuring done (0.2s)
-- Generating done (0.1s)
-- Build files have been written to: C:/Users/kimbe/Development/infra/colcon/build/test_gnumakeflags
Change Dir: 'C:/Users/kimbe/Development/infra/colcon/build/test_gnumakeflags'

Run Build Command(s): C:/Users/kimbe/AppData/Local/Microsoft/WinGet/Packages/Ninja-build.Ninja_Microsoft.Winget.Source_8wekyb3d8bbwe/ninja.exe -v -j16 -l16
[1/1] C:\WINDOWS\system32\cmd.exe /C "cd /D  ...\build\test_gnumakeflags && "C:\Program Files\CMake\bin\cmake.exe" -E echo "Build completed successfully""
Build completed successfully

-- Install configuration: ""
-- Installing: ...n/install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [1.27s]

Summary: 1 package finished [1.78s]
WNDPROC return value cannot be converted to LRESULT
TypeError: WPARAM is simple, so must be an int object (got NoneType)

PASSED
test_gnumakeflags.py::test_with_gnumakeflags
=== Test 2: Building WITH GNUMAKEFLAGS set ===
Expected: ninja WITHOUT extra -j and -l flags (GNUMAKEFLAGS should prevent colcon from adding them)

Starting >>> test_gnumakeflags
-- Configuring done (0.2s)
-- Generating done (0.0s)
-- Build files have been written to: .../build/test_gnumakeflags
Change Dir: .../build/test_gnumakeflags'

Run Build Command(s): C:/Users/kimbe/AppData/Local/Microsoft/WinGet/Packages/Ninja-build.Ninja_Microsoft.Winget.Source_8wekyb3d8bbwe/ninja.exe -v -j16 -l16
[1/1] C:\WINDOWS\system32\cmd.exe /C "cd /D ...build\test_gnumakeflags && "C:\Program Files\CMake\bin\cmake.exe" -E echo "Build completed successfully""
Build completed successfully

-- Install configuration: ""
-- Installing: .../install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [1.10s]

Summary: 1 package finished [1.47s]
WNDPROC return value cannot be converted to LRESULT
TypeError: WPARAM is simple, so must be an int object (got NoneType)

FAILED
test_gnumakeflags.py::test_with_makeflags
=== Test 3: Building WITH MAKEFLAGS set ===
Expected: ninja WITHOUT extra -j and -l flags (MAKEFLAGS should prevent colcon from adding them)

Starting >>> test_gnumakeflags
-- Configuring done (0.2s)
-- Generating done (0.0s)
-- Build files have been written to: .../build/test_gnumakeflags
Change Dir: '.../build/test_gnumakeflags'

Run Build Command(s): C:/Users/kimbe/AppData/Local/Microsoft/WinGet/Packages/Ninja-build.Ninja_Microsoft.Winget.Source_8wekyb3d8bbwe/ninja.exe -v
[1/1] C:\WINDOWS\system32\cmd.exe /C "cd /D ...\build\test_gnumakeflags && "C:\Program Files\CMake\bin\cmake.exe" -E echo "Build completed successfully""
Build completed successfully

-- Install configuration: ""
-- Installing: .../install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [1.09s]

Summary: 1 package finished [1.45s]
WNDPROC return value cannot be converted to LRESULT
TypeError: WPARAM is simple, so must be an int object (got NoneType)

PASSED

================================================================= FAILURES ================================================================= 
__________________________________________________________ test_with_gnumakeflags __________________________________________________________

    def test_with_gnumakeflags():
        """Test 2: With GNUMAKEFLAGS - colcon should NOT add -j and -l flags."""
        print("\n=== Test 2: Building WITH GNUMAKEFLAGS set ===")
        print("Expected: ninja WITHOUT extra -j and -l flags (GNUMAKEFLAGS should prevent colcon from adding them)\n")

        output, returncode = run_colcon_build({"GNUMAKEFLAGS": "-j2 -l2"})
        print(output)

        # Check that ninja is invoked with -v but WITHOUT -j and -l flags
        assert re.search(r"ninja.*-v", output), \
            "Expected ninja to be invoked with -v flag"

>       assert not re.search(r"ninja.*-j\d+.*-l\d+", output), \
            "Expected ninja WITHOUT -j and -l flags when GNUMAKEFLAGS is set (BUG: this currently fails)"
E       AssertionError: Expected ninja WITHOUT -j and -l flags when GNUMAKEFLAGS is set (BUG: this currently fails)
E       assert not <re.Match object; span=(397, 419), match='ninja.exe -v -j16 -l16'>
E        +  where <re.Match object; span=(397, 419), match='ninja.exe -v -j16 -l16'> = <function search at 0x000001CB37C77060>('ninja.*-j\\d+.*-l\\d+', 'Starting >>> test_gnumakeflags\n-- Configuring done (0.2s)\n-- Generating done (0.0s)\n-- Build files have been written to: .../build/test_gnumakeflags\nChange Dir: .../build/test_gnumakeflags\'\n\nRun Build Command(s): C:/Users/kimbe/AppData/Local/Microsoft/WinGet/Packages/Ninja-build.Ninja_Microsoft.Winget.Source_8wekyb3d8bbwe/ninja.exe -v -j16 -l16\n[1/1] C:\\WINDOWS\\system32\\cmd.exe /C "cd /D C:\\Users\\kimbe\\Development\\infra\\colcon\\build\\test_gnumakeflags && "C:\\Program Files\\CMake\\bin\\cmake.exe" -E echo "Build completed successfully""\nBuild completed successfully\n\n-- Install configuration: ""\n-- Installing: .../install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt\nFinished <<< test_gnumakeflags [1.10s]\n\nSummary: 1 package finished [1.47s]\nWNDPROC return value cannot be converted to LRESULT\nTypeError: WPARAM is simple, so must be an int object (got NoneType)\n')  
E        +    where <function search at 0x000001CB37C77060> = re.search

test_gnumakeflags.py:74: AssertionError
========================================================= short test summary info ==========================================================
FAILED test_gnumakeflags.py::test_with_gnumakeflags - AssertionError: Expected ninja WITHOUT -j and -l flags when GNUMAKEFLAGS is set (BUG: this currently fails)
======================================================= 1 failed, 2 passed in 23.20s =======================================================

This is on ubuntu 24.04:

$ pixi run test-all
 
✨ Pixi task (test-all): pytest test_gnumakeflags.py -v -s
================================================================= test session starts =================================================================
platform linux -- Python 3.14.3, pytest-9.0.2, pluggy-1.6.0 -- .../.pixi/envs/default/bin/python3.14
cachedir: .pytest_cache
rootdir: ...
plugins: rerunfailures-16.1, colcon-core-0.20.1, cov-7.1.0, repeat-0.9.4
collected 3 items                                                                                                                                     

test_gnumakeflags.py::test_baseline_without_env_vars 
=== Test 1: Building WITHOUT GNUMAKEFLAGS ===
Expected: ninja with -j and -l flags

Starting >>> test_gnumakeflags
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: .../build/test_gnumakeflags
Change Dir: '.../build/test_gnumakeflags'

Run Build Command(s): /usr/bin/ninja -v -j16 -l16
[1/1] cd .../build/test_gnumakeflags && /usr/bin/cmake -E echo Build\ completed\ successfully
Build completed successfully

-- Install configuration: ""
-- Installing: .../install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [0.37s]

Summary: 1 package finished [1.18s]

PASSED
test_gnumakeflags.py::test_with_gnumakeflags 
=== Test 2: Building WITH GNUMAKEFLAGS set ===
Expected: ninja WITHOUT extra -j and -l flags (GNUMAKEFLAGS should prevent colcon from adding them)

Starting >>> test_gnumakeflags
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: .../build/test_gnumakeflags
Change Dir: '.../build/test_gnumakeflags'

Run Build Command(s): /usr/bin/ninja -v
[1/1] cd .../build/test_gnumakeflags && /usr/bin/cmake -E echo Build\ completed\ successfully
Build completed successfully

-- Install configuration: ""
-- Installing: .../install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [0.18s]

Summary: 1 package finished [0.38s]

PASSED
test_gnumakeflags.py::test_with_makeflags 
=== Test 3: Building WITH MAKEFLAGS set ===
Expected: ninja WITHOUT extra -j and -l flags (MAKEFLAGS should prevent colcon from adding them)

Starting >>> test_gnumakeflags
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: /home/kimbe/dev/infra/colcon/build/test_gnumakeflags
Change Dir: '/home/kimbe/dev/infra/colcon/build/test_gnumakeflags'

Run Build Command(s): /usr/bin/ninja -v
[1/1] cd /home/kimbe/dev/infra/colcon/build/test_gnumakeflags && /usr/bin/cmake -E echo Build\ completed\ successfully
Build completed successfully

-- Install configuration: ""
-- Installing: /home/kimbe/dev/infra/colcon/install/test_gnumakeflags/share/test_gnumakeflags/CMakeLists.txt
Finished <<< test_gnumakeflags [0.17s]

Summary: 1 package finished [0.35s]

PASSED

================================================================== 3 passed in 4.59s =================================================================

so this PR is for ninja which we don't use on windows (but we might?) so it is perhaps a good question to ask if this is an acceptable failure

test_flag_pr.zip

[knmcguire]

So if I understand correctly, the environment make variables take precedent always? I guess this was already behavior that existed, but perhaps maybe not what I might expect. On the other hand, it should perhaps be the responsible thing of the user to have a clean dev environment for this.

Anyway, just a design question mostly!

[cottsay]

Sort of. We want to change the "default" behavior of colcon's invocations of make. Passing command line arguments to make will take precedence over MAKEFLAGS env var, but make will implicitly consume MAKEFLAGS by default.

We're really just using this information to decide whether or not to pass those command line arguments.