[RFC] New patch to review for coreboot: a61a8fe util/board_status: Use capture and not commit time

[Paul Menzel]

Dear coreboot folks,

just a heads-up that I sent the following patch set touch board status’
directory structure for review.

Am Samstag, den 19.04.2014, 22:27 +0200 schrieb Paul Menzel:
> Paul Menzel ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/5553
>
> -gerrit
>
> commit a61a8fedadf418fc380743844572c0d9ea0f6eed
> Author: Paul Menzel 
> Date:   Sat Apr 19 21:31:27 2014 +0200
> util/board_status: Use capture and not commit time
>
> Currently the directory structure is
>
> 	<vendor>/<board>/<revision>/<timestamp>
>
> where the time stamp is the commit date. This does not allow to upload
> results for the same commit and same board with different configuration.
> The hardware could differ, for example the used processor, or the
> configuration of coreboot or the used payload or its configuration.
>
> Also it is theoretically possible, though not very easy to achieve with
> the current Gerrit workflow, that two commits have the same commit date.
>
> So instead of using the date of the commit simply use the date of
> running the script `board_status.sh`.
>
> 	<vendor>/<board>/<revision>/<time_now>
>
> In theory that could still give collisions, but the chances of two
> people having the same board running coreboot build from the same commit
> and running `board_status.sh` at the same time are very low.
>
> Getting the current time is not easy, as the interface of `date` is not
> standardized, though `date -u` probably should work the same. As
> `getrevision.sh` already takes care of the formatting, pass the
> non-tracked file `/bin/true` to `getrevision.sh`, which causes it to
> return the current time properly formatted. Get rid of the resulting
> warning
>
> 	Warning: Path "/bin/true" is not under version control.
>
> by removing the line from `getrevision.sh`.
>
> Change-Id: Id92a4b660e910b524f15297472ff570ef797039e
> Signed-off-by: Paul Menzel <[e-mail]>
>
> util/board_status/board_status.sh | 7 ++++---
>  util/board_status/getrevision.sh  | 3 ---
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/util/board_status/board_status.sh b/util/board_status/board_status.sh
> index 3cff231..19c69d1 100644
> --- a/util/board_status/board_status.sh
> +++ b/util/board_status/board_status.sh
> @@ -141,7 +141,7 @@ fi
>  tmpdir=$(mktemp -d)
>
> # Obtain board and revision info to form the directory structure:
> -# <vendor>/<board>/<revision>/<timestamp>
> +# <vendor>/<board>/<revision>/<time_now>
>  cbfstool_cmd="util/cbfstool/cbfstool"
>  test_cmd $LOCAL "$cbfstool_cmd"
>  $cbfstool_cmd build/coreboot.rom extract -n config -f ${tmpdir}/config.txt
> @@ -153,8 +153,9 @@ getrevision="util/board_status/getrevision.sh"
>  test_cmd $LOCAL $getrevision
>  tagged_version=$($getrevision -T)
>  timestamp=$($getrevision -t)
> +time_now=$($getrevision -t /bin/true)

That is an ugly hack, which lead me to remove the warning in
`getrevision.sh`. Should I just use `date -u` and format that somehow?
Any ideas?

Should the time stamp of the commit date of the commit still be put into
`revision.txt`?
> -results="${vendor}/${mainboard}/${tagged_version}/${timestamp}"
> +results="${vendor}/${mainboard}/${tagged_version}/${time_now}"
>
> echo "Temporarily placing output in ${tmpdir}/${results}"
>  mkdir -p "${tmpdir}/${results}"
> @@ -202,7 +203,7 @@ if [ $UPLOAD_RESULTS -eq 1 ]; then
>
> echo "Uploading results"
>    git add "${vendor}"
> git commit -a -m "${mainboard_dir}/${tagged_version}/${timestamp}"
>
> git commit -a -m "${mainboard_dir}/${tagged_version}/${time_now}"
> git push origin
>
> # Results have been uploaded so it's pointless to keep the
>
> diff --git a/util/board_status/getrevision.sh b/util/board_status/getrevision.sh
> index 0a6b6c6..bdd4748 100755
> --- a/util/board_status/getrevision.sh
> +++ b/util/board_status/getrevision.sh
> @@ -233,9 +233,6 @@ main() {
>    if [ -z "$query_path" ] ; then
>    	query_path=.
>    fi
> if ! is_file_tracked "$query_path" ; then
> echo "Warning: Path \"${query_path}\" is not under version control.">&2
> fi
> if [ -z "$action" ] ; then
> show_help
> echo "Error: No actions specified"

Thanks,

Paul