Patchwork license.bbclass: remove existing license.manifest before appending new data

login
register
mail settings
Submitter Eric BENARD
Date March 29, 2012, 12:22 p.m.
Message ID <1333023749-31830-1-git-send-email-eric@eukrea.com>
Download mbox | patch
Permalink /patch/24847/
State Accepted
Commit 2d49a8f659694b60cdb706e8993cd9550e2002bd
Headers show

Comments

Eric BENARD - March 29, 2012, 12:22 p.m.
without this fix, we append license each time we build again the same image,
ending with a large not up to date file.

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 meta/classes/license.bbclass |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Richard Purdie - March 29, 2012, 9:24 p.m.
On Thu, 2012-03-29 at 14:22 +0200, Eric Bénard wrote:
> without this fix, we append license each time we build again the same image,
> ending with a large not up to date file.
> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  meta/classes/license.bbclass |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)

Merged to master, thanks.

Richard
Chris Larson - March 29, 2012, 11:22 p.m.
On Thu, Mar 29, 2012 at 5:22 AM, Eric Bénard <eric@eukrea.com> wrote:
> without this fix, we append license each time we build again the same image,
> ending with a large not up to date file.
>
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  meta/classes/license.bbclass |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index 394a6d4..c85233c 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -79,6 +79,10 @@ license_create_manifest() {
>        # Get list of installed packages
>        list_installed_packages | grep -v "locale" |sort > ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest
>        INSTALLED_PKGS=`cat ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest`
> +       # remove existing license.manifest file
> +       if [ -f ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest ]; then
> +               rm ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest
> +       fi
>        # list of installed packages is broken for deb

Probably not a concern in this particular case, but in general you
should avoid this sort of construct, as it's racy.
Eric BENARD - March 30, 2012, 6:20 a.m.
Le Thu, 29 Mar 2012 16:22:06 -0700,
Chris Larson <clarson@kergoth.com> a écrit :

> On Thu, Mar 29, 2012 at 5:22 AM, Eric Bénard <eric@eukrea.com> wrote:
> > without this fix, we append license each time we build again the same image,
> > ending with a large not up to date file.
> >
> > Signed-off-by: Eric Bénard <eric@eukrea.com>
> > ---
> >  meta/classes/license.bbclass |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> > index 394a6d4..c85233c 100644
> > --- a/meta/classes/license.bbclass
> > +++ b/meta/classes/license.bbclass
> > @@ -79,6 +79,10 @@ license_create_manifest() {
> >        # Get list of installed packages
> >        list_installed_packages | grep -v "locale" |sort > ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest
> >        INSTALLED_PKGS=`cat ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest`
> > +       # remove existing license.manifest file
> > +       if [ -f ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest ]; then
> > +               rm ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest
> > +       fi
> >        # list of installed packages is broken for deb
> 
> Probably not a concern in this particular case, but in general you
> should avoid this sort of construct, as it's racy.

ok, what would be the right way to do that to avoid the race problem ?

Eric
Chris Larson - March 30, 2012, 10:41 p.m.
On Thu, Mar 29, 2012 at 11:20 PM, Eric Bénard <eric@eukrea.com> wrote:
> Le Thu, 29 Mar 2012 16:22:06 -0700,
> Chris Larson <clarson@kergoth.com> a écrit :
>
>> On Thu, Mar 29, 2012 at 5:22 AM, Eric Bénard <eric@eukrea.com> wrote:
>> > without this fix, we append license each time we build again the same image,
>> > ending with a large not up to date file.
>> >
>> > Signed-off-by: Eric Bénard <eric@eukrea.com>
>> > ---
>> >  meta/classes/license.bbclass |    4 ++++
>> >  1 files changed, 4 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
>> > index 394a6d4..c85233c 100644
>> > --- a/meta/classes/license.bbclass
>> > +++ b/meta/classes/license.bbclass
>> > @@ -79,6 +79,10 @@ license_create_manifest() {
>> >        # Get list of installed packages
>> >        list_installed_packages | grep -v "locale" |sort > ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest
>> >        INSTALLED_PKGS=`cat ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest`
>> > +       # remove existing license.manifest file
>> > +       if [ -f ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest ]; then
>> > +               rm ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest
>> > +       fi
>> >        # list of installed packages is broken for deb
>>
>> Probably not a concern in this particular case, but in general you
>> should avoid this sort of construct, as it's racy.
>
> ok, what would be the right way to do that to avoid the race problem ?

Generally speaking rather than doing "if this, then do that"
particularly with the filesystem, just do it and handle any errors. In
this case, rm -f, which exits silently if the file was already
removed.

Patch

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index 394a6d4..c85233c 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -79,6 +79,10 @@  license_create_manifest() {
 	# Get list of installed packages
 	list_installed_packages | grep -v "locale" |sort > ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest
 	INSTALLED_PKGS=`cat ${LICENSE_DIRECTORY}/${IMAGE_NAME}/package.manifest`
+	# remove existing license.manifest file
+	if [ -f ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest ]; then
+		rm ${LICENSE_DIRECTORY}/${IMAGE_NAME}/license.manifest
+	fi
 	# list of installed packages is broken for deb
 	for pkg in ${INSTALLED_PKGS}; do
 		# not the best way to do this but licenses are not arch dependant iirc