git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v1 1/1] t0003: Call dd with portable blocksize
@ 2023-01-21 11:05 tboegi
  2023-01-22  0:08 ` Junio C Hamano
  2023-01-22  6:28 ` [PATCH v2 " tboegi
  0 siblings, 2 replies; 5+ messages in thread
From: tboegi @ 2023-01-21 11:05 UTC (permalink / raw)
  To: tboegi, git

From: Torsten Bögershausen <tboegi@web.de>

The command `dd -bs=101M count=1` is not portable.
Use `bs=1048576 count=101`, which does the same, instead.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t0003-attributes.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index d0284fe2d7..394a08e6d6 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -400,7 +400,7 @@ test_expect_success 'large attributes line ignores trailing content in tree' '

 test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
 	test_when_finished "rm .gitattributes" &&
-	dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
+	dd if=/dev/zero of=.gitattributes bs=1048576 count=101 2>/dev/null &&
 	git check-attr --all path >/dev/null 2>err &&
 	echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
 	test_cmp expect err
@@ -428,7 +428,7 @@ test_expect_success 'large attributes line ignores trailing content in index' '

 test_expect_success EXPENSIVE 'large attributes file ignored in index' '
 	test_when_finished "git update-index --remove .gitattributes" &&
-	blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
+	blob=$(dd if=/dev/zero bs=1048576 count=101 2>/dev/null | git hash-object -w --stdin) &&
 	git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
 	git check-attr --cached --all path >/dev/null 2>err &&
 	echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
--
2.39.1.254.g904d404274


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 1/1] t0003: Call dd with portable blocksize
  2023-01-21 11:05 [PATCH v1 1/1] t0003: Call dd with portable blocksize tboegi
