Bug 205960 - lang/python35 lang/python34: Backport upstream issue 24557
Summary: lang/python35 lang/python34: Backport upstream issue 24557
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Bernard Spil
URL: http://bugs.python.org/issue24557
Keywords: patch
Depends on:
Blocks: 199050
  Show dependency treegraph
 
Reported: 2016-01-06 16:24 UTC by Mark.Martinec
Modified: 2016-07-09 12:50 UTC (History)
8 users (show)

See Also:
koobs: maintainer-feedback+
koobs: merge-quarterly+


Attachments
svn diff for lang/python35 (2.96 KB, patch)
2016-01-06 20:40 UTC, Bernard Spil
no flags Details | Diff
patch-Modules__ssl.c(python34) (690 bytes, patch)
2016-07-04 23:43 UTC, Walter Schwarzenfeld
no flags Details | Diff
ac_cv_lib_crypto_RAND_egd=no (858 bytes, patch)
2016-07-05 16:01 UTC, Adam Weinberger
no flags Details | Diff
svn diff for lang/python35 (3.17 KB, patch)
2016-07-05 20:18 UTC, Bernard Spil
no flags Details | Diff
svn diff for lang/python35 Additionally: - USE_OPENSSL -> USES= ssl - CPE_VERSION defaults to PORTVERSION -> remove (2.69 KB, patch)
2016-07-07 09:24 UTC, Bernard Spil
koobs: maintainer-approval-
Details | Diff
svn diff for lang/python34 (1.83 KB, patch)
2016-07-07 09:25 UTC, Bernard Spil
koobs: maintainer-approval+
Details | Diff
svn diff for lang/python3* (307 bytes, patch)
2016-07-08 16:07 UTC, Bernard Spil
no flags Details | Diff
svn diff for lang/python3* (5.19 KB, patch)
2016-07-08 16:37 UTC, Bernard Spil
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark.Martinec 2016-01-06 16:24:49 UTC
This seems to be the same issue as reported in Bug 195513
for lang/python32 :

  https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=195513
  https://svnweb.freebsd.org/changeset/ports/404378

