PR#15: Unbundle stb_image - rpms/CuraEngine

rpms / CuraEngine

#15 Unbundle stb_image

Merged 2 years ago by churchyard. Opened 2 years ago by music.

rpms/ music/CuraEngine stb into rawhide

Unbundle stb_image

Benjamin A. Beasley • 2 years ago

c2aa95c

CuraEngine.spec

file modified

+9 -10

		`@@ -1,18 +1,12 @@`
		`Name: CuraEngine`
		`Epoch: 1`
		`Version: 4.10.0`
		`- Release: 1%{?dist}`
		`+ Release: 2%{?dist}`
		`Summary: Engine for processing 3D models into G-code instructions for 3D printers`
		`License: AGPLv3+`
		`URL: https://github.com/Ultimaker/%{name}`
		`Source0: %{url}/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz`

		`- # The cmake stuff would attempt to git clone this:`
		`- # TODO package on it's own`
		`- %global stb_commit e6afb9cbae4064da8c3e69af3ff5c4629579c1d2`
		`- Source1: https://github.com/nothings/stb/archive/%{stb_commit}.tar.gz`
		`- Provides: bundled(stb) = %stb_commit`
		`-`
		`BuildRequires: gcc`
		`BuildRequires: gcc-c++`
		`BuildRequires: libarcus-devel == %{version}`
		`@@ -22,6 +16,10 @@`
		`BuildRequires: cmake`
		`BuildRequires: git-core`

		`+ # Header-only package; -static version is for tracking per guidelines`
		`+ BuildRequires: stb_image-devel`
		`+ BuildRequires: stb_image-static`
		`+`
		`Patch0: %{name}-rpath.patch`
		`Patch1: %{name}-static-libstdcpp.patch`

		`@@ -37,8 +35,6 @@`

		`%prep`
		`%autosetup -p1 -S git`
		`- tar -xf %{SOURCE1}`
		`- mv stb-%{stb_commit} stb`

		`# bundled libraries`
		`rm -rf libs`
		`@@ -47,7 +43,7 @@`
		`sed -i 's/"DEV"/"%{version}"/' src/settings/Settings.h`

		`%build`
		`- %cmake -DBUILD_SHARED_LIBS:BOOL=OFF -DCURA_ENGINE_VERSION:STRING=%{version} -DUSE_SYSTEM_LIBS:BOOL=ON -DCMAKE_CXX_FLAGS_RELEASE_INIT:STRING="%{optflags} -fPIC" -DStb_INCLUDE_DIRS:PATH=${PWD}`
		`+ %cmake -DBUILD_SHARED_LIBS:BOOL=OFF -DCURA_ENGINE_VERSION:STRING=%{version} -DUSE_SYSTEM_LIBS:BOOL=ON -DCMAKE_CXX_FLAGS_RELEASE_INIT:STRING="%{optflags} -fPIC" -DStb_INCLUDE_DIRS:PATH=%{_includedir}`
		`%cmake_build`


		`@@ -64,6 +60,9 @@`
		`%{_bindir}/%{name}`

		`%changelog`
		`+ * Mon Aug 23 2021 Benjamin A. Beasley <code@musicinmybrain.net> - 1:4.10.0-2`
		`+ - Unbundle stb_image`
		`+`
		`* Mon Aug 16 2021 Gabriel Féron <feron.gabriel@gmail.com> - 4.10.0-1`
		`- Update to 4.10.0`

sources

file modified

-1

		`@@ -1,2 +1,1 @@`
		`SHA512 (CuraEngine-4.10.0.tar.gz) = 45d32be165da98c945c71b2f803a68e06b60eeacf9ad7fcbdde2659cb05455c8f45d69939662de086348129b0d3a40c484819915aa0c6953937a48d53848b652`
		`- SHA512 (e6afb9cbae4064da8c3e69af3ff5c4629579c1d2.tar.gz) = 232ef301d4d6c82c7c5f0e4234b9160cc815f3b6bcc35d341cdf8738646f2f0887ee9838680699f4c9f4274b1390036b2c4fb3ebc2d663af8ff888114dc9f04b`

music commented 2 years ago

The new stb package with stb_image-devel subpackage is available in all releases, via a newpackage update plus a buildroot override that expires in a week, so feel free to backport this change to other branches.

churchyard commented on line 38 of CuraEngine.spec 2 years ago

Can we pass this in via some CMake variable instead?

churchyard commented 2 years ago

Awesome, thanks! I've asked one question inline.

music commented 2 years ago

Well… it’s awkward. The new stb/stb_image-devel package installs the header directly in /usr/include. This made sense to me, as each stb library is a totally independent single-file header-only library, and each has a “namespacing” stb_ prefix to avoid conflicts. But the CuraEngine package (like some but not all packages that use stb libraries) has #include <stb/stb_image.h>. It’s not possible to make that work by messing with include paths alone, because there’s no /usr/include/stb/.

So one of the following is required:

Projects with includes that start with stb/, like this one, can keep an stb directory in their source tree where the bundled headers were, with headers symlinked to those in /usr/include. Alternatively, they can replace the stb directory with a symlink to /usr/include.
Projects with includes that start with stb/ could patch them.
I could change the stb-image-devel package to install each header both as /usr/include/stb_image.h and as /usr/include/stb/stb_image.h, with one a symlink to the other, and so on for all the other stb subpackages. This is both practical, and ugly in its own way.

With a time machine, or a breaking change, I could have chosen to install all stb headers under /usr/include/stb. In that case, this project would work out of the box, but projects that use #include <stb_foo.h> would need -I%{_includedir}/stb added to their build flags.

All of this is of course a consequence of the stb libraries being intended as copylibs. As far as upstream is concerned, there is no standard installation path. You copy what you need into your source tree and move on.

Given all of that, suggestions remain welcome.

music commented 2 years ago

Two packages that did not use #include <stb/…>, and needed no special accommodations beyond removing the bundled headers:

One other that did, like CuraEngine:

https://src.fedoraproject.org/rpms/mlpack/pull-request/2#request_diff

There’s another I have no PR for yet, retroarch. It’s a mess of #include <stb/stb_vorbis.h>, #include "../../deps/stb/stb_truetype.h", #include <stb_image.h>, and so on, so symlinking or patching will be the only plausible approaches there.

churchyard commented 2 years ago

I could change the stb-image-devel package to install each header both as /usr/include/stb_image.h and as /usr/include/stb/stb_image.h, with one a symlink to the other, and so on for all the other stb subpackages. This is both practical, and ugly in its own way.

I like that approach.

music commented 2 years ago

I like that approach.

Well, it’s a reasonable request. No need for me to search my feelings too deeply for an aesthetic judgement on it. Stand by for an update, then.

churchyard commented 2 years ago

Consider this: You are making 1 package unaesthetic so other N packages don't need to :)

rebased onto c2aa95c

2 years ago

music commented 2 years ago

I’ve updated the stb package and the PR. It will be a little while before all the Bodhi updates and buildroot overrides for stable releases and EPEL’s are caught up.