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

kernel plugin: if dtb target == NULL and arch == (arm||arm64), build and install all dtbs #1148

Merged
merged 2 commits into from Mar 5, 2017

Conversation

piso77
Copy link
Contributor

@piso77 piso77 commented Feb 17, 2017

If dtb target == NULL and arch == (arm||arm64), build and install all dtbs by default.

Signed-off-by: Paolo Pisati paolo.pisati@canonical.com

LP: #1665656

@sergiusens
Copy link
Collaborator

mind updating the unit tests here?

piso77 and others added 2 commits February 22, 2017 16:05
…and install

all dtbs

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
@@ -863,15 +863,15 @@ def test_kernel_image_target_as_map(self):
plugin = kernel.KernelPlugin('test-part', self.options,
project_options)

self.assertEqual(plugin.make_targets, ['Image', 'modules'])
self.assertEqual(plugin.make_targets, ['Image', 'modules', 'dtbs'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any tests that cover when not building on these architectures, and thus makes sure dtbs aren't built for them?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes we do although not directly visible on https://github.com/snapcore/snapcraft/pull/1148/files#diff-6f5410d7a6a3500fab904417fc36123dR779

We should probably refactor the kernel tests into many smaller classes but Paolo shouldn't be the one to do that.

@sergiusens sergiusens merged commit 2426177 into canonical:master Mar 5, 2017
self.make_targets.append('dtbs')
self.make_install_targets.extend([
'dtbs_install',
'INSTALL_DTBS_PATH={}/dtbs'.format(self.installdir)])
Copy link
Contributor

Choose a reason for hiding this comment

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

We also run this tests in armhf and arm64. The tests were not updated to take into account that they could be running on these architectures, so now we have new failures:

https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-xenial-snappy-dev-snapcraft-daily/xenial/armhf/s/snapcraft/20170307_030329_f94e4@/log.gz

This is pretty important, because we can't land in the archives if the tests start failing. @piso77, can you please take a look?

come-maiz pushed a commit that referenced this pull request Mar 27, 2017
…hem all. (#1148)

Build all dtbs if no specific dtb is requested on architectures that support dtbs.

LP: #1665656

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
…hem all. (canonical#1148)

Build all dtbs if no specific dtb is requested on architectures that support dtbs.

LP: #1665656

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
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

4 participants