Details

Reviewers

koobs
sunpoet
antoine

Group Reviewers

Python

Commits

rP455585: add devel/py-backports - namespace shim for Python backports

Summary

All backports.* python modules install the same file.
The proposal is to remove this file from the packages
and install it separately. This is the first prototype.

Step 2 could be one of:
o provide USES=python:backports, which will automatically remove this file from plist and add dependency on devel/py-backports (and maybe additionally imply python:2)
o make USES=python react to PORTNAME and do the right thing
o patch all py-backports.* files to remove this file

I like the first approach above

Diff Detail

Repository

rP FreeBSD ports repository

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 9740
Build 10179: arc lint + arc unit

Event Timeline

lifanov created this revision.Jun 8 2017, 4:59 PM

Herald added a subscriber: mat. · View Herald TranscriptJun 8 2017, 4:59 PM

Harbormaster completed remote builds in B9740: Diff 29343.Jun 8 2017, 4:59 PM

lantw44_gmail.com added a subscriber: lantw44_gmail.com.Jun 9 2017, 12:30 PM

mat added inline comments.Jun 9 2017, 1:10 PM

devel/py-backports/Makefile
18–20	Never use %%foo%% placeholders in PLIST_FILES, always use the variable it comes from, so, here, ${PYTHON_SITELIBDIR} and ${PYTHON_PYOEXTENSION}.
devel/py-backports/pkg-descr
1	pkg-descr must, must, be longer than COMMENT, at least a couple of lines.

address mat's feedback:

o use variables in PLIST_FILES directly
o provide package description that is more descriptive than the comment

Harbormaster completed remote builds in B10514: Diff 30843.Jul 16 2017, 10:37 PM

lifanov marked 2 inline comments as done.Jul 16 2017, 10:37 PM

lifanov edited the summary of this revision. (Show Details)Jul 16 2017, 10:40 PM

lifanov added a reviewer: sunpoet.

lifanov edited the summary of this revision. (Show Details)Jul 16 2017, 10:43 PM

mat added inline comments.Jul 17 2017, 1:18 PM

devel/py-backports/Makefile

It just a thought, but this will probably not do what you think:

$ echo '__path__ = __import__('pkgutil').extend_path(__path__, __name__)'
__path__ = __import__(pkgutil).extend_path(__path__, __name__)

Notice the lack of quotes. Instead of playing around with quotes, it might be better to drop the file in the files directory and INSTALL_DATA it.

Also, never, ever, use ${ECHO} in the ports tree. From Mk/bsd.commands.mk:

# ECHO is defined in /usr/share/mk/sys.mk, which can either be "echo",
# or "true" if the make flag -s is given.  Use ECHO_CMD where you mean
# the echo command.
ECHO_CMD?=              echo    # Shell builtin

# Used to print all the '===>' style prompts - override this to turn them off.
ECHO_MSG?=              ${ECHO_CMD}

address mat's feedback:

o use ${ECHO_CMD} instead of ${ECHO}, which is defined in mk/sys.mk
o fix quoting around 'pkgutil'

I would like to not create a separate file in files/ for just one line
of content, but I don't feel strongly about it either.

Owners added a reviewer: O5: Ports Framework.Jul 17 2017, 1:32 PM

Herald added a reviewer: portmgr. · View Herald TranscriptJul 17 2017, 1:32 PM

Harbormaster completed remote builds in B10523: Diff 30861.Jul 17 2017, 1:32 PM

whoops, included too much

Harbormaster completed remote builds in B10524: Diff 30862.Jul 17 2017, 1:33 PM

What is holding this?

lwhsu added a subscriber: lwhsu.Jul 20 2017, 10:50 PM

Here is an idea for a different approach.

Have a package manager silently ignore file conflicts when files are identical. pkg can keep track of how many times that file is installed, and remove it after the last package that installed it is removed. This can be implemented with a simple reference count.

Then py-backports-* can just make sure the contents are the same.

Pros: 1. Solves this py-backports-* problem without creating extra packages, 2. might be useful in some other situations.
Cons: Looks like a far-fetched, fancy feature.

The absence of something holding a change/proposal is necessary but insufficient (alone) to promote a change.

In this case, sufficient review and investigation of potential alternatives/improvements

In D11095#242627, @yuri_rawbw.com wrote:

Here is an idea for a different approach.

Have a package manager silently ignore file conflicts when files are identical. pkg can keep track of how many times that file is installed, and remove it after the last package that installed it is removed. This can be implemented with a simple reference count.

Then py-backports-* can just make sure the contents are the same.

Pros: 1. Solves this py-backports-* problem without creating extra packages, 2. might be useful in some other situations.
Cons: Looks like a far-fetched, fancy feature.

-1 on pkg being the place to solve this. Though there may be merit in improved conflicts handling (in certain limited cases) in pkg in general, I'd like not to have the path of least resistance for porters/maintainers be let a tool handle an issue rather than do it properly (where properly means ports should not conflict in the first place)

@lifanov Could you add an example consumer port to this changeset please

In D11095#242930, @koobs wrote:

-1 on pkg being the place to solve this. Though there may be merit in improved conflicts handling (in certain limited cases) in pkg in general, I'd like not to have the path of least resistance for porters/maintainers be let a tool handle an issue rather than do it properly (where properly means ports should not conflict in the first place)

Least resistance wasn't my motivation. Adding a new pkg feature is actually many times more work, and longer wait. I really believe that this feature is reasonable to have in pkg, and can be useful in other cases as well.

