Summary: | app-text/texlive-core: should not define a macro for abs() | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Anthony Basile <blueness> |
Component: | Current packages | Assignee: | Alexis Ballier <aballier> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | tex, vapier, vitto.giova |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
See Also: | https://bugs.gentoo.org/show_bug.cgi?id=510766 | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Attachments: |
Patch against SVN head for upstream.
Patch against texlive-core-2012 Updated patch against SVN head for upstream Updated patch against SVN head for upstream Patch that passes make check Patch v2 |
Description
Anthony Basile
2014-05-19 16:25:46 UTC
please submit a patch upstream if you want this fixed, 2014 seems to still have it (In reply to Alexis Ballier from comment #1) i'm assuming you're familiar with texlive as a maintainer, which means you're familiar with upstream processes, which means it'd be easy for you to send the patch to them ? (In reply to SpanKY from comment #2) > (In reply to Alexis Ballier from comment #1) > > i'm assuming you're familiar with texlive as a maintainer, which means > you're familiar with upstream processes, which means it'd be easy for you to > send the patch to them ? i can do that too if i have a patch (In reply to Anthony Basile from comment #0) > > Its okay with glibc (and musl) because of the different way the headers > stack. While that commit by uclibc, which includes stdlib.h in > bits/sched.h, is questionable, still its not a good idea to define a macro > with the same name as a POSIX function. Its safe on any posix system to > just remove that line, or, if you really want to be safe, you can add > AC_CHECK_FUNCS([abs]) to configure.ac. You can just drop the line, but I'll do it the safest way possible, ie check if abs is available and if it isn't define the macro. (In reply to Anthony Basile from comment #4) > (In reply to Anthony Basile from comment #0) > > You can just drop the line, but I'll do it the safest way possible, ie check > if abs is available and if it isn't define the macro. i.e. I'll get you a patch :) (In reply to Anthony Basile from comment #5) > (In reply to Anthony Basile from comment #4) > > (In reply to Anthony Basile from comment #0) > > > > You can just drop the line, but I'll do it the safest way possible, ie check > > if abs is available and if it isn't define the macro. > > i.e. I'll get you a patch :) thanks; if you can send this to tex-live@tug.org cc'ing me so that i'll follow it, it's even better :) (i can proxy send it for you, but it'd be harder for me to defend or test it since i don't have any uclibc system) Created attachment 396902 [details, diff]
Patch against SVN head for upstream.
Okay this patch is meant for upstream. After applying the patch, they'll have to `cd utils/pmx && autoreconf`.
I don't recommend this patch for Gentoo since we can avoid eautoreconf-ing by just removing the macro for abs(). I can't think of anything we support where you'll need it. That's my next patch.
Created attachment 396904 [details, diff]
Patch against texlive-core-2012
This patch just removes the macro for abs() from 2012. It'll work against the other versions, 2013 and 2014, provided you fix up the pathname to f2c.h.
I tested both patches.
(In reply to Anthony Basile from comment #7) > Created attachment 396902 [details, diff] [details, diff] > Patch against SVN head for upstream. thanks; is the config.h.in / autoheader usage part really needed ? IMHO it's better to keep changes at a minimum, and I think autoconf passes -DMACRO when autoheader isn't used. > I don't recommend this patch for Gentoo since we can avoid eautoreconf-ing > by just removing the macro for abs(). I can't think of anything we support > where you'll need it. That's my next patch. Yep, anyway, I don't think autoreconf-ing works without upstream's scripts for doing it (or at least, it didn't work in the past) (In reply to Alexis Ballier from comment #9) > (In reply to Anthony Basile from comment #7) > > Created attachment 396902 [details, diff] [details, diff] [details, diff] > > Patch against SVN head for upstream. > > thanks; is the config.h.in / autoheader usage part really needed ? > IMHO it's better to keep changes at a minimum, and I think autoconf passes > -DMACRO when autoheader isn't used. Yeah you can do it that way too. Do you want me to generate that? > > > > I don't recommend this patch for Gentoo since we can avoid eautoreconf-ing > > by just removing the macro for abs(). I can't think of anything we support > > where you'll need it. That's my next patch. > > Yep, anyway, I don't think autoreconf-ing works without upstream's scripts > for doing it (or at least, it didn't work in the past) Works "locally" ie on a per directory basis. texlive is a beast as you know. (In reply to Anthony Basile from comment #10) > (In reply to Alexis Ballier from comment #9) > > (In reply to Anthony Basile from comment #7) > > > Created attachment 396902 [details, diff] [details, diff] [details, diff] [details, diff] > > > Patch against SVN head for upstream. > > > > thanks; is the config.h.in / autoheader usage part really needed ? > > IMHO it's better to keep changes at a minimum, and I think autoconf passes > > -DMACRO when autoheader isn't used. > > Yeah you can do it that way too. Do you want me to generate that? yes please, so that I'll know it has been tested. > > > > > > > I don't recommend this patch for Gentoo since we can avoid eautoreconf-ing > > > by just removing the macro for abs(). I can't think of anything we support > > > where you'll need it. That's my next patch. > > > > Yep, anyway, I don't think autoreconf-ing works without upstream's scripts > > for doing it (or at least, it didn't work in the past) > > Works "locally" ie on a per directory basis. texlive is a beast as you know. it used to require different versions for different directories too... oh, and also, could you please commit your for-tree patch to: gentoo/src/patchsets/texlive/${year}/texlive-core (and update the quilt series); extra kudos if you roll up a new patchset and commit it, but this I can do if you prefer a double-check. Note: I know all this might sound like I don't want to help, but I've noticed from the texlive 2014 bump that I'm really the bottleneck here, it is simple and not so much time consuming for me to do it but when people try to help without guidance they waste a lot of time; so I'm trying to get more people able to push fixes there. Created attachment 397674 [details, diff] Updated patch against SVN head for upstream This has been tested as follows: 1) checkout as per upstream instrucitons: rsync -va --delete --exclude=.svn tug.org::tldevsrc/Build/source . 2) apply patch 3) cd source/utils/pmx ; patch -p1 < /pathto/patch 4) autoreconf ; ./configure ; make ; readelf -s pmxab | grep abs You get that pmxab does obtain abs() from your libc. on glibc you get: 14: 0000000000000000 0 FUNC GLOBAL DEFAULT UND abs@GLIBC_2.2.5 (2) 2009: 0000000000000000 0 FUNC GLOBAL DEFAULT UND abs@@GLIBC_2.2.5 Created attachment 397676 [details, diff]
Updated patch against SVN head for upstream
Sorry I forgot to add the commit message. (SVN is wierd.)
(In reply to Alexis Ballier from comment #12) > oh, and also, could you please commit your for-tree patch to: > gentoo/src/patchsets/texlive/${year}/texlive-core > > (and update the quilt series); extra kudos if you roll up a new patchset and > commit it, but this I can do if you prefer a double-check. > done. Please test texlive-core-2012-r2 texlive-core-2013-r2 texlive-core-2014-r2 (In reply to Anthony Basile from comment #15) > (In reply to Alexis Ballier from comment #12) > > oh, and also, could you please commit your for-tree patch to: > > gentoo/src/patchsets/texlive/${year}/texlive-core > > > > (and update the quilt series); extra kudos if you roll up a new patchset and > > commit it, but this I can do if you prefer a double-check. > > > > done. Please test texlive-core-2012-r2 texlive-core-2013-r2 > texlive-core-2014-r2 actually, while your at it, can we start stabilization on one of these, preferably like the 2014 one if it fits with the other tex packages so we don't fall behind. well, guess there's something we've overseen: bug #541934 is caused by this patch I'd bet that's because abs from stdlib in int -> int and thus truncates floats while the macro doesn't... I'll revert the patch soonish waiting for a proper one (In reply to Alexis Ballier from comment #17) > well, guess there's something we've overseen: bug #541934 is caused by this > patch > > I'd bet that's because abs from stdlib in int -> int and thus truncates > floats while the macro doesn't... > > I'll revert the patch soonish waiting for a proper one I didn't run the tests. Hang on a bit because this may be an easy fix. I'll have to look at it tomorrow. Created attachment 400996 [details, diff]
Patch that passes make check
(In reply to Felix Janda from comment #19) > Created attachment 400996 [details, diff] [details, diff] > Patch that passes make check Felix thanks for looking at this. Is that patch all that is required? Doesn't the code refer directly to abs() itself elsewhere? I haven't looked in a while. If that's all that's required, I'll yank my patch, replace it with yours and respin the patchset. (In reply to Anthony Basile from comment #20) > (In reply to Felix Janda from comment #19) > > Created attachment 400996 [details, diff] [details, diff] [details, diff] > > Patch that passes make check > > Felix thanks for looking at this. Is that patch all that is required? > Doesn't the code refer directly to abs() itself elsewhere? I haven't looked > in a while. > > If that's all that's required, I'll yank my patch, replace it with yours and > respin the patchset. It's that is required to get a working pmx. However it gives an implicit function declaration warning. Below will be a patch fixing this. abs() is not directly used in the libf2c directory but only in scor2prt.c and pmxab.c and there it is only used for integer arguments. As an aside: The libf2c directory is dev-libs/libf2c, which likely also does not build with uclibc. Created attachment 401048 [details, diff]
Patch v2
(In reply to Felix Janda from comment #22) > Created attachment 401048 [details, diff] [details, diff] > Patch v2 This is fine for gentoo. For upstream don't you think we need the AC_CHECK_FUNCS([abs]) from comment #14? I can't think of any situaiton where abs() is not available in libc, but at the same time, they did add it for some reason. C89 already specifies abs in the stdlib.h header. Most of the problems with the libf2c code are due to the fact that it tries also to cater for compilers only understanding K&R C. The rest of texlive will surely not compile with a non-ANSI C compiler. It would be nice if this, together with the things the other patches in texlive address, could be fixed upstream (netlib.org). The latest upstream change was in 2013, so there is some chance to get things fixed there. For upstream some KR_headers #ifdefery would be necessary. On the other hand, a Changelog entry from 2002 suggests that the author considered these K&R compilers rare even then.) I've sent patches to netlib.org. When they are incorporated into upstream libf2c, I'll send a patch to texlive updating their copy. (In reply to Felix Janda from comment #25) > I've sent patches to netlib.org. When they are incorporated into > upstream libf2c, I'll send a patch to texlive updating their copy. I pushed out the fix to the patchsets and ebuilds and removed the old buggy ones. I confirmed that the tests work for 2013 and 2014 on both glibc and uclibc. Let's just see what upstream has to say. *texlive-core-2014-r4 (28 Apr 2015) *texlive-core-2012-r3 (28 Apr 2015) *texlive-core-2013-r3 (28 Apr 2015) 28 Apr 2015; Anthony G. Basile <blueness@gentoo.org> +texlive-core-2012-r3.ebuild, +texlive-core-2013-r3.ebuild, +texlive-core-2014-r4.ebuild, -texlive-core-2012-r2.ebuild, -texlive-core-2013-r2.ebuild, -texlive-core-2014-r2.ebuild: Fix bump 2012/2013/2014 for bug #510770. this is done and texlive-core-2014-r4 is going stable now. |