Apparently the fix was only applied to lang/python32 but not
to python34 and python35 ports. (not sure about python33,
haven't tried it).


Full pudriere logs for python34 and python35 are at:

  https://www.ijs.si/~mark/tmp/python34-3.4.4.log
  https://www.ijs.si/~mark/tmp/python35-3.5.1.log

Relevant excerpts from a poudriere build:

building '_ssl' extension
cc -fPIC -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -Werror=declaration-after-statement -I./Include -I. -IInclude -I/usr/local/include -I/wrkdirs/usr/ports/lang/python35/work/Python-3.5.1/Include -I/wrkdirs/usr/ports/lang/python35/work/Python-3.5.1 -c /wrkdirs/usr/ports/lang/python35/work/Python-3.5.1/Modules/_ssl.c -o build/temp.freebsd-10.2-RELEASE-p7-amd64-3.5/wrkdirs/usr/ports/lang/python35/work/Python-3.5.1/Modules/_ssl.o
/wrkdirs/usr/ports/lang/python35/work/Python-3.5.1/Modules/_ssl.c:3935:17: warning: implicit declaration of function 'RAND_egd' is invalid in C99 [-Wimplicit-function-declaration]
    int bytes = RAND_egd(PyBytes_AsString(path));
                ^
1 warning generated.
cc -shared -lpthread -Wl,-rpath,/usr/local/lib -Wl,-rpath=/usr/lib:/usr/local/lib -fstack-protector -lpthread -Wl,-rpath,/usr/local/lib -Wl,-rpath=/usr/lib:/usr/local/lib -fstack-protector -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing build/temp.freebsd-10.2-RELEASE-p7-amd64-3.5/wrkdirs/usr/ports/lang/python35/work/Python-3.5.1/Modules/_ssl.o -L. -L/usr/local/lib -lssl -lcrypto -lpython3.5m -o build/lib.freebsd-10.2-RELEASE-p7-amd64-3.5/_ssl.so
*** WARNING: renaming "_ssl" since importing it failed: build/lib.freebsd-10.2-RELEASE-p7-amd64-3.5/_ssl.so: Undefined symbol "RAND_egd"

[...]
Following modules built successfully but were removed because they could not be imported:
_ssl

[...]
===>  Building package for python35-3.5.1
pkg-static: Unable to access file /wrkdirs/usr/ports/lang/python35/work/stage/usr/local/lib/python3.5/lib-dynload/_ssl.so: No such file or directory
*** Error code 1
Comment 1 Bernard Spil freebsd_committer freebsd_triage 2016-01-06 19:04:07 UTC
Hi Mark,

Just checked the 3.4 sources and the required changes for LibreSSL are in the tarball (same applies to 3.5). Can you please send the config.log and config.h? There's code in configure to test for the existence of the RAND_egd method in libcrypto which must have somehow failed. Additionally, please provide your /etc/make.conf

As a fix for this in  Modules/_ssl.c you could replace
#ifdef HAVE_RAND_EGD
with
#ifndef OPENSSL_NO_EGD
(NB ifdef -> ifNdef) that should work for sure.

I still need to upstream this improvement but the result should be exactly the same.
Comment 2 Mark.Martinec 2016-01-06 20:02:12 UTC
> Just checked the 3.4 sources and the required changes for LibreSSL
> are in the tarball (same applies to 3.5). Can you please send the
> config.log and config.h? There's code in configure to test for
> the existence of the RAND_egd method in libcrypto which must have
> somehow failed.

Can't find config.h (will look some more), but here is
the config.log (this time the 'make' was run interactively
in /usr/ports/lang/python35, resulting in the same failure
as in poudriere) :

  https://www.ijs.si/~mark/tmp/config.log


> Additionally, please provide your /etc/make.conf

The of makefiles can be seen near the beginning of the provided
build log files (follow links), between "---Begin make.conf---"
and "---End make.conf---".
Comment 3 VK 2016-01-06 20:17:12 UTC
I can't replicate this, I have Python 3.5.1 building happily against LibreSSL. Latest build log:

http://cherri.irealone.hr/poudriere/data/102x64-default-servers/2016-01-06_17h28m32s/logs/python35-3.5.1.log
Comment 4 Bernard Spil freebsd_committer freebsd_triage 2016-01-06 20:40:46 UTC
Created attachment 165181 [details]
svn diff for lang/python35

There's definitely something wrong with the CFLAGS, configure is using the libs from base not ports.

configure:9276: checking for RAND_egd in -lcrypto
configure:9301: cc -o conftest -O2 -pipe -fno-omit-frame-pointer  -fstack-protector -fno-strict-aliasing   -lpthread -Wl,-rpath,/usr/local/lib -Wl,-rpath=/usr/lib:/usr/local/lib -fstack-protector conftest.c -lcrypto   >&5
configure:9301: $? = 0
configure:9310: result: yes

This patch changes behaviour to use the define from opensslfeatures.h. At build time the correct include is done.
Comment 5 Bernard Spil freebsd_committer freebsd_triage 2016-01-06 20:41:43 UTC
I've just replicated this behaviour on my own system in a build jail (not poudriere) as well. After applying the patch the build is OK. Still to upstream!
Comment 6 Bernard Spil freebsd_committer freebsd_triage 2016-01-06 20:57:05 UTC
Just checked, this build works just fine with security/libressl-devel on PC-BSD
https://builds.pcbsd.org/avenger/data/pcbsd-101-RELEASE-master/latest-per-pkg/python35-3.5.0_1.log

I recall this bug from early on when fixing LibreSSL issues and that was fixed with passing extra flags for configure. The currently attached patch is preferred.
Comment 7 Adam Weinberger freebsd_committer freebsd_triage 2016-03-07 17:53:54 UTC
Can somebody please commit this patch? I'd really like to use python35.
Comment 8 Pierre Guinoiseau 2016-05-05 10:36:17 UTC
The build still fails with the latest libressl, can someone commit this patch?
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2016-06-27 01:04:11 UTC
Re-assign to Bernard, who submitted the patch. I'm still concerned about the global CFLAGS addition and I don't have time to investigate potential issues. I don't know believe it's necessary given libressl is a drop in replacement for openssl (in ports)
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2016-06-27 01:04:45 UTC
Apologies for the delay.
Comment 11 freebsd 2016-07-04 16:53:51 UTC
python34-3.4.5 compiles fine with LibreSSL on my system, but fails on make install with the _ssl.so message when NLS option is deactivated. When NLS is active it installs without a problem.
Comment 12 Walter Schwarzenfeld 2016-07-04 23:43:18 UTC
Created attachment 172123 [details]
patch-Modules__ssl.c(python34)
Comment 13 Walter Schwarzenfeld 2016-07-04 23:45:00 UTC
Python34 only needs this one patched files (little modification of python35 patch, no change in the Makefile).
Comment 14 Adam Weinberger freebsd_committer freebsd_triage 2016-07-05 02:33:09 UTC
I still feel like comment #4 is the underlying problem here. configure is detecting RAND_egd when libressl is installed, which means it's testing it from /usr/lib/libcrypto.so, not /usr/local/lib/libcrypto.so.

Could the problem be that at around line ~9500 in configure, it's testing:

  LIBS="-lcrypto $LIBS"

Does changing that to
  LIBS="%%LDFLAGS%% -lcrypto $LIBS"
or
  LIBS="$LIBS -lcrypto"
or
  LIBS="$LIBS %%LDFLAGS%% -lcrypto"

help it detect it correctly in the first place?
Comment 15 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-05 09:33:33 UTC
-1 on change in attachment 165181 [details]

Any solution should avoid the further use of global {C,CPP,LD}FLAGS modifications as it's a solution too widely scoped for the problem and has caused issues in the past, such as identifying other libraries (we don't want found) in LOCALBASE, or include order issues causing failures in other modules (see the comment in the python3* port makefile)

The reason why this fails when NLS is disabled is the following line in the Makefile:

NLS_CPPFLAGS= -I${LOCALBASE}/include

When NLS is enabled (the default), I believe the ssl libraries from LOCALBASE are found and used. This doesn't however mean that the solution is to modify global CFLAGS.

Long term, this use of CPPFLAGS for gettext needs to go as well, and having another part of the port use it (or CFLAGS) makes that more difficult

For further information, see:

https://svnweb.freebsd.org/ports?view=revision&revision=326729
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=181721

I haven't reviewed or tested attachment 172123 [details] provided by Walter
Comment 16 Adam Weinberger freebsd_committer freebsd_triage 2016-07-05 16:01:16 UTC
Created attachment 172145 [details]
ac_cv_lib_crypto_RAND_egd=no

As another option, if python is building against LibreSSL, you could just tell configure that it doesn't have RAND_egd. Attaching a patch that demonstrates this.
Comment 17 Walter Schwarzenfeld 2016-07-05 16:44:38 UTC
I obsolete my patch. Adam Weinbergers patch seems to work on python34 and python35. I think is a more clear way to solve it (and better to change if needed in the future).
Comment 18 Bernard Spil freebsd_committer freebsd_triage 2016-07-05 20:18:14 UTC
Created attachment 172152 [details]
svn diff for lang/python35

(In reply to w.schwarzenfeld from comment #17)

Adam's patch would indeed work. That just forces EGD to be disabled in configure.

The permanent (and correct) solution is to use my patch but discard the Makefile change (CFLAGS) as that's not required.

Both OpenSSL 1.1 and LibreSSL define OPENSSL_NO_EGD (just as they use OPENSSL_NO_* for any other removed/disabled feature)

Python can be a bit slow in adopting these changes, let me poke them again.
http://bugs.python.org/issue24557
https://github.com/python/cpython/pull/26

Cheers,

Bernard
Comment 19 Adam Weinberger freebsd_committer freebsd_triage 2016-07-06 17:18:28 UTC
(In reply to Bernard Spil from comment #18)

> Both OpenSSL 1.1 and LibreSSL define OPENSSL_NO_EGD (just as they use
> OPENSSL_NO_* for any other removed/disabled feature)

That hadn't clicked for me. In that case, then I completely agree with you that your patch is the correct method.
Comment 20 Walter Schwarzenfeld 2016-07-06 17:35:42 UTC
If we take this patch for python35 I have to "reactivate" my patch for python34 cause it is different (the python35 patch does not work for python34).
Comment 21 Bernard Spil freebsd_committer freebsd_triage 2016-07-07 09:24:11 UTC
Created attachment 172190 [details]
svn diff for lang/python35

Additionally:
  - USE_OPENSSL -> USES= ssl
  - CPE_VERSION defaults to PORTVERSION -> remove

Updated patch with upstream commit

The configure changes are harmless, do not affect build/run
Comment 22 Bernard Spil freebsd_committer freebsd_triage 2016-07-07 09:25:49 UTC
Created attachment 172191 [details]
svn diff for lang/python34

Based on the upstream commit

Additionally
  - USE_OPENSSL -> USES= ssl
  - CPE_VERSION defaults to PORTVERSION so remove
Comment 23 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-08 15:03:31 UTC
CPE_VERSION should remain as DISTVERSION (its a public Python version identifier)

Please confirm these changes also pass QA (portlint, poudriere)
Comment 24 Bernard Spil freebsd_committer freebsd_triage 2016-07-08 16:07:37 UTC
Created attachment 172245 [details]
svn diff for lang/python3*

- Combine into single patch
- Revert change for CPE_VERSION in python35

lang/python3*: Fix build issues without EGD

  - Add backport of patch for EGD issue
  - Change USE_OPENSSL to USES= ssl
  - Don't set CPE_VERSION to the default PORTVERSION
Comment 25 Bernard Spil freebsd_committer freebsd_triage 2016-07-08 16:37:33 UTC
Created attachment 172247 [details]
svn diff for lang/python3*
Comment 26 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-08 16:42:38 UTC
Comment on attachment 172247 [details]
svn diff for lang/python3*

Approved, pending QA (poudriere) on all supported versions/archs, as this will be MFH'd
Comment 27 commit-hook freebsd_committer freebsd_triage 2016-07-08 19:08:10 UTC
A commit references this bug:

Author: brnrd
Date: Fri Jul  8 19:07:42 UTC 2016
New revision: 418240
URL: https://svnweb.freebsd.org/changeset/ports/418240

Log:
  lang/python3*: Backport upstream issue 24557

    - Add backport of patch for EGD issue [1]
    - Change USE_OPENSSL to USES= ssl
    - Don't set CPE_VERSION to the default PORTVERSION

  PR:		205960 [1], 199050 [1]
  Reviewed by:	koobs (python)
  Approved by:	python (koobs)
  Obtained from:	https://hg.python.org/cpython/rev/7c0432cf1f2e [1]
  MFH:		2016Q3

Changes:
  head/lang/python33/Makefile
  head/lang/python34/Makefile
  head/lang/python34/files/patch-issue24557
  head/lang/python35/Makefile
  head/lang/python35/files/patch-issue24557
Comment 28 commit-hook freebsd_committer freebsd_triage 2016-07-08 20:09:21 UTC
A commit references this bug:

Author: brnrd
Date: Fri Jul  8 20:08:48 UTC 2016
New revision: 418241
URL: https://svnweb.freebsd.org/changeset/ports/418241

Log:
  MFH: r418240

  lang/python3*: Backport upstream issue 24557

    - Add backport of patch for EGD issue [1]
    - Change USE_OPENSSL to USES= ssl
    - Don't set CPE_VERSION to the default PORTVERSION

  PR:		205960 [1], 199050 [1]
  Reviewed by:	koobs (python)
  Approved by:	python (koobs)
  Obtained from:	https://hg.python.org/cpython/rev/7c0432cf1f2e [1]

  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2016Q3/
  branches/2016Q3/lang/python33/Makefile
  branches/2016Q3/lang/python34/Makefile
  branches/2016Q3/lang/python34/files/patch-issue24557
  branches/2016Q3/lang/python35/Makefile
  branches/2016Q3/lang/python35/files/patch-issue24557
Comment 29 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-09 12:50:34 UTC
Correctly classify post-resolution (mfh+)