[PATCH] sstate.bbclass: update .siginfo atime

[PATCH] sstate.bbclass: update .siginfo atime

[Ed Bartosh]

.siginfo files are not being accessed from local or NFS-mounted
sstate mirrors when sstate package is installed, so their atime
is not updated. If sstate mirror is cleaned based on access time,
they get deleted, even though they are still being used.

Updated atime of .siginfo symlinks with 'touch -a'. This command
dereferences symlinks pointing to the local mirror and updates
atime of the .siginfo file on the mirror.

[YOCTO #10857]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/sstate.bbclass | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 465fdcf..aa95964 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -753,6 +753,8 @@ sstate_unpack_package () {
 	[ ! -w ${SSTATE_PKG} ] || touch --no-dereference ${SSTATE_PKG}
 	[ ! -w ${SSTATE_PKG}.sig ] || [ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig
 	[ ! -w ${SSTATE_PKG}.siginfo ] || [ ! -e ${SSTATE_PKG}.siginfo ] || touch --no-dereference ${SSTATE_PKG}.siginfo
+	# update .siginfo atime on local/NFS mirror
+	[ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
 }
 
 BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
-- 
2.1.4

Re: [PATCH] sstate.bbclass: update .siginfo atime

[Richard Purdie]

On Wed, 2017-03-01 at 16:28 +0200, Ed Bartosh wrote:
> .siginfo files are not being accessed from local or NFS-mounted
> sstate mirrors when sstate package is installed, so their atime
> is not updated. If sstate mirror is cleaned based on access time,
> they get deleted, even though they are still being used.
> 
> Updated atime of .siginfo symlinks with 'touch -a'. This command
> dereferences symlinks pointing to the local mirror and updates
> atime of the .siginfo file on the mirror.
> 
> [YOCTO #10857]
> 
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> ---
>  meta/classes/sstate.bbclass | 2 ++
>  1 file changed, 2 insertions(+)

https://autobuilder.yoctoproject.org/main/builders/nightly-x86-64/builds/1138/steps/Running%20ESDK%20Sanity%20Tests/logs/stdio

rpurdie@opensuse132:~> less  /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64/build/build/tmp/work/qemux86_64-poky-linux/core-image-sato/1.0-r0/testsdkext/tmp/work/core2-64-poky-linux/meta-extsdk-toolchain/1.0-r0/temp/log.do_populate_sysroot_setscene.31530
DEBUG: Executing python function do_populate_sysroot_setscene
DEBUG: Executing shell function sstate_unpack_package
sysroot-destdir/
sysroot-destdir/sysroot-providers/
sysroot-destdir/sysroot-providers/meta-extsdk-toolchain
WARNING: /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64/build/build/tmp/work/qemux86_64-poky-linux/core-image-sato/1.0-r0/testsdkext/tmp/work/core2-64-poky-linux/meta-extsdk-toolchain/1.0-r0/temp/run.sstate_unpack_package.31530:1 exit 1 from '[ -h /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64/build/build/tmp/work/qemux86_64-poky-linux/core-image-sato/1.0-r0/testsdkext/sstate-cache/56/sstate:meta-extsdk-toolchain:core2-64-poky-linux:1.0:r0:core2-64:3:563a7c8c6206b04c5b117d6df021982b_populate_sysroot.tgz.siginfo ]'
DEBUG: Python function do_populate_sysroot_setscene finished

So it looks like this failure is from this patch somehow...

Cheers,

Richard

Re: [PATCH] sstate.bbclass: update .siginfo atime

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

On 1 March 2017 at 14:28, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:

> +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
>

I think you need to end this with || true to eat the failure if the test
fails.

Ross

[-- Attachment #2: Type: text/html, Size: 668 bytes --]

Re: [PATCH] sstate.bbclass: update .siginfo atime

[Ed Bartosh]

On Mon, Mar 06, 2017 at 11:14:49AM +0000, Burton, Ross wrote:
> On 1 March 2017 at 14:28, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> 
> > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> >
> 
> I think you need to end this with || true to eat the failure if the test
> fails.

Thank you for pointing out to this.
I guess it should work also if I move this line up. I'll send v2 soon.

--
Regards,
Ed

[PATCH v2] sstate.bbclass: update .siginfo atime

[Ed Bartosh]

.siginfo files are not being accessed from local or NFS-mounted
sstate mirrors when sstate package is installed, so their atime
is not updated. If sstate mirror is cleaned based on access time,
they get deleted, even though they are still being used.

Updated atime of .siginfo symlinks with 'touch -a'. This command
dereferences symlinks pointing to the local mirror and updates
atime of the .siginfo file on the mirror.

[YOCTO #10857]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/sstate.bbclass | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 23dda08..e415f68 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -747,6 +747,8 @@ python sstate_sign_package () {
 #
 sstate_unpack_package () {
 	tar -xvzf ${SSTATE_PKG}
+	# update .siginfo atime on local/NFS mirror
+	[ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
 	# Use "! -w ||" to return true for read only files
 	[ ! -w ${SSTATE_PKG} ] || touch --no-dereference ${SSTATE_PKG}
 	[ ! -w ${SSTATE_PKG}.sig ] || [ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig
-- 
2.1.4

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

On 6 March 2017 at 15:31, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:

> +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
>

$ ( [ -h / ] && echo something ); echo $?
1

If the -h fails then this is still going to cause sstate_unpack_package to
trap.

Ross

[-- Attachment #2: Type: text/html, Size: 880 bytes --]

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Ed Bartosh]

On Mon, Mar 06, 2017 at 04:49:25PM +0000, Burton, Ross wrote:
> On 6 March 2017 at 15:31, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> 
> > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> >
> 
> $ ( [ -h / ] && echo something ); echo $?
> 1
> 
> If the -h fails then this is still going to cause sstate_unpack_package to
> trap.

I don't think it will as '&&' and '||' lists are among other special cases where
shell doesn't exits on error.

Here is a proof:

$ cat test.sh

set -e

trap 'ouch!' ERR

foo() {
    [ -h /etc/fstab ] && echo 'fstab is a symlink'
    echo 'end of foo'
}

foo
echo $?

$ $ sh -e test.sh
end of foo
0


--
Regards,
Ed

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Patrick Ohly]

On Mon, 2017-03-06 at 19:50 +0200, Ed Bartosh wrote:
> On Mon, Mar 06, 2017 at 04:49:25PM +0000, Burton, Ross wrote:
> > On 6 March 2017 at 15:31, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > 
> > > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> > >
> > 
> > $ ( [ -h / ] && echo something ); echo $?
> > 1
> > 
> > If the -h fails then this is still going to cause sstate_unpack_package to
> > trap.
> 
> I don't think it will as '&&' and '||' lists are among other special cases where
> shell doesn't exits on error.

True, but if the clause happens to be the last one in a function or
script (for example, after restructuring code), then the return code of
that becomes non-zero, which isn't intended.

$ cat test.sh 
foo() {
    [ -h /foobar ] && echo 'fstab is a symlink'
}

foo
echo $?
$ bash -xe test.sh 
+ set -e
+ foo
+ '[' -h /foobar ']'
$ echo $?
1

Making the intend explicit with || true avoids that.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Ed Bartosh]

On Mon, Mar 06, 2017 at 08:31:55PM +0100, Patrick Ohly wrote:
> On Mon, 2017-03-06 at 19:50 +0200, Ed Bartosh wrote:
> > On Mon, Mar 06, 2017 at 04:49:25PM +0000, Burton, Ross wrote:
> > > On 6 March 2017 at 15:31, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > > 
> > > > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> > > >
> > > 
> > > $ ( [ -h / ] && echo something ); echo $?
> > > 1
> > > 
> > > If the -h fails then this is still going to cause sstate_unpack_package to
> > > trap.
> > 
> > I don't think it will as '&&' and '||' lists are among other special cases where
> > shell doesn't exits on error.
> 
> True, but if the clause happens to be the last one in a function or
> script (for example, after restructuring code), then the return code of
> that becomes non-zero, which isn't intended.

Exactly because of this I moved my changes up in the function in v2.

> 
> $ cat test.sh 
> foo() {
>     [ -h /foobar ] && echo 'fstab is a symlink'
> }
> 
> foo
> echo $?
> $ bash -xe test.sh 
> + set -e
> + foo
> + '[' -h /foobar ']'
> $ echo $?
> 1
> 
> Making the intend explicit with || true avoids that.
> 
> -- 
> Best Regards, Patrick Ohly
> 
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
> 
> 
> 

-- 
--
Regards,
Ed

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

On 6 March 2017 at 17:50, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:

> I don't think it will as '&&' and '||' lists are among other special cases
> where
> shell doesn't exits on error.
>

Ah, fair enough.  Another quirk of sh I wasn't aware of :)

Ross

[-- Attachment #2: Type: text/html, Size: 718 bytes --]

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Patrick Ohly]

On Mon, 2017-03-06 at 21:58 +0200, Ed Bartosh wrote:
> On Mon, Mar 06, 2017 at 08:31:55PM +0100, Patrick Ohly wrote:
> > On Mon, 2017-03-06 at 19:50 +0200, Ed Bartosh wrote:
> > > On Mon, Mar 06, 2017 at 04:49:25PM +0000, Burton, Ross wrote:
> > > > On 6 March 2017 at 15:31, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > > > 
> > > > > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> > > > >
> > > > 
> > > > $ ( [ -h / ] && echo something ); echo $?
> > > > 1
> > > > 
> > > > If the -h fails then this is still going to cause sstate_unpack_package to
> > > > trap.
> > > 
> > > I don't think it will as '&&' and '||' lists are among other special cases where
> > > shell doesn't exits on error.
> > 
> > True, but if the clause happens to be the last one in a function or
> > script (for example, after restructuring code), then the return code of
> > that becomes non-zero, which isn't intended.
> 
> Exactly because of this I moved my changes up in the function in v2.

And someone else might accidentally move it down again, without
understanding the consequences. It's simply safer to be explicit, IMHO.
Not worth a v3, but perhaps in the future.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] sstate.bbclass: update .siginfo atime

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Patrick Ohly
> Sent: den 6 mars 2017 22:46
> To: ed.bartosh@linux.intel.com
> Cc: OE-core
> Subject: Re: [OE-core] [PATCH v2] sstate.bbclass: update .siginfo atime
> 
> On Mon, 2017-03-06 at 21:58 +0200, Ed Bartosh wrote:
> > On Mon, Mar 06, 2017 at 08:31:55PM +0100, Patrick Ohly wrote:
> > > On Mon, 2017-03-06 at 19:50 +0200, Ed Bartosh wrote:
> > > > On Mon, Mar 06, 2017 at 04:49:25PM +0000, Burton, Ross wrote:
> > > > > On 6 March 2017 at 15:31, Ed Bartosh
> <ed.bartosh@linux.intel.com> wrote:
> > > > >
> > > > > > +       [ -h ${SSTATE_PKG}.siginfo ] && touch -a
> ${SSTATE_PKG}.siginfo
> > > > > >
> > > > >
> > > > > $ ( [ -h / ] && echo something ); echo $?
> > > > > 1
> > > > >
> > > > > If the -h fails then this is still going to cause
> sstate_unpack_package to
> > > > > trap.
> > > >
> > > > I don't think it will as '&&' and '||' lists are among other
> special cases where
> > > > shell doesn't exits on error.
> > >
> > > True, but if the clause happens to be the last one in a function or
> > > script (for example, after restructuring code), then the return
> code of
> > > that becomes non-zero, which isn't intended.
> >
> > Exactly because of this I moved my changes up in the function in v2.
> 
> And someone else might accidentally move it down again, without
> understanding the consequences. It's simply safer to be explicit, IMHO.
> Not worth a v3, but perhaps in the future.

This is why our shell expert always recommends that tests like this 
should be written this way instead:

     [ ! -h /foobar ] || echo 'fstab is a symlink'

I.e., reverse the first test and always use || rather than &&.

//Peter