Bug 243937 - lang/python38: Fails to package in certain conditions due to DISABLED_EXTENSIONS not working
Summary: lang/python38: Fails to package in certain conditions due to DISABLED_EXTENSI...
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Kubilay Kocak
URL:
Keywords: buildisok, needs-qa
Depends on:
Blocks:
 
Reported: 2020-02-06 19:28 UTC by John Hein
Modified: 2020-07-15 18:20 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (python)


Attachments
[patch] fix broken disable modules (v0) (1.45 KB, patch)
2020-02-06 19:28 UTC, John Hein
no flags Details | Diff
[patch] fix broken disable modules (v1) (995 bytes, patch)
2020-02-06 19:29 UTC, John Hein
no flags Details | Diff
[patch] fix broken disable modules (v2) (1019 bytes, patch)
2020-02-07 04:01 UTC, John Hein
jcfyecrayz: maintainer-approval? (python)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2020-02-06 19:28:22 UTC
Created attachment 211420 [details]
[patch] fix broken disable modules (v0)

make check-plist gives errors for lang/python38 ...


====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: lib/python%%XYDOT%%/lib-dynload/_gdbm.so
Error: Orphaned: lib/python%%XYDOT%%/lib-dynload/_sqlite3.so
Error: Orphaned: lib/python%%XYDOT%%/lib-dynload/_tkinter.so
===> Checking for items in pkg-plist which are not in STAGEDIR
===> Error: Plist issues found.
*** Error code 1


The attached patch fixes the check-plist error (and avoids building the modules saving a bit of build time) by using Modules/Setup.

See also https://bugs.python.org/issue20210

The fix could also be applied to python37 (and then we could remove the files/patch-issue20210 and DISABLED_EXTENSIONS in Makefile).
Comment 1 John Hein 2020-02-06 19:29:51 UTC
Created attachment 211421 [details]
[patch] fix broken disable modules (v1)

Here's another version using Setup.local.  Pick your poison.
Comment 2 John Hein 2020-02-06 19:30:58 UTC
Neither patch should require a PORTREVISION bump.
Comment 3 John Hein 2020-02-07 02:35:34 UTC
QA:

 - v1 patch ok in poudriere 11-stable/amd64.

 - portlint:

   That patch (v1) should use ${PRINTF} vs. printf.  Noticed by portlint.
Comment 4 John Hein 2020-02-07 04:01:37 UTC
Created attachment 211437 [details]
[patch] fix broken disable modules (v2)

v2:

 - Use ${PRINTF} (as suggested by portlint)

 - Use DISABLED_EXTENSIONS in the printf so now the WITHOUT_NIS case works for python38, too (it doesn't before this patch for the same reason that disabling the other extensions did not work).

Confirmed in poudriere with and without WITHOUT_NIS set.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2020-02-07 10:52:38 UTC
Thank you for the report John

Could you clarify if the origin of this issue was identified? Was it a port or upstream change that regressed the DISABLED_MODULES functionality? Are any other lang/python* versions affected in the same way?

Also, if I understand correctly, these modules will build (and result in pkg-plist) errors, if the associated third party headers/libraries are installed (such as tkinter, gdbm, etc), is that correct.
Comment 6 John Hein 2020-02-07 11:44:26 UTC
(In reply to Kubilay Kocak from comment #5)
python37 has patch-issue20210 to "solve" how to disable building/installing the extensions.  That got repocopied from python36 and python35/34 before that.  You probably already know this.

I don't know why it was not pulled into python38.  Maybe the patch no longer applied?  I didn't check.  But from reading the upstream issue 20210, it's clear they went another way than the original patch we've been carrying along.  The  *disabled* marker method in Modules/Setup* was committed (looks like it was committed in python37 in 2017).  So I think using that going forward is the right way to go (instead of trying to resurrect the old patch which upstream decided not to use).

I just read bug 241416.  The extensions problem was known and deferred (until someone could develop the fix).  So I guess it just hadn't gotten addressed yet.

I think (but have not tried) that this patch would work for our latest python37.  We could choose to apply it there as well or just live with the old patch as long as it still works.  pro for the new patch in python37: future python37 updates might break the old patch.  con argument: unnecessary work/thrash (wait until current flavor stops working for py37, if ever).  In any case, that decision can be separated from this fix for py38.

Re: your second question re: third party headers.  I don't know if having gdbm, et. al., ports installed before building py38 affects whether those extensions are built or not.  I suspect it does, but I have not done that test to confirm.  In my environment, they were installed.
Comment 7 John Hein 2020-02-07 12:10:55 UTC
(In reply to John Hein from comment #6)
I just tried poudriere with an unpatched python38 without having gdb, sqlite, tk ports installed.  The extensions were not built, and thus no check-plist error, confirming our suspicions. 

WITHOUT_NIS=1 does not prevent building/installing nis.so, however.  That's to be expected since (before the patch here) DISABLED_EXTENSIONS does nothing for py38 (which has nothing like patch-issue20210 in python37, and earlier, that will use DISABLED_EXTENSIONS).  There is not a plist error since @comment in the plist just makes plist checking ignore the nis.so file in the staging area.  So it's just a little extra wasted build time.  But the current v2 patch here solves that, too.
Comment 8 Automation User 2020-04-10 01:28:36 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/134601403
Comment 9 John Hein 2020-07-15 18:16:08 UTC
ping.

This still works as intended with 3.8.4, by the way.
Comment 10 John Hein 2020-07-15 18:20:44 UTC
And regarding 'needs-qa', this passed testing and works as intended with various options set in poudriere (lang/python38, databases/py-sqlite3, databases/py-gdbm, . x11-toolkits/py-tkinter and FLAVOR=py38).