From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] core/legal-info: use hash file from version sub-dir
Date: Fri, 14 Jul 2017 10:41:31 +0200 [thread overview]
Message-ID: <20170714084131.GB3373@scaer> (raw)
In-Reply-To: <ebb72a4e-7e47-1d4a-9a69-1fe05a1e3ec8@lucaceresoli.net>
Luca, All,
On 2017-07-14 09:39 +0200, Luca Ceresoli spake thusly:
> On 14/07/2017 01:32, Joshua Henderson wrote:
> > Yann,
> >
> > On 07/13/2017 03:19 PM, Yann E. MORIN wrote:
> >
> >> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> >> index accf48c464..f285395267 100644
> >> --- a/package/pkg-utils.mk
> >> +++ b/package/pkg-utils.mk
> >> @@ -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
> >>
> >
> > 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.
>
> 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.
> 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.
--
.-----------------.--------------------.------------------.--------------------.
| 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. |
'------------------------------^-------^------------------^--------------------'
next prev parent reply other threads:[~2017-07-14 8:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-13 22:19 [Buildroot] [PATCH] core/legal-info: use hash file from version sub-dir Yann E. MORIN
2017-07-13 23:32 ` Joshua Henderson
2017-07-14 7:39 ` Luca Ceresoli
2017-07-14 8:41 ` Yann E. MORIN [this message]
2017-07-14 8:38 ` Yann E. MORIN
2017-07-18 19:48 ` Arnout Vandecappelle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170714084131.GB3373@scaer \
--to=yann.morin.1998@free.fr \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.