Bug 208486

Summary:

lang/python27 lang/python33 lang/python34 lang/python35: correct __FreeBSD_version check for ctype UTF-8 bug workaround

Product:

Ports & Packages

Reporter:

Dimitry Andric <dim>

Component:

Individual Port(s)

Assignee:

Dimitry Andric <dim>

Status:

Closed FIXED

Severity:

Affects Many People

CC:

dim

Priority:

---

Flags:

bugzilla: maintainer-feedback? (python)
dim: exp-run?

Version:

Latest

Hardware:

Any

OS:

Any

Bug Depends on:

Bug Blocks:

208158

Attachments:

Description	Flags
Fix python issue 10910 for lang/python[27,33,34,35]	none
Get rid of obsolete UTF-8 workaround for lang/python[27,33,34,35]	none
Fix python issue 10910 for lang/python[27,33,34,35], with bump	none

Description Dimitry Andric freebsd_committer

2016-04-03 17:49:44 UTC

Created attachment 168932 [details]
Fix python issue 10910 for lang/python[27,33,34,35]

During the exp-run in bug 208158, it was found that pyport.h (from the various lang/python ports) result in compilation errors with libc++ 3.8.0 [1], for instance with science/py-scipy:

In file included from scipy/interpolate/src/_interpolate.cpp:4:
In file included from scipy/interpolate/src/interpolate.h:3:
In file included from /usr/include/c++/v1/iostream:38:
In file included from /usr/include/c++/v1/ios:216:
/usr/include/c++/v1/__locale:468:15: error: C++ requires a type specifier for all declarations
    char_type toupper(char_type __c) const
              ^
/usr/local/include/python2.7/pyport.h:731:29: note: expanded from macro 'toupper'
#define toupper(c) towupper(btowc(c))
                            ^

This is because pyport.h contains a rather nasty workaround for very old FreeBSD ctype issue, which was added in August 2004 [2]:

/* On 4.4BSD-descendants, ctype functions serves the whole range of
 * wchar_t character set rather than single byte code points only.
 * This characteristic can break some operations of string object
 * including str.upper() and str.split() on UTF-8 locales.  This
 * workaround was provided by Tim Robbins of FreeBSD project.
 */

#ifdef __FreeBSD__
#include <osreldate.h>
#if __FreeBSD_version > 500039
# define _PY_PORT_CTYPE_UTF8_ISSUE
#endif
#endif


#if defined(__APPLE__)
# define _PY_PORT_CTYPE_UTF8_ISSUE
#endif

#ifdef _PY_PORT_CTYPE_UTF8_ISSUE
#include <ctype.h>
#include <wctype.h>
#undef isalnum
#define isalnum(c) iswalnum(btowc(c))
#undef isalpha
#define isalpha(c) iswalpha(btowc(c))
#undef islower
#define islower(c) iswlower(btowc(c))
#undef isspace
#define isspace(c) iswspace(btowc(c))
#undef isupper
#define isupper(c) iswupper(btowc(c))
#undef tolower
#define tolower(c) towlower(btowc(c))
#undef toupper
#define toupper(c) towupper(btowc(c))
#endif

Re-defining the macros like this causes trouble with some standard C++ libraries, which explicitly undefine such macros, and create them as inline functions instead.  This problem was already reported to upstream Python in 2011, as issue 10910 [3].

The approach there is to wrap the workaround in an #ifdef __cplusplus, but after some digging, I found out that the original bug in ctype, which for example gave back 'True' for isspace('\xa0'), has already been solved in October 2007 by Andrey Chernov, in r172619 [4].  The ctype fixes were MFC'd to stable/6 and stable/7 in r172929 [5].

I propose to add the attached patch to lang/python[27,33,34,35], which instead corrects the __FreeBSD_version check to the following:

#if (__FreeBSD_version >= 500040 && __FreeBSD_version < 602113) || \
    (__FreeBSD_version >= 700000 && __FreeBSD_version < 700054) || \
    (__FreeBSD_version >= 800000 && __FreeBSD_version < 800001)
# define _PY_PORT_CTYPE_UTF8_ISSUE
#endif

Alternatively, since we don't really support such old FreeBSD versions anymore, the whole check and workaround can be removed instead.  However, I also submitted the same change to upstream Python, in issue 10910, so hopefully this fix will appear in future Python releases.

[1] http://package18.nyi.freebsd.org/data/headamd64PR208158-default/2016-03-22_18h30m05s/logs/errors/py27-scipy-0.16.1.log
[2] https://hg.python.org/cpython/rev/adfe7d39a049
[3] http://bugs.python.org/issue10910
[4] https://svnweb.freebsd.org/base?view=revision&revision=172619
[5] https://svnweb.freebsd.org/base?view=revision&revision=172929

Comment 1 Dimitry Andric freebsd_committer

2016-04-09 13:53:36 UTC

So, any feedback on this?  It is a relatively minor change, and this will unblock all pyport.h-using ports when I import a new libc++ drop.

(Unfortunately upstream also didn't really pick up my report, but maybe this is because it is an issue which is already 5 years old...)

Comment 2 Mathieu Arnold freebsd_committer

2016-04-21 21:57:15 UTC

I don't quite understand the patch, it feels like it doesn't affect any supported version of FreeBSD, it seems easier to just remove the #ifdef __FreeBSD__ block.

Comment 3 Dimitry Andric freebsd_committer

2016-04-21 22:40:36 UTC

(In reply to Mathieu Arnold from comment #2)
> I don't quite understand the patch, it feels like it doesn't affect any
> supported version of FreeBSD, it seems easier to just remove the #ifdef
> __FreeBSD__ block.

Well, I didn't want to inconvenience anybody running some older version of FreeBSD, which is why I submitted this patch as-is upstream.

But I'm also just fine with deleting the whole #ifdef block, or for that matter the whole code part for the workaround.

Comment 4 Mathieu Arnold freebsd_committer

2016-04-22 07:35:13 UTC

Well, it's fine for upstream, you never know what antique version of OS they supports.  But for the ports tree, we only support building on 9.3, 10.1, 10.2, 10.3 and head, and no one can really use a release before 9.3 (or 9.something at least) because of infrastructure changes so it's not necessary.

Anyway, if you are certain nothing will break with this, it can be committed as-is, if you are not sure and things might break, just add the exp-run flag (at the top click on "more flags") it only takes a day or so.

Comment 5 Dimitry Andric freebsd_committer

2016-04-22 19:24:29 UTC

Created attachment 169578 [details]
Get rid of obsolete UTF-8 workaround for lang/python[27,33,34,35]

Let's just go for deleting the whole UTF-8 workaround then, since it is obsolete for all supported FreeBSD versions for ports.  I also bumped the port revisions, forcing dependents to use the fixed pyport.h.

Comment 6 Dimitry Andric freebsd_committer

2016-04-22 19:25:22 UTC

And just to be 100% sure nothing breaks, do an exp-run.

Comment 7 Antoine Brodin freebsd_committer

2016-04-23 08:49:20 UTC

This changes breaks at least databases/py-psycopg2 :
http://package22.nyi.freebsd.org/data/101i386-default-PR208486/2016-04-22_21h12m24s/logs/errors/py27-psycopg2-2.6.1_1.log

Comment 8 Antoine Brodin freebsd_committer

2016-04-23 08:56:16 UTC

(In reply to Antoine Brodin from comment #7)
The failure seems to be because osreldate.h is no longer included,  so the previous patch may be less intrusive

Comment 9 Dimitry Andric freebsd_committer

2016-04-23 12:23:20 UTC

(In reply to Antoine Brodin from comment #8)
> (In reply to Antoine Brodin from comment #7)
> The failure seems to be because osreldate.h is no longer included,

Hmm, indeed; psycopg2-2.6.1/psycopg/config.h has this fragment:

#if (defined(__FreeBSD__) && __FreeBSD_version < 503000) \
    || (defined(_WIN32) && !defined(__GNUC__)) \
    || (defined(sun) || defined(__sun__)) \
        && (defined(__SunOS_5_8) || defined(__SunOS_5_9))
/* what's this, we have no round function either? */
static double round(double num)
{
  return (num >= 0) ? floor(num + 0.5) : ceil(num - 0.5);
}
#endif

but it doesn't include <osreldate.h> by itself.  Apparently such python modules simply assume the existence of __FreeBSD_version, since it was always there.


> so the
> previous patch may be less intrusive

Ok, let's go for that one then.  I'll update it with revision bumps.

Comment 10 Dimitry Andric freebsd_committer

2016-04-23 12:27:38 UTC

Created attachment 169596 [details]
Fix python issue 10910 for lang/python[27,33,34,35], with bump

Comment 11 Antoine Brodin freebsd_committer

2016-04-25 19:12:46 UTC

Exp-run looks good, approved by portmgr.

Comment 12 commit-hook freebsd_committer

2016-04-25 20:23:01 UTC

A commit references this bug:

Author: dim
Date: Mon Apr 25 20:22:21 UTC 2016
New revision: 414029
URL: https://svnweb.freebsd.org/changeset/ports/414029

Log:
  For the various lang/python* ports, improve the __FreeBSD_version
  check in pyport.h for working around a very old ctype issue.

  If the workaround for this issue is enabled, pyport.h redefines
  toupper() and some other ctype macros, and this wreaks havoc when
  including newer libc++ headers (or any other system header which tries
  to declare those functions).

  Approved by:	portmgr (antoine)
  PR:		208486
  MFH:		2016Q2

Changes:
  head/lang/python27/Makefile
  head/lang/python27/files/patch-Include__pyport.h
  head/lang/python33/Makefile
  head/lang/python33/files/patch-Include__pyport.h
  head/lang/python34/Makefile
  head/lang/python34/files/patch-Include__pyport.h
  head/lang/python35/Makefile
  head/lang/python35/files/patch-Include__pyport.h

Comment 13 commit-hook freebsd_committer

2016-04-27 11:07:56 UTC

A commit references this bug:

Author: dim
Date: Wed Apr 27 11:07:22 UTC 2016
New revision: 414099
URL: https://svnweb.freebsd.org/changeset/ports/414099

Log:
  MFH: r414029

  For the various lang/python* ports, improve the __FreeBSD_version
  check in pyport.h for working around a very old ctype issue.

  If the workaround for this issue is enabled, pyport.h redefines
  toupper() and some other ctype macros, and this wreaks havoc when
  including newer libc++ headers (or any other system header which tries
  to declare those functions).

  Approved by:	portmgr (junovitch)
  PR:		208486

Changes:
_U  branches/2016Q2/
  branches/2016Q2/lang/python27/Makefile
  branches/2016Q2/lang/python27/files/patch-Include__pyport.h
  branches/2016Q2/lang/python33/Makefile
  branches/2016Q2/lang/python33/files/patch-Include__pyport.h
  branches/2016Q2/lang/python34/Makefile
  branches/2016Q2/lang/python34/files/patch-Include__pyport.h
  branches/2016Q2/lang/python35/Makefile
  branches/2016Q2/lang/python35/files/patch-Include__pyport.h