| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: headerscheck ccache support |
| Date: | 2025-11-28 11:38:59 |
| Message-ID: | cce91d86-bf43-46a1-9429-113fe748f70e@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 21.11.25 13:14, Álvaro Herrera wrote:
>> Now ccache works.
>
> Sounds reasonable. I notice that you're cleaning this file in a `rm`
> line in the loop,
>
>> @@ -253,10 +249,11 @@ do
>> if ! $COMPILER $COMPILER_FLAGS -I $builddir -I $srcdir \
>> -I $builddir/src/include -I $srcdir/src/include \
>> -I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
>> - $EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.$ext -o $tmp/test.o
>> + $EXTRAINCLUDES $EXTRAFLAGS -c $test_file_name.$ext -o $test_file_name.o
>> then
>> exit_status=1
>> fi
>> + rm -f "$test_file_name.$ext" "$test_file_name.o"
>> done
>
> but this means that if the script is interrupted halfway through, one
> file or two files might remain in place. Would it be possible to have
> the current file name in a variable, so that the `trap` line can delete
> them?
Here is another patch set. I have made some tweaks to address the issue
you raise, and I took some code and inspiration from Thomas Munro's
patch. The solution I chose is to create a temporary subdirectory in
the build directory, and create the test files in there. That way the
trap can just blow away the directory, as before.
> I've been also wondering about testing whether `parallel` is installed,
> and use that if so.
Another approach I had in mind for some time is to just write out a
makefile with the test compile commands, and run that with make -j.
Demo patch attached. (I'm not seriously proposing this. For one thing,
we probably wouldn't want to introduce a dependency on make. But you
could probably write an equivalent ninja.build file.)
But this doesn't seem to buy very much. The overhead of the shell
script to write out the test files appears to become significant
compared the the actual compile commands.
Another simple idea is to run headerscheck and cpluspluscheck in
parallel. You can already do that manually, and we could do that on CI
to save about 50% wall-clock time. Patch attached.
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-headerscheck-ccache-support.patch | text/plain | 3.7 KB |
| v2-0002-ci-Run-headerscheck-and-cplusplucheck-in-parallel.patch | text/plain | 917 bytes |
| v2-0003-headerscheck-Parallelize-by-writing-out-a-makefil.patch | text/plain | 2.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ajit Awekar | 2025-11-28 11:39:04 | Periodic authorization expiration checks using GoAway message |
| Previous Message | Nazir Bilal Yavuz | 2025-11-28 11:16:45 | Re: Proposal: adding --enable-shadows-warning |