@ 2023-01-22  0:08 ` Junio C Hamano
  2023-01-22  6:28 ` [PATCH v2 " tboegi
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-01-22  0:08 UTC (permalink / raw)
  To: tboegi; +Cc: git

tboegi@web.de writes:

> From: Torsten Bögershausen <tboegi@web.de>
>
> The command `dd -bs=101M count=1` is not portable.

No need for '-'; the UI of dd was meant as a joke and deliberately
deviates from UNIX norm to use '-' as an option introducer.

> Use `bs=1048576 count=101`, which does the same, instead.

Thanks for catching this.  It always is hard to catch these mistakes
made in code that was cooked behind embargo, as there aren't many
eyeballs on the changes.

Strictly speaking, "bs=1048576 count=101" does not do the same thing
(unlike the original that does a single write(2)system call of a
huge buffer, it issues 101 smaller write(2)).

It definitely is an improvement, independently from the portability
issues, to rewrite it like you did.  Unnecessarily large an I/O
should be avoided.

Will queue.  Thanks.

> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>  t/t0003-attributes.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> index d0284fe2d7..394a08e6d6 100755
> --- a/t/t0003-attributes.sh
> +++ b/t/t0003-attributes.sh
> @@ -400,7 +400,7 @@ test_expect_success 'large attributes line ignores trailing content in tree' '
>
>  test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
>  	test_when_finished "rm .gitattributes" &&
> -	dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
> +	dd if=/dev/zero of=.gitattributes bs=1048576 count=101 2>/dev/null &&
>  	git check-attr --all path >/dev/null 2>err &&
>  	echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
>  	test_cmp expect err
> @@ -428,7 +428,7 @@ test_expect_success 'large attributes line ignores trailing content in index' '
>
>  test_expect_success EXPENSIVE 'large attributes file ignored in index' '
>  	test_when_finished "git update-index --remove .gitattributes" &&
> -	blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
> +	blob=$(dd if=/dev/zero bs=1048576 count=101 2>/dev/null | git hash-object -w --stdin) &&
>  	git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
>  	git check-attr --cached --all path >/dev/null 2>err &&
>  	echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
> --
> 2.39.1.254.g904d404274

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/1] t0003: Call dd with portable blocksize
  2023-01-21 11:05 [PATCH v1 1/1] t0003: Call dd with portable blocksize tboegi
  2023-01-22  0:08 ` Junio C Hamano
@ 2023-01-22  6:28 ` tboegi
  2023-01-22 16:21   ` Junio C Hamano
  1 sibling, 1 reply; 5+ messages in thread
From: tboegi @ 2023-01-22  6:28 UTC (permalink / raw)
  To: tboegi, git

From: Torsten Bögershausen <tboegi@web.de>

The command `dd bs=101M count=1` is not portable,
e.g. dd shipped with MacOs does not understand the 'M'.

Use `bs=1048576 count=101`, which achives the same, instead.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t0003-attributes.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index d0284fe2d7..394a08e6d6 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -400,7 +400,7 @@ test_expect_success 'large attributes line ignores trailing content in tree' '

 test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
 	test_when_finished "rm .gitattributes" &&
-	dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
+	dd if=/dev/zero of=.gitattributes bs=1048576 count=101 2>/dev/null &&
 	git check-attr --all path >/dev/null 2>err &&
 	echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
 	test_cmp expect err
@@ -428,7 +428,7 @@ test_expect_success 'large attributes line ignores trailing content in index' '

 test_expect_success EXPENSIVE 'large attributes file ignored in index' '
 	test_when_finished "git update-index --remove .gitattributes" &&
-	blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
+	blob=$(dd if=/dev/zero bs=1048576 count=101 2>/dev/null | git hash-object -w --stdin) &&
 	git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
 	git check-attr --cached --all path >/dev/null 2>err &&
 	echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
--
2.39.1.254.g904d404274


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] t0003: Call dd with portable blocksize
  2023-01-22  6:28 ` [PATCH v2 " tboegi
@ 2023-01-22 16:21   ` Junio C Hamano
  2023-01-22 18:00     ` Torsten Bögershausen
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2023-01-22 16:21 UTC (permalink / raw)
  To: tboegi; +Cc: git

tboegi@web.de writes:

> From: Torsten Bögershausen <tboegi@web.de>
>
> The command `dd bs=101M count=1` is not portable,
> e.g. dd shipped with MacOs does not understand the 'M'.

Very good piece of information to have here.

> Use `bs=1048576 count=101`, which achives the same, instead.

I'd locally tweak (read: no need to resend) it to

    Use `dd bs=1048576 count=101`, which ...

and downcase "Call" on the title line.

A tangent.  I wonder how portable 

    dd bs=1024x1024 count=101
    dd bs=1kx1k count=101

are in practice.  "Two or more positive decimal numbers (with or
without 'k' or 'b') separated by 'x', specifying the product of the
indicated values" is from POSIX, but I haven't used it myself (I
know GNU dd groks it).

In any case, thanks for the update.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] t0003: Call dd with portable blocksize
  2023-01-22 16:21   ` Junio C Hamano
@ 2023-01-22 18:00     ` Torsten Bögershausen
  0 siblings, 0 replies; 5+ messages in thread
From: Torsten Bögershausen @ 2023-01-22 18:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sun, Jan 22, 2023 at 08:21:45AM -0800, Junio C Hamano wrote:
> tboegi@web.de writes:
>
> > From: Torsten Bögershausen <tboegi@web.de>
> >
> > The command `dd bs=101M count=1` is not portable,
> > e.g. dd shipped with MacOs does not understand the 'M'.
>
> Very good piece of information to have here.
>
> > Use `bs=1048576 count=101`, which achives the same, instead.
>
> I'd locally tweak (read: no need to resend) it to
>
>     Use `dd bs=1048576 count=101`, which ...
>
> and downcase "Call" on the title line.
>
> A tangent.  I wonder how portable
>
>     dd bs=1024x1024 count=101
This works all Unix-ish system I have: both on MacOs and FreeBSD.

>     dd bs=1kx1k count=101
Works on FreeBSD.
Does not work under MacOS: "dd: bs: illegal numeric value"


> In any case, thanks for the update.

Thanks for the review and the tweaking.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-22 18:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-21 11:05 [PATCH v1 1/1] t0003: Call dd with portable blocksize tboegi
2023-01-22  0:08 ` Junio C Hamano
2023-01-22  6:28 ` [PATCH v2 " tboegi
2023-01-22 16:21   ` Junio C Hamano
2023-01-22 18:00     ` Torsten Bögershausen

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).