Add S2_USE_SYSTEM_INCLUDES cmake options to avoid s2 compile time warnings…

[MBkkt]

… from s2 headers

[jmr]

What warnings are you seeing? Maybe we can just fix them.

[MBkkt]

@jmr

What warnings are you seeing? Maybe we can just fix them.

I think you're fixing part of them in update from Google PR.

But in general I don't think it's convenient for us as users to create PR for every warning (especially for msvc, array[0], undef, etc):

Please check commit

https://github.com/arangodb/arangodb/pull/17893/commits/47b0f232b5843ee8e1eb58d861288f8a2d5e15d0

[jmr]

Please check commit https://github.com/arangodb/arangodb/commit/47b0f232b5843ee8e1eb58d861288f8a2d5e15d0

How do I see the warnings?

Can you paste the output of what you see without SYSTEM, or something similar?

[MBkkt]

@jmr

You can see what it cause:

Zero sized array https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-levels-2-and-4-c4200?view=msvc-170

define windows related macros before undef they

not cast to void* in memset trivial struct

A lot of int/size_t

Can you paste the output of what you see without SYSTEM, or something similar?

I don't have windows local, so I need run it in our CI.

And we don't compile s2 with warnings, so issue with werror fails because of s2, commonly depends on which headers I include. I don't include all, so if we fix some warnings now, it's not guarantee that in future I didn't face with same issue but in other header.

[MBkkt]

In general It's not really difficult to me have such patch local.

I just thought it can be useful for other users.

If you think it's really unnecessary (why?) I can close it.

If you want fix warnings, I think better to add it to S2 tests and CI

[jmr]

abseil-cpp does it, so that's good enough for me.

Can you make it an option?

https://github.com/abseil/abseil-cpp/blob/2ed6963f2b67a68d94303cafb571d3d039a49f9a/CMakeLists.txt#L75

[MBkkt]

Ok!

[MBkkt]

@jmr Done