Re: [PATCH 07/15] t4011: abstract away SHA-1-specific constants
On 2019-10-08 at 12:33:43, Bert Wesarg wrote: > On Tue, Oct 8, 2019 at 2:21 PM Derrick Stolee wrote: > > > > On 10/5/2019 5:12 PM, brian m. carlson wrote: > > > Adjust the test so that it computes variables for object IDs instead of > > > using hard-coded hashes. > > > > [snip] > > > > > @@ -137,14 +141,17 @@ test_expect_success SYMLINKS 'setup symlinks with > > > attributes' ' > > > ' > > > > > > test_expect_success SYMLINKS 'symlinks do not respect userdiff config by > > > path' ' > > > - cat >expect <<-\EOF && > > > + file=$(git rev-parse --short $(git hash-object file.bin)) && > > > + link=$(git rev-parse --short \ > > > + $(printf file.bin | git hash-object --stdin)) && > > > > Why this change from $(git hash-object file.bin) to > > $(printf file.bin | git hash-object --stdin)? > > thats two different things. The first hashes the content of file > "file.bin". The second hashes the literal string "file.bin". To avoid > this confusion, may I suggest to use 'printf "%s" "file.bin"', so that > it is clear, that the literal string is meant in this context? This is completely correct, and, yes, I can definitely make that change. In fact, the fact that this is confusing probably means I should use a suitably named shell function for this, so I'll make that change when I reroll. > > For that matter, why are you using the "printf|git hash-object" > > pattern throughout your change? Seems like an unnecessary hurdle > > to me. I would normally use echo for these types of things (because that's our style and it's more customary), but in this case the symlink contents don't contain a newline, so printf is required. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH 07/15] t4011: abstract away SHA-1-specific constants
On 10/8/2019 8:33 AM, Bert Wesarg wrote: > On Tue, Oct 8, 2019 at 2:21 PM Derrick Stolee wrote: >> >> On 10/5/2019 5:12 PM, brian m. carlson wrote: >>> Adjust the test so that it computes variables for object IDs instead of >>> using hard-coded hashes. >> >> [snip] >> >>> @@ -137,14 +141,17 @@ test_expect_success SYMLINKS 'setup symlinks with >>> attributes' ' >>> ' >>> >>> test_expect_success SYMLINKS 'symlinks do not respect userdiff config by >>> path' ' >>> - cat >expect <<-\EOF && >>> + file=$(git rev-parse --short $(git hash-object file.bin)) && >>> + link=$(git rev-parse --short \ >>> + $(printf file.bin | git hash-object --stdin)) && >> >> Why this change from $(git hash-object file.bin) to >> $(printf file.bin | git hash-object --stdin)? > > thats two different things. The first hashes the content of file > "file.bin". The second hashes the literal string "file.bin". To avoid > this confusion, may I suggest to use 'printf "%s" "file.bin"', so that > it is clear, that the literal string is meant in this context? Ah, and because the resulting hash is for the contents of the symlink (not the contents of the file), it makes sense to use printf here. Thanks for the clarification! -Stolee
Re: [PATCH 07/15] t4011: abstract away SHA-1-specific constants
On Tue, Oct 8, 2019 at 2:21 PM Derrick Stolee wrote: > > On 10/5/2019 5:12 PM, brian m. carlson wrote: > > Adjust the test so that it computes variables for object IDs instead of > > using hard-coded hashes. > > [snip] > > > @@ -137,14 +141,17 @@ test_expect_success SYMLINKS 'setup symlinks with > > attributes' ' > > ' > > > > test_expect_success SYMLINKS 'symlinks do not respect userdiff config by > > path' ' > > - cat >expect <<-\EOF && > > + file=$(git rev-parse --short $(git hash-object file.bin)) && > > + link=$(git rev-parse --short \ > > + $(printf file.bin | git hash-object --stdin)) && > > Why this change from $(git hash-object file.bin) to > $(printf file.bin | git hash-object --stdin)? thats two different things. The first hashes the content of file "file.bin". The second hashes the literal string "file.bin". To avoid this confusion, may I suggest to use 'printf "%s" "file.bin"', so that it is clear, that the literal string is meant in this context? Bert > > For that matter, why are you using the "printf|git hash-object" > pattern throughout your change? Seems like an unnecessary hurdle > to me. > > -Stolee
Re: [PATCH 07/15] t4011: abstract away SHA-1-specific constants
On 10/5/2019 5:12 PM, brian m. carlson wrote: > Adjust the test so that it computes variables for object IDs instead of > using hard-coded hashes. [snip] > @@ -137,14 +141,17 @@ test_expect_success SYMLINKS 'setup symlinks with > attributes' ' > ' > > test_expect_success SYMLINKS 'symlinks do not respect userdiff config by > path' ' > - cat >expect <<-\EOF && > + file=$(git rev-parse --short $(git hash-object file.bin)) && > + link=$(git rev-parse --short \ > + $(printf file.bin | git hash-object --stdin)) && Why this change from $(git hash-object file.bin) to $(printf file.bin | git hash-object --stdin)? For that matter, why are you using the "printf|git hash-object" pattern throughout your change? Seems like an unnecessary hurdle to me. -Stolee
