AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.05k stars 347 forks source link

Remove remaining traces of boost #1808

Closed fpsunflower closed 2 months ago

fpsunflower commented 2 months ago

Description

Replace the last few uses of boost with robin-map (which is already a dependency for OIIO, although it is new for OSL itself) and functions in OIIO.

Tests

Testsuite passes on my mac laptop. Will keep an eye on how the CI steps go.

Checklist:

lgritz commented 2 months ago

I would recommend copying OIIO's FindRobinmap.cmake and the parts of externalpackages.cmake that deal with robinmap, just exactly copy all of that because it works.

fpsunflower commented 2 months ago

I would recommend copying OIIO's FindRobinmap.cmake and the parts of externalpackages.cmake that deal with robinmap, just exactly copy all of that because it works.

I gave a quick try to FetchContent since I was reading about it earlier and this is a particularly "easy" case with a header only dependency that has a modern cmake build system with exported targets. It seems to work as advertised, and will automatically do a find_package first to check if you already have the library on your system before executing FetchContent.

It doesn't seem to work properly on the CI machines though -- and the error message is not helpful at all. Is there a trick to accessing the internet from the CI machines perhaps?

Of course I'm happy to clone the existing setup from OIIO if this fails, but the amount of boilerplate is definitely reduced with this approach.

lgritz commented 2 months ago

There is no trick, the CI runners have direct internet access.

lgritz commented 2 months ago

I would recommend just copying the old way OIIO did it, it has worked perfectly for a long time.

When I finish perfecting the new OIIO build system, we'll port it over here, too.

At that point, it will be easy to experiment with FetchContent if we want, and if that works any better or simpler instead of my build_dependency_with_cmake approach, I'm happy to switch.

fpsunflower commented 2 months ago

Will do.

fpsunflower commented 2 months ago

I believe this is working now. There are still some failures on OSX but they appear to be unrelated.

fpsunflower commented 2 months ago

I think partio was broken because it was previously pulling in zlib implictly via boost. It seems a bit weird that the cmake setup for partio doesn't handle this for us, but perhaps it isn't fully correct on their end?

Adding zlib to the target_link_libraries fixes it locally for me (was building without partio previously).

lgritz commented 2 months ago

I believe this is working now. There are still some failures on OSX but they appear to be unrelated.

That's unrelated. I expected it to be fixed after the OIIO release a couple days ago. I'll need to look into it separately, but it's definitely not at all related to your PR.