add bazel support

[jmr]

We will need to do something about what are currently external dependencies: openssl, ~~glog, gflags~~.

This might be useful: https://github.com/openssl/openssl/issues/3840

[prem-nuro]

Maybe a dumb question, but could we just exclusively use boringssl with Bazel? https://github.com/google/boringssl/tree/master-with-bazel

Then all deps already have Bazel support.

[wyfSunflower]

1. @ericveach Could you please give me a solid reason why did S2 not use regular hexahedron projection, but use cube?

2. What's the distortion ratio and calculation time of regular hexahedron for convert cell ids to points or vice versa, compare to cube?

[chanced]

It seems openssl can be built with bazel using rule_foreign_cc https://github.com/bazelbuild/rules_foreign_cc/issues/337

I'm not familiar with bazel or c++ but I cruised down that rabbit hole.

[chanced]

@wyfSunflower You can't use hexagons like that. You end up needing pentagons. If that's what you are after, go with h3.

[chrismgrayftsinc]

For what it's worth, it appears that boringssl is supported already. See https://github.com/google/s2geometry/blob/5b5eccd54a08ae03b4467e79ffbb076d0b5f221e/src/s2/util/math/exactfloat/exactfloat.cc#L67 which appears to be the only place that OpenSSL / boringssl is used.

[chrismgrayftsinc]

I created https://github.com/chrismgrayftsinc/s2geometry/tree/bazel as a proof of concept. It compiles, but I have no idea if it works. If a maintainer is interested in a PR, let me know and I can clean it up.

[jmr]

@chrismgrayftsinc Your example doesn't show how to use openssl. We already have a bazel/blaze BUILD file we could release, which would save a lot of work. I'm just not sure how openssl needs to be handled.

[chrismgrayftsinc]

@jmr I use boringssl here: https://github.com/chrismgrayftsinc/s2geometry/commit/df42fefaf6aafe2b66ee072f7529a11cd7ec0bb7#diff-75c3ab7c9e0e33383d592e445ca6474cf69fc07110a420f0069c7e2e2798691dR14

In my reading, that's the only place where OpenSSL is used, but I could be wrong.

[chrismgrayftsinc]

Ping

[jmr]

I use boringssl here: https://github.com/chrismgrayftsinc/s2geometry/commit/df42fefaf6aafe2b66ee072f7529a11cd7ec0bb7#diff-75c3ab7c9e0e33383d592e445ca6474cf69fc07110a420f0069c7e2e2798691dR14

Ok. I missed that.

It compiles, but I have no idea if it works.

If you can add the tests, and they pass, I'll merge it.

I can also get the Google-internal BUILD files working externally. It's lower on my list than doing a new release. You could just want for that if cleaning everything up is too much work.

[chrismgrayftsinc]

I have added the tests with this commit: https://github.com/chrismgrayftsinc/s2geometry/commit/a64ac1e1b076c35a386d7b7016c4dd4c99ca93bc

I'd probably be happier with the Google-internal BUILD files because they're probably structured better.

[spendres]

Thanks for doing this @chrismgrayftsinc and @jmr.

running:

bazel test //:s2loop_measures_test

fails due to error accumulation in ss_loop.cc. See attached log file. s2loop_measures_test_log.txt

Here is the complete set of tests that I ran: test.sh.txt

I believe that this issue is already open as a compatibility issue with the latest version of abseil.

[spendres]

@chrismgrayftsinc and @jmr...

Before I was able to use this bazel build, I had to remove an older absl and s2 install by cmake in /usr/local/lib and /usr/local/include. Do you have a bazel way to restrict the build to use the google toolchain?

[spendres]

@chrismgrayftsinc and @jmr

The error is coming from the s2geometry/src/src/util/math/exactfloat.cc module. This is where the library tests for floating point overflow and underflow for various versions of boringssl. exactfloat.cc may need revision to support the most recent version of boringssl.

I ran the s2loop_test with several versions of boringssl on google and clang toolchains. Same failure and same amount of error accumulation on all versions. The error accumulation is roughly 6% over the threshold.

I am building on MacOSX 14.4.1 on an M1 chip, so no matter what architecture I build for, my system will still rely on darwin.sandbox. It would be good to try on an Intel or AMD machine to see if this error is isolated to floating point on Apple silicon/darwin.sandbox.

Let me know if you are using x86 hardware and if you see the same failure. In the meantime, I'll look through the code and try to find other hardware.

[chrismgrayftsinc]

I am on x86 and all the tests succeed for me.

[spendres]

Great. The only other suggestion would be to update abseil-cpp version to 20240116.2 in your MODULE.bazel file.

@jmr: Is there anything else you need to complete the PR?

[jmr]

Which PR? I don't see a bazel one.

[chrismgrayftsinc]

Yeah I intentionally haven't made one. I think the google BUILD structure is likely a lot better and @jmr is welcome to take inspiration from my branch for any bazel-specific stuff.

[spendres]

I understand your thoughts, @chrismgrayftsinc, and greatly appreciate your work on this as it is very timely for my project.

I will work with @jmr to merge your bazel build into what the bazel.central.repository team needs for version 11. They have no maintainer specified at https://registry.bazel.build/modules/s2geometry, but that module was first published 29 days ago for version 0.10.0, so someone is working it. Their current bazel (build https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/s2geometry/0.10.0/patches/add_build_file.patch) is not as complete as yours, so we will "use yours for inspiration". You've done a lot of heavy lifting here, so you should receive the credit.

Changes planned in order of importance:

1. require bazel version 7.0.2 and bzlmod through .bazelrc, remove WORKSPACE file
2. specify googletoolchain
3. support bazel test //:*
4. build from HEAD
5. create a "hello-world from s2" example that imports the library
6. build with docker container using bazel build image

@jmr: I will be tackling this merge over the next few days from a freshly forked s2geometry repo. I'll request a PR once the structure of the build is complete.

Thanks again, @chrismgrayftsinc!

[spendres]

@jmr: I created a PR with a good candidate for a bazel build for v0.11.1.

I located the root for the bazel files under the s2 directory as the bazel central repository seems to favor – separate from cmake.

@chrismgrayftsinc: let me know if you can git clone https://github.com/spendres/s2geometry and run the tests.

[spendres]

@jmr and @chrismgrayftsinc:

1. I just updated https://github.com/spendres/s2geometry to use s2 instead of s2lib. Bazel outputs libs2lib otherwise, and now creates libs2.

2. I invited both of you to collaborate on http://github.com/spendres/s2-user – which provides a simple example of importing the s2lib into a bazel c++ project.

[spendres]

@jmr and @chrismgrayftsinc:

@jmr asked about the strategy for the layout of the bazel build in https://github.com/spendres/s2geometry

The idea was to merge the first attempt in the bazel central repository with that made by @chrismgrayftsinc. In the end ,the bazel central repository version duplicated the packages and subpackages created by @chrismgrayftsinc in subdirectories under s2. So the :base library used in the bcr, was displaced in favor of s2/base:base, :mathutil by s2/util/mathutil:mathutil etc... each with it's own BUILD. This more closely follows bazel guidelines. The version in the bcr was not very far along.

Both initial designs lacked the BUILD file in the s2 directory. Bazel complained about not exporting the files, so I created that package and then build/test caching started to work. I arranged the s2 library into hierarchical libraries to speed up development builds. With the BUILD file in s2, I was able to remove the "s2-includes" global.

None of my attempts have found a way to exit on bazel version below X. If either of you know how to do this, let me know.

This design uses bzlmod so I left out the WORKSPACE file from @chrismgrayftsinc's repo.

[spendres]

Never mind on the version below X, I found the setting. I added – to root MODULE.bazel – the line specifying the minimum bazel version to the module definition and tested it with a currently non-existent version of 7.1.2 to make sure bazel errors out if the condition is not met.