Bug 257111 - databases/py-apsw: Add option to enable EXTENSION loading
Summary: databases/py-apsw: Add option to enable EXTENSION loading
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: Po-Chuan Hsieh
URL:
Keywords: needs-qa
Depends on:
Blocks: 257112
  Show dependency treegraph
 
Reported: 2021-07-11 15:30 UTC by Guido Falsi
Modified: 2021-07-20 14:35 UTC (History)
1 user (show)

See Also:
sunpoet: maintainer-feedback+


Attachments
Proposed patch (925 bytes, patch)
2021-07-11 15:30 UTC, Guido Falsi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guido Falsi freebsd_committer 2021-07-11 15:30:09 UTC
Created attachment 226371 [details]
Proposed patch

Looks like this software enables by default EXPERIMENTAL features, but disables extension loading support.

I've added an option to the port, enabled by default, just like the sqlite3 port, to enable such feature.

This feature is required by deskutils/calibre new version 5.23, as pointed out by the developer:

https://bugs.launchpad.net/calibre/+bug/1935747

I hope this can be imported as is.

Thanks in advance.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-12 02:43:37 UTC
What is the difference for users for a build without EXTENSION support? 

In normal cases, if a package depends on another packages 'optional' characteristics or dependencies, the 'requiring' port should depend on the transitive dependencies directly. Unfortunately this option is a build time change, which can't be leveraged for that.

Ports that depend on other ports OPTION'al semantics can be problematic, such as when users disable those options without realising the consequences, and ports/pkg does not have a mechanism to make users aware of these scenarios: 'Hey, this package needs this other packages 'feature', which is not included in this package build'

If there isn't a sufficiently compelling reason to disable extension loading, I'd suggest/propose removing it.
Comment 2 Guido Falsi freebsd_committer 2021-07-12 07:23:55 UTC
(In reply to Kubilay Kocak from comment #1)

As I reported in the linked bug report upstream (adding it to see also) calibre will simply fail to start if load_extension is not enabled in py-apsw.

I proposed this patch with an OPTION because it looked reasonable to me. I can leave the option and propose also a slave port, forcing the option on, for calibre to depend on, or also do the same removing options and using flavors (not sure which way is the preferred one at present).

Since the slave port or the flavors would be in conflict with each other these also look problematic though.

I'm also ok with removing the OPTION and make extension loading always on simply adding an unconditional PYDISTUTILS_BUILDARGS+=--enable=load_extension to the port. Not sure how well this would play if the EXTENSION option is disabled in sqlite3 port though.
Comment 3 Po-Chuan Hsieh freebsd_committer 2021-07-15 20:43:41 UTC
LGTM. Thanks. IMHO, an option is just fine. It is on by default since calibre requires it. And users can disable it if necessary.
Comment 4 Guido Falsi freebsd_committer 2021-07-16 10:54:21 UTC
(In reply to Po-Chuan Hsieh from comment #3)

So I consider this as maintainer approval and commit my patch adding the option?
Comment 5 Po-Chuan Hsieh freebsd_committer 2021-07-19 13:06:00 UTC
(In reply to Guido Falsi from comment #4)

Yes, I changed maintainer-feedback flag to +. Thanks!
Comment 6 commit-hook freebsd_committer 2021-07-20 14:33:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=da566bc8534e93a6c9c15e3db46c315d3cd96a62

commit da566bc8534e93a6c9c15e3db46c315d3cd96a62
Author:     Guido Falsi <madpilot@FreeBSD.org>
AuthorDate: 2021-07-20 14:28:42 +0000
Commit:     Guido Falsi <madpilot@FreeBSD.org>
CommitDate: 2021-07-20 14:28:42 +0000

    databases/py-apsw: Add option to enable EXTENSION loading.

    The new option adds support to load DB extensions at runtime.

    The option is enabled by default because it is required by the
    latest version of deskutils/calibre.

    PR:             257111
    Approved by:    sunpoet (maintainer)

 databases/py-apsw/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 7 Guido Falsi freebsd_committer 2021-07-20 14:35:09 UTC
Patch committed.

Thanks!