Re: [PATCH v2 05/11] t0027: make hash size independent'

2018-08-20 Thread Torsten Bögershausen
On 20.08.18 00:10, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson
>  wrote:
>> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
>>> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
 -   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
 +   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp+" &&
>>>
>>> My immediate thought upon reading this was whether "+" is valid in
>>> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
>>> would make sense to use a character less likely to cause brain
>>> hiccups; for instance, "exp0'.
>>
>> The reason I picked that is that we use it quite a bit in the Makefile,
>> so it seemed like an obvious choice for a temporary file name.  If you
>> feel strongly I can pick something else, but I thought it would be
>> reasonably intuitive for other developers.  Maybe I was wrong, though.
> 
> I don't feel strongly about it. My brain tripped over it probably
> because it's not an idiom in Git tests. In fact, I see just one place
> where "+" has been used like this, in t6026.
> 

Probably "tmp" is a better name than "exp+" 
(Why the '+' ? Is it better that the non-'+' ?)

Anyway,
If we re-order a little bit, can we use a simple '|' instead ?

tr '\015\000abcdef0123456789' QN0 <"$2" |
sed -e "s/+/$ZERO_OID/"   >"$exp" &&
tr '\015\000abcdef0123456789' QN0 <"$3" >"$act" &&


Re: [PATCH v2 05/11] t0027: make hash size independent'

2018-08-19 Thread Eric Sunshine
On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson
 wrote:
> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
> > On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> > > -   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
> > > +   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp+" 
> > > &&
> >
> > My immediate thought upon reading this was whether "+" is valid in
> > Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
> > would make sense to use a character less likely to cause brain
> > hiccups; for instance, "exp0'.
>
> The reason I picked that is that we use it quite a bit in the Makefile,
> so it seemed like an obvious choice for a temporary file name.  If you
> feel strongly I can pick something else, but I thought it would be
> reasonably intuitive for other developers.  Maybe I was wrong, though.

I don't feel strongly about it. My brain tripped over it probably
because it's not an idiom in Git tests. In fact, I see just one place
where "+" has been used like this, in t6026.


Re: [PATCH v2 05/11] t0027: make hash size independent'

2018-08-19 Thread brian m. carlson
On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
>  wrote:
> > We transform various object IDs into all-zero object IDs for comparison.
> > Adjust the length as well so that this works for all hash algorithms.
> >
> > Signed-off-by: brian m. carlson 
> > ---
> > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> > @@ -15,8 +15,9 @@ compare_ws_file () {
> > -   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
> > +   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp+" &&
> 
> My immediate thought upon reading this was whether "+" is valid in
> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
> would make sense to use a character less likely to cause brain
> hiccups; for instance, "exp0'.

The reason I picked that is that we use it quite a bit in the Makefile,
so it seemed like an obvious choice for a temporary file name.  If you
feel strongly I can pick something else, but I thought it would be
reasonably intuitive for other developers.  Maybe I was wrong, though.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: [PATCH v2 05/11] t0027: make hash size independent

2018-08-19 Thread Eric Sunshine
On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
 wrote:
> We transform various object IDs into all-zero object IDs for comparison.
> Adjust the length as well so that this works for all hash algorithms.
>
> Signed-off-by: brian m. carlson 
> ---
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> @@ -15,8 +15,9 @@ compare_ws_file () {
> -   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
> +   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp+" &&

My immediate thought upon reading this was whether "+" is valid in
Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
would make sense to use a character less likely to cause brain
hiccups; for instance, "exp0'.

> tr '\015\000abcdef0123456789' QN0 <"$3" >"$act" &&
> +   sed -e "s/+/$ZERO_OID/" "$exp+" >"$exp" &&


[PATCH v2 05/11] t0027: make hash size independent

2018-08-19 Thread brian m. carlson
We transform various object IDs into all-zero object IDs for comparison.
Adjust the length as well so that this works for all hash algorithms.

Signed-off-by: brian m. carlson 
---
 t/t0027-auto-crlf.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index beb5927f77..e748060489 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -15,8 +15,9 @@ compare_ws_file () {
pfx=$1
exp=$2.expect
act=$pfx.actual.$3
-   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
+   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp+" &&
tr '\015\000abcdef0123456789' QN0 <"$3" >"$act" &&
+   sed -e "s/+/$ZERO_OID/" "$exp+" >"$exp" &&
test_cmp "$exp" "$act" &&
rm "$exp" "$act"
 }