Adding python-minimal to plugins that need it #316

Merged
merged 1 commit into from Feb 11, 2016

Conversation

Projects
None yet
3 participants
Collaborator

sergiusens commented Feb 11, 2016

python-minimal is needed by the python2 and the catkin plugin.

LP: #1541451

Signed-off-by: Sergio Schvezov sergio.schvezov@ubuntu.com

Member

elopio commented Feb 11, 2016

lgtm. The tests failed because of a hash mismatch. Retriggered...

snapcraft/plugins/python2.py
@@ -67,6 +67,7 @@ def __init__(self, name, options):
super().__init__(name, options)
self.stage_packages.extend([
'python-dev',
+ 'python-minimal',
@kyrofa

kyrofa Feb 11, 2016

Member

I'm afraid the fix for this one isn't so easy (fix doesn't work for me on xenial Classic Dimension). Perhaps the env() is used before pluginhandler.pull()?

@sergiusens

sergiusens Feb 11, 2016

Collaborator

It indeed might be, I pushed it here to see what happened on travis; I'm not sure how to fix this one since run is called from the class which generates an env by calling env.

Member

kyrofa commented Feb 11, 2016

Yeah that looks better. 👍

Collaborator

sergiusens commented Feb 11, 2016

@kyrofa did it work for you? Or is it just looks 😃

Member

kyrofa commented Feb 11, 2016

@sergiusens testing it out now, but yeah, based on how it looks it seems to solve the issue I had.

snapcraft/plugins/python2.py
@@ -137,23 +145,11 @@ def build(self):
os.makedirs(self.dist_packages_dir, exist_ok=True)
@kyrofa

kyrofa Feb 11, 2016

Member

You're still calling this, but you removed it. The examples tests will fail shortly 😃 .

@sergiusens

sergiusens Feb 11, 2016

Collaborator

Weird that it would need to be created; no I see our unit test coverage sucks for these python plugins 😭

@kyrofa

kyrofa Feb 11, 2016

Member

Indeed 😞

Member

kyrofa commented Feb 11, 2016

@sergiusens alright, tested and verified: 👍 from me.

snapcraft/common.py
@@ -133,6 +134,17 @@ def get_schemadir():
return _schemadir
+def get_python2_path(root):
+ """Returns a valid PYTHONPATH or raises an exception."""
@elopio

elopio Feb 11, 2016

Member

s/Returns/Return

@sergiusens

sergiusens Feb 11, 2016

Collaborator

and "or raise an" I suppose

snapcraft/plugins/catkin.py
+ try:
+ env.append('PYTHONPATH={0}'.format(common.get_python2_path(root)))
+ except EnvironmentError as e:
+ logger.debug(e)
@elopio

elopio Feb 11, 2016

Member

So no python is not a fatal error? Sounds weird.
But if that's the was, I think it should be warn, not debug.

@kyrofa

kyrofa Feb 11, 2016

Member

It's because this WILL happen when run is called, until the stage-packages have been installed.

@sergiusens

sergiusens Feb 11, 2016

Collaborator

@elopio I debated the case of using warn; but as @kyrofa mentions, it will happen a couple of times before it doesn't

@elopio

elopio Feb 11, 2016

Member

Got it. That's worth commenting in the code.

+ try:
+ return glob.glob(os.path.join(root, 'usr', 'include', 'python2*'))[0]
+ except IndexError:
+ raise EnvironmentError('python development headers not installed')
@elopio

elopio Feb 11, 2016

Member

What about a clue about how to fix this? Like "Install the package ..."

@sergiusens

sergiusens Feb 11, 2016

Collaborator

This is a programmatic error though as python-dev should be a required package for the python plugin (as it is now).

Member

elopio commented Feb 11, 2016

Some minor comments. 👍 from me.

Replacing the need for pyversions
pyversions creates a chicken and egg problem, so we use globbing
to work around that.

LP: #1541451

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>

sergiusens added a commit that referenced this pull request Feb 11, 2016

Merge pull request #316 from sergiusens/bugfix/1541451/python-minimal
Adding python-minimal to plugins that need it

@sergiusens sergiusens merged commit 61cc7a6 into snapcore:master Feb 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 93.755%
Details

@sergiusens sergiusens deleted the sergiusens:bugfix/1541451/python-minimal branch Feb 11, 2016

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

Merge pull request #316 from sergiusens/bugfix/1541451/python-minimal
Adding python-minimal to plugins that need it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment