Repository navigation
duckdb: fix unittest progress output exceeding Hydra log limit - #579992
jonasgloning wants to merge 2 commits into
Conversation
041298c to
d88d380
Compare
Review dismissed automatically
| # Fix uninitialized terminal width in the unittest progress reporter. | ||
| # It padded every progress line to a garbage width when stdin is not a TTY and | ||
| # blew up the build log past Hydra's limit. | ||
| # Patch is upstream for DuckDB >2.0 |
There was a problem hiding this comment.
@jonasgloning , you note this is already resolved upstream at >=2.0. If that's the case would you defer this PR to
or not?
There was a problem hiding this comment.
The patch is on main and on track to land in the bug fix release after 2.0, so 2.0.1 mid-November. So the patch is still a prerequisite for 2.0.0 afaik.
That was intended as a strictly larger sign; I should have worded this more clearly!
| ] | ||
| ++ lib.optionals stdenv.cc.isGNU [ | ||
| # causes massive amounts of logs, exceeding hydra's limits on some platforms | ||
| (lib.cmakeFeature "CMAKE_CXX_FLAGS" "-Wno-sfinae-incomplete") | ||
| ]; |
There was a problem hiding this comment.
| ]; |
feel free to drop this, then; i added this in an attempt to fix this issue, but this seems to have been the wrong solution. this pr seems like a much more correct solution (though i have no opinion on upgrade vs patch).
edit: i mean the entire isGNU + CMAKE_CXX_FLAGS block, github is rendering this wrong.
Things done
Resolves #570642
The unittest progress reporter reads the terminal width from an uninitialised struct. So every progress line can be padded with tens of thousands of spaces exceeding Hydra's log limit.
This PR backports the upstream fix and drops the x86_64 workaround.
./result/bin/.nixpkgs-reviewon this PR. See nixpkgs-review usage.passthru.tests.