Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for mesa libraries. #204

Merged
merged 1 commit into from Jan 7, 2016

Conversation

kyrofa
Copy link
Contributor

@kyrofa 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


tempdirObj = tempfile.TemporaryDirectory()
self.addCleanup(tempdirObj.cleanup)
os.chdir(tempdirObj.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@come-maiz
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

I re triggered the examples test run

@@ -0,0 +1,50 @@
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright (C) 2015 Canonical Ltd
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2016

@sergiusens
Copy link
Collaborator

What wishlist bug is this closing? 😉

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
@kyrofa
Copy link
Contributor Author

kyrofa commented Jan 6, 2016

@sergiusens heh. Fixed 😃 .

@sergiusens
Copy link
Collaborator

👍

sergiusens added a commit that referenced this pull request Jan 7, 2016
@sergiusens sergiusens merged commit f8d8ee7 into canonical:master Jan 7, 2016
@kyrofa kyrofa deleted the ld_library_path_opengl branch January 7, 2016 13:49
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants