Re: archivers/snappy: unbreak on sparc64
On Tue, Oct 17, 2023 at 07:07:12PM +0100, Stuart Henderson wrote:
> I think this is a better fix (checked there's no change to the produced
> package so I didn't bump).
I concur, thanks.
OK kn
>
> Index: Makefile
> ===
> RCS file: /cvs/ports/archivers/snappy/Makefile,v
> retrieving revision 1.20
> diff -u -p -r1.20 Makefile
> --- Makefile 16 Oct 2023 21:17:52 - 1.20
> +++ Makefile 17 Oct 2023 17:56:42 -
> @@ -25,8 +25,12 @@ CONFIGURE_ARGS += -DBUILD_SHARED_LIBS=ON
> -DINSTALL_GTEST=OFF \
> -DSNAPPY_BUILD_BENCHMARKS=OFF
>
> -EXTRA_ports-gcc += -I${LOCALBASE}/include
> -CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
> +# used in tests
> +BUILD_DEPENDS = archivers/lz4 \
> + archivers/lzo2
> +
> +CXXFLAGS += -I${LOCALBASE}/include
> +MODCMAKE_LDFLAGS = -L${LOCALBASE}/lib
>
> # expects submodule/cannot use system library
> DIST_TUPLE = github google googletest
> e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
>
Re: archivers/snappy: unbreak on sparc64
On Tue, Oct 17, 2023 at 07:07:12PM +0100, Stuart Henderson wrote:
> On 2023/10/17 17:36, Klemens Nanni wrote:
> > On Tue, Oct 17, 2023 at 04:32:19PM +0200, Theo Buehler wrote:
> > > On Tue, Oct 17, 2023 at 03:27:26PM +0100, Stuart Henderson wrote:
> > > > On 2023/10/16 21:07, Klemens Nanni wrote:
> > > > > OK kn when that works on sparc64 for you and got wrapped in a
> > > > > CHOSEN_COMPILER or MACHINE_ARCH check.
> > > >
> > > > CHOSEN_COMPILER is messy because it has to go after the final
> > > > .include
> > > >
> > > > Using MACHINE_ARCH and enumerating the base-gcc archs is a
> > > > terrible idea for this.
> > > >
> > > > Could do !${PROPERTIES:Mclang} though my strong preference would
> > > > be to avoid difference between flags on the different archs and
> > > > always add to CXXFLAGS..
> > >
> > > I would also have preferred that since I don't really see the reason
> > > for treating gcc and clang differently, just because clang happens to
> > > work for a reason that I did not investigate.
> > >
> > > I used this (since I just used something similar for shared-mime-info)
> > > and it does the expected:
> > >
> > > EXTRA_ports-gcc +=-I${LOCALBASE}/include
> > > CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
> >
> > No objection from me doing this unconditionally if this is too messy.
> >
> > It just made it easier to reason about and, imho, also reads clearer than
> > an unconditional include that implies configure/build script bugs when there
> > are none (at least for clang archs).
> >
>
> I don't see a configure script bug.
>
> lz4 and lzo2 are picked up on ports-gcc archs at configure time.
> From the failed build log:
>
> -- Looking for lzo1x_1_15_compress in lzo2
> -- Looking for lzo1x_1_15_compress in lzo2 - not found
> -- Looking for LZ4_compress_default in lz4
> -- Looking for LZ4_compress_default in lz4 - found
>
> IIRC ports-gcc's library search path includes /usr/local/lib which would
> explain this. If that's not it, there's probably something deep in
> /usr/local/share/cmake responsbile, but I no longer have the correct
> glasses from the Infocom HHGTTG packaging to spend much time in
> there.
>
> These libraries aren't linked to the main snappy library but are used
> in tests.
>
> Adding the -I without doing anything else means that if lz4 or lzo2 are
> present during configure and then junked, build will fail. (Probably
> better if things like this are seen on base-clang archs too, rather
> than failing very occasionally on a less common arch).
The problem I was addressing is that lz4 will always make sanppy fail on
sparc64 because it's always there:
cmake -> libarchive -> lz4.
In particular, it won't be junked. ports-gcc then stumbles over the weird
#include "lz4.h"
in the test (which should be fixed upstream by using I suppose).
For some reason clang is happy with "" while ports-gcc isn't, so I made
sure gcc can find it.
I did not look for further problems, but I do see the problem with lzo2.
> I think this is a better fix (checked there's no change to the produced
> package so I didn't bump).
ok tb
> Index: Makefile
> ===
> RCS file: /cvs/ports/archivers/snappy/Makefile,v
> retrieving revision 1.20
> diff -u -p -r1.20 Makefile
> --- Makefile 16 Oct 2023 21:17:52 - 1.20
> +++ Makefile 17 Oct 2023 17:56:42 -
> @@ -25,8 +25,12 @@ CONFIGURE_ARGS += -DBUILD_SHARED_LIBS=ON
> -DINSTALL_GTEST=OFF \
> -DSNAPPY_BUILD_BENCHMARKS=OFF
>
> -EXTRA_ports-gcc += -I${LOCALBASE}/include
> -CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
> +# used in tests
> +BUILD_DEPENDS = archivers/lz4 \
> + archivers/lzo2
> +
> +CXXFLAGS += -I${LOCALBASE}/include
> +MODCMAKE_LDFLAGS = -L${LOCALBASE}/lib
>
> # expects submodule/cannot use system library
> DIST_TUPLE = github google googletest
> e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
>
Re: archivers/snappy: unbreak on sparc64
On 2023/10/17 17:36, Klemens Nanni wrote:
> On Tue, Oct 17, 2023 at 04:32:19PM +0200, Theo Buehler wrote:
> > On Tue, Oct 17, 2023 at 03:27:26PM +0100, Stuart Henderson wrote:
> > > On 2023/10/16 21:07, Klemens Nanni wrote:
> > > > OK kn when that works on sparc64 for you and got wrapped in a
> > > > CHOSEN_COMPILER or MACHINE_ARCH check.
> > >
> > > CHOSEN_COMPILER is messy because it has to go after the final
> > > .include
> > >
> > > Using MACHINE_ARCH and enumerating the base-gcc archs is a
> > > terrible idea for this.
> > >
> > > Could do !${PROPERTIES:Mclang} though my strong preference would
> > > be to avoid difference between flags on the different archs and
> > > always add to CXXFLAGS..
> >
> > I would also have preferred that since I don't really see the reason
> > for treating gcc and clang differently, just because clang happens to
> > work for a reason that I did not investigate.
> >
> > I used this (since I just used something similar for shared-mime-info)
> > and it does the expected:
> >
> > EXTRA_ports-gcc += -I${LOCALBASE}/include
> > CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
>
> No objection from me doing this unconditionally if this is too messy.
>
> It just made it easier to reason about and, imho, also reads clearer than
> an unconditional include that implies configure/build script bugs when there
> are none (at least for clang archs).
>
I don't see a configure script bug.
lz4 and lzo2 are picked up on ports-gcc archs at configure time.
>From the failed build log:
-- Looking for lzo1x_1_15_compress in lzo2
-- Looking for lzo1x_1_15_compress in lzo2 - not found
-- Looking for LZ4_compress_default in lz4
-- Looking for LZ4_compress_default in lz4 - found
IIRC ports-gcc's library search path includes /usr/local/lib which would
explain this. If that's not it, there's probably something deep in
/usr/local/share/cmake responsbile, but I no longer have the correct
glasses from the Infocom HHGTTG packaging to spend much time in
there.
These libraries aren't linked to the main snappy library but are used
in tests.
Adding the -I without doing anything else means that if lz4 or lzo2 are
present during configure and then junked, build will fail. (Probably
better if things like this are seen on base-clang archs too, rather
than failing very occasionally on a less common arch).
I think this is a better fix (checked there's no change to the produced
package so I didn't bump).
Index: Makefile
===
RCS file: /cvs/ports/archivers/snappy/Makefile,v
retrieving revision 1.20
diff -u -p -r1.20 Makefile
--- Makefile16 Oct 2023 21:17:52 - 1.20
+++ Makefile17 Oct 2023 17:56:42 -
@@ -25,8 +25,12 @@ CONFIGURE_ARGS +=-DBUILD_SHARED_LIBS=ON
-DINSTALL_GTEST=OFF \
-DSNAPPY_BUILD_BENCHMARKS=OFF
-EXTRA_ports-gcc += -I${LOCALBASE}/include
-CXXFLAGS +=${EXTRA_${CHOSEN_COMPILER}}
+# used in tests
+BUILD_DEPENDS =archivers/lz4 \
+ archivers/lzo2
+
+CXXFLAGS +=-I${LOCALBASE}/include
+MODCMAKE_LDFLAGS = -L${LOCALBASE}/lib
# expects submodule/cannot use system library
DIST_TUPLE = github google googletest
e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
Re: archivers/snappy: unbreak on sparc64
On Tue, Oct 17, 2023 at 04:32:19PM +0200, Theo Buehler wrote:
> On Tue, Oct 17, 2023 at 03:27:26PM +0100, Stuart Henderson wrote:
> > On 2023/10/16 21:07, Klemens Nanni wrote:
> > > OK kn when that works on sparc64 for you and got wrapped in a
> > > CHOSEN_COMPILER or MACHINE_ARCH check.
> >
> > CHOSEN_COMPILER is messy because it has to go after the final
> > .include
> >
> > Using MACHINE_ARCH and enumerating the base-gcc archs is a
> > terrible idea for this.
> >
> > Could do !${PROPERTIES:Mclang} though my strong preference would
> > be to avoid difference between flags on the different archs and
> > always add to CXXFLAGS..
>
> I would also have preferred that since I don't really see the reason
> for treating gcc and clang differently, just because clang happens to
> work for a reason that I did not investigate.
>
> I used this (since I just used something similar for shared-mime-info)
> and it does the expected:
>
> EXTRA_ports-gcc +=-I${LOCALBASE}/include
> CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
No objection from me doing this unconditionally if this is too messy.
It just made it easier to reason about and, imho, also reads clearer than
an unconditional include that implies configure/build script bugs when there
are none (at least for clang archs).
Re: archivers/snappy: unbreak on sparc64
On Tue, Oct 17, 2023 at 03:27:26PM +0100, Stuart Henderson wrote:
> On 2023/10/16 21:07, Klemens Nanni wrote:
> > OK kn when that works on sparc64 for you and got wrapped in a
> > CHOSEN_COMPILER or MACHINE_ARCH check.
>
> CHOSEN_COMPILER is messy because it has to go after the final
> .include
>
> Using MACHINE_ARCH and enumerating the base-gcc archs is a
> terrible idea for this.
>
> Could do !${PROPERTIES:Mclang} though my strong preference would
> be to avoid difference between flags on the different archs and
> always add to CXXFLAGS..
I would also have preferred that since I don't really see the reason
for treating gcc and clang differently, just because clang happens to
work for a reason that I did not investigate.
I used this (since I just used something similar for shared-mime-info)
and it does the expected:
EXTRA_ports-gcc += -I${LOCALBASE}/include
CXXFLAGS += ${EXTRA_${CHOSEN_COMPILER}}
Re: archivers/snappy: unbreak on sparc64
On 2023/10/16 21:07, Klemens Nanni wrote:
> OK kn when that works on sparc64 for you and got wrapped in a
> CHOSEN_COMPILER or MACHINE_ARCH check.
CHOSEN_COMPILER is messy because it has to go after the final
.include
Using MACHINE_ARCH and enumerating the base-gcc archs is a
terrible idea for this.
Could do !${PROPERTIES:Mclang} though my strong preference would
be to avoid difference between flags on the different archs and
always add to CXXFLAGS..
Re: archivers/snappy: unbreak on sparc64
On Mon, Oct 16, 2023 at 10:51:37PM +0200, Theo Buehler wrote:
> sorry for missing that.
>
> we should keep the existing CXXFLAGS, especially -O2 -pipe. This diff
> no dynamic export changes and doesn't change sizees:
>
> 38075 164840 39763 9b53../build-amd64/libsnappy.so
> 38075 164840 39763 9b53../build-amd64/libsnappy.so.3.0
Oh I see, you forced CXXFLAGS instead of amending them.
Still, I'd be happier do to this only where needed such that potential
future bugs won't get fixed silently by that.
> Index: Makefile
> ===
> RCS file: /cvs/ports/archivers/snappy/Makefile,v
> retrieving revision 1.19
> diff -u -p -r1.19 Makefile
> --- Makefile 16 Oct 2023 20:31:13 - 1.19
> +++ Makefile 16 Oct 2023 20:49:09 -
> @@ -3,7 +3,7 @@ COMMENT = fast compression/decompression
> GH_TAGNAME = 1.1.10
> GH_PROJECT = snappy
> GH_ACCOUNT = google
> -REVISION = 0
> +REVISION = 1
>
> SHARED_LIBS =snappy 3.0
>
> @@ -24,6 +24,8 @@ MODULES = devel/cmake
> CONFIGURE_ARGS +=-DBUILD_SHARED_LIBS=ON \
> -DINSTALL_GTEST=OFF \
> -DSNAPPY_BUILD_BENCHMARKS=OFF
> +
> +CONFIGURE_ENV += CXXFLAGS="${CXXFLAGS} -I${LOCALBASE}/include"
CXXFLAGS += -I/usr/local/include
This is enough and gets picked up by cmake.
OK kn when that works on sparc64 for you and got wrapped in a
CHOSEN_COMPILER or MACHINE_ARCH check.
>
> # expects submodule/cannot use system library
> DIST_TUPLE = github google googletest
> e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
> Index: patches/patch-third_party_googletest_googletest_src_gtest_cc
> ===
> RCS file: patches/patch-third_party_googletest_googletest_src_gtest_cc
> diff -N patches/patch-third_party_googletest_googletest_src_gtest_cc
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-third_party_googletest_googletest_src_gtest_cc 16 Oct
> 2023 20:49:09 -
> @@ -0,0 +1,17 @@
> +Fix build on sparc64:
> +
> +gtest.cc:5342:7:
> + error: 'raise' was not declared in this scope
> + raise(SIGTRAP);
> +
> +Index: third_party/googletest/googletest/src/gtest.cc
> +--- third_party/googletest/googletest/src/gtest.cc.orig
> third_party/googletest/googletest/src/gtest.cc
> +@@ -43,6 +43,7 @@
> + #include
> + #include // NOLINT
> + #include
> ++#include
> + #include
> + #include
> + #include
>
Re: archivers/snappy: unbreak on sparc64
On Mon, Oct 16, 2023 at 08:36:07PM +, Klemens Nanni wrote:
> On Mon, Oct 16, 2023 at 09:47:42PM +0200, Theo Buehler wrote:
> > For some reason ports-gcc can't find "lz4.h", so pass -I in CXXFLAGS.
> > Then there's the usual missing include thing in gtest. I should see if I
> > can upstream that...
>
> You could contain it with to avoid subtle change on clang archs, no?
>
> .if ${CHOSEN_COMPILER} == "ports-gcc"
> CXXFLAGS += -I/usr/local/include
> .endif
>
> >
> > I don't think this changes the package where it already builds, but I
> > can bump REVISION if that's preferred.
>
> Generally, I'd say bump if in doubt.
>
> On amd64, this diff tripples the *built* libsnappy.so.* in size.
> I've built twice without and twice with the diff:
>
> $ size lib*
> textdatabss dec hex
> 38075 164840 39763 9b53libsnappy.so.3.0.cur
> 38075 164840 39763 9b53libsnappy.so.3.0.cur2
> 109844 520832 115084 1c18c libsnappy.so.3.0.new
> 109844 520832 115084 1c18c libsnappy.so.3.0.new2
sorry for missing that.
we should keep the existing CXXFLAGS, especially -O2 -pipe. This diff
no dynamic export changes and doesn't change sizees:
38075 164840 39763 9b53../build-amd64/libsnappy.so
38075 164840 39763 9b53../build-amd64/libsnappy.so.3.0
Index: Makefile
===
RCS file: /cvs/ports/archivers/snappy/Makefile,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile
--- Makefile16 Oct 2023 20:31:13 - 1.19
+++ Makefile16 Oct 2023 20:49:09 -
@@ -3,7 +3,7 @@ COMMENT = fast compression/decompression
GH_TAGNAME = 1.1.10
GH_PROJECT = snappy
GH_ACCOUNT = google
-REVISION = 0
+REVISION = 1
SHARED_LIBS = snappy 3.0
@@ -24,6 +24,8 @@ MODULES = devel/cmake
CONFIGURE_ARGS += -DBUILD_SHARED_LIBS=ON \
-DINSTALL_GTEST=OFF \
-DSNAPPY_BUILD_BENCHMARKS=OFF
+
+CONFIGURE_ENV += CXXFLAGS="${CXXFLAGS} -I${LOCALBASE}/include"
# expects submodule/cannot use system library
DIST_TUPLE = github google googletest
e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
Index: patches/patch-third_party_googletest_googletest_src_gtest_cc
===
RCS file: patches/patch-third_party_googletest_googletest_src_gtest_cc
diff -N patches/patch-third_party_googletest_googletest_src_gtest_cc
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-third_party_googletest_googletest_src_gtest_cc16 Oct
2023 20:49:09 -
@@ -0,0 +1,17 @@
+Fix build on sparc64:
+
+gtest.cc:5342:7:
+ error: 'raise' was not declared in this scope
+ raise(SIGTRAP);
+
+Index: third_party/googletest/googletest/src/gtest.cc
+--- third_party/googletest/googletest/src/gtest.cc.orig
third_party/googletest/googletest/src/gtest.cc
+@@ -43,6 +43,7 @@
+ #include
+ #include // NOLINT
+ #include
++#include
+ #include
+ #include
+ #include
Re: archivers/snappy: unbreak on sparc64
On Mon, Oct 16, 2023 at 09:47:42PM +0200, Theo Buehler wrote:
> For some reason ports-gcc can't find "lz4.h", so pass -I in CXXFLAGS.
> Then there's the usual missing include thing in gtest. I should see if I
> can upstream that...
You could contain it with to avoid subtle change on clang archs, no?
.if ${CHOSEN_COMPILER} == "ports-gcc"
CXXFLAGS += -I/usr/local/include
.endif
>
> I don't think this changes the package where it already builds, but I
> can bump REVISION if that's preferred.
Generally, I'd say bump if in doubt.
On amd64, this diff tripples the *built* libsnappy.so.* in size.
I've built twice without and twice with the diff:
$ size lib*
textdatabss dec hex
38075 164840 39763 9b53libsnappy.so.3.0.cur
38075 164840 39763 9b53libsnappy.so.3.0.cur2
109844 520832 115084 1c18c libsnappy.so.3.0.new
109844 520832 115084 1c18c libsnappy.so.3.0.new2
>
> Index: Makefile
> ===
> RCS file: /cvs/ports/archivers/snappy/Makefile,v
> retrieving revision 1.18
> diff -u -p -r1.18 Makefile
> --- Makefile 15 Oct 2023 07:31:06 - 1.18
> +++ Makefile 16 Oct 2023 19:35:14 -
> @@ -24,6 +24,8 @@ MODULES = devel/cmake
> CONFIGURE_ARGS +=-DBUILD_SHARED_LIBS=ON \
> -DSNAPPY_BUILD_BENCHMARKS=OFF
>
> +CONFIGURE_ENV += CXXFLAGS="-I${LOCALBASE}/include"
> +
Need to investigate why that adds so many symbols.
> # expects submodule/cannot use system library
> DIST_TUPLE = github google googletest
> e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
> third_party/googletest/ # BSD
> Index: patches/patch-third_party_googletest_googletest_src_gtest_cc
> ===
> RCS file: patches/patch-third_party_googletest_googletest_src_gtest_cc
> diff -N patches/patch-third_party_googletest_googletest_src_gtest_cc
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-third_party_googletest_googletest_src_gtest_cc 16 Oct
> 2023 19:38:50 -
> @@ -0,0 +1,17 @@
> +Fix build on sparc64:
> +
> +gtest.cc:5342:7:
> + error: 'raise' was not declared in this scope
> + raise(SIGTRAP);
That part is OK kn and clearly unrelated.
> +
> +Index: third_party/googletest/googletest/src/gtest.cc
> +--- third_party/googletest/googletest/src/gtest.cc.orig
> third_party/googletest/googletest/src/gtest.cc
> +@@ -43,6 +43,7 @@
> + #include
> + #include // NOLINT
> + #include
> ++#include
> + #include
> + #include
> + #include
archivers/snappy: unbreak on sparc64
For some reason ports-gcc can't find "lz4.h", so pass -I in CXXFLAGS.
Then there's the usual missing include thing in gtest. I should see if I
can upstream that...
I don't think this changes the package where it already builds, but I
can bump REVISION if that's preferred.
Index: Makefile
===
RCS file: /cvs/ports/archivers/snappy/Makefile,v
retrieving revision 1.18
diff -u -p -r1.18 Makefile
--- Makefile15 Oct 2023 07:31:06 - 1.18
+++ Makefile16 Oct 2023 19:35:14 -
@@ -24,6 +24,8 @@ MODULES = devel/cmake
CONFIGURE_ARGS += -DBUILD_SHARED_LIBS=ON \
-DSNAPPY_BUILD_BENCHMARKS=OFF
+CONFIGURE_ENV += CXXFLAGS="-I${LOCALBASE}/include"
+
# expects submodule/cannot use system library
DIST_TUPLE = github google googletest
e40661d89b051e9ef4eb8a2420b74bf78b39ef41 \
third_party/googletest/ # BSD
Index: patches/patch-third_party_googletest_googletest_src_gtest_cc
===
RCS file: patches/patch-third_party_googletest_googletest_src_gtest_cc
diff -N patches/patch-third_party_googletest_googletest_src_gtest_cc
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-third_party_googletest_googletest_src_gtest_cc16 Oct
2023 19:38:50 -
@@ -0,0 +1,17 @@
+Fix build on sparc64:
+
+gtest.cc:5342:7:
+ error: 'raise' was not declared in this scope
+ raise(SIGTRAP);
+
+Index: third_party/googletest/googletest/src/gtest.cc
+--- third_party/googletest/googletest/src/gtest.cc.orig
third_party/googletest/googletest/src/gtest.cc
+@@ -43,6 +43,7 @@
+ #include
+ #include // NOLINT
+ #include
++#include
+ #include
+ #include
+ #include
