core/legal-info: use hash file from version sub-dir
PATCH 1/1 core/legal-info: use hash file from version sub-dir
When we have multiple versions for a package, and the licensing terms depend on the version actually selected (e.g. like Qt5), storing the hashes for those license files in the .hash file is broken: the infra will ensure that all hashes for a file do match, which would not be the case here.
We fix that by first looking for a hash file in the version sub-dir first, and if that directory does not exist, then we use the main hash file. Note that if the version sub-dir exists, the main hash file is not used even if there is no per-version hash file.
Update the documentation accordingly.
--- docs/manual/adding-packages-directory.txt | 4 +++- package/pkg-utils.mk | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-)
docs/manual/adding-packages-directory.txt +3 -1
@@ -482,7 +482,9 @@ this in a comment line above the hashes. .Note The hashes for license files are used to detect a license change when a -package version is bumped. +package version is bumped. For a package with multiple versions (like Qt5), +create the hash file in a subdirectory +<packageversion>+ of that package +(see also xref:patch-apply-order[]). .Note The number of spaces does not matter, so one can use spaces (or tabs) to
package/pkg-utils.mk +6 -3
@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \ - case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \ } && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endef
Signature · 1 line
2.11.0
PATCH 1/1 core/legal-info: use hash file from version sub-dir
Yann,
Quoted 11 lines · Yann E. MORIN
package/pkg-utils.mk +0
-2@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \
package/pkg-utils.mk endef · line 97
Quoted 10 lines
package/pkg-utils.mk +6
-1- case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \ } && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endef
I think this has unintended side effects. If there is already a $(PKG)_VERSION directory for patches, it will look for the hash file there even if there are no hash file differences, which results in missing license file hash.
I believe it would work if you test -f on the actual .hash file instead of just the directory. But, do note that if there are multiple license files per package, and only one of them is different, this still results in putting all hashes including duplicate ones in separate hash files.
Josh
PATCH 1/1 core/legal-info: use hash file from version sub-dir
Hi Yann, Joshua,
Quoted 30 lines · Joshua Henderson
Yann,
Quoted 11 lines · Yann E. MORIN
package/pkg-utils.mk +0
-2@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \package/pkg-utils.mk endef · line 97
Quoted 10 lines
package/pkg-utils.mk +6
-1- case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \ } && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endefI think this has unintended side effects. If there is already a $(PKG)_VERSION directory for patches, it will look for the hash file there even if there are no hash file differences, which results in missing license file hash.
Quoted 3 lines
I believe it would work if you test -f on the actual .hash file instead of just the directory. But, do note that if there are multiple license files per package, and only one of them is different, this still results in putting all hashes including duplicate ones in separate hash files.
I agree with Josh here.
A cleaner solution would be to put only version-bound hashes in version-named subdirs, and leave all other hashes in the base directory. This would remove all duplication and avoid unintended disabling of all hashes when one <version>/*.hash file is created, which would happen only for those versions that actually have that file.
I think this can be achieved in a simple way by cat-ing together $(3)/$(1).hash and $(3)/$($(PKG)_VERSION)/$(1).hash in a temporary file and feed that to check-hash. Additionally if check-hash were able to read the hashes from stdin, no temporary file would be even needed, but this is not necessarily a good idea.
Signature · 1 line
Luca
PATCH 1/1 core/legal-info: use hash file from version sub-dir
Luca, All,
On 2017-07-14 09:39 +0200, Luca Ceresoli spake thusly:
Quoted 35 lines · Joshua Henderson
Quoted 30 lines
Yann,
Quoted 11 lines · Yann E. MORIN
package/pkg-utils.mk +0
-2@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \package/pkg-utils.mk endef · line 97
Quoted 10 lines
package/pkg-utils.mk +6
-1- case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \ } && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endefI think this has unintended side effects. If there is already a $(PKG)_VERSION directory for patches, it will look for the hash file there even if there are no hash file differences, which results in missing license file hash.
Quoted 3 lines
I believe it would work if you test -f on the actual .hash file instead of just the directory. But, do note that if there are multiple license files per package, and only one of them is different, this still results in putting all hashes including duplicate ones in separate hash files.
Quoted 7 lines
I agree with Josh here.
A cleaner solution would be to put only version-bound hashes in version-named subdirs, and leave all other hashes in the base directory. This would remove all duplication and avoid unintended disabling of all hashes when one <version>/*.hash file is created, which would happen only for those versions that actually have that file.
See my reply to Joshua: I would favour the opposite, in fact, to move all hashes to the per-version directory, even if there is duplication.
Quoted 5 lines
I think this can be achieved in a simple way by cat-ing together $(3)/$(1).hash and $(3)/$($(PKG)_VERSION)/$(1).hash in a temporary file and feed that to check-hash. Additionally if check-hash were able to read the hashes from stdin, no temporary file would be even needed, but this is not necessarily a good idea.
cat-ing to a temporary file will bneed that we arrange for a trickier code, because we need a unique filename for when we want to support TLPB.
As for stdin, this would be doable quite easily, we just need to recognise '-' as stdin, like almost all of the rest of the world. ;-)
But still, my preference would be to move hashes to the per-version directory. All hashes, not just license ones.
In the meantime, I've marked the patch as 'changes requested'.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 core/legal-info: use hash file from version sub-dir
In reply to Joshua HendersonJoshua, All,
On 2017-07-13 16:32 -0700, Joshua Henderson spake thusly:
Quoted 19 lines · Yann E. MORIN
Quoted 18 lines
package/pkg-utils.mk +6
-3@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \ - case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \
Quoted 8 lines
package/pkg-utils.mk endef · line 97
Quoted 4 lines
package/pkg-utils.mk +0
-0} && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endefI think this has unintended side effects. If there is already a $(PKG)_VERSION directory for patches, it will look for the hash file there even if there are no hash file differences, which results in missing license file hash.
This is the intended behaviour, yes: if there is a version choice, then we _want_ the license hashes to be per version, even if they are the same across versions. We do not want them in the 'main' hash file.
Yes, this means duplicated entries.
I was also wondering if we should also move the hashes for the downlaoded files to be per-version as well, to keep things together.
Ie., we would have something like:
package/qt5/qt5base/Config.in
package/qt5/qt5base/qt5base.mk
package/qt5/qt5base/5.6.2/*.patch
package/qt5/qt5base/5.6.2/qt5base.hash
package/qt5/qt5base/5.8.0/*.patch
package/qt5/qt5base/5.8.0/qt5base.hashQuoted 1 line
I believe it would work if you test -f on the actual .hash file instead of just the directory.
Again, if the directory exists, I intended that the main hash file was not used at all.
Quoted 3 lines
But, do note
that if there are multiple license files per package, and only one of them is different, this still results in putting all hashes including duplicate ones in separate hash files.
Yes, this is what I had in mind in this case.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 core/legal-info: use hash file from version sub-dir
Quoted 40 lines · Yann E. MORIN
Joshua, All,
On 2017-07-13 16:32 -0700, Joshua Henderson spake thusly:
Quoted 19 lines · Yann E. MORIN
Quoted 18 lines
package/pkg-utils.mk +6
-3@@ -86,9 +86,12 @@ endef define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET} mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \ { \ - support/download/check-hash $(3)/$(1).hash $(5) $(4); \ - ret=$${?}; \ - case $${ret} in (0|3) ;; (*) exit 1;; esac; \ + if [ -d $(3)/$($(PKG)_VERSION) ]; then \ + support/download/check-hash $(3)/$($(PKG)_VERSION)/$(1).hash $(5) $(4); \ + else \ + support/download/check-hash $(3)/$(1).hash $(5) $(4); \ + fi; \ + case $${?} in (0|3) ;; (*) exit 1;; esac; \Quoted 8 lines
package/pkg-utils.mk endef · line 97
Quoted 4 lines
package/pkg-utils.mk +0
-0} && \ cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4) endefI think this has unintended side effects. If there is already a $(PKG)_VERSION directory for patches, it will look for the hash file there even if there are no hash file differences, which results in missing license file hash.
This is the intended behaviour, yes: if there is a version choice, then we _want_ the license hashes to be per version, even if they are the same across versions. We do not want them in the 'main' hash file.
Yes, this means duplicated entries.
I was also wondering if we should also move the hashes for the downlaoded files to be per-version as well, to keep things together.
Quoted 8 lines
Ie., we would have something like:
package/qt5/qt5base/Config.in package/qt5/qt5base/qt5base.mk package/qt5/qt5base/5.6.2/*.patch package/qt5/qt5base/5.6.2/qt5base.hash package/qt5/qt5base/5.8.0/*.patch package/qt5/qt5base/5.8.0/qt5base.hash
This indeed sounds like the right approach. But then both uses of the hash file
(download and legal-info) should use the same approach, as you say.
In fact, this can be a workaround for cases where we often don't have a hash
because of custom version specification, like the kernel...
Perhaps the nicest approach is to move the intelligence down into check-hash
itself (the less shell code in the makefiles the better). So check-hash would not get a hash file as argument, but it would get package directory and version.
Note by the way that pkg-stats should probably be updated as well.
Sorry to give you more work :-)
Regards,
Arnout
Quoted 17 lines
Quoted 4 lines
I believe it would work if you test -f on the actual .hash file instead of just the directory.
Again, if the directory exists, I intended that the main hash file was not used at all.
Quoted 3 lines
But, do note
that if there are multiple license files per package, and only one of them is different, this still results in putting all hashes including duplicate ones in separate hash files.Yes, this is what I had in mind in this case.
Regards,
Yann E. MORIN.
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF