Copy entire source, even if source-subdir is specified. #344

Merged
merged 1 commit into from Feb 29, 2016

Conversation

Projects
None yet
2 participants
Member

kyrofa commented Feb 27, 2016

Currently, if source-subdir is specified, Snapcraft copies ONLY that subdir (and excludes the rest of the source) and attempts to build from it. This only works for some projects, not all. A better solution is to copy the entire source and simply build from WITHIN the subdir. This PR fixes LP: #1549676 by introducing exactly that change. It does so without changing the plugin API or semantics.

snapcraft/tests/test_base_plugin.py
- tmpdir = tempfile.TemporaryDirectory()
- self.addCleanup(tmpdir.cleanup)
- plugin.builddir = tmpdir.name
+ self.assertEqual(os.path.join(plugin.buildbasedir,
@sergiusens

sergiusens Feb 27, 2016

Collaborator

It seems there will be less indentation eyeball matching if done like

self.assertEqual(
    os.path.join(plugin.buildbasedir, options.source_subdir),
    plugin.builddir)
snapcraft/tests/test_base_plugin.py
plugin.build()
- self.assertTrue(os.path.exists(
- os.path.join(plugin.builddir, 'src', 'file')))
+ self.assertTrue(os.path.exists(os.path.join(plugin.buildbasedir,
@sergiusens

sergiusens Feb 27, 2016

Collaborator

Same comment about eyeballs :-)

self.assertTrue(
    os.path.exists(os.path.join(plugin.buildbasedir, 'file')))

or

self.assertTrue(os.path.exists(
    os.path.join(plugin.buildbasedir, 'file')))
snapcraft/tests/test_base_plugin.py
plugin.build()
- self.assertTrue(os.path.exists(os.path.join(plugin.builddir, 'file')))
+ self.assertTrue(os.path.exists(os.path.join(plugin.buildbasedir,
@sergiusens

sergiusens Feb 27, 2016

Collaborator

ditto

self.assertTrue(os.path.exists(
    os.path.join(plugin.buildbasedir, 'file1')))
snapcraft/__init__.py
self.installdir = os.path.join(self.partdir, 'install')
+ self.buildbasedir = os.path.join(self.partdir, 'build')
@sergiusens

sergiusens Feb 27, 2016

Collaborator

Can we rename buildbasedir to build_basedir?

Collaborator

sergiusens commented Feb 27, 2016

I am 👍 but would really like those cosmetics fixed as I have difficulties aligning if it is too deep 😉

Member

kyrofa commented Feb 27, 2016

Easy changes! I'll keep that in mind for the future-- I've always tried to stretch the line as far as it'll go but you're right, it has its limits. Thanks for the review 😃 .

Copy entire source, even if source-subdir is specified.
Currently, if source-subdir is specified, Snapcraft copies
ONLY that subdir (and excludes the rest of the source) and
attempts to build from it. This only works for some projects,
not all. A better solution is to copy the entire source and
simply build from WITHIN the subdir. That is the change
introduced here.

LP: #1549676

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

kyrofa commented Feb 28, 2016

Grr... over quota or some such thing...

Collaborator

sergiusens commented Feb 29, 2016

retest this please

Collaborator

sergiusens commented Feb 29, 2016

retest this please

Member

kyrofa commented Feb 29, 2016

Tested locally-- examples pass.

kyrofa added a commit that referenced this pull request Feb 29, 2016

Merge pull request #344 from kyrofa/bugfix/1549676/source_subdir_stil…
…l_copy_all

Copy entire source, even if source-subdir is specified.

@kyrofa kyrofa merged commit 27e36fe into snapcore:master Feb 29, 2016

2 of 3 checks passed

Examples tests 13 tests run, 0 skipped, 2 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 94.702%
Details

@kyrofa kyrofa deleted the kyrofa:bugfix/1549676/source_subdir_still_copy_all branch Feb 29, 2016

smoser pushed a commit to smoser/snapcraft that referenced this pull request Sep 14, 2016

rdma: log exception thrown in device processing (#344)
Exceptions thrown from rdma processing thread running in
the background were not logged properly.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>

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

Merge pull request #344 from kyrofa/bugfix/1549676/source_subdir_stil…
…l_copy_all

Copy entire source, even if source-subdir is specified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment