Re: p7zip: add support for more archs

[Jérémie Courrèges-Anglas]

Christian Weisgerber  writes:

> On 2015-11-11, Jérémie Courrèges-Anglas  wrote:
>
>> Right now p7zip fails to build on several archs, because of the rather
>> lame approach to endianness taken by this port.  This patch should allow
>> to build p7zip on all archs supported by OpenBSD.
>
> Have you considered getting the information from endian.h?

Yup.  But that's something that I will propose upstream, not in the
ports tree.

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

Re: p7zip: add support for more archs

[Jérémie Courrèges-Anglas]

Stuart Henderson  writes:

> On 2015/11/11 22:19, Josh Grosse wrote:
>> On Wed, Nov 11, 2015 at 09:45:22PM -0500, Josh Grosse wrote:
>> > Fixing the code may be the correct solution, but it is beyond me, as 
>> > I don't have the technical skills to deal with the portability issues 
>> > that were raised and to my understanding still exist.
>> 
>> I've had an out-of-band discussion with Theo.  Fixing the code is the
>> correct solution.  So I will invest the effort to obtain the skills 
>> and knowledge needed to fix the code.
>
> It seems that upstream would like to be portable - it's in the project
> name, and their list of unofficial packages includes Amiga and BeOS -
> it might well need little more work than reporting it upstream.
>
> And indeed they may have even already fixed it in the time since it
> was previously tested.
>
> Is there anyone with an alpha that can try regress tests with jca's
> diff and send a backtrace if it still segfaults?

Three successful regress tests runs on sparc64.  I doubt that we'll get
many alpha test reports, these days.

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

Re: p7zip: add support for more archs

[Stuart Henderson]

On 2015/11/11 22:19, Josh Grosse wrote:
> On Wed, Nov 11, 2015 at 09:45:22PM -0500, Josh Grosse wrote:
> > Fixing the code may be the correct solution, but it is beyond me, as 
> > I don't have the technical skills to deal with the portability issues 
> > that were raised and to my understanding still exist.
> 
> I've had an out-of-band discussion with Theo.  Fixing the code is the
> correct solution.  So I will invest the effort to obtain the skills 
> and knowledge needed to fix the code.

It seems that upstream would like to be portable - it's in the project
name, and their list of unofficial packages includes Amiga and BeOS -
it might well need little more work than reporting it upstream.

And indeed they may have even already fixed it in the time since it
was previously tested.

Is there anyone with an alpha that can try regress tests with jca's
diff and send a backtrace if it still segfaults?

Re: p7zip: add support for more archs

[Christian Weisgerber]

On 2015-11-11, Jérémie Courrèges-Anglas  wrote:

> Right now p7zip fails to build on several archs, because of the rather
> lame approach to endianness taken by this port.  This patch should allow
> to build p7zip on all archs supported by OpenBSD.

Have you considered getting the information from endian.h?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: p7zip: add support for more archs

[Josh Grosse]

On Thu, Nov 12, 2015 at 02:02:50AM +0100, J??r??mie Courr??ges-Anglas wrote:

> Back to your concern: I don't think it's a reasonable approach to ask
> for tests on all architectures before introducing a change in a port.
> If we did that, the ports tree would be ridiculously small and lagging
> behind upstream.
> 
> This fixes a *build* issue.  A package broken at runtime on one arch or
> two is better than no package at all on ten architectures.

Back when the port was first proposed in 2007, there were several Project 
members who weighed in, on this subject, primarily due to the quality of 
this particular application.  

While a maintainer's role is limited it does include maintenance and support, 
and it is the latter which is the basis for my concern.  This application 
didn't run on Alpha or Vax eight years ago, and to my knowledge hasn't been 
retested on either.  

Of course I will abide by the governance controls the Project has for ports, 
which I understand to be committers' consensus, overseen by Theo.  

Re: p7zip: add support for more archs

[Josh Grosse]

On Thu, Nov 12, 2015 at 12:34:03AM +0100, J??r??mie Courr??ges-Anglas wrote:
> 
> Right now p7zip fails to build on several archs, because of the rather
> lame approach to endianness taken by this port.  This patch should allow
> to build p7zip on all archs supported by OpenBSD.
> 
> ok?

jca@,

I'm fine with the changes, once  the architectures can all be 
tested.  Back in the spring of 2007, this was tested on many
archs. And naddy@ had reported a failure on Alpha, which is included
in the patch to C/CpuArch.h. 

I'm just a maintainer.  So I've added naddy in copy, as I'd like
someone with *prior* anxiety about the quality of the application
for portability to weigh in on this recommended revision.

I'm fine with it, if it can be tested on these architectures. And I'm not
sure a vax/simh test would pass naddy's sniff test. I've heard it time
and again that real hardware may behave differently than an emulator.

If this *can* run on vax, then the SHARED_ONLY can be dropped from the 
Makefile, too.  

   -Josh-

> 
> Index: Makefile
> ===
> RCS file: /cvs/ports/archivers/p7zip/Makefile,v
> retrieving revision 1.25
> diff -u -p -r1.25 Makefile
> --- Makefile  21 Oct 2015 10:45:08 -  1.25
> +++ Makefile  11 Nov 2015 23:32:22 -
> @@ -6,6 +6,7 @@ COMMENT-main= file archiver with high co
>  COMMENT-rar= rar modules for p7zip
>  
>  V=   15.09
> +REVISION=0
>  DISTNAME=p7zip_${V}_src_all
>  PKGNAME= p7zip-${V}
>  PKGNAME-main=p7zip-${V}
> Index: patches/patch-C_CpuArch_h
> ===
> RCS file: patches/patch-C_CpuArch_h
> diff -N patches/patch-C_CpuArch_h
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-C_CpuArch_h 11 Nov 2015 23:32:22 -
> @@ -0,0 +1,31 @@
> +$OpenBSD$
> +
> +Add support for more OpenBSD architectures.
> +
> +--- C/CpuArch.h.orig Sun Sep 27 21:31:20 2015
>  C/CpuArch.h  Thu Nov 12 00:28:15 2015
> +@@ -65,7 +65,10 @@ If MY_CPU_LE_UNALIGN is not defined, we don't know abo
> + || defined(__AARCH64EL__) \
> + || defined(__MIPSEL__) \
> + || defined(__MIPSEL) \
> +-|| defined(_MIPSEL)
> ++|| defined(_MIPSEL) \
> ++|| defined(__alpha__) \
> ++|| defined(__sh__) \
> ++|| defined(__vax__)
> +   #define MY_CPU_LE
> + #endif
> + 
> +@@ -77,7 +80,11 @@ If MY_CPU_LE_UNALIGN is not defined, we don't know abo
> + || defined(__MIPSEB) \
> + || defined(_MIPSEB) \
> + || defined(__m68k__) \
> +-|| defined(__s390x__)
> ++|| defined(__m88k__) \
> ++|| defined(__s390x__) \
> ++|| defined(__hppa__) \
> ++|| defined(__mips64__) \
> ++|| defined(__sparc__)
> +   #define MY_CPU_BE
> + #endif
> + 
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: p7zip: add support for more archs

[Theo de Raadt]

> On Thu, Nov 12, 2015 at 02:02:50AM +0100, J??r??mie Courr??ges-Anglas wrote:
> 
> > Back to your concern: I don't think it's a reasonable approach to ask
> > for tests on all architectures before introducing a change in a port.
> > If we did that, the ports tree would be ridiculously small and lagging
> > behind upstream.
> > 
> > This fixes a *build* issue.  A package broken at runtime on one arch or
> > two is better than no package at all on ten architectures.
> 
> Back when the port was first proposed in 2007, there were several Project 
> members who weighed in, on this subject, primarily due to the quality of 
> this particular application.  
> 
> While a maintainer's role is limited it does include maintenance and support, 
> and it is the latter which is the basis for my concern.  This application 
> didn't run on Alpha or Vax eight years ago, and to my knowledge hasn't been 
> retested on either.  
> 
> Of course I will abide by the governance controls the Project has for ports, 
> which I understand to be committers' consensus, overseen by Theo.  

I really don't tell the ports people what to do.

But... I also don't think they should be martyrs.

Re: p7zip: add support for more archs

[Jérémie Courrèges-Anglas]

Josh Grosse  writes:

> On Thu, Nov 12, 2015 at 12:34:03AM +0100, J??r??mie Courr??ges-Anglas wrote:
>> 
>> Right now p7zip fails to build on several archs, because of the rather
>> lame approach to endianness taken by this port.  This patch should allow
>> to build p7zip on all archs supported by OpenBSD.
>> 
>> ok?
>
> jca@,
>
> I'm fine with the changes, once  the architectures can all be 
> tested.  Back in the spring of 2007, this was tested on many
> archs. And naddy@ had reported a failure on Alpha, which is included
> in the patch to C/CpuArch.h. 

IIUC this was a runtime failure, not a build failure.  Right now
p7zip doesn't *build* on those archs.

> I'm just a maintainer.  So I've added naddy in copy, as I'd like
> someone with *prior* anxiety about the quality of the application
> for portability to weigh in on this recommended revision.
>
> I'm fine with it, if it can be tested on these architectures. And I'm not
> sure a vax/simh test would pass naddy's sniff test. I've heard it time
> and again that real hardware may behave differently than an emulator.
>
> If this *can* run on vax, then the SHARED_ONLY can be dropped from the 
> Makefile, too.  

I didn't notice the SHARED_ONLY marker, but I'm tempted to keep the vax
addition in this patch.  It doesn't add maintenance burden, and perhaps
one day vax will have shared libs, and unicorns will fly.

Back to your concern: I don't think it's a reasonable approach to ask
for tests on all architectures before introducing a change in a port.
If we did that, the ports tree would be ridiculously small and lagging
behind upstream.

This fixes a *build* issue.  A package broken at runtime on one arch or
two is better than no package at all on ten architectures.

http://build-failures.rhaalovely.net/alpha/2015-10-30/archivers/p7zip.log
http://build-failures.rhaalovely.net/sparc64/2015-10-30/archivers/p7zip.log
http://build-failures.rhaalovely.net/hppa/2015-10-30/archivers/p7zip.log

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

Re: p7zip: add support for more archs

[Josh Grosse]

On Wed, Nov 11, 2015 at 07:26:14PM -0700, Theo de Raadt wrote:
> I really don't tell the ports people what to do.
> 
> But... I also don't think they should be martyrs.

Naddy said, after seeing an alignment error on Alpha: [1] 

"How about actually fixing the code rather than slapping on a list
of archs where this accidentally happens to run?"

I think the question is whether to have build errors, or run errors. 
And as stated I'll abide by either decision.  It is the run errors
that will come my way as maintainer, and I will at least now be able
to refer any inquiries to this thread, so there won't be any
significant support burden.

Fixing the code may be the correct solution, but it is beyond me, as 
I don't have the technical skills to deal with the portability issues 
that were raised and to my understanding still exist.

---

[1] http://marc.info/?l=openbsd-ports=117638516307110=2

Re: p7zip: add support for more archs

[Josh Grosse]

On Wed, Nov 11, 2015 at 09:45:22PM -0500, Josh Grosse wrote:
> Fixing the code may be the correct solution, but it is beyond me, as 
> I don't have the technical skills to deal with the portability issues 
> that were raised and to my understanding still exist.

I've had an out-of-band discussion with Theo.  Fixing the code is the
correct solution.  So I will invest the effort to obtain the skills 
and knowledge needed to fix the code.

p7zip: add support for more archs

[Jérémie Courrèges-Anglas]

Right now p7zip fails to build on several archs, because of the rather
lame approach to endianness taken by this port.  This patch should allow
to build p7zip on all archs supported by OpenBSD.

ok?

Index: Makefile
===
RCS file: /cvs/ports/archivers/p7zip/Makefile,v
retrieving revision 1.25
diff -u -p -r1.25 Makefile
--- Makefile21 Oct 2015 10:45:08 -  1.25
+++ Makefile11 Nov 2015 23:32:22 -
@@ -6,6 +6,7 @@ COMMENT-main=   file archiver with high co
 COMMENT-rar=   rar modules for p7zip
 
 V= 15.09
+REVISION=  0
 DISTNAME=  p7zip_${V}_src_all
 PKGNAME=   p7zip-${V}
 PKGNAME-main=  p7zip-${V}
Index: patches/patch-C_CpuArch_h
===
RCS file: patches/patch-C_CpuArch_h
diff -N patches/patch-C_CpuArch_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-C_CpuArch_h   11 Nov 2015 23:32:22 -
@@ -0,0 +1,31 @@
+$OpenBSD$
+
+Add support for more OpenBSD architectures.
+
+--- C/CpuArch.h.orig   Sun Sep 27 21:31:20 2015
 C/CpuArch.hThu Nov 12 00:28:15 2015
+@@ -65,7 +65,10 @@ If MY_CPU_LE_UNALIGN is not defined, we don't know abo
+ || defined(__AARCH64EL__) \
+ || defined(__MIPSEL__) \
+ || defined(__MIPSEL) \
+-|| defined(_MIPSEL)
++|| defined(_MIPSEL) \
++|| defined(__alpha__) \
++|| defined(__sh__) \
++|| defined(__vax__)
+   #define MY_CPU_LE
+ #endif
+ 
+@@ -77,7 +80,11 @@ If MY_CPU_LE_UNALIGN is not defined, we don't know abo
+ || defined(__MIPSEB) \
+ || defined(_MIPSEB) \
+ || defined(__m68k__) \
+-|| defined(__s390x__)
++|| defined(__m88k__) \
++|| defined(__s390x__) \
++|| defined(__hppa__) \
++|| defined(__mips64__) \
++|| defined(__sparc__)
+   #define MY_CPU_BE
+ #endif
+ 


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