Re: [PATCH] [round 3] [All done :-)] Upstreaming Debian patches

On 06/07/2016 10:28 AM, Thorsten Glaser wrote:
> On Mon, 6 Jun 2016, Christian Seiler wrote:
>> I can verify that that's all, thank you very much!
> 
> Out of the Debian patches, this part is still missing
> and needed upstream:
> 
> --- a/diet.c
> +++ b/diet.c
> @@ -214,8 +214,12 @@ int main(int argc,char *argv[]) {
>        shortplatform="parisc";
>  #endif
>  #ifdef __x86_64__
> +#ifdef __ILP32__
> +      shortplatform=(m==32?"i386":(m==64?"x86_64":"x32"));
> +#else
>        shortplatform=(m==32?"i386":(m==33?"x32":"x86_64"));
>  #endif
> +#endif
>  #ifdef __ia64__
>        shortplatform="ia64";
>  #endif

No, Felix shot down native x32 support, see [1]:

| I don't think it makes sense to have this kind of thing in the mainline
| dietlibc, but feel free to keep on doing this for your x32 distro.

(And for using -mx32 on an x86_64 compiler it's not required;
and Felix did apply a patch for detecting x32 cross compilers.)

> debian/patches/debian/multiarch.diff should also be considered,
> in a way that it is inert when the distro does not use Multi-Arch.
> Note that, even on M-A enabled Debian, dashL_debian_old must NOT
> be retired; in the regular toolchain (system gcc/glibc), /usr/lib
> is still searched as well, it’s just that Debian packages don’t
> install anything into that any more eventually, but it will never
> be delisted as search path.

First of all you need to separate two things: I added the
dashL_debian_old stuff to capture (for compatibility reasons) the
non-standard directory that the older Debian packages were using
(/usr/lib/diet/lib). So that's definitely not something that
should go upstream, because upstream never supported that.

The second part mangles the default DIETHOME based on the arch
of the compiler to be used. (You can still override it via the
environment variable, of course.) The only reason I actually have
this patch in there is because Debian has multiple cases where
two archs have the same dietlibc name, so the installed paths
would clash. (Debian's armhf and armel map to dietlibc's arm,
Debian's powerpc and powerpcspe map to dietlibc's ppc. Plus, in
earlier versions, mipsel and mips mapped to the same dietlibc
architecture name, mips, though that doesn't apply anymore.)

If that had not been the case, just installing dietlibc to
/usr/lib/diet would have worked just fine, the i386 package would
have taken /usr/lib/diet/lib-i386, the x86_64 package lib-x86_64,
etc.

So this is in fact Debian-specific, which is why I haven't
forwarded it.

> debian/patches/debian/dont-strip.diff could probably go away if
> upstream introduces a way to tell the Makefile to not strip but
> not define -DDEBUG… actually, it doesn’t do that, so Debian can
> probably just remove that patch and convert the debian/rules line
> 	ifneq (,$(findstring debug,$(DEB_BUILD_OPTIONS)))
> into an always-true condition, or even inline the DEBUG=1 below.

DEBUG=1 also drops -Os/-O2, which is why I've done it that way.
But if you combine it with the next point you bring up, it's
probably something that can be simplified:

> debian/patches/debian/hardening.diff is probably wrong; CFLAGS
> should be provided by debian/rules, not created in the Makefile.
> Use DEB_BUILD_MAINT_OPTIONS, dpkg-buildflags, et al. for that.

Yes, it's on my TODO list. (But also Debian-specific.)

> debian/patches/debian/sparc32-mcpu-v9.diff is a workaround for
> a bug in recent Linux/GCC versions to not support sparc32 CPUs
> in the sparc32 architecture any more (don’t laugh), and thus
> should be kept Debian-specific

Actually, as far as I understood it, Debian did compile gcc on
sparc32 for sparc64 CPUs only, so the atomic intrinsic emulation
functions required on sparc32 CPUs were not added to -lgcc
anymore. But IIUC, upstream gcc still supports them, so this is
definitely something Debian-specific, because you don't need to
compile your sparc32 gcc in that way, and then you'd have the
emulation functions available.

Regards,
Christian

[1] http://news.gmane.org/find-root.php?message_id=20160329160620.GA16244%40qarx.de