/snapcraft

sources: support symlinks in deb sources #941

Merged
merged 4 commits into from Jan 3, 2017
+29 −7

Conversation

Reviewers

@sergiusens sergiusens

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@kyrofa @codecov-io @flexiondotorg @sergiusens
@kyrofa
Member

kyrofa commented Dec 2, 2016

This PR fixes LP: #1634813 by using python3-debian to extract debs instead of python3-apt, whose extractall method creates directories when it encounters symlinks.

Note that this does add another dependency-- did I put it everywhere?
@codecov-io

codecov-io commented Dec 2, 2016 •
Edited 1 time

Current coverage is 96.33% (diff: 100%)

Merging #941 into master will not change coverage

@@             master       #941   diff @@
==========================================
  Files           190        190          
  Lines         16794      16794          
  Methods           0          0          
  Messages          0          0          
  Branches       1275       1275          
==========================================
  Hits          16179      16179          
  Misses          422        422          
  Partials        193        193

Powered by Codecov. Last update e093f6b...9e8a01e
@kyrofa
Support symlinks within deb sources. 
Use python3-debian to extract debs instead of python3-apt, whose
extractall method creates directories when it encounters symlinks.

LP: #1634813

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits

All checks have passed

4 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
@snappy-m-o
xenial-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
yakkety-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
zesty-amd64 — autopkgtest finished (success)
Details
f7cace0
@sergiusens

sergiusens requested changes Dec 2, 2016
View changes

Nice find, just a little more work and it will get there 😉

.travis.yml
@@ -14,8 +14,8 @@ env:
- secure: "gqtqTji8cie0Q2O+sRhE4MbTXGI0qTq8yPgRGFd9XlT/lB/EttFQKu72qycr/jyrvt809wjWM13QVqa5/71SoJd+Xzrmr1/leevx9Z/Wnv+IYkRAuGHW7iIDQb7MhQvpq3tw8hbGJzGxw03cUmjKJ89AAlGbwaURMat47lPsRXus8R7pl9S6r5owhBbmrQNaP9io0oPQDOAUf4pmJma1FTHAjXg0EdUwdXFUWToj15c7UJtB/MQNNTfjlwGA+/sPDqgthUEAzXmvUfXAZWnjQFZmq4ebvBIJEOQEPdLCXWGYdN2DAL7zp7WthrwFfgFFZb579rOBh0ETIMebUgBLoVSiPcn/bfzdYHcYKGf7lTJpoug5QENl+kZcuVyK7GUjf8O9tamhkYeMtUOy5Ubrcnv+Lfy9NsDPhKY05n+7tzzUVB1dePTrMHPuRZLl4OKku1AUN/S3A2xMrLO8vsWPVxcfxeb+4Y5ikYiHHpOozJHHDdPmj5raRIf3IH87W2PX0nJhg+gEgNHV1v3HBoyeqOPl4hl6/Fb9sCS/JAbbfcixkC54MHHI+opNSgZRvY0RORGHmuhHRGvfMxnwHmeOD51oV+SRGJS6A7qUq6GlBIy3/YlAY3LqqkWrHwm4EYttd4yM1FZ5s9pVnoSSJkgQ5vYK7A8a9AgZJlCcZnbzXtA="
matrix:
- TEST_SUITE=static DEPENDENCIES="apt install -y python3-pip && python3 -m pip install -r requirements-devel.txt"
- - TEST_SUITE=unit DEPENDENCIES="apt install -y libnacl-dev libsodium-dev libffi-dev libapt-pkg-dev libarchive-dev python3-pip squashfs-tools xdelta3 && python3 -m pip install -r requirements-devel.txt -r requirements.txt && python3 -m pip uninstall -y coverage && apt install -y python3-coverage"
- - TEST_SUITE=integration TEST_STORE=fake DEPENDENCIES="apt install -y bzr curl git libnacl-dev libsodium-dev libffi-dev libapt-pkg-dev libarchive-dev mercurial python3-pip subversion squashfs-tools sudo snapd xdelta3 && python3 -m pip install -r requirements-devel.txt -r requirements.txt"
+ - TEST_SUITE=unit DEPENDENCIES="apt install -y libnacl-dev libsodium-dev libffi-dev libapt-pkg-dev libarchive-dev python3-pip python3-debian squashfs-tools xdelta3 && python3 -m pip install -r requirements-devel.txt -r requirements.txt && python3 -m pip uninstall -y coverage && apt install -y python3-coverage"
@sergiusens

sergiusens Dec 2, 2016

Collaborator

is this really needed? Can't it be solved by adding it to requirements.txt?

Also missing an entry in HACKING.md

@kyrofa

kyrofa Dec 2, 2016

Member

Yeah good call, done.

@kyrofa
Put python-debian in requirements.txt. 
Signed-off-by: Kyle Fazzari <kyle@canonical.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits

Some checks were not successful

1 failing and 3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
@snappy-m-o
xenial-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
yakkety-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
zesty-amd64 — autopkgtest finished (success)
Details
2a0bd7c
@kyrofa
Member

kyrofa commented Dec 7, 2016

python-debian is not installing from pip. This needs a little more investigation.

@kyrofa kyrofa closed this Dec 7, 2016

@flexiondotorg
Contributor

flexiondotorg commented Dec 16, 2016 •
Edited 1 time

@kyrofa After looking at https://travis-ci.org/snapcore/snapcraft/jobs/180859624 I noticed chardet is failing to import. If python-debian is pip installed into a clean virtualenv it does not require or install chardet, even though it clearly uses it. I suggest adding chardet to one of the requirements files.

In contrast python3-debian Depends: python3-chardet.

kyrofa added some commits Dec 16, 2016

@kyrofa
Merge remote-tracking branch 'origin/master' into bugfix/1634813/supp… 
…ort_debs_containing_symlinks

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits
5cee369
@kyrofa
Apply changes to new deb source. 
Signed-off-by: Kyle Fazzari <kyle@canonical.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits

All checks have passed

5 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
@snappy-m-o
xenial-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
xenial-armhf — autopkgtest finished (success)
Details
@snappy-m-o
yakkety-amd64 — autopkgtest finished (success)
Details
@snappy-m-o
zesty-amd64 — autopkgtest finished (success)
Details
9e8a01e

@kyrofa kyrofa reopened this Dec 16, 2016

@kyrofa
Member

kyrofa commented Dec 16, 2016

If python-debian is pip installed into a clean virtualenv it does not require or install chardet, even though it clearly uses it. I suggest adding chardet to one of the requirements files.

Indeed, it turns out it's a known bug. Barry's on it. I've added chardet to requirements.txt until that fix is available.

@sergiusens sergiusens changed the title from Support symlinks within deb sources. to sources: support symlinks in deb sources Jan 3, 2017

@sergiusens sergiusens merged commit 2ed733d into snapcore:master Jan 3, 2017
5 checks passed

5 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-armhf autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

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

@kyrofa @sergiusens
sources: support symlinks in deb sources (#941) 
LP: #1634813
81ba54a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment