From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 11 Jun 2018 07:47:43 GMT
Subject: Re: [PATCH 01/10] t: add tool to translate hash-related values
Message-ID: <20180611074743.GA24066@flurp.local>
In-Reply-To: <20180604235229.279814-2-sandals@crustytoothpaste.net>

On Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:
> Add a test function helper, test_translate, that will produce its first
> argument if the hash in use is SHA-1 and the second if its argument is
> NewHash.  Implement a mode that can read entries from a file as well for
> reusability across tests.

The word "translate" is very generic and is (at least in my mind)
strongly associated with i18n/l10n, so the name test_translate() may
be confusing for readers. Perhaps test_oid_lookup() or test_oid_get()
or even just test_oid()?

> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -1147,3 +1147,43 @@ depacketize () {
> +test_translate_f_ () {
> +	local file="$TEST_DIRECTORY/translate/$2" &&
> +	perl -e '
> +		$delim = "\t";
> +		($hoidlen, $file, $arg) = @ARGV;
> +		open($fh, "<", $file) or die "open: $!";
> +		while (<$fh>) {
> +			# Allow specifying other delimiters.
> +			$delim = $1 if /^#!\sdelimiter\s(.)/;
> +			next if /^#/;
> +			@fields = split /$delim/, $_, 3;
> +			if ($fields[0] eq $arg) {
> +				print($hoidlen == 40 ? $fields[1] : $fields[2]);
> +				last;
> +			}
> +		}
> +	' "$1" "$file" "$3"
> +}

This is a very expensive lookup since it invokes a heavyweight command
(perl, in this case) for *every* OID it needs to retrieve from the
file. Windows users, especially, will likely not be happy about this.
See below for an alternative.

> +# Without -f, print the first argument if we are using SHA-1 and the second if
> +# we're using NewHash.
> +# With -f FILE ARG, read the (by default) tab-delimited file from
> +# t/translate/FILE, finding the first field matching ARG and printing either the
> +# second or third field depending on the hash in use.
> +test_translate () {
> +	local hoidlen=$(printf "%s" "$EMPTY_BLOB" | wc -c) &&
> +	if [ "$1" = "-f" ]
> +	then
> +		shift &&
> +		test_translate_f_ "$hoidlen" "$@"
> +	else
> +		if [ "$hoidlen" -eq 40 ]
> +		then
> +			printf "%s" "$1"
> +		else
> +			printf "%s" "$2"
> +		fi
> +	fi
> +}

This is less flexible than I had expected, allowing for only SHA1 and
NewHash. When you had written about OID lookup table functionality in
email previously, my impression was that the tables would allow values
for arbitrary hash algorithms. Such flexibility would allow people to
experiment with hash algorithms without having to once again retrofit
the test suite machinery.

Here's what I had envisioned when reading your emails about OID lookup
table functionality:

--- >8 ---
test_detect_hash () {
    test_hash_algo=...
}
    
test_oid_cache () {
    while read tag rest
    do
        case $tag in \#*) continue ;; esac

        for x in $rest
        do
            k=${x%:*}
            v=${x#*:}
            if test "$k" = $test_hash_algo
            then
                eval "test_oid_$tag=$v"
                break
            fi
        done
    done
}

test_oid () {
    if test $# -gt 1
    then
        test_oid_cache <<-EOF
        $*
        EOF
    fi
    eval "echo \$test_oid_$1"
}
--- >8 ---

test_detect_hash() would detect the hash algorithm and record it
instead of having to determine it each time an OID needs to be
"translated". It probably would be called by test-lib.sh.

test_oid_cache() reads a table of OID's and caches them so that
subsequent look-ups are fast. It would be called (possibly multiple
times) by any script needing to "translate" OID's. Each line of the
table has form "tag algo:value ...". Lines starting with '#' are
comments (as in your implementation). Since it reads stdin, it works
equally well reading OID tables from files and here-docs, which
provides extra flexibility for test authors. For instance:

    test_oid_cache <translate/hash-info

or:

    test_oid_cache <<-\EOF
    rawsz sha1:20 NewHash:32
    hexsz sha1:40 NewHash:64
    EOF

test_oid() does the actual OID lookup/translation. It looks up a
pre-cached value or, for convenience (as per your implementation), can
choose between values specified at invocation time. For example, the
simple case:

    $(test_oid hexsz)

And, when specifying values from which to choose based upon hash
algorithm:

    $(test_oid bored sha1:deadbeef NewHash:feedface)

A nice property of how this implementation caches values is that you
don't need test_oid() for really simple cases. You can just access the
variable directly. For instance: $test_oid_hexsz

Another nice property of how caching is implemented is that someone
testing a new hash algorithm doesn't have edit the existing tables to
tack the value for the new algorithm onto the end of each line. It
works equally well to place those values in a new file or new here-doc
or simply append new lines to existing files or here-docs. For
instance, someone testing algorithm "NewShiny" can just add those
lines without having to modify existing lines:

    test_oid_cache <<-\EOF
    rawsz sha1:20 NewHash:32
    hexsz sha1:40 NewHash:64
    # testing NewShiny algorithm
    rawsz: NewShiny:42
    hexsz: NewShiny:84
    EOF