Redo pkg-config handling with library collecting #335

sergiusens · 2016-02-25T02:36:10Z

pkg-config only handled PKG_CONFIG_SYSROOT_DIR as the stage dir
providing many issues when using packages from installdir for a part
and also build-packages from the host.

For a full implementation, elf dependency tracking from the host
to the snap is also implemented here

LP: #1549570

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

come-maiz · 2016-02-25T06:01:16Z

libraries/16.04

+libustr-1.0.so.1
+libustr-1.0.so.1.0.4
+libyaml-0.so.2
+libyaml-0.so.2.0.4


How did you generate this list?
A README file next to this one explaining why and how would be great.

Sounds like a good idea, so far I did it manually with find from an OS snap. This will probably need updating from my Qt comments from last night

Agreed, this could use some documentation.

@ElOpio and @kyrofa done https://github.com/ubuntu-core/snapcraft/pull/335/files#diff-2cb561c5e46512465f54a0f84d421c80R66

But is also makes me think if it is worth to make this dynamic.

How would you go about making it dynamic? Would you have to download the OS snap every time?

come-maiz · 2016-02-25T06:11:40Z

lgtm, but the elf bits are flying over my head. I'll read more about that tomorrow.
You might want to get chipaca or mvo to take a look.

come-maiz · 2016-02-25T06:15:43Z

snapcraft/yaml.py

+    try:
+        check_call(['/usr/bin/pkg-config', '--exists'] + modules, env=env)
+        return True
+    except:


except subprocess.CalledProcessError.
We don't want to ignore any other exceptions.

sergiusens · 2016-02-25T12:10:23Z

@ElOpio wrt ELF I can ldd against ELF (Executable Linking Format) files. I am walking the whole tree in case someone goes with a non standard layout.

chipaca · 2016-02-25T12:49:51Z

snapcraft/libraries.py

+    ldd_out = [l[2] for l in ldd_out if len(l) > 2 and os.path.exists(l[2])]
+
+    # Anything that is part of the project we don't need to check for
+    libs = list(filter(lambda x: not x.startswith(os.getcwd()), ldd_out))


two (silly) things that bother me about this line: you're calling os.getcwd in a loop without expecting it to change, and you switched from list comprehensions to filter+lambda.

sergiusens · 2016-02-25T13:24:16Z

thanks @chipaca

sergiusens · 2016-02-26T06:50:24Z

retest this please

sergiusens · 2016-02-28T18:49:00Z

retest this please

sergiusens · 2016-02-28T21:10:20Z

retest this please

sergiusens · 2016-02-28T22:58:15Z

examples_tests/__init__.py

-                [self.snapcraft_command, 'snap'], cwd=working_dir,
-                stderr=subprocess.STDOUT).decode('utf-8')
+                [self.snapcraft_command, '--debug', 'snap'], cwd=working_dir,
+                stderr=subprocess.STDOUT)


@ElOpio I'm lost as with what is going on here:

Traceback (most recent call last): File "/home/jenkins-slave/workspace/github-snapcraft-examples-tests-cloud/examples_tests/tests.py", line 98, in test_example self.build_snap(self.example_dir) File "/home/jenkins-slave/workspace/github-snapcraft-examples-tests-cloud/examples_tests/__init__.py", line 129, in build_snap stderr=subprocess.STDOUT).decode('utf-8') File "/usr/lib/python3.5/subprocess.py", line 626, in check_output **kwargs).stdout File "/usr/lib/python3.5/subprocess.py", line 708, in run output=stdout, stderr=stderr)

coveralls · 2016-03-24T03:54:14Z

Coverage decreased (-0.2%) to 95.34% when pulling 6d9d14c on sergiusens:feature/1549570/ultimate-pkg-config into 2037d9c on ubuntu-core:master.

sergiusens · 2016-03-24T05:00:06Z

snapcraft/pluginhandler.py

@@ -399,6 +398,7 @@ def strip(self, force=False):
        self.notify_stage('Stripping')
        snap_files, snap_dirs = self.migratable_fileset_for('snap')
        _migrate_files(snap_files, snap_dirs, self.stagedir, self.snapdir)
+        _copy_dependencies(self.snapdir)


Needs tracking and library paths used need to be exported to env
hint for @kyrofa

coveralls · 2016-03-24T05:01:09Z

Coverage decreased (-0.2%) to 95.674% when pulling f31718f on sergiusens:feature/1549570/ultimate-pkg-config into 88d21b1 on ubuntu-core:master.

sergiusens · 2016-03-24T05:01:27Z

snapcraft/pluginhandler.py

@@ -265,7 +262,7 @@ def pull(self, force=False):
        package_files = set()
        package_directories = set()
        if self.code.stage_packages:
-            package_files, package_directories = self._setup_stage_packages()
+            self._setup_stage_packages()
        self.code.pull()

        # Record the files and directories unpacked from the stage packages


We probably don't need this anymore
hint @kyrofa

sergiusens · 2016-03-29T01:26:00Z

snapcraft/pluginhandler.py

+            destination_path = os.path.join(destination, relpath)
+
+            if os.path.isdir(source_path) and not os.path.islink(source_path):
+                os.makedirs(destination_path, exist_ok=True)


@kyrofa do we need to create these potential empty dirs? The else clause seems will be able to ensure the directories exist and not migrated until build happens which is too late for the python plugins.

coveralls · 2016-03-29T01:27:01Z

Coverage decreased (-0.3%) to 95.564% when pulling 5ba5674 on sergiusens:feature/1549570/ultimate-pkg-config into b18e4e9 on ubuntu-core:master.

sergiusens · 2016-03-29T01:27:13Z

@ElOpio given these pass on amd64 can we give this a run on armhf ?

sergiusens · 2016-03-29T01:56:30Z

@ElOpio I'm still seeing _testtools.testresult.real.StringException: output: {{{No module named 'magic'}}}

sergiusens · 2016-03-29T02:25:38Z

snapcraft/yaml.py

            env += part.env(part.installdir)
            env += _runtime_env(stagedir)
            env += _runtime_env(part.installdir)
+            env += _runtime_env(part.ubuntu_unpackdir)


@kyrofa python plugins fail to work, I'm thinking it might be due to this, did it work for you?

come-maiz · 2016-03-29T06:12:03Z

retest this please

come-maiz · 2016-03-29T07:00:45Z

retest this please

sergiusens · 2016-03-29T11:29:38Z

Thanks @ElOpio

pkg-config only handled PKG_CONFIG_SYSROOT_DIR as the stage dir providing many issues when using packages from installdir for a part and also build-packages from the host. For a full implementation, elf dependency tracking from the host to the snap is also implemented here Add dependency tracking and use it to determine LD_LIBRARY_PATH. Also unpacked stage-packages into an intermediate area in pull() and copied them into installdir in build() to make per-step cleaning work better. LP: #1549570 Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com> Signed-off-by: Kyle Fazzari <kyle@canonical.com>

sergiusens · 2016-03-29T21:16:43Z

@ElOpio final call is on you 😉

come-maiz · 2016-03-29T22:05:51Z

flashing...

sergiusens · 2016-03-30T01:39:45Z

@ElOpio how is it doing?

come-maiz · 2016-03-30T01:46:58Z

@sergiusens slooooow... Running in the snappy classic in rpi2. All unit tests passed. The integration tests have been running for an hour:

(classic)ubuntu@localhost:~/snapcraft$ ./runtests.sh integration
.................[sudo] password for ubuntu:
EEEE.........[sudo] password for ubuntu:
E....[sudo] password for ubuntu:
E..

sergiusens · 2016-03-30T01:48:25Z

@ElOpio hmm, maybe we can fix the sudo issue for armhf this cycle :-)

come-maiz · 2016-03-30T02:40:45Z

@sergiusens I can take a look at that.
The tests that could run, ran successfully. The tests that need sudo failed.
👍 from me to land this. I'll see if I can get the failed tests to pass. If they uncover an unlikely error, we can still fix it before the release.

…mate-pkg-config Redo pkg-config handling with library collecting

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch 3 times, most recently from 3b7bf1b to 3d1aa66 Compare February 25, 2016 04:46

come-maiz reviewed Feb 25, 2016
View reviewed changes

chipaca reviewed Feb 25, 2016
View reviewed changes

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch 2 times, most recently from 5c4d3c0 to 10f8e96 Compare February 25, 2016 13:23

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch 4 times, most recently from 831e80b to 8c27da5 Compare February 25, 2016 18:56

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch 4 times, most recently from ffea685 to 411faa3 Compare February 28, 2016 18:35

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch from 411faa3 to 6218bcc Compare February 28, 2016 21:09

sergiusens reviewed Feb 28, 2016
View reviewed changes

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch 4 times, most recently from 7ccea33 to 6d9d14c Compare March 24, 2016 03:51

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch from 6d9d14c to f31718f Compare March 24, 2016 04:58

sergiusens reviewed Mar 24, 2016
View reviewed changes

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch from f31718f to 5ba5674 Compare March 29, 2016 01:23

sergiusens reviewed Mar 29, 2016
View reviewed changes

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch from 5ba5674 to bcc0b45 Compare March 29, 2016 19:53

sergiusens force-pushed the feature/1549570/ultimate-pkg-config branch from bcc0b45 to 4a5de24 Compare March 29, 2016 20:10

sergiusens merged commit 854431f into canonical:master Mar 30, 2016

sergiusens deleted the feature/1549570/ultimate-pkg-config branch March 30, 2016 03:07

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

Merge pull request canonical#335 from sergiusens/feature/1549570/ulti…

691dd0a

…mate-pkg-config Redo pkg-config handling with library collecting

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redo pkg-config handling with library collecting #335

Redo pkg-config handling with library collecting #335

sergiusens commented Feb 25, 2016

come-maiz Feb 25, 2016

sergiusens Feb 25, 2016

kyrofa Feb 25, 2016

sergiusens Feb 29, 2016

kyrofa Feb 29, 2016

come-maiz commented Feb 25, 2016

come-maiz Feb 25, 2016

sergiusens commented Feb 25, 2016

chipaca Feb 25, 2016

sergiusens commented Feb 25, 2016

sergiusens commented Feb 26, 2016

sergiusens commented Feb 28, 2016

sergiusens commented Feb 28, 2016

sergiusens Feb 28, 2016

coveralls commented Mar 24, 2016

sergiusens Mar 24, 2016

coveralls commented Mar 24, 2016

sergiusens Mar 24, 2016

sergiusens Mar 29, 2016

coveralls commented Mar 29, 2016

sergiusens commented Mar 29, 2016

sergiusens commented Mar 29, 2016

sergiusens Mar 29, 2016

come-maiz commented Mar 29, 2016

come-maiz commented Mar 29, 2016

sergiusens commented Mar 29, 2016

sergiusens commented Mar 29, 2016

come-maiz commented Mar 29, 2016

sergiusens commented Mar 30, 2016

come-maiz commented Mar 30, 2016

sergiusens commented Mar 30, 2016

come-maiz commented Mar 30, 2016

Redo pkg-config handling with library collecting #335

Redo pkg-config handling with library collecting #335

Conversation

sergiusens commented Feb 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

come-maiz commented Feb 25, 2016

Choose a reason for hiding this comment

sergiusens commented Feb 25, 2016

Choose a reason for hiding this comment

sergiusens commented Feb 25, 2016

sergiusens commented Feb 26, 2016

sergiusens commented Feb 28, 2016

sergiusens commented Feb 28, 2016

Choose a reason for hiding this comment

coveralls commented Mar 24, 2016

Choose a reason for hiding this comment

coveralls commented Mar 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 29, 2016

sergiusens commented Mar 29, 2016

sergiusens commented Mar 29, 2016

Choose a reason for hiding this comment

come-maiz commented Mar 29, 2016

come-maiz commented Mar 29, 2016

sergiusens commented Mar 29, 2016

sergiusens commented Mar 29, 2016

come-maiz commented Mar 29, 2016

sergiusens commented Mar 30, 2016

come-maiz commented Mar 30, 2016

sergiusens commented Mar 30, 2016

come-maiz commented Mar 30, 2016