If "source: .." do not copy the current directory into itself. #645

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

evandandrea commented Jul 9, 2016

This fixes LP: #1600428 by skipping over the directory containing the snapcraft.yaml when source: ..

Can one of the admins verify this patch?

Can one of the admins verify this patch?

Collaborator

sergiusens commented Jul 11, 2016

ok to test

Collaborator

sergiusens commented Jul 13, 2016

Hmm, this looks weird, @elopio ideas?

======================================================================
ERROR: test_pull_with_source_a_parent_of_current_dir (snapcraft.tests.test_sources.TestLocal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/adt-run.oP2bCg/build.4WQ/real-tree/snapcraft/tests/test_sources.py", line 391, in test_pull_with_source_a_parent_of_current_dir
    local.pull()
  File "/tmp/adt-run.oP2bCg/build.4WQ/real-tree/snapcraft/internal/sources.py", line 353, in pull
    copy_function=common.link_or_copy, ignore=ignore)
  File "/usr/lib/python3.5/shutil.py", line 353, in copytree
    raise Error(errors)
shutil.Error: [('/tmp/systemd-private-f5594a71a9ba4ccaac3e284940845926-systemd-timesyncd.service-MUKQz7', 'src/snapcraft/systemd-private-f5594a71a9ba4ccaac3e284940845926-systemd-timesyncd.service-MUKQz7', "[Errno 13] Permission denied: '/tmp/systemd-private-f5594a71a9ba4ccaac3e284940845926-systemd-timesyncd.service-MUKQz7'")]
Member

elopio commented Jul 13, 2016

The cwd for the test is /tmp/{randomdir}. When you tell it that the source is .., it will try to copy all /tmp into /tmp/{randomdir}/src/snapcraft. Some files in tmp are protected, thus the error.

I suggest to os.chdir('src/snapcraft') before calling pull.

And I think this is missing an integration test. Please take a look at the integration_tests dir, and let me know if you need a hand with this.

Thanks for the code Evan!

Member

kyrofa commented Jul 14, 2016

And I think this is missing an integration test.

Ah, I was going to say just that. Thanks Evan, looks great other than that!

Contributor

evandandrea commented Jul 14, 2016

On it 😄

Member

kyrofa commented Aug 3, 2016

@evandandrea do you think you can get that integration test in? Or would you like one of us to take over?

Member

kyrofa commented Aug 12, 2016

Closed in favor of #725 due to inactivity.

@kyrofa kyrofa closed this Aug 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment