All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jérôme Pouiller" <jezz@sysmic.org>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] fakedate: simplify logic
Date: Wed, 08 Feb 2017 15:50:31 +0100	[thread overview]
Message-ID: <3055501.XEoAAfpss1@sagittea> (raw)
In-Reply-To: <20170207220817.5781-1-peter@korsgaard.com>

Hello Peter,

On Tuesday 07 February 2017 23:08:17 Peter Korsgaard wrote:
[...]
> diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate
> index 4a9b9b5e6..abe6b38f9 100755
> --- a/package/fakedate/fakedate
> +++ b/package/fakedate/fakedate
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  # vim: set sw=4 expandtab:
>  #
>  # This program is free software; you can redistribute it and/or 
modify
> @@ -18,18 +18,12 @@
>  # Copyright (C) 2016 J?r?me Pouiller <jezz@sysmic.org>
>  #
>  
> -# Sanity check
> -if ! readlink -f "$0" | grep -q fakedate; then
> -    echo "fakedate: Please name this script \`fakedate'"
> -    exit 1
> -fi
> -
>  DATE_BIN=false
>  # Do not call `date' directly since it will produce an infinite 
recursion.
>  # Instead, find path of true `date' binary.
> -for P in `echo $PATH | tr ':' ' '`; do
> +IFS=':'; for P in $PATH; do

I think you have to reset $IFS to its previous value after used
it.

Globally, I dislike use of $IFS. I think it may produce unattended side 
effects.


>      if [ -x "$P/date" ]; then
> -        if readlink -f "$P/date" | grep -qv fakedate; then
> +        if ! [ "$P/date" -ef "$0" ]; then

Nice! I didn't know '-ef'.

>              DATE_BIN="$P/date"
>              break;
>          fi
> @@ -50,8 +44,8 @@ if [ -n "$SOURCE_DATE_EPOCH" ]; then
>      done
>      if [ $FORCE_EPOCH -eq 1 ]; then
>          echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of 
true time" >&2
> -        exec $DATE_BIN -d "@$SOURCE_DATE_EPOCH" "$@"
> +	ARGS="-d @$SOURCE_DATE_EPOCH"

Indentation seems incorrect: s/\t/        /

>      fi
>  fi
>  
> -exec $DATE_BIN "$@"
> +exec $DATE_BIN $ARGS "$@"

It's matter of taste.


> diff --git a/package/fakedate/fakedate.mk 
b/package/fakedate/fakedate.mk
> index 61d4bd702..e81ce5dac 100644
> --- a/package/fakedate/fakedate.mk
> +++ b/package/fakedate/fakedate.mk
> @@ -8,8 +8,7 @@
>  HOST_FAKEDATE_LICENSE = GPLv2+
>  
>  define HOST_FAKEDATE_INSTALL_CMDS
> -	$(INSTALL) -D -m 755 package/fakedate/fakedate 
$(HOST_DIR)/usr/bin/fakedate
> -	ln -sfn fakedate $(HOST_DIR)/usr/bin/date
> +	$(INSTALL) -D -m 755 package/fakedate/fakedate 
$(HOST_DIR)/usr/bin/date

Hmm... In order to not confuse user, I prefer to keep fakedate as 
original name and adding a symlink beside.

>  endef
>  
>  $(eval $(host-generic-package))
> 

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

  reply	other threads:[~2017-02-08 14:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07 22:08 [Buildroot] [PATCH] fakedate: simplify logic Peter Korsgaard
2017-02-08 14:50 ` Jérôme Pouiller [this message]
2017-03-27 21:46   ` Arnout Vandecappelle
2017-07-03 13:14     ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3055501.XEoAAfpss1@sagittea \
    --to=jezz@sysmic.org \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.