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

Add a possibility to create multi-image device tree blobs for Amlogic #2286

Merged
merged 3 commits into from Dec 11, 2017

Conversation

kszaq
Copy link
Contributor

@kszaq kszaq commented Dec 1, 2017

This is useful if a few devices need to be supported by one project.

If there is only one device tree created from KERNEL_UBOOT_EXTRA_TARGET it is appended as-is, e.g. without multidtb tool header.

@@ -0,0 +1,35 @@
################################################################################
# This file is part of LibreELEC - https://libreelec.tv
# Copyright (C) 2016 Team LibreELEC
Copy link
Member

Choose a reason for hiding this comment

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

Copyright (C) 2016-present Team LibreELEC

PKG_SECTION="tools"
PKG_SHORTDESC="AML DTB Tools"
PKG_LONGDESC="AML DTB Tools"
PKG_AUTORECONF="no"
Copy link
Member

Choose a reason for hiding this comment

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

PKG_AUTORECONF="no" is dropped entirely
maybe you need to add PKG_TOOLCHAIN="manual" if it won't compile without

Copy link
Contributor

Choose a reason for hiding this comment

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

PKG_TOOLCHAIN is not needed but PKG_AUTORECONF needs to be dropped

################################################################################

PKG_NAME="aml-dtbtools"
PKG_VERSION="cce100f"
Copy link
Contributor

Choose a reason for hiding this comment

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

needs PKG_SHA256="8bcaa83fcc9e85c9c04930e7411447d96a97da0809c5ecd9af91c8b554133c41"

Copy link
Contributor

Choose a reason for hiding this comment

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

Checksums aren't reliable when using github archives, they get regenerated at random intervals.

Copy link
Contributor

@spycat88 spycat88 Dec 2, 2017

Choose a reason for hiding this comment

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

This is now required as per #1597 and #1806 there has been no problem with them as of yet and over 300 packages use a github.

PKG_ARCH="any"
PKG_LICENSE="free"
PKG_SITE="https://github.com/Wilhansen/aml-dtbtools"
PKG_URL="${PKG_SITE}/archive/${PKG_VERSION}.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

sticking with conventions this should be PKG_URL="https://github.com/Wilhansen/aml-dtbtools/archive/${PKG_VERSION}.tar.gz"

Copy link
Contributor

Choose a reason for hiding this comment

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

I know that is the convention, but I think that @kszaq way should be the new convention. Now that we are using github as preferred source, makes more easy use another repository, avoid the mistake of changing the PKG_SITE and not the PKG_URL or vice-versa.

Copy link
Contributor

@spycat88 spycat88 Dec 2, 2017

Choose a reason for hiding this comment

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

This is why PR's have reviews so mistakes don't get made :) I don't think this is a good idea, it would add fragmentation as only 300/700 packages use github.

Copy link
Member

Choose a reason for hiding this comment

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

PKG_SITE is not necessarily github even if the download comes from github. Proper fix would be a special var for gh archives. But not too sure we should add complexity to that.

@kszaq
Copy link
Contributor Author

kszaq commented Dec 2, 2017

Thank you everyone for reviewing. Suggestions applied.

@Ray-future
Copy link
Contributor

Thanks @kszaq. Looks good so far I will include that PR in my tree/build to test WP2 and WH.
Good job.

@spycat88
Copy link
Contributor

spycat88 commented Dec 3, 2017

Looks spot on now, tested against S905 and no problems, haven't tested other projects.

@Ray-future Ray-future merged commit 7d31b17 into LibreELEC:master Dec 11, 2017
@kszaq kszaq deleted the aml_multidtb branch December 11, 2017 21:38
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

6 participants