threads / patch / 2228

patchAvoid using dc in git-count-objects

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

## tl;dr

18 messages between Oct 25, 2005 and Oct 28, 2005. Diffs are folded; open one to read it.

replies: 17people: 8as markdown or json

Johannes Schindelin· Oct 25, 2005, 23:22 UTC · lore

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(-)
Show changes to git-count-objects.sh +10 −4
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
Junio C Hamano· Oct 26, 2005, 00:12 UTC · re: Johannes Schindelin · lore

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

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?

Johannes Schindelin· Oct 26, 2005, 08:33 UTC · re: Junio C Hamano · lore

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

Hi,
On Tue, 25 Oct 2005, Junio C Hamano wrote:
Show 9 quoted lines
> 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

Petr Baudis· Oct 26, 2005, 08:36 UTC · re: Johannes Schindelin · lore

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

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.
Matthias Urlichs· Oct 27, 2005, 08:50 UTC · re: Petr Baudis · lore

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

Hi, Petr Baudis wrote:
Show 6 quoted lines
> 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.
Johannes Schindelin· Oct 27, 2005, 09:56 UTC · re: Matthias Urlichs · lore

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

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

Linus Torvalds· Oct 27, 2005, 15:12 UTC · re: Matthias Urlichs · lore

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

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
Junio C Hamano· Oct 27, 2005, 18:00 UTC · re: Linus Torvalds · lore

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

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.

Herbert Xu· Oct 28, 2005, 03:20 UTC · re: Linus Torvalds · lore

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

Linus Torvalds <torvalds@osdl.org> wrote:
Show 5 quoted lines
> 
> 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
H. Peter Anvin· Oct 28, 2005, 04:14 UTC · re: Herbert Xu · lore

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

Herbert Xu wrote:
Show 11 quoted lines
> 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
Herbert Xu· Oct 28, 2005, 04:32 UTC · re: H. Peter Anvin · lore

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

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


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 $?
H. Peter Anvin· Oct 28, 2005, 04:34 UTC · re: Herbert Xu · lore

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

Herbert Xu wrote:
Show 16 quoted lines
> 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
Herbert Xu· Oct 28, 2005, 04:58 UTC · re: H. Peter Anvin · lore

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

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
H. Peter Anvin· Oct 28, 2005, 05:22 UTC · re: Herbert Xu · lore

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

Herbert Xu wrote:
Show 26 quoted lines
> 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
Johannes Schindelin· Oct 26, 2005, 08:58 UTC · re: Johannes Schindelin · lore

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

Hi,
On Wed, 26 Oct 2005, Johannes Schindelin wrote:
Show 8 quoted lines
> 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:
Show changes to git-count-objects.sh +13 −0
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
Andreas Ericsson· Oct 26, 2005, 09:23 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 12 quoted lines
> 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
Johannes Schindelin· Oct 26, 2005, 14:44 UTC · re: Andreas Ericsson · lore

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

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

Andreas Ericsson· Oct 26, 2005, 14:55 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 14 quoted lines
> 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.

Show 6 quoted lines
> 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

← back to recent threads