Re: [PATCH] Fix archivers/p7zip endianness detection

[Josh Grosse]

On Sun, Jul 02, 2017 at 05:46:07PM +0200, Jeremie Courreges-Anglas wrote:

> I'd like to commit this.

$MAINTAINER OK, for what that is worth.

Re: [PATCH] Fix archivers/p7zip endianness detection

[Jeremie Courreges-Anglas]

Josh Grosse  writes:

> On Sun, Jul 02, 2017 at 12:41:48PM +0200, Donovan Watteau wrote:
>> Hi,
>> 
>> archivers/p7zip currently fails to build on loongson, because of a
>> mistake in patch-C_CpuArch_h: we were implying that __mips64__ always
>> means that we're running big-endian, but that's wrong for loongson
>> (which is mips64el).
>> 
>> __MIPSEL__ and __mips64__ are both detected by CpuArch.h, so p7zip
>> tries to build thinking that it's both big- and little-endian, and
>> the build fails right at the start.
>> 
>> The following diff just patches CpuArch.h to use  instead,
>> which makes things simpler and correct.
>
> Thank you, Donovan.  I've tested on amd64, and the patch seems to be 
> working.  The built-in tests all function.  I am in the process of 
> testing on i386 and armv7 (the latter under qemu), which I should
> be able to complete later today.

make test passes on armv7.

>> I don't think REVISION needs to be bumped, because it shouldn't change
>> anything for the architectures where it didn't build (except if there
>> was an arch where we were successfully building, but with the wrong
>> endianness).
>
> I think a revision bump is needed and have attached a revise patch
> which includes one.

Bumps are cheap, when in doubt, use them.

Here, the architecture list did not include powerpc, where neither
MY_CPU_BE and MY_CPU_LE would end up defined (the build would succeed,
though).  Since p7zip contains code that depends on those macros,
a REVISION bump is due.

I'd like to commit this.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: [PATCH] Fix archivers/p7zip endianness detection

[Josh Grosse]

On Sun, Jul 02, 2017 at 09:32:26AM -0400, Josh Grosse wrote:
> ...I am in the process of 
> testing on i386 and armv7 (the latter under qemu), which I should
> be able to complete later today.

Testing complete on amd64, i386, and armv7.  No regressions.

Re: [PATCH] Fix archivers/p7zip endianness detection

[Josh Grosse]

On Sun, Jul 02, 2017 at 12:41:48PM +0200, Donovan Watteau wrote:
> Hi,
> 
> archivers/p7zip currently fails to build on loongson, because of a
> mistake in patch-C_CpuArch_h: we were implying that __mips64__ always
> means that we're running big-endian, but that's wrong for loongson
> (which is mips64el).
> 
> __MIPSEL__ and __mips64__ are both detected by CpuArch.h, so p7zip
> tries to build thinking that it's both big- and little-endian, and
> the build fails right at the start.
> 
> The following diff just patches CpuArch.h to use  instead,
> which makes things simpler and correct.

Thank you, Donovan.  I've tested on amd64, and the patch seems to be 
working.  The built-in tests all function.  I am in the process of 
testing on i386 and armv7 (the latter under qemu), which I should
be able to complete later today.
 
> I don't think REVISION needs to be bumped, because it shouldn't change
> anything for the architectures where it didn't build (except if there
> was an arch where we were successfully building, but with the wrong
> endianness).

I think a revision bump is needed and have attached a revise patch
which includes one.

  Josh Grosse, $MAINTAINER
Index: Makefile
===
RCS file: /systems/cvs/ports/archivers/p7zip/Makefile,v
retrieving revision 1.39
diff -u -p -r1.39 Makefile
--- Makefile10 Apr 2017 11:45:22 -  1.39
+++ Makefile2 Jul 2017 12:32:47 -
@@ -4,7 +4,7 @@ COMMENT-main=   file archiver with high co
 COMMENT-rar=   rar modules for p7zip
 
 V= 16.02
-REVISION-main= 1
+REVISION-main= 2
 REVISION-rar=  0
 DISTNAME=  p7zip_${V}_src_all
 PKGNAME=   p7zip-${V}
Index: patches/patch-C_CpuArch_h
===
RCS file: /systems/cvs/ports/archivers/p7zip/patches/patch-C_CpuArch_h,v
retrieving revision 1.2
diff -u -p -r1.2 patch-C_CpuArch_h
--- patches/patch-C_CpuArch_h   10 Apr 2016 19:53:09 -  1.2
+++ patches/patch-C_CpuArch_h   2 Jul 2017 12:31:10 -
@@ -1,25 +1,52 @@
 $OpenBSD: patch-C_CpuArch_h,v 1.2 2016/04/10 19:53:09 naddy Exp $
 
-Add support for more OpenBSD architectures.
+Use  to determine endianness, instead of a complex and 
+incorrect list of architectures.
 
 C/CpuArch.h.orig   Wed Feb 17 01:27:16 2016
-+++ C/CpuArch.hSun Apr  3 19:05:55 2016
-@@ -66,6 +66,8 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
- || defined(__MIPSEL__) \
- || defined(__MIPSEL) \
- || defined(_MIPSEL) \
-+|| defined(__alpha__) \
-+|| defined(__sh__) \
- || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == 
__ORDER_LITTLE_ENDIAN__))
-   #define MY_CPU_LE
+Index: C/CpuArch.h
+--- C/CpuArch.h.orig
 C/CpuArch.h
+@@ -5,6 +5,7 @@
+ #define __CPU_ARCH_H
+ 
+ #include "7zTypes.h"
++#include 
+ 
+ EXTERN_C_BEGIN
+ 
+@@ -56,33 +57,9 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
+ #define MY_CPU_IA64_LE
  #endif
-@@ -82,6 +84,9 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
- || defined(__s390x__) \
- || defined(__zarch__) \
- || defined(__sparc) \
-+|| defined(__sparc__) \
-+|| defined(__hppa__) \
-+|| defined(__mips64__) \
- || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
+ 
+-#if defined(MY_CPU_X86_OR_AMD64) \
+-|| defined(MY_CPU_ARM_LE) \
+-|| defined(MY_CPU_IA64_LE) \
+-|| defined(__LITTLE_ENDIAN__) \
+-|| defined(__ARMEL__) \
+-|| defined(__THUMBEL__) \
+-|| defined(__AARCH64EL__) \
+-|| defined(__MIPSEL__) \
+-|| defined(__MIPSEL) \
+-|| defined(_MIPSEL) \
+-|| (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == 
__ORDER_LITTLE_ENDIAN__))
++#if BYTE_ORDER == LITTLE_ENDIAN
+   #define MY_CPU_LE
+-#endif
+-
+-#if defined(__BIG_ENDIAN__) \
+-|| defined(__ARMEB__) \
+-|| defined(__THUMBEB__) \
+-|| defined(__AARCH64EB__) \
+-|| defined(__MIPSEB__) \
+-|| defined(__MIPSEB) \
+-|| defined(_MIPSEB) \
+-|| defined(__m68k__) \
+-|| defined(__s390__) \
+-|| defined(__s390x__) \
+-|| defined(__zarch__) \
+-|| defined(__sparc) \
+-|| (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
++#elif BYTE_ORDER == BIG_ENDIAN
#define MY_CPU_BE
  #endif
+ 

[PATCH] Fix archivers/p7zip endianness detection

[Donovan Watteau]

Hi,

archivers/p7zip currently fails to build on loongson, because of a
mistake in patch-C_CpuArch_h: we were implying that __mips64__ always
means that we're running big-endian, but that's wrong for loongson
(which is mips64el).

__MIPSEL__ and __mips64__ are both detected by CpuArch.h, so p7zip
tries to build thinking that it's both big- and little-endian, and
the build fails right at the start.

The following diff just patches CpuArch.h to use  instead,
which makes things simpler and correct.

I don't think REVISION needs to be bumped, because it shouldn't change
anything for the architectures where it didn't build (except if there
was an arch where we were successfully building, but with the wrong
endianness).

Index: patches/patch-C_CpuArch_h
===
RCS file: /cvs/ports/archivers/p7zip/patches/patch-C_CpuArch_h,v
retrieving revision 1.2
diff -u -p -r1.2 patch-C_CpuArch_h
--- patches/patch-C_CpuArch_h   10 Apr 2016 19:53:09 -  1.2
+++ patches/patch-C_CpuArch_h   2 Jul 2017 09:14:29 -
@@ -1,25 +1,52 @@
 $OpenBSD: patch-C_CpuArch_h,v 1.2 2016/04/10 19:53:09 naddy Exp $
 
-Add support for more OpenBSD architectures.
+Use  to determine endianness, instead of a complex and 
+incorrect list of architectures.
 
 C/CpuArch.h.orig   Wed Feb 17 01:27:16 2016
-+++ C/CpuArch.hSun Apr  3 19:05:55 2016
-@@ -66,6 +66,8 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
- || defined(__MIPSEL__) \
- || defined(__MIPSEL) \
- || defined(_MIPSEL) \
-+|| defined(__alpha__) \
-+|| defined(__sh__) \
- || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == 
__ORDER_LITTLE_ENDIAN__))
-   #define MY_CPU_LE
+Index: C/CpuArch.h
+--- C/CpuArch.h.orig
 C/CpuArch.h
+@@ -5,6 +5,7 @@
+ #define __CPU_ARCH_H
+ 
+ #include "7zTypes.h"
++#include 
+ 
+ EXTERN_C_BEGIN
+ 
+@@ -56,33 +57,9 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
+ #define MY_CPU_IA64_LE
  #endif
-@@ -82,6 +84,9 @@ MY_CPU_LE_UNALIGN means that CPU is LITTLE ENDIAN and 
- || defined(__s390x__) \
- || defined(__zarch__) \
- || defined(__sparc) \
-+|| defined(__sparc__) \
-+|| defined(__hppa__) \
-+|| defined(__mips64__) \
- || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
+ 
+-#if defined(MY_CPU_X86_OR_AMD64) \
+-|| defined(MY_CPU_ARM_LE) \
+-|| defined(MY_CPU_IA64_LE) \
+-|| defined(__LITTLE_ENDIAN__) \
+-|| defined(__ARMEL__) \
+-|| defined(__THUMBEL__) \
+-|| defined(__AARCH64EL__) \
+-|| defined(__MIPSEL__) \
+-|| defined(__MIPSEL) \
+-|| defined(_MIPSEL) \
+-|| (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == 
__ORDER_LITTLE_ENDIAN__))
++#if BYTE_ORDER == LITTLE_ENDIAN
+   #define MY_CPU_LE
+-#endif
+-
+-#if defined(__BIG_ENDIAN__) \
+-|| defined(__ARMEB__) \
+-|| defined(__THUMBEB__) \
+-|| defined(__AARCH64EB__) \
+-|| defined(__MIPSEB__) \
+-|| defined(__MIPSEB) \
+-|| defined(_MIPSEB) \
+-|| defined(__m68k__) \
+-|| defined(__s390__) \
+-|| defined(__s390x__) \
+-|| defined(__zarch__) \
+-|| defined(__sparc) \
+-|| (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
++#elif BYTE_ORDER == BIG_ENDIAN
#define MY_CPU_BE
  #endif
+