/snapcraft

Add support for mesa libraries. #204

Merged
merged 1 commit into from Jan 7, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@kyrofa @elopio @sergiusens
@kyrofa
Member

kyrofa commented Jan 6, 2016

Currently Snapcraft adds only a few standard paths to LD_LIBRARY_PATH. This PR expands that to include paths used by mesa packages.

This will need to be backported to 1.x once approved.

Resolves LP: #1531620
snapcraft/tests/test_yaml.py
+
+ tempdirObj = tempfile.TemporaryDirectory()
+ self.addCleanup(tempdirObj.cleanup)
+ os.chdir(tempdirObj.name)
@elopio

elopio Jan 6, 2016

Member

We already have this on the base setup, when this test runs the cwd is already a tempdir that will be deleted. Tha path to that dir is in self.path.
https://github.com/ubuntu-core/snapcraft/blob/master/snapcraft/tests/__init__.py#L33

@kyrofa

kyrofa Jan 6, 2016

Member

Ah good find! I was copying what was already there-- I've fixed the rest of the tests as well.

snapcraft/tests/test_yaml.py
+ self.addCleanup(tempdirObj.cleanup)
+ os.chdir(tempdirObj.name)
+ with open('icon.svg', 'w') as icon:
+ icon.write('<svg />')
@elopio

elopio Jan 6, 2016

Member

aren't the icons optional now? I can't remember if that landed or not yet.

@kyrofa

kyrofa Jan 6, 2016

Member

Oh right, I forgot about that! I do believe it's landed.

@elopio

elopio Jan 6, 2016

Member

ah, yes it is because you don't have it in your example! So this can be removed.

@kyrofa

kyrofa Jan 6, 2016

Member

Fixed.

@elopio
Member

elopio commented Jan 6, 2016

lgtm. Maybe, I would move the library handling to a new module. snapcraft/libraries?
I don't like that we have the mesa lib hardcoded and that we'll have to keep adding paths there, but I don't know much about that and can't think of a better way to do it.

Thanks for the example!
@kyrofa
Member

kyrofa commented Jan 6, 2016

@elopio alright I moved it into snapcraft.libraries. I agree with you on the hard-coding, but there doesn't seem to be another way to do it since the .debs typically configure themselves in ld.so.conf.d via postinst scripts, which don't get run with how we're unpacking them.
@sergiusens
Collaborator

sergiusens commented Jan 6, 2016

I re triggered the examples test run
snapcraft/libraries.py
@@ -0,0 +1,50 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2015 Canonical Ltd
@sergiusens

sergiusens Jan 6, 2016

Collaborator

2016

snapcraft/tests/test_libraries.py
@@ -0,0 +1,45 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2015 Canonical Ltd
@sergiusens

sergiusens Jan 6, 2016

Collaborator

2016

snapcraft/tests/test_yaml.py
@@ -317,7 +300,6 @@ def test_config_expands_filesets(self, mock_loadPlugin):
version: "1"
summary: test
description: test
-icon: my-icon.png
@sergiusens

sergiusens Jan 6, 2016

Collaborator

shouldn't these be a separate commit?

@kyrofa

kyrofa Jan 6, 2016

Member

They could be. These small refactors were to simply make the tests consistent with the ones I was adding rather than being consistent the other way and adding more cruft.

@sergiusens
Collaborator

sergiusens commented Jan 6, 2016

What wishlist bug is this closing? 😉
@kyrofa
Add support for mesa libraries. 
Currently Snapcraft adds only a few standard paths to
LD_LIBRARY_PATH. This commit expands that to include
paths used by mesa packages.

Also add an opencv example to exercise this.

Signed-off-by: Kyle Fazzari <kyle@canonical.com>

LP: #1531620
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.3%) to 89.165%
Details
da00836
@kyrofa
Member

kyrofa commented Jan 6, 2016

@sergiusens heh. Fixed 😃 .

@kyrofa kyrofa referenced this pull request Jan 6, 2016

Merged

Update debian/changelog for the 1.0 release. #198

@sergiusens
Collaborator

sergiusens commented Jan 7, 2016

👍

sergiusens added a commit that referenced this pull request Jan 7, 2016

@sergiusens
Merge pull request #204 from kyrofa/ld_library_path_opengl 
Add support for mesa libraries.
f8d8ee7

@sergiusens sergiusens merged commit f8d8ee7 into snapcore:master Jan 7, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 89.165%
Details

@kyrofa kyrofa referenced this pull request Jan 7, 2016

Merged

Backport to 1.x: Add support for mesa libraries. #207

@kyrofa kyrofa deleted the kyrofa:ld_library_path_opengl branch Jan 7, 2016

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

@sergiusens
Merge pull request #204 from kyrofa/ld_library_path_opengl 
Add support for mesa libraries.
d0ab400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment