Page MenuHomeFreeBSD

add shared py-backports namespace file for backports.* modules
ClosedPublic

Authored by lifanov on Jun 8 2017, 4:59 PM.

Details

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

See Also:

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lifanov created this revision.Jun 8 2017, 4:59 PM
mat added inline comments.Jun 9 2017, 1:10 PM
devel/py-backports/Makefile
18–20 ↗(On Diff #29343)

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 ↗(On Diff #29343)

pkg-descr must, must, be longer than COMMENT, at least a couple of lines.

lifanov updated this revision to Diff 30843.Jul 16 2017, 10:37 PM

address mat's feedback:

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

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
24 ↗(On Diff #30843)

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}
lifanov updated this revision to Diff 30861.Jul 17 2017, 1:32 PM

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.

lifanov updated this revision to Diff 30862.Jul 17 2017, 1:33 PM

whoops, included too much

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.

koobs edited edge metadata.Jul 25 2017, 3:20 AM

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

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

-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.

koobs edited the summary of this revision. (Show Details)Jul 25 2017, 3:32 AM
koobs edited reviewers, added: Python; removed: O5: Ports Framework, portmgr.

-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 inline comments.
devel/py-backports/Makefile
26 ↗(On Diff #30862)

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
26 ↗(On Diff #30862)

And yes, more than half of the ports do it wrong... sigh...

lifanov updated this revision to Diff 32443.Aug 28 2017, 1:20 PM

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

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.

lifanov added a comment.EditedAug 29 2017, 1:54 PM

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?

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.

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!

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

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...

lifanov updated this revision to Diff 32924.Sep 11 2017, 5:24 PM

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

lifanov updated this revision to Diff 32928.Sep 11 2017, 5:49 PM

really fix for and test with Python 3

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

Thanks Antoine!

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

lifanov updated this revision to Diff 36238.Dec 5 2017, 2:36 PM

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.

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

antoine accepted this revision.Dec 5 2017, 2:38 PM

You can commit approved by portmgr.

This revision was automatically updated to reflect the committed changes.