All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
@ 2021-12-15 17:34 Ignacy Gawędzki
  2021-12-16 20:03 ` Arnout Vandecappelle
  2022-01-05 20:37 ` Yann E. MORIN
  0 siblings, 2 replies; 6+ messages in thread
From: Ignacy Gawędzki @ 2021-12-15 17:34 UTC (permalink / raw)
  To: buildroot

If the PATH initially contains host/bin, then the right date
executable is to be found after the *first* occurrence of fakedate in
normal (forward) PATH order.  Fix the wrapper so that that first
occurrence is found indeed.

Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
---
 package/fakedate/fakedate | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
index 9bef113357..03a4f04079 100755
--- a/package/fakedate/fakedate
+++ b/package/fakedate/fakedate
@@ -21,13 +21,18 @@
 DATE_BIN=false
 # Do not call any 'date' before us in the PATH, or that would create
 # an infinite recursion.
+last_date=false
 for date in $(which -a date |tac); do
     if [ "${date}" -ef "$0" ]; then
-        break
+	DATE_BIN=$last_date
     fi
-    DATE_BIN="${date}"
+    last_date="${date}"
 done
 
+if [ "$DATE_BIN" = false ]; then
+    DATE_BIN=$last_date
+fi
+
 if [ -n "$SOURCE_DATE_EPOCH" ]; then
     FORCE_EPOCH=1
     for i in "$@"; do
-- 
2.32.0
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
  2021-12-15 17:34 [Buildroot] [PATCH] package/fakedate: fix finding the right date executable Ignacy Gawędzki
@ 2021-12-16 20:03 ` Arnout Vandecappelle
  2021-12-16 21:08   ` Yann E. MORIN
  2022-01-05 20:37 ` Yann E. MORIN
  1 sibling, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2021-12-16 20:03 UTC (permalink / raw)
  To: Ignacy Gawędzki, buildroot; +Cc: Yann E. MORIN



On 15/12/2021 18:34, Ignacy Gawędzki wrote:
> If the PATH initially contains host/bin, then the right date
> executable is to be found after the *first* occurrence of fakedate in
> normal (forward) PATH order.  Fix the wrapper so that that first
> occurrence is found indeed.

  Just to be clear: the problem is that if you do

PATH=$PATH:$PWD/output/host/bin make

that it doesn't find the real date program, right?

> 
> Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> ---
>   package/fakedate/fakedate | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> index 9bef113357..03a4f04079 100755
> --- a/package/fakedate/fakedate
> +++ b/package/fakedate/fakedate
> @@ -21,13 +21,18 @@
>   DATE_BIN=false
>   # Do not call any 'date' before us in the PATH, or that would create
>   # an infinite recursion.
> +last_date=false
>   for date in $(which -a date |tac); do
>       if [ "${date}" -ef "$0" ]; then
> -        break
> +	DATE_BIN=$last_date
>       fi
> -    DATE_BIN="${date}"
> +    last_date="${date}"
>   done

  Although this works, wouldn't it be simpler to throw away the tac invocation 
and instead just take the first one after ourselves? I.e., something like:

found=false
for date in $(which -a date); do
     if [ "${date}" -ef "$@" ]; then
         found=true
     elif [ "$found" = true ]; then
         DATE_BIN="${date}"
     fi
done

(as usual, untested).

  Yann? Remember what was the reason for the tac?

  Regards,
  Arnout


>   
> +if [ "$DATE_BIN" = false ]; then
> +    DATE_BIN=$last_date
> +fi

> +
>   if [ -n "$SOURCE_DATE_EPOCH" ]; then
>       FORCE_EPOCH=1
>       for i in "$@"; do
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
  2021-12-16 20:03 ` Arnout Vandecappelle
@ 2021-12-16 21:08   ` Yann E. MORIN
  2021-12-17 22:28     ` Ignacy Gawędzki
  0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2021-12-16 21:08 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: Ignacy Gawędzki, buildroot

Arnout, All,

On 2021-12-16 21:03 +0100, Arnout Vandecappelle spake thusly:
> On 15/12/2021 18:34, Ignacy Gawędzki wrote:
> >If the PATH initially contains host/bin, then the right date
> >executable is to be found after the *first* occurrence of fakedate in
> >normal (forward) PATH order.  Fix the wrapper so that that first
> >occurrence is found indeed.
>  Just to be clear: the problem is that if you do
> PATH=$PATH:$PWD/output/host/bin make
> that it doesn't find the real date program, right?

This is very not nice to do sto, indeed... :-(
Unless this is again this SDK thing that is bitting us back?

> >Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> >---
> >  package/fakedate/fakedate | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> >diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> >index 9bef113357..03a4f04079 100755
> >--- a/package/fakedate/fakedate
> >+++ b/package/fakedate/fakedate
> >@@ -21,13 +21,18 @@
> >  DATE_BIN=false
> >  # Do not call any 'date' before us in the PATH, or that would create
> >  # an infinite recursion.
> >+last_date=false
> >  for date in $(which -a date |tac); do
> >      if [ "${date}" -ef "$0" ]; then
> >-        break
> >+	DATE_BIN=$last_date
> >      fi
> >-    DATE_BIN="${date}"
> >+    last_date="${date}"
> >  done
> 
>  Although this works, wouldn't it be simpler to throw away the tac
> invocation and instead just take the first one after ourselves? I.e.,
> something like:
> 
> found=false
> for date in $(which -a date); do
>     if [ "${date}" -ef "$@" ]; then
>         found=true
>     elif [ "$found" = true ]; then

You can simplify the elif test with (after all, that's the whole point
of using false/true instead of 0/1):

    elif ${found}; then

>         DATE_BIN="${date}"

And then, you can break the loop because we did find it.

>     fi
> done

This is usually what I would have suggested... However, see below...

> (as usual, untested).

>  Yann? Remember what was the reason for the tac?

0fe536b797c (package/fakedate: avoid recursion if not first in the PATH)

I used tac because it avoided using an intermediate variable like you
suggest above, with "found=true":

    We fix that by iterating, in reverse order, over all the 'date' we can
    find in PATH, and when we find ourselves, then we know the one we found
    in the iteration just before is the one that we should call.

This was a hack, and in restrospect, not that smart as it seemed to be...

So this means that we now have the reverse problem: we recurse if we are
also the last...

Your (Arnout's) solution is the best I can think of: it is obvious,
straightforward, and it is not trying to be smart, so I like it.

Ignacy, when you respin with Arnout's proposal, do not forget to add a
line like that to your commit log, please:

    Fixes: 0fe536b797cc0ae11bd54ed7046cd39b73780c18

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> 
> >+if [ "$DATE_BIN" = false ]; then
> >+    DATE_BIN=$last_date
> >+fi
> 
> >+
> >  if [ -n "$SOURCE_DATE_EPOCH" ]; then
> >      FORCE_EPOCH=1
> >      for i in "$@"; do
> >

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
  2021-12-16 21:08   ` Yann E. MORIN
@ 2021-12-17 22:28     ` Ignacy Gawędzki
  0 siblings, 0 replies; 6+ messages in thread
From: Ignacy Gawędzki @ 2021-12-17 22:28 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

On Thu, Dec 16, 2021 at 10:08:08PM +0100, thus spake Yann E. MORIN:
> Arnout, All,
> 
> On 2021-12-16 21:03 +0100, Arnout Vandecappelle spake thusly:
> > On 15/12/2021 18:34, Ignacy Gawędzki wrote:
> > >If the PATH initially contains host/bin, then the right date
> > >executable is to be found after the *first* occurrence of fakedate in
> > >normal (forward) PATH order.  Fix the wrapper so that that first
> > >occurrence is found indeed.
> >  Just to be clear: the problem is that if you do
> > PATH=$PATH:$PWD/output/host/bin make
> > that it doesn't find the real date program, right?
> 
> This is very not nice to do sto, indeed... :-(
> Unless this is again this SDK thing that is bitting us back?

In a nutshell, I have host/bin appended to my PATH because I often use
the buildroot-built cross-compiler chain.

> > >Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> > >---
> > >  package/fakedate/fakedate | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> > >index 9bef113357..03a4f04079 100755
> > >--- a/package/fakedate/fakedate
> > >+++ b/package/fakedate/fakedate
> > >@@ -21,13 +21,18 @@
> > >  DATE_BIN=false
> > >  # Do not call any 'date' before us in the PATH, or that would create
> > >  # an infinite recursion.
> > >+last_date=false
> > >  for date in $(which -a date |tac); do
> > >      if [ "${date}" -ef "$0" ]; then
> > >-        break
> > >+	DATE_BIN=$last_date
> > >      fi
> > >-    DATE_BIN="${date}"
> > >+    last_date="${date}"
> > >  done
> > 
> >  Although this works, wouldn't it be simpler to throw away the tac
> > invocation and instead just take the first one after ourselves? I.e.,
> > something like:
> > 
> > found=false
> > for date in $(which -a date); do
> >     if [ "${date}" -ef "$@" ]; then
> >         found=true
> >     elif [ "$found" = true ]; then
> 
> You can simplify the elif test with (after all, that's the whole point
> of using false/true instead of 0/1):
> 
>     elif ${found}; then
> 
> >         DATE_BIN="${date}"
> 
> And then, you can break the loop because we did find it.
> 
> >     fi
> > done

The problem with this version is that it won't choose the first
non-fakedate date following fakedate in the PATH, but the last.
Moveover, if, for any reason, fakedate is not in the PATH, it won't
find any date.

This is mostly why I kept the reverse iteration, to find the first
non-fakedate date that follows the first fakedate in the PATH, and to
fall back on the first date in the PATH if fakedate is not found at
all.

Regards,

Ignacy

-- 
Ignacy Gawędzki
R&D Engineer
Green Communications
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
  2021-12-15 17:34 [Buildroot] [PATCH] package/fakedate: fix finding the right date executable Ignacy Gawędzki
  2021-12-16 20:03 ` Arnout Vandecappelle
@ 2022-01-05 20:37 ` Yann E. MORIN
  2022-01-06 10:32   ` Ignacy Gawędzki
  1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2022-01-05 20:37 UTC (permalink / raw)
  To: Ignacy Gawędzki; +Cc: buildroot

Ignacy, All,

On 2021-12-15 18:34 +0100, Ignacy Gawędzki spake thusly:
> If the PATH initially contains host/bin, then the right date
> executable is to be found after the *first* occurrence of fakedate in
> normal (forward) PATH order.  Fix the wrapper so that that first
> occurrence is found indeed.
> 
> Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>

After discussing this with Arnout and Thomas, we concluded that Arnout's
proposal, with my ltile amendment, was the proper solution. Except that
the loop had to be broken on first hit, of course, so I added the proper
break.

I also added a test that we actually did find a date executable.

Of course, if we missed something, then feel free to yel back! ;-)

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/fakedate/fakedate | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> index 9bef113357..03a4f04079 100755
> --- a/package/fakedate/fakedate
> +++ b/package/fakedate/fakedate
> @@ -21,13 +21,18 @@
>  DATE_BIN=false
>  # Do not call any 'date' before us in the PATH, or that would create
>  # an infinite recursion.
> +last_date=false
>  for date in $(which -a date |tac); do
>      if [ "${date}" -ef "$0" ]; then
> -        break
> +	DATE_BIN=$last_date
>      fi
> -    DATE_BIN="${date}"
> +    last_date="${date}"
>  done
>  
> +if [ "$DATE_BIN" = false ]; then
> +    DATE_BIN=$last_date
> +fi
> +
>  if [ -n "$SOURCE_DATE_EPOCH" ]; then
>      FORCE_EPOCH=1
>      for i in "$@"; do
> -- 
> 2.32.0
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable
  2022-01-05 20:37 ` Yann E. MORIN
@ 2022-01-06 10:32   ` Ignacy Gawędzki
  0 siblings, 0 replies; 6+ messages in thread
From: Ignacy Gawędzki @ 2022-01-06 10:32 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

On Wed, Jan 05, 2022 at 09:37:27PM +0100, thus spake Yann E. MORIN:
> Ignacy, All,
> 
> On 2021-12-15 18:34 +0100, Ignacy Gawędzki spake thusly:
> > If the PATH initially contains host/bin, then the right date
> > executable is to be found after the *first* occurrence of fakedate in
> > normal (forward) PATH order.  Fix the wrapper so that that first
> > occurrence is found indeed.
> > 
> > Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> 
> After discussing this with Arnout and Thomas, we concluded that Arnout's
> proposal, with my ltile amendment, was the proper solution. Except that
> the loop had to be broken on first hit, of course, so I added the proper
> break.
> 
> I also added a test that we actually did find a date executable.
> 
> Of course, if we missed something, then feel free to yel back! ;-)

With this code, fakedate is not usable without having its path in
PATH.  If that will never be a problem, then I have no reason to yell. =)

> 
> Applied to master, thanks.
> 
> Regards,
> Yann E. MORIN.
> 
> > ---
> >  package/fakedate/fakedate | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> > index 9bef113357..03a4f04079 100755
> > --- a/package/fakedate/fakedate
> > +++ b/package/fakedate/fakedate
> > @@ -21,13 +21,18 @@
> >  DATE_BIN=false
> >  # Do not call any 'date' before us in the PATH, or that would create
> >  # an infinite recursion.
> > +last_date=false
> >  for date in $(which -a date |tac); do
> >      if [ "${date}" -ef "$0" ]; then
> > -        break
> > +	DATE_BIN=$last_date
> >      fi
> > -    DATE_BIN="${date}"
> > +    last_date="${date}"
> >  done
> >  
> > +if [ "$DATE_BIN" = false ]; then
> > +    DATE_BIN=$last_date
> > +fi
> > +
> >  if [ -n "$SOURCE_DATE_EPOCH" ]; then
> >      FORCE_EPOCH=1
> >      for i in "$@"; do
> > -- 
> > 2.32.0
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> 
> 

-- 
Ignacy Gawędzki
R&D Engineer
Green Communications
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-01-06 10:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-15 17:34 [Buildroot] [PATCH] package/fakedate: fix finding the right date executable Ignacy Gawędzki
2021-12-16 20:03 ` Arnout Vandecappelle
2021-12-16 21:08   ` Yann E. MORIN
2021-12-17 22:28     ` Ignacy Gawędzki
2022-01-05 20:37 ` Yann E. MORIN
2022-01-06 10:32   ` Ignacy Gawędzki

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.