In D11095#242931, @yuri_rawbw.com wrote:

In D11095#242930, @koobs wrote:

-1 on pkg being the place to solve this. Though there may be merit in improved conflicts handling (in certain limited cases) in pkg in general, I'd like not to have the path of least resistance for porters/maintainers be let a tool handle an issue rather than do it properly (where properly means ports should not conflict in the first place)

Least resistance wasn't my motivation. Adding a new pkg feature is actually many times more work, and longer wait. I really believe that this feature is reasonable to have in pkg, and can be useful in other cases as well.

I meant as a potential downside symptom, not that that was an intent or desirable property of the proposal

koobs edited the summary of this revision. (Show Details)Jul 25 2017, 3:38 AM

antoine added a subscriber: antoine.Aug 27 2017, 8:30 PM

antoine added inline comments.

devel/py-backports/Makefile
27	The correct way to compileall is: ${PYTHON_CMD} -m compileall -d ${PYTHON_SITELIBDIR} ${STAGEDIR}${PYTHON_SITELIBDIR} ${PYTHON_CMD} -O -m compileall -d ${PYTHON_SITELIBDIR} ${STAGEDIR}${PYTHON_SITELIBDIR}

antoine added inline comments.Aug 27 2017, 8:37 PM

devel/py-backports/Makefile
27	And yes, more than half of the ports do it wrong... sigh...

o use compileall with -d flag
o add a sample consumer to the patch

Harbormaster completed remote builds in B11266: Diff 32443.Aug 28 2017, 1:20 PM

lifanov marked 3 inline comments as done.Aug 28 2017, 1:21 PM

antoine added inline comments.Aug 28 2017, 1:23 PM

devel/py-backports.csv/Makefile
23 ↗	(On Diff #32443)	Removing it from stagedir is not enough, you have to remove it from plist too.

I would like some help with filtering files out of generated plist. I can't figure out how to do this. I tested conversion of a port that actually needs this and it *is* necessary.

antoine added inline comments.Aug 29 2017, 2:01 PM

devel/py-backports.csv/Makefile
23 ↗	(On Diff #32443)	Note that backports.csv is a bad example, it doesn't install any __init__.py

Yep. I tried it with security/py-backports.ssl_match_hostname and... it doesn't work.
I'll switch the test port to that one once I figure out a correct fix.

I *could* just sed it out of egginfo post-extract. Is this a good idea?

In D11095#252326, @lifanov wrote:

I would like some help with filtering files out of generated plist. I can't figure out how to do this. I tested conversion of a port that actually needs this and it *is* necessary.

You may use something like:

POST_PLIST= remove-init-from-plist
remove-init-from-plist:

${REINPLACE_CMD} '/backports\/__init__.py/d' ${TMPPLIST}

Note that with python 3 some files are installed differently, in backports/__pycache__/__init__.*

Cool, thanks! The backports.* stuff is backports of Python 3 features to Python 2, so these should not be necessary for Python 3 at all.

In D11095#252330, @lifanov wrote:

Cool, thanks! The backports.* stuff is backports of Python 3 features to Python 2, so these should not be necessary for Python 3 at all.

No, for instance backports.ssl_match_hostname is a backport from python3.5 so it is necessary on python3.3 and python3.4

Ahh, you're right!

leres_ee.lbl.gov added a subscriber: leres_ee.lbl.gov.Sep 4 2017, 8:32 PM

FYI openbsd ports solved the problem by adding py-backports too : http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/devel/py-backports/

leres_ee.lbl.gov removed a subscriber: leres_ee.lbl.gov.Sep 11 2017, 4:30 PM

leres added a subscriber: leres.Sep 11 2017, 4:31 PM

Oooh, there is a Gentoo mirror for this package. Hold on while I shamelessly rip off the OpenBSD port...

fix stage and plist issues using a post-stage and post-plist target

really fix for and test with Python 3

Harbormaster completed remote builds in B11497: Diff 32928.Sep 11 2017, 5:49 PM

At this point, the patch works and I can commit this port and security/py-backports.ssl_match_hostname if they are ready.

Are we going to get to the point when it's possible to have devel/pylint and devel/ipython installed at the same time again?

Is there some progress on this issue?

With the current ports tree I have the problem with backports.shutil_get_terminal_size vs backports.functools_lru_cache:

pkg-static: py27-backports.shutil_get_terminal_size-1.0.0 conflicts with py27-backports.functools_lru_cache-1.4 (installs files into the same place). Problematic file: /usr/local/lib/python2.7/site-packages/backports/__init__.py

I updated your patch and put it on https://people.freebsd.org/~antoine/ports/backports.diff

Thanks Antoine!

I was waiting for an approval from someone from Python.
I will re-test your patch and update this revision.

update patch with the one fixed by antoine

This includes conversion of two more ports:
devel/py-backports.functools_lru_cache
devel/py-backports.shutil_get_terminal_size

I tested it and all ports work with all flavors.

Harbormaster completed remote builds in B13352: Diff 36238.Dec 5 2017, 2:37 PM

Your patch works, thank you!
This is pending approval by Python.

You can commit approved by portmgr.

Closed by commit rP455585: add devel/py-backports - namespace shim for Python backports (authored by lifanov). · Explain WhyDec 5 2017, 3:00 PM

This revision was automatically updated to reflect the committed changes.

Thank you!