From e778682731783e69bbdff79d102fb8112d658efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Thu, 5 Sep 2024 10:43:58 +0200 Subject: [PATCH 1/5] Always pass -fdiagnostics-color=always to cc-style compilers to avoid useless recompilations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- src/dune_rules/cxx_flags.ml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dune_rules/cxx_flags.ml b/src/dune_rules/cxx_flags.ml index 22aa0ea1a2a..d4daffd73ea 100644 --- a/src/dune_rules/cxx_flags.ml +++ b/src/dune_rules/cxx_flags.ml @@ -22,8 +22,7 @@ let base_cxx_flags ~for_ cc = ;; let fdiagnostics_color = function - | (Gcc | Clang) when Lazy.force Ansi_color.stderr_supports_color -> - [ "-fdiagnostics-color=always" ] + | Gcc | Clang -> [ "-fdiagnostics-color=always" ] | _ -> [] ;; From ff412966438915a1217075b387028c37991613ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Thu, 5 Sep 2024 10:46:20 +0200 Subject: [PATCH 2/5] Changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- doc/changes/10883.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/changes/10883.md diff --git a/doc/changes/10883.md b/doc/changes/10883.md new file mode 100644 index 00000000000..1a50bfc0044 --- /dev/null +++ b/doc/changes/10883.md @@ -0,0 +1,2 @@ +- Fix an issue where C stubs would be rebuilt whenever the stderr of Dune was + redirected. (#10883, @nojb) From d063f8bb73487edceb92341085997e74d8205313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Fri, 6 Sep 2024 13:47:30 +0200 Subject: [PATCH 3/5] Adapt test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- test/blackbox-tests/test-cases/variables/var-cc.t/run.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/blackbox-tests/test-cases/variables/var-cc.t/run.t b/test/blackbox-tests/test-cases/variables/var-cc.t/run.t index 8c3772b08c9..93542b5110d 100644 --- a/test/blackbox-tests/test-cases/variables/var-cc.t/run.t +++ b/test/blackbox-tests/test-cases/variables/var-cc.t/run.t @@ -51,7 +51,7 @@ No env > (action (echo %{cc}))) > EOF - $ dune build @cc28 | sed "s,${O_CC} ${O_CCF} ${O_CCPPF},OK," + $ dune build @cc28 | sed "s,${O_CC} ${O_CCF} ${O_CCPPF} -fdiagnostics-color=always,OK," OK With added env flags @@ -59,7 +59,7 @@ With added env flags > (env (_ (c_flags :standard -fPIC))) > EOF - $ dune build @cc28 | sed "s,${O_CC} ${O_CCF} ${O_CCPPF} -fPIC,OK," + $ dune build @cc28 | sed "s,${O_CC} ${O_CCF} ${O_CCPPF} -fdiagnostics-color=always -fPIC,OK," OK With redefining env flags From b4253de92601c1b09799908ba7fc0640c92ad2da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Fri, 6 Sep 2024 14:06:40 +0200 Subject: [PATCH 4/5] Adapt test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- .../c-flags-diagnostics-color.t/run.t | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t b/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t index 1489d1f8c21..fe9369db564 100644 --- a/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t +++ b/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t @@ -1,9 +1,6 @@ This test won't work with MSVC as it's using GCC and clang CLI parameters. -We have to force Dune outputting colors, for that we use -`CLICOLOR_FORCE=1`. - The flag should be present in the command, as we want it in the :standard set. @@ -11,8 +8,8 @@ We don't actually test that the compiler outputs colors, just that Dune correctly passes the flag when required, and doesn't when it's not. -test, color enabled, color flag default (enabled) -================================================= +test that we pass the flag +========================== $ cat >dune < (library @@ -20,11 +17,11 @@ test, color enabled, color flag default (enabled) > (foreign_stubs (language c) (names stub))) > EOF - $ CLICOLOR_FORCE=1 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always" + $ dune rules -m stub.o | grep -ce "-fdiagnostics-color=always" 1 -test, color enabled, color flag disabled -======================================== +test color flag disabled +======================== $ cat >dune < (library @@ -34,12 +31,12 @@ test, color enabled, color flag disabled > (language c) (names stub))) > EOF - $ CLICOLOR_FORCE=1 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always" + $ dune rules -m stub.o | grep -ce "-fdiagnostics-color=always" 0 [1] -test, color disabled, color flag default (enabled) -================================================== +test that we correctly filter out the color codes +================================================= $ cat >dune < (library @@ -48,6 +45,11 @@ test, color disabled, color flag default (enabled) > (language c) (names stub))) > EOF - $ CLICOLOR=0 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always" - 0 + $ dune build + File "dune", line 4, characters 22-26: + 4 | (language c) (names stub))) + ^^^^ + stub.c:1:2: error: #error "error message" + 1 | #error "error message" + | ^~~~~ [1] From 897a9d2207a5e37f2a12ae257f6300e9983a8020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Fri, 6 Sep 2024 14:43:27 +0200 Subject: [PATCH 5/5] Fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- .../foreign-stubs/c-flags-diagnostics-color.t/run.t | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t b/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t index fe9369db564..0191ca49fa1 100644 --- a/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t +++ b/test/blackbox-tests/test-cases/foreign-stubs/c-flags-diagnostics-color.t/run.t @@ -45,11 +45,7 @@ test that we correctly filter out the color codes > (language c) (names stub))) > EOF - $ dune build - File "dune", line 4, characters 22-26: - 4 | (language c) (names stub))) - ^^^^ - stub.c:1:2: error: #error "error message" - 1 | #error "error message" - | ^~~~~ +We check there is no ESC [ sequence. + + $ dune build 2>&1 | grep `printf '\033\\['` [1]