ginkgo-project / ginkgo

Numerical linear algebra software package
https://ginkgo-project.github.io/
BSD 3-Clause "New" or "Revised" License
401 stars 88 forks source link

Rename test binaries that collide with C++ standard library header names #1531

Closed upsj closed 8 months ago

upsj commented 8 months ago

This gets rid of all the CPATH issues we've been having from time to time by renaming all test binaries that match a C++ standard library header name.

Fixes #1530

pratikvn commented 8 months ago

I think it might be a bit annoying to maintain these lists ? Maybe we can prefix all test executables with a unique prefix/suffix like gkotest/gko ? So the executables would be named of the form gkotest_array/gko_array etc ?

upsj commented 8 months ago

I don't think it's that much effort, the list of C++ standard library headers is pretty short and doesn't change a lot, and we don't have many tests that overlap the list. That's something we can do during code review.

upsj commented 8 months ago

To go a bit more into detail why I dislike adding a prefix/suffix to all tests: I like being able to just build a single test binary via ninja rcm_gpu, the prefix/suffix makes that more annoying. The cause of this PR is a rather obscure issue that rarely impacts us, so I would like to make the workaround as unintrusive as possible.

pratikvn commented 8 months ago

I see. That makes sense. Usually in my case, it is multiple targets that are relevant, so I end up doing a regex match with

ninja -t targets | grep ^foo | sed 's/:.*//' > my_targets

so a suffix/prefix does not affect this.

and then you can just run ninja $(cat my_targets) whenever necessary. This could also be something we could add to our CMake build step accepting a string from the user, but maybe that is over-engineering this.

upsj commented 8 months ago

What we could do there (and what I did before) is add meta-targets that compile certain parts of the library, e.g. a benchmarks target that compiles all benchmark binaries, a tests target that compiles all tests or stuff like that.