[master,langdale,kirkstone] gcc-shared-source: ensure that S exists in do_deploy_source_date_epoch

From: Peter Marko <peter.marko@siemens.com>

This include overrides do_deploy_source_date_epoch function
and is additionally referencing ${S}.
If ${S} does not exist yet, the function will fail because
it cannot evaluate path .. from non-existing directory.

Reproducer (verified in kirkstone):
  bitbake gcc -c deploy_source_date_epoch
  bitbake gcc -c cleansstate
  rm -rf build/tmp
  bitbake gcc -c deploy_source_date_epoch

Signed-off-by: Peter Marko <peter.marko@siemens.com>
---
 meta/recipes-devtools/gcc/gcc-shared-source.inc | 1 +
 1 file changed, 1 insertion(+)

On Thu, 2023-03-23 at 12:01 +0100, Peter Marko wrote:
> From: Peter Marko <peter.marko@siemens.com>
> 
> This include overrides do_deploy_source_date_epoch function
> and is additionally referencing ${S}.
> If ${S} does not exist yet, the function will fail because
> it cannot evaluate path .. from non-existing directory.
> 
> Reproducer (verified in kirkstone):
>   bitbake gcc -c deploy_source_date_epoch
>   bitbake gcc -c cleansstate
>   rm -rf build/tmp
>   bitbake gcc -c deploy_source_date_epoch
> 
> Signed-off-by: Peter Marko <peter.marko@siemens.com>
> ---
>  meta/recipes-devtools/gcc/gcc-shared-source.inc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/meta/recipes-devtools/gcc/gcc-shared-source.inc b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> index cd2e341099..7c84ea088d 100644
> --- a/meta/recipes-devtools/gcc/gcc-shared-source.inc
> +++ b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> @@ -10,6 +10,7 @@ SRC_URI = ""
>  do_configure[depends] += "gcc-source-${PV}:do_preconfigure"
>  do_populate_lic[depends] += "gcc-source-${PV}:do_unpack"
>  do_deploy_source_date_epoch[depends] += "gcc-source-${PV}:do_deploy_source_date_epoch"
> +do_deploy_source_date_epoch[dirs] += "${S}"
>  
>  # Copy the SDE from the shared workdir to the recipe workdir
>  do_deploy_source_date_epoch () {

I have a suspicion this was fixed differently in master.

The fix looks incorrect since it will just make the directory exist,
which will hide the error but won't fix whatever the race is causing
the underlying determinism problem.

Can you confirm the problem exists on master to start with please?

Cheers,

Richard

Hi Richard,

I have verified that it fails the reproducer also on master (poky: 23212c8c44 (HEAD, mirror/master) rpm: fix RPM_ETCCONFIGDIR value in SDK)

I don't think it hides anything.
The content of ${S} is completely irrelevant for the task except that it is used to create paths to files based on "${S}/..".

Best Regards,
  Peter

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Thursday, March 23, 2023 12:40
To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core][master][langdale][kirkstone][PATCH] gcc-shared-source: ensure that S exists in do_deploy_source_date_epoch

> On Thu, 2023-03-23 at 12:01 +0100, Peter Marko wrote:
> > From: Peter Marko <peter.marko@siemens.com>
> > 
> > This include overrides do_deploy_source_date_epoch function and is 
> > additionally referencing ${S}.
> > If ${S} does not exist yet, the function will fail because it cannot 
> > evaluate path .. from non-existing directory.
> > 
> > Reproducer (verified in kirkstone):
> >   bitbake gcc -c deploy_source_date_epoch
> >   bitbake gcc -c cleansstate
> >   rm -rf build/tmp
> >   bitbake gcc -c deploy_source_date_epoch
> > 
> > Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > ---
> >  meta/recipes-devtools/gcc/gcc-shared-source.inc | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/meta/recipes-devtools/gcc/gcc-shared-source.inc 
> > b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > index cd2e341099..7c84ea088d 100644
> > --- a/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > +++ b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > @@ -10,6 +10,7 @@ SRC_URI = ""
> >  do_configure[depends] += "gcc-source-${PV}:do_preconfigure"
> >  do_populate_lic[depends] += "gcc-source-${PV}:do_unpack"
> >  do_deploy_source_date_epoch[depends] += "gcc-source-${PV}:do_deploy_source_date_epoch"
> > +do_deploy_source_date_epoch[dirs] += "${S}"
> >  
> >  # Copy the SDE from the shared workdir to the recipe workdir  
> > do_deploy_source_date_epoch () {
>
> I have a suspicion this was fixed differently in master.
>
> The fix looks incorrect since it will just make the directory exist, which will hide the error but won't fix whatever the race is causing the underlying determinism problem.
>
> Can you confirm the problem exists on master to start with please?
> >
> Cheers,
>
> Richard

Would this be more acceptable change?

-       cp -p ${S}/../$sde_file ${SDE_DEPLOYDIR}
-       cp -p ${S}/../$sde_file ${SDE_FILE}
+       cp -p $(dirname ${S})/$sde_file ${SDE_DEPLOYDIR}
+       cp -p $(dirname ${S})/$sde_file ${SDE_FILE}

Peter

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Peter Marko via lists.openembedded.org
> Sent: Thursday, March 23, 2023 12:49
> To: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][master][langdale][kirkstone][PATCH] gcc-shared-source: ensure that S exists in do_deploy_source_date_epoch
>
> Hi Richard,
>
> I have verified that it fails the reproducer also on master (poky: 23212c8c44 (HEAD, mirror/master) rpm: fix RPM_ETCCONFIGDIR value in SDK)
>
> I don't think it hides anything.
> The content of ${S} is completely irrelevant for the task except that it is used to create paths to files based on "${S}/..".
>
> Best Regards,
>   Peter
>
> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: Thursday, March 23, 2023 12:40
> To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][master][langdale][kirkstone][PATCH] gcc-shared-source: ensure that S exists in do_deploy_source_date_epoch
>
> > On Thu, 2023-03-23 at 12:01 +0100, Peter Marko wrote:
> > > From: Peter Marko <peter.marko@siemens.com>
> > > 
> > > This include overrides do_deploy_source_date_epoch function and is 
> > > additionally referencing ${S}.
> > > If ${S} does not exist yet, the function will fail because it cannot 
> > > evaluate path .. from non-existing directory.
> > > 
> > > Reproducer (verified in kirkstone):
> > >   bitbake gcc -c deploy_source_date_epoch
> > >   bitbake gcc -c cleansstate
> > >   rm -rf build/tmp
> > >   bitbake gcc -c deploy_source_date_epoch
> > > 
> > > Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > > ---
> > >  meta/recipes-devtools/gcc/gcc-shared-source.inc | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > > b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > > index cd2e341099..7c84ea088d 100644
> > > --- a/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > > +++ b/meta/recipes-devtools/gcc/gcc-shared-source.inc
> > > @@ -10,6 +10,7 @@ SRC_URI = ""
> > >  do_configure[depends] += "gcc-source-${PV}:do_preconfigure"
> > >  do_populate_lic[depends] += "gcc-source-${PV}:do_unpack"
> > >  do_deploy_source_date_epoch[depends] += "gcc-source-${PV}:do_deploy_source_date_epoch"
> > > +do_deploy_source_date_epoch[dirs] += "${S}"
> > >  
> > >  # Copy the SDE from the shared workdir to the recipe workdir 
> > > do_deploy_source_date_epoch () {
> >
> > I have a suspicion this was fixed differently in master.
> >
> > The fix looks incorrect since it will just make the directory exist, which will hide the error but won't fix whatever the race is causing the underlying determinism problem.
> >
> > Can you confirm the problem exists on master to start with please?
> > >
> > Cheers,
> >
> > Richard

On Thu, 2023-03-23 at 11:59 +0000, Marko, Peter wrote:
> Would this be more acceptable change?
> 
> -       cp -p ${S}/../$sde_file ${SDE_DEPLOYDIR}
> -       cp -p ${S}/../$sde_file ${SDE_FILE}
> +       cp -p $(dirname ${S})/$sde_file ${SDE_DEPLOYDIR}
> +       cp -p $(dirname ${S})/$sde_file ${SDE_FILE}

Yes, that makes it clearer what was going on.

We did have a different bug in this area which I thought this was
perhaps related to but it does sound like a different issue.

Cheers,

Richard

I have sent out the alternative implementation.

Thanks for your hints.
  Peter

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org> 
> Sent: Thursday, March 23, 2023 13:03
> To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][master][langdale][kirkstone][PATCH] gcc-shared-source: ensure that S exists in do_deploy_source_date_epoch
>
> On Thu, 2023-03-23 at 11:59 +0000, Marko, Peter wrote:
> > Would this be more acceptable change?
> > 
> > -       cp -p ${S}/../$sde_file ${SDE_DEPLOYDIR}
> > -       cp -p ${S}/../$sde_file ${SDE_FILE}
> > +       cp -p $(dirname ${S})/$sde_file ${SDE_DEPLOYDIR}
> > +       cp -p $(dirname ${S})/$sde_file ${SDE_FILE}
>
> Yes, that makes it clearer what was going on.
>
> We did have a different bug in this area which I thought this was perhaps related to but it does sound like a different issue.
>
> Cheers,
>
> Richard