Re: [Buildroot] [PATCH] package/fakedate: fix finding the right date executable

["Yann E. MORIN" <yann.morin.1998@free.fr>]

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