| Message ID | 20211215173432.r75invoe3ff6mthg@zenon.in.qult.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | package/fakedate: fix finding the right date executable | expand |
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 >
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 > >
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, 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
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. | > '------------------------------^-------^------------------^--------------------' > >
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
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(-)