[PATCH] Avoid using dc in git-count-objects

[PATCH] Avoid using dc in git-count-objects

[Johannes Schindelin]

Using dc is not really necessary, since expr understands summing 32 bit 
signed integers. Which means that git-count-objects will now fail when 2 
GB of unpacked objects have accumulated.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	This is (again) something I found on a default cygwin setup:
	dc is not installed. I do not even know if you can install
	dc on cygwin without compiling it yourself.

	FYI: I use git-count-objects in t5500-fetch-pack to find out
	if the correct number of objects was fetched.

 git-count-objects.sh |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/git-count-objects.sh b/git-count-objects.sh
index 843d2fd..7f9d8e5 100755
--- a/git-count-objects.sh
+++ b/git-count-objects.sh
@@ -2,12 +2,18 @@
 
 . git-sh-setup
 
+function sum () {
+	local result=0
+	while read line; do
+		result=$(expr $result + $line)
+	done
+	echo $result
+}
+
 echo $(find "$GIT_DIR/objects"/?? -type f -print 2>/dev/null | wc -l) objects, \
 $({
-    echo 0
     # "no-such" is to help Darwin folks by not using xargs -r.
     find "$GIT_DIR/objects"/?? -type f -print 2>/dev/null |
     xargs du -k "$GIT_DIR/objects/no-such" 2>/dev/null |
-    sed -e 's/[ 	].*/ +/'
-    echo p
-} | dc) kilobytes
+    sed -e 's/[ 	].*//'
+} | sum) kilobytes

Re: [PATCH] Avoid using dc in git-count-objects

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Using dc is not really necessary, since expr understands summing 32 bit 
> signed integers. Which means that git-count-objects will now fail when 2 
> GB of unpacked objects have accumulated.

Sorry, but I am not very happy about this patch.  "local"
bashism aside, doesn't this spawn expr for every unpacked
object?

Re: [PATCH] Avoid using dc in git-count-objects

[Johannes Schindelin]

Hi,

On Tue, 25 Oct 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Using dc is not really necessary, since expr understands summing 32 bit 
> > signed integers. Which means that git-count-objects will now fail when 2 
> > GB of unpacked objects have accumulated.
> 
> Sorry, but I am not very happy about this patch.  "local"
> bashism aside, doesn't this spawn expr for every unpacked
> object?

Aargh! I had the impression "expr" was a builtin... Just forget about the 
patch, okay?

Ciao,
Dscho

Re: [PATCH] Avoid using dc in git-count-objects

[Petr Baudis]

Dear diary, on Wed, Oct 26, 2005 at 10:33:33AM CEST, I got a letter
where Johannes Schindelin <Johannes.Schindelin@gmx.de> told me that...
> Aargh! I had the impression "expr" was a builtin... Just forget about the 
> patch, okay?

I think that builtin or not, $() will always spawn a subshell. ...?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

Re: [PATCH] Avoid using dc in git-count-objects

[Matthias Urlichs]

Hi, Petr Baudis wrote:

> Dear diary, on Wed, Oct 26, 2005 at 10:33:33AM CEST, I got a letter
> where Johannes Schindelin <Johannes.Schindelin@gmx.de> told me that...
>> Aargh! I had the impression "expr" was a builtin... Just forget about the 
>> patch, okay?
> 
> I think that builtin or not, $() will always spawn a subshell. ...?

It'll fork a subshell, but if the $() is a builtin, it won't exec.

(My built-in dictionary says: "spawn"=="fork+exec"; sorry if that
disagrees with yours.)

That being said, "echo $((1 + 2 + $((3 + 4))))" will not even fork.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Prunes give you a run for your money.

Re: [PATCH] Avoid using dc in git-count-objects

[Johannes Schindelin]

Hi,

On Thu, 27 Oct 2005, Matthias Urlichs wrote:

> That being said, "echo $((1 + 2 + $((3 + 4))))" will not even fork.

<germanenglish>
But zats not troo. It forks fine for mee: ze result is 10.
</germanenglish>

;-)

Ciao,
Dscho

Re: [PATCH] Avoid using dc in git-count-objects

[Linus Torvalds]

On Thu, 27 Oct 2005, Matthias Urlichs wrote:
> 
> It'll fork a subshell, but if the $() is a builtin, it won't exec.
> 
> That being said, "echo $((1 + 2 + $((3 + 4))))" will not even fork.

Right. Don't confuse $(..) with $((..)).

They have absolutely nothing to do with each other, except for looking a 
bit similar. 

The $((..)) is shell-builtin arithmetic. I could imagine a broken shell 
forking and executing "dc" for it, but quite frankly, that sounds so 
unlikely as to be totally silly. So realistically, a shell either supports 
it or not. 

Oh - there's another similarity between $(..) and $((..)). They're both 
POSIX 1003.2 standard features, so any "modern" shell should support them. 
Although in practice I don't know if that means anything else than ksh 
(where both syntaxes came from, actually).

		Linus

Re: [PATCH] Avoid using dc in git-count-objects

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Oh - there's another similarity between $(..) and $((..)). They're both 
> POSIX 1003.2 standard features, so any "modern" shell should support them. 
> Although in practice I don't know if that means anything else than ksh 
> (where both syntaxes came from, actually).

OK, you prodded me enough to come to 21st century ;-).

I was hoping that doing .git/objects/ traversal and disk block
accounting ourselves maybe in C or Perl would be enough for that
particular program, but I would not object if the community
concensus is we would run on only POSIX shells anymore.

I still resist saying that we run only on Bash, though.  I
suspect we already have some bashism on the periphery, but
eradicating them has been lower priority for me.

Re: [PATCH] Avoid using dc in git-count-objects

[Herbert Xu]

Linus Torvalds <torvalds@osdl.org> wrote:
> 
> Oh - there's another similarity between $(..) and $((..)). They're both 
> POSIX 1003.2 standard features, so any "modern" shell should support them. 
> Although in practice I don't know if that means anything else than ksh 
> (where both syntaxes came from, actually).

There is also dash (http://gondor.apana.org.au/~herbert/dash/).
It complies with POSIX, is less than half the size of pdksh (83960 bytes
currently on i386), and is faster than pdksh.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Avoid using dc in git-count-objects

[H. Peter Anvin]

Herbert Xu wrote:
> Linus Torvalds <torvalds@osdl.org> wrote:
> 
>>Oh - there's another similarity between $(..) and $((..)). They're both 
>>POSIX 1003.2 standard features, so any "modern" shell should support them. 
>>Although in practice I don't know if that means anything else than ksh 
>>(where both syntaxes came from, actually).
> 
> 
> There is also dash (http://gondor.apana.org.au/~herbert/dash/).
> It complies with POSIX, is less than half the size of pdksh (83960 bytes
> currently on i386), and is faster than pdksh.

Current ash also has these features, and is about that size compiled 
static with klibc :)

	-hpa

Re: [PATCH] Avoid using dc in git-count-objects

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Thu, Oct 27, 2005 at 09:14:51PM -0700, H. Peter Anvin wrote:
> 
> Current ash also has these features, and is about that size compiled 
> static with klibc :)

Interesting.  I just tried it on my POSIX testsuite and unfortunately
it failed a number of simple tests including the case statement and
segfaulted on quite a few occasions too.  I've included one of the
segfault scripts here.

Dash is already used in busybox where it has been linked with ulibc.
So it can't be that hard to make it work with klibc.

Perhaps you could use dash for your project as well?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: dotcmd --]
[-- Type: text/plain, Size: 172 bytes --]

trap 'rm -f $TMP' EXIT
TMP=$(tempfile)

cat <<- EOF > $TMP
	return
	echo not here
EOF

a() {
	. $TMP
	echo here
}

a

cat <<- EOF > $TMP
	exit
EOF

(false; . $TMP)
echo $?

Re: [PATCH] Avoid using dc in git-count-objects

[H. Peter Anvin]

Herbert Xu wrote:
> On Thu, Oct 27, 2005 at 09:14:51PM -0700, H. Peter Anvin wrote:
> 
>>Current ash also has these features, and is about that size compiled 
>>static with klibc :)
> 
> 
> Interesting.  I just tried it on my POSIX testsuite and unfortunately
> it failed a number of simple tests including the case statement and
> segfaulted on quite a few occasions too.  I've included one of the
> segfault scripts here.
> 
> Dash is already used in busybox where it has been linked with ulibc.
> So it can't be that hard to make it work with klibc.
> 
> Perhaps you could use dash for your project as well?
> 

Depends on how big it is.

	-hpa

Re: [PATCH] Avoid using dc in git-count-objects

[Herbert Xu]

On Thu, Oct 27, 2005 at 09:34:30PM -0700, H. Peter Anvin wrote:
>
> >Perhaps you could use dash for your project as well?
> 
> Depends on how big it is.

I just noticed that you were building it with -Os.  So I built
dash with -Os and this is what I get:

$ size dash
   text    data     bss     dec     hex filename
  64397     884   10240   75521   12701 dash
$ size klibc-1.1.1/ash/sh.shared
   text    data     bss     dec     hex filename
  66237     424   70908  137569   21961 klibc-1.1.1/ash/sh.shared

So it looks like as long as dash can link with klibc then:

1) It should be smaller in size.
2) It should be more stable due to much wider testing coverage (Debian).
3) You get some extra bells & whistles thrown in for free like printf.

I'll let you know when I have a version that links with klibc.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Avoid using dc in git-count-objects

[H. Peter Anvin]

Herbert Xu wrote:
> On Thu, Oct 27, 2005 at 09:34:30PM -0700, H. Peter Anvin wrote:
> 
>>>Perhaps you could use dash for your project as well?
>>
>>Depends on how big it is.
> 
> 
> I just noticed that you were building it with -Os.  So I built
> dash with -Os and this is what I get:
> 
> $ size dash
>    text    data     bss     dec     hex filename
>   64397     884   10240   75521   12701 dash
> $ size klibc-1.1.1/ash/sh.shared
>    text    data     bss     dec     hex filename
>   66237     424   70908  137569   21961 klibc-1.1.1/ash/sh.shared
> 
> So it looks like as long as dash can link with klibc then:
> 
> 1) It should be smaller in size.
> 2) It should be more stable due to much wider testing coverage (Debian).
> 3) You get some extra bells & whistles thrown in for free like printf.
> 
> I'll let you know when I have a version that links with klibc.
> 
> Cheers,

Cool, thanks!

	-hpa

Re: [PATCH] Avoid using dc in git-count-objects

[Johannes Schindelin]

Hi,

On Wed, 26 Oct 2005, Johannes Schindelin wrote:

> On Tue, 25 Oct 2005, Junio C Hamano wrote:
> 
> > Sorry, but I am not very happy about this patch.  "local"
> > bashism aside, doesn't this spawn expr for every unpacked
> > object?
> 
> Aargh! I had the impression "expr" was a builtin... Just forget about 
> the patch, okay?

This patch should be better:

diff --git a/git-count-objects.sh b/git-count-objects.sh
index 843d2fd..ceb1ff2 100755
--- a/git-count-objects.sh
+++ b/git-count-objects.sh
@@ -2,6 +2,19 @@
 
 . git-sh-setup
 
+case $SHELL in
+*bash)
+	function dc () {
+		while read a b; do
+			case $a,$b in
+			0,) result=0;;
+			*,+) result=$(($result+$a));;
+			p,) echo $result
+			esac
+		done
+	}
+esac
+
 echo $(find "$GIT_DIR/objects"/?? -type f -print 2>/dev/null | wc -l) objects, \
 $({
     echo 0

Re: [PATCH] Avoid using dc in git-count-objects

[Andreas Ericsson]

Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> 
>>Using dc is not really necessary, since expr understands summing 32 bit 
>>signed integers. Which means that git-count-objects will now fail when 2 
>>GB of unpacked objects have accumulated.
> 
> 
> Sorry, but I am not very happy about this patch.  "local"
> bashism aside, doesn't this spawn expr for every unpacked
> object?
> 

I'd be more worried about the fact that the kilobytes count is way off 
as it is. du (at least from coreutils-5.2.1) rounds up to nearest 
kilobyte *for each file* when printing kb-count.

Try these:
    du -skc .git/objects/?? | grep total
    du -skc .git/objects/??/* | grep total
    du -sbc .git/objects/?? | grep total
    du -sbc .git/objects/??/* | grep total

which will all yield different values.

I have no idea which of those values people expect to get back, so it 
might be correct right now, although I doubt it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] Avoid using dc in git-count-objects

[Johannes Schindelin]

Hi,

On Wed, 26 Oct 2005, Andreas Ericsson wrote:

> I'd be more worried about the fact that the kilobytes count is way off 
> as it is. du (at least from coreutils-5.2.1) rounds up to nearest 
> kilobyte *for each file* when printing kb-count.

The rationale behind this: You want to know how much space it takes on 
your hard disk. Remember, git-count-objects should give you a clue whether 
to repack or not.

Actually, "du -k" in my tests rounds up to nearest block size or kilobytes 
(whichever is greater): For example, "du -k" on a very small file (53 
bytes) says "1" on an ext2fs yields "1", "4" on hfs, and 32 on a big 
FAT32. Of course, you may get different values, since the block sizes 
sometimes depend on the total size of the media.

Hth,
Dscho

Re: [PATCH] Avoid using dc in git-count-objects

[Andreas Ericsson]

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 26 Oct 2005, Andreas Ericsson wrote:
> 
> 
>>I'd be more worried about the fact that the kilobytes count is way off 
>>as it is. du (at least from coreutils-5.2.1) rounds up to nearest 
>>kilobyte *for each file* when printing kb-count.
> 
> 
> The rationale behind this: You want to know how much space it takes on 
> your hard disk. Remember, git-count-objects should give you a clue whether 
> to repack or not.
> 

Oh. I thought it was so I would know how much data would be sent over 
the network. Diskspace is cheap, bandwidth is... well, that's cheap too 
(in Sweden at least), but it's boring to wait.

> Actually, "du -k" in my tests rounds up to nearest block size or kilobytes 
> (whichever is greater): For example, "du -k" on a very small file (53 
> bytes) says "1" on an ext2fs yields "1", "4" on hfs, and 32 on a big 
> FAT32. Of course, you may get different values, since the block sizes 
> sometimes depend on the total size of the media.
> 

 From my du man-page:

  -k     like --block-size=1K

I think *most* du implementations work like this, but apparently not all 
of them. I'll hack something up in C instead so it's at least consistent 
regardless of what version of du is used.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231