Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
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.  |
'------------------------------^-------^------------------^--------------------'

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox