Stop reinventing the wheel. about rawspeed HOT 17 CLOSED

Hey there! Just dropping in from the Abseil team. Just some thoughts I had reading your post --

Abseil has everything specified in your table except for Endian support:

• Note that the interface of absl::Span is more like that of std::string_view than anything in the std::span proposal

** before C++17, absl::optional is our own implementation. In C++17 and beyond, it becomes a simple typedef for std::optional. The same goes for absl::string_view, absl::any, etc

*** We also have FixedArray, a runtime-sized small-object-optimized array with fixed size

Currently our CMake support is a bit wonky, but I'm currently in the process of improving it as a high-priority task -- we should have better support for using us via ExternalProject, for example.

In terms of external dependencies, we do currently rely on cctz for Time support, but that is also being worked on as a high priority task.

My question for you, then, as we work to turn Abseil into a more mature open source project, is what are the pain points currently which would keep you from depending on Abseil for your low level abstractions?

from rawspeed.

As a user of rawspeed I'd like to point out that I wouldn't be happy to see the build system download stuff from the Internet. At all. For google test that's a non-issue as users don't have to build the tests. But a library that is a hard dependency has to be either shipped by the OS or bundled in the source tree.

Apart from that I am happy with anything that isn't Boost.

from rawspeed.

I have this figured out with GoogleTest.cmakee + GoogleTest.cmake.in

The directions at https://github.com/google/googletest/tree/master/googletest#incorporating-into-an-existing-cmake-project should work just fine for Abseil already. If you decide to go this route and have issues please let me know by filing a bug against abseil.

But let's take the step back, shall we. I did not pick abseil. I did not not pick abseil.
It's still an open question as of right now.

Oh of course. But picking up this issue seemed like a great way for us to reach out and get more feelers about how different people are feeling about Abseil :) I hope this didn't feel like pressuring you to use us, obviously you should do whatever works best for you.

from rawspeed.

Thanks for dropping by!

Abseil has everything specified in your table

Nice :)

except for Endian support:

That is mostly non-problem, that one is trivial.

ArrayRef | absl::Span | absl/types/span.h

Oh, nice, i did not thought of that!

Currently our CMake support is a bit wonky, but I'm currently in the process of improving it as a high-priority task -- we should have better support for using us via ExternalProject, for example.

Ideally i'd expect it to be similar to that of googletest, with the same flexible set of variants.

what are the pain points currently which would keep you from depending on Abseil for your low level abstractions?

That's The question here :)

The versioning scheme (live at head / copylib / just use the trunk) is likely the biggest sorrow point.
Don't get me wrong, i totally get the reasoning, after all, the current RawSpeed is using that very same model... It kinda works so far (except with Fedora, see e.g. [1], google fedora + darktable + rawspeed can show you more gore), because there are rather good reasons to do it this way.

But if a copylib starts using a copylib, i'm not so sure how things will work out...
If abseil is to have a proper 'release' tags (like it already promises to, see),
then there would be some room to breathe.

At minimum, one could simply manually track said 'release tag' via submodule.
At maximum, distros could package new abseil release tarball (source tarball, like debian does for googletest) every half a year.
Though i doubt they will like it. But i'm also not sure they will like all those bundled copies :)

TLDR: can't check the water temperature without somehow measuring the water temperature.

Did that answer the question, or was the question different?

from rawspeed.

I'm not sure I understand why building Abseil from trunk impedes your own ability to build from trunk unless we accidentally break you -- I assume that is the issue? Anyways, as was pointed out in our repo, we are very soon having a serious discussion about stable release branches / tags. So stay tuned there. This also makes me start to think about some way to specify "try to build / test with trunk abseil. If it passes, replace my copy of abseil with HEAD, else keep my current version"

As for CMake, it seems like the only real issue there is that googletest has example code for you to download and build them at configure time, and then asks you to add_subdirectory just as we currently do. We can definitely look into supporting that -- if you are willing to use bleeding-edge cmake 3.11 then you may want to look into FetchContent(), which does the get-my-dependency-at-configure time for you but allows you to then build and link it at the same time as the rest of your build.

from rawspeed.

I'm not sure I understand why building Abseil from trunk impedes your own ability to build from trunk unless we accidentally break you -- I assume that is the issue?

Not really. I'm not sure how to explain it to the people who don't already understand it, sorry :/
Not even looking at the distro side of things, this 'just use current git master' is a serious change from the usual approach. Plus, i don't actually know the opinion of the actual users of this library on that matter.
(CC @hanatos / @houz, @LibRaw, @aferrero2707, ..., possibly @Beep6581 in the future, ....; all the interested parties are likely watching the repo anyway)

Anyways, as was pointed out in our repo, we are very soon having a serious discussion about stable release branches / tags. So stay tuned there.

Good. Let's see what happens there :)

FetchContent(), which does the get-my-dependency-at-configure time for you but allows you to then build and link it at the same time as the rest of your build.

I have this figured out with GoogleTest.cmakee + GoogleTest.cmake.in
If abseil supports that approach (if it does not, then this is all moot point and i definitively can't pick abseil...), it will just work with s/GoogleTest/abseil/i so to speak.

But let's take the step back, shall we. I did not pick abseil. I did not not pick abseil.
It's still an open question as of right now.
(Though, yeah, unless something rather drastic happens, i guess abseil will be the answer here..)

from rawspeed.

my 2 cents ... as a packager.

No releases just sucks. For many distros it means we will have at least 2 copies of abseil on the distro (rawtherapee and darktable). And I really don't get what would be so hard with tagging releases and allow proper packaging that way. If you expect your users to be able to use HEAD all the time then all your QA should be done already and it is all about making a tag.

In the long term I hope we can also get rawspeed releases at some point to avoid the duplication between darktable and rawtherapee. But that is a discussion for another day.

from rawspeed.

@houz The current abseil CMake directions suggest using a local fork and add_subdirectory() already. A way to do this while staying current to Abseil HEAD is to set up a local mirror of abseil which is pulled from during the build process, for example.

@darix the 5 second version is that Abseil is so low-level that if we have tagged releases, we are almost guaranteeing ODR violations -- suppose your project uses Abseil v1, but you also use protobuf which uses Abseil v2 and gRPC which uses Abseil v3. But we are likely to have something like a 6 month stable release timeline which is a simple tag on our Github repo. We also want to avoid binary distribution for the same reason. See our TL Titus Winters' cppCon keynote for more detail

from rawspeed.

using a local fork and add_subdirectory() already. A way to do this while staying current to Abseil HEAD is to set up a local mirror of abseil which is pulled from during the build process, for example.

The 'how' part is quite clear to us. We won't invent a new wheel there (LOL), but just add an 'abseil' submodule into https://github.com/darktable-org/rawspeed/tree/develop/src/external
Since right now darktable is using rawspeed as a submodule already, this won't add any new problems. It's a charted territory.

I do understand the gravity of the ODR issue, and the reluctance to do releases.
I absolutely do NOT want binary releases. Only a tag/source releases.

I'm hazy on the part about abseil being used by the library, and then by the software using that library.
I do understand that it will cause problems. Right now i'm not aware of any RS users already [[in]directly] using abseil, so i'd prefer to leave this out until that actually happens.
But when it does happen, there are some easy solutions i see:

1. An cmake switch telling rawspeed not to build it's own version of abseil, but just use the one provided (via cmake target) by the parent build system.
2. An option to specify the path to the abseil sources to be used
3. ???

More than one option can exist at the same time. The second option will be available anyway from the start, i think.

staying current to Abseil HEAD

Let me be blunt: i don't see this happening.
Some specific commit/tag will be tracked, and updated manually. Ideally, that 6-month tag.
BUT the packagers will be free to replace the copy with some other version to address the ODR concerns/etc. That's the Key.

from rawspeed.

@LebedevRI @darix This is a great conversation from the perspective of packagers -- would you all mind moving this over to the abseil github so the rest of the team can be in on the discussion?

from rawspeed.

Absolutely do feel free to @LebedevRI highlight me in the relevant discussions! (packaging, and the cmake stuff to)

from rawspeed.

@JonathanDCohen where do you want me do output all these thoughts?
(i guess, it would be best to do that before abseil/abseil-cpp#87 (comment) is done..)

from rawspeed.

By the way, Abseil team released an LTS branch: https://abseil.io/about/releases

from rawspeed.

Still no release though https://github.com/abseil/abseil-cpp/releases

from rawspeed.

https://github.com/abseil/abseil-cpp/releases/tag/20180600

from rawspeed.

abseil/abseil-cpp#155

from rawspeed.

A lot of water has flown since.
The world isn't what it used to me.
I think this isn't going to be happening.

from rawspeed.