Re: removing tophat from Debian

2017-12-17 Thread Andreas Tille
On Mon, Dec 18, 2017 at 02:35:21AM +0100, Steffen Möller wrote:
> 
> We are packaging software, not workflows (at the very moment, mostly).
> Anybody
> requesting tophat shall get exactly that, however unfortunate that decision
> may be.

Fair enough.
 
> I am happy with a post-inst warning, or a debian/NEWS entry, but what the
> user gets has to be what the user asks for.

Good idea.

> > 
> > 1. tophat Depends hisat2 (solves the issue that the replacement
> >will be installed
> You may suggest hisat2, or even recommend it, but you should not depend on
> it. And there is no need to enforce a replacement in the first place.

I admit I see no real harm done to use stronger relations as long as the
user really gets what was requested.

> > 2. provide instead of /usr/bin/tophat a shell script issuing
> >a warning first before the actual tophat call

What do you think about this in addition to a debian/NEWS entry?

> > What about this?
> 
> This would undermine the trust that our users have in our packages.
> 
> Please not. It is better remove the package from the archive than to disturb
> the integrity of the package.

I do not think that an extra dependency - for whatever motivation it was
added, will disturb the integrity of the package.
 
> I propose to discuss any general policy towards such desirable package
> substitutions at our upcoming Sprint in Barcelona.

+1

Kind regards

   Andreas. 

-- 
http://fam-tille.de



Re: removing tophat from Debian

2017-12-17 Thread Steffen Möller

Hello,

On 16.12.17 22:15, Andreas Tille wrote:

Hi Afif,

On Sat, Dec 16, 2017 at 12:31:28PM -0800, Afif Elghraoui wrote:

Since I'm not a user of all this software I do not have any objections.
However, I wonder whether we should provide kind of a sensible
"migration path" and add "Replaces: tophat" to hisat2 or something like
this.

Ack. Although I don't believe Replaces: is correct here (looking at Debian 
policy).

Its definitely incorrect.



However, we need to make sure that users who
had installed tophat get hisat2 installed.


I disagree.

We are packaging software, not workflows (at the very moment, mostly). 
Anybody
requesting tophat shall get exactly that, however unfortunate that 
decision may be.


I am happy with a post-inst warning, or a debian/NEWS entry, but what 
the user gets has to be what the user asks for.


The situation is different once we provide workflow packages. Then we 
will have some NGS workflows that will update tophat underneath as the 
maintainer of that workflow thinks (and perfectly documents) to best 
perform the task at hand.






Maybe a transitional package is needed, which would have to sit through another 
stable release cycle. That would also satisfy the concerns about notification 
time that the others brought up on this thread.

After reading this thread I think the better way to go is:

1. tophat Depends hisat2 (solves the issue that the replacement
   will be installed
You may suggest hisat2, or even recommend it, but you should not depend 
on it. And there is no need to enforce a replacement in the first place.

2. provide instead of /usr/bin/tophat a shell script issuing
   a warning first before the actual tophat call

What about this?


This would undermine the trust that our users have in our packages.

Please not. It is better remove the package from the archive than to 
disturb the integrity of the package.


I propose to discuss any general policy towards such desirable package 
substitutions at our upcoming Sprint in Barcelona.


Cheers,

Steffen



Re: ismrmrd FTBFS on armhf

2017-12-17 Thread Adrian Bunk
On Sun, Dec 17, 2017 at 04:49:48PM +, Ghislain Vaillant wrote:
> Le 17/12/17 à 15:40, Adrian Bunk a écrit :
>...
> > Unaligned floating point access on armhf is expected to fail,
> > and that's exactly what happens here:
> > unknown location(0): fatal error: in 
> > "AcquisitionsTest/test_acquisition_header": memory access violation at 
> > address: 0xbecd3b6a: invalid address alignment
> > 
> > Running the test in gdb confirms that this is floating point code.
> > 
> > sparc is generally unhappy with unaligned access:
> > unknown location(0): fatal error: in 
> > "AcquisitionsTest/test_acquisition_header": memory access violation at 
> > address: 0x7feffb7c936: invalid address alignment
> > 
> > Note that even on architectures where unaligned access is permitted
> > it can be slower than aligned access.
> 
> So, what would be the right course of action moving forward? Removing the
> package for both armhf and sparc64?

It was already removed there ages ago, the only question is whether this
is worth fixing.

The root cause is that these structs seem to have been defined
by people without much knowledge about C alignment.

arm64 is more forgiving, the only affected release architecture
is armhf.

The armel packages could be used through multiarch on all armhf
hardware if someone really needs them.

IMHO the most reasonable action forward would be to do nothing,
and leave this bug open to document that the armhf FTBFS is expected.

> Ghis

cu
Adrian

-- 

   "Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
   "Only a promise," Lao Er said.
   Pearl S. Buck - Dragon Seed



Re: ismrmrd FTBFS on armhf

2017-12-17 Thread Ghislain Vaillant



Le 17/12/17 à 15:40, Adrian Bunk a écrit :

On Sun, Dec 17, 2017 at 02:33:03PM +, Ghislain Vaillant wrote:

ISMRMRD uses a non-portable instruction (#pragma pack) which modifies the
memory alignment of its data structures. It seems neither armhf nor sparc64
supports it, hence the failure of the test suite for both architectures.


#pragma pack is supported everywhere,
and this pragma is the cause of the FTBFS.


Ack.


I am not sure what the best course of action is. Either leaving things as is
assuming a future rebuild with a newer compiler could improve it, disabling
the tests or even dropping the packages for the failing architectures.

Opinions welcome.


With #pragma pack you are forcing the compiler to do the wrong thing,
the only thing a newer compiler could possibly improve would be to
error on such code.


Ack.


Unaligned floating point access on armhf is expected to fail,
and that's exactly what happens here:
unknown location(0): fatal error: in 
"AcquisitionsTest/test_acquisition_header": memory access violation at address: 
0xbecd3b6a: invalid address alignment

Running the test in gdb confirms that this is floating point code.

sparc is generally unhappy with unaligned access:
unknown location(0): fatal error: in 
"AcquisitionsTest/test_acquisition_header": memory access violation at address: 
0x7feffb7c936: invalid address alignment

Note that even on architectures where unaligned access is permitted
it can be slower than aligned access.


So, what would be the right course of action moving forward? Removing 
the package for both armhf and sparc64?


Ghis



Re: ismrmrd FTBFS on armhf

2017-12-17 Thread Adrian Bunk
On Sun, Dec 17, 2017 at 02:33:03PM +, Ghislain Vaillant wrote:
> ISMRMRD uses a non-portable instruction (#pragma pack) which modifies the
> memory alignment of its data structures. It seems neither armhf nor sparc64
> supports it, hence the failure of the test suite for both architectures.

#pragma pack is supported everywhere,
and this pragma is the cause of the FTBFS.

> I am not sure what the best course of action is. Either leaving things as is
> assuming a future rebuild with a newer compiler could improve it, disabling
> the tests or even dropping the packages for the failing architectures.
> 
> Opinions welcome.

With #pragma pack you are forcing the compiler to do the wrong thing,
the only thing a newer compiler could possibly improve would be to
error on such code.

Unaligned floating point access on armhf is expected to fail,
and that's exactly what happens here:
unknown location(0): fatal error: in 
"AcquisitionsTest/test_acquisition_header": memory access violation at address: 
0xbecd3b6a: invalid address alignment

Running the test in gdb confirms that this is floating point code.

sparc is generally unhappy with unaligned access:
unknown location(0): fatal error: in 
"AcquisitionsTest/test_acquisition_header": memory access violation at address: 
0x7feffb7c936: invalid address alignment

Note that even on architectures where unaligned access is permitted
it can be slower than aligned access.

> Ghis

cu
Adrian

-- 

   "Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
   "Only a promise," Lao Er said.
   Pearl S. Buck - Dragon Seed



Re: ismrmrd FTBFS on armhf

2017-12-17 Thread Ghislain Vaillant
ISMRMRD uses a non-portable instruction (#pragma pack) which modifies 
the memory alignment of its data structures. It seems neither armhf nor 
sparc64 supports it, hence the failure of the test suite for both 
architectures.


I am not sure what the best course of action is. Either leaving things 
as is assuming a future rebuild with a newer compiler could improve it, 
disabling the tests or even dropping the packages for the failing 
architectures.


Opinions welcome.

Ghis