Git development
 help / color / mirror / Atom feed
* Re: git-feed-mail-list.sh
From: Linus Torvalds @ 2006-05-09  0:19 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Junio C Hamano, git
In-Reply-To: <1147131877.2694.37.camel@shinybook.infradead.org>



On Tue, 9 May 2006, David Woodhouse wrote:
> 
> The remaining problem is that the invocation of 'date' doesn't work with
> new versions of coreutils. This...
> 
>    date=(${rest#*> })
>    sec=${date[0]}; tz=${date[1]}
>    dtz=${tz/+/+ }; dtz=${dtz/-/- }
>    pdate="$(date -Rud "1970-01-01 UTC + $sec sec $dtz" 2>/dev/null)"
> 
> ... doesn't work any more on FC-5, because:

Well, you might choose to just not use "git-cat-file commit" but instead 
ask git to format the thing for you.

Ie you could probably more easily parse the data from something like

	git show -B --patch-with-stat --pretty=fuller $commit

instead of using "git-cat-file commit $commit" and generating the stat and 
diff manually.

That way you get the dates etc pretty-printed for you by git.

			Linus

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: sean @ 2006-05-09  0:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0605090142280.5778@wbgn013.biozentrum.uni-wuerzburg.de>

On Tue, 9 May 2006 01:44:31 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Hi,
> 
> On Mon, 8 May 2006, sean wrote:
> 
> > There's no good reason for git to break with traditional and common 
> > practice in this case.
> 
> It is well established common practice that ini files (and everything in 
> config resembles an ini file) have case insensitive section headers as 
> well as keys.

Not in ini files that support section headers that represent filenames
and directories.  Exactly because those things _are_ case sensitive 
and include more characters than just alnums.  But we're not just 
talking about the config file syntax we're talking about how the 
user must ultimately refer to branches with uppercase character.  
Now everytime a person wants to add a branch attribute via repo-config
they have to remember that git thinks uppercase characters should
be quoted.  Doesn't that sound ridiculous to you?

The point is that we should make the config file and the repo-config
command easy for the _users_.   Instead we're going to make them use
some crazy extra syntax because we refuse to come to terms with the
limitations of the choices we've made so far.

One option, which I don't really like and comes with its own set of 
problems, would be to do something like:

[branch1]
    streetname = "p4/BrAnCH"
[branch2]
    streetname = "origin"

And then allow reference to it from git-repo-config by either branch#
or the value of street name.  While it would take some extra coding
but at least it lives within the current restrictions for key names.

It just seems that once you have to even consider options like this,
a big red flag should be raised about some of the assumptions we've
built into the system.

Sean

^ permalink raw reply

* Re: git-feed-mail-list.sh
From: David Woodhouse @ 2006-05-08 23:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzdy9zl2.fsf@assigned-by-dhcp.cox.net>

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

On Wed, 2006-05-03 at 21:35 -0700, Junio C Hamano wrote:
> If you were to do bashism local, don't you want to also localize
> other variables like key, SUBHEX, NEWSUB,...?
> 
> It may make sense to enhance format-patch to do the Q encoding,
> so that you do not have to do this part by hand... 

Yes, that would be useful. We should perhaps to the From: and To:
headers too. Here's my current version (thanks for the feedback)...

The remaining problem is that the invocation of 'date' doesn't work with
new versions of coreutils. This...

   date=(${rest#*> })
   sec=${date[0]}; tz=${date[1]}
   dtz=${tz/+/+ }; dtz=${dtz/-/- }
   pdate="$(date -Rud "1970-01-01 UTC + $sec sec $dtz" 2>/dev/null)"

... doesn't work any more on FC-5, because:

 $ date -Rud '1970-01-01 UTC + 1147104611 sec + 0100'
date: invalid date `1970-01-01 UTC + 1147104611 sec + 0100'

-- 
dwmw2

[-- Attachment #2: git-feed-mail-list.sh --]
[-- Type: application/x-shellscript, Size: 3600 bytes --]

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: Johannes Schindelin @ 2006-05-08 23:44 UTC (permalink / raw)
  To: sean; +Cc: git
In-Reply-To: <BAYC1-PASMTP0453E2D70B10C6D116167EAEA80@CEZ.ICE>

Hi,

On Mon, 8 May 2006, sean wrote:

> There's no good reason for git to break with traditional and common 
> practice in this case.

It is well established common practice that ini files (and everything in 
config resembles an ini file) have case insensitive section headers as 
well as keys.

Ciao,
Dscho

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: Johannes Schindelin @ 2006-05-08 23:40 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605081905240.6713@iabervon.org>

Hi,

On Mon, 8 May 2006, Daniel Barkalow wrote:

> You could tell people always to use:
> 
>  [branch."name"]

I find this utterly ugly.

> I don't think that people are likely to use older versions of git on the 
> same repository they've used newer versions on. (Clones of it, sure, but 
> that doesn't matter here.) But we should, in any case, make the code 
> ignore sections or lines with syntax errors, under the assumption that 
> they're a later extension and possibly legal but not anything the code 
> could be interested in getting from a parser that doesn't support them.

I have to bisect git sometimes, just because I have some local changes. 
Older gits do barf with a fatal error when encountering a bad config.

Further, it is probably not a good idea to relax error-checking. It is too 
easy to overlook.

Ciao,
Dscho

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: sean @ 2006-05-08 23:30 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: junkio, git
In-Reply-To: <Pine.LNX.4.64.0605081905240.6713@iabervon.org>

On Mon, 8 May 2006 19:20:17 -0400 (EDT)
Daniel Barkalow <barkalow@iabervon.org> wrote:

> You could tell people always to use:
> 
>  [branch."name"]
> 
> even if the branch name is all lowercase anyway. They could even use:
> 
>  [Branch."MyMixedCaseBranch"]
> 
> Then when you refer to something case-sensitive with the possibility of 
> funny characters, you put it in quotes, regardless of what it is.
> 
> For that matter, we could retain the quotes when we parse the file, and 
> reject [branch.master] for lacking the quotes, so that people who are only 
> exposed to branch names all in lowercase letters don't get habits that 
> will fail when they have a v2.6.16.x branch.
> 
> I don't think that people are likely to use older versions of git on the 
> same repository they've used newer versions on. (Clones of it, sure, but 
> that doesn't matter here.) But we should, in any case, make the code 
> ignore sections or lines with syntax errors, under the assumption that 
> they're a later extension and possibly legal but not anything the code 
> could be interested in getting from a parser that doesn't support them.


Unfortunately that's not the only place where you have to use the 
excessive quoting; you also have to do the same when using the git
repo-config command line.   All because we're clinging to case
insensitivity and a very restrictive set of characters for 
identifiers in the config file.  And just why are we clinging?

Believe when Linus offered the code originally he said that it was
easier to start out very restrictive and loosen up later than it was
to start loose and become restrictive later.  That makes a lot of
sense, but somewhere along the way we seem to have made a virtue
out of something that is actually getting in the way of natural
syntactic usage.  There's no good reason for git to break with 
traditional and common practice in this case.

Sean

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: Daniel Barkalow @ 2006-05-08 23:20 UTC (permalink / raw)
  To: sean; +Cc: Junio C Hamano, git
In-Reply-To: <BAYC1-PASMTP110777A694DAF1D7623895AEA80@CEZ.ICE>

On Sun, 7 May 2006, sean wrote:

> On Sun, 07 May 2006 19:29:50 -0700
> Junio C Hamano <junkio@cox.net> wrote:
> 
> 
> > Not at all.  Whatever Porcelain that runs repo-config to record
> > the branch name needs to spell that branch name with proper
> > quoting, like:
> 
> Okay.  It just seems nuts to require quoting because you happen
> to use an uppercase character.  People are used to quoting 
> special characters like * and $, not uppercase letters.

You could tell people always to use:

 [branch."name"]

even if the branch name is all lowercase anyway. They could even use:

 [Branch."MyMixedCaseBranch"]

Then when you refer to something case-sensitive with the possibility of 
funny characters, you put it in quotes, regardless of what it is.

For that matter, we could retain the quotes when we parse the file, and 
reject [branch.master] for lacking the quotes, so that people who are only 
exposed to branch names all in lowercase letters don't get habits that 
will fail when they have a v2.6.16.x branch.

I don't think that people are likely to use older versions of git on the 
same repository they've used newer versions on. (Clones of it, sure, but 
that doesn't matter here.) But we should, in any case, make the code 
ignore sections or lines with syntax errors, under the assumption that 
they're a later extension and possibly legal but not anything the code 
could be interested in getting from a parser that doesn't support them.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH/RFC] Teach git-clean optional <paths>... parameters.
From: Pavel Roskin @ 2006-05-08 22:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wv4gx0r.fsf@assigned-by-dhcp.cox.net>

On Mon, 2006-05-08 at 12:02 -0700, Junio C Hamano wrote:
> When optional paths arguments are given, git-clean passes them
> to underlying git-ls-files; with this, you can say:
> 
> 	git clean 'temp-*'
> 
> to clean only the garbage files whose names begin with 'temp-'.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Pavel Roskin <proski@gnu.org>

>  * I usually do not use clean myself, so I am not sure if this
>    is the kind of thing people who do use 'clean' regularly
>    would generally want, hence this RFC.

I'm not likely to use this feature, but I think it's OK to have it.

It would be nice to have "--" support (see e.g. git-commit).

> +	-X	remove only ignored files as well

That's my stupid error, "as well" should be removed.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: (patch) calloc->xcalloc in read-cache.c
From: Junio C Hamano @ 2006-05-08 20:54 UTC (permalink / raw)
  To: Yakov Lerner; +Cc: git
In-Reply-To: <f36b08ee0605081101w3dc3a60cof5a524e9b4a3f555@mail.gmail.com>

"Yakov Lerner" <iler.ml@gmail.com> writes:

> --- read-cache.c.000    2006-05-08 15:13:57.000000000 +0000
> +++ read-cache.c        2006-05-08 15:15:35.000000000 +0000
> @@ -557,7 +557,7 @@
>
>        active_nr = ntohl(hdr->hdr_entries);
>        active_alloc = alloc_nr(active_nr);
> -       active_cache = calloc(active_alloc, sizeof(struct cache_entry *));
> +       active_cache = xcalloc(active_alloc, sizeof(struct cache_entry *));
>
>        offset = sizeof(*hdr);
>        for (i = 0; i < active_nr; i++) {
>
> Yakov

While I do not mind hand generated patch, your mailer setting
seems to be seriously broken.  

Mind checking Documentation/SubmittingPatches and try again?

^ permalink raw reply

* Re: [patch] munmap-before-rename, cygwin need
From: Junio C Hamano @ 2006-05-08 20:52 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Yakov Lerner, Junio C Hamano, git
In-Reply-To: <81b0412b0605080635r40868f18ua88b33558368f82b@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> You really want  "NO_MMAP = YesPlease" in your config.mak.
> Take a look into compat/mmap.c. That's why Junio has never seen
> the breakage.

I do not have custom config.mak for my Cygwin builds and NO_MMAP
is disabled in the default Makefile, so that does not explain
why it does not break for me.  Very puzzled.

^ permalink raw reply

* Re: [patch] munmap-before-rename, cygwin need
From: Junio C Hamano @ 2006-05-08 20:51 UTC (permalink / raw)
  To: Yakov Lerner; +Cc: Junio C Hamano, git
In-Reply-To: <f36b08ee0605080747r24668152t20cc406e017454a9@mail.gmail.com>

"Yakov Lerner" <iler.ml@gmail.com> writes:

> You are right. Apply did not work when I gave it more than one
> patchfile on commandline (and --index option). I fixed this by
> zeroing active_nr and freeing active_cache in
> unmap_cache().

I suspect that essentially means you are forcing the cache to be
read again after writing it out, in which case I think you are
better off using NO_MMAP as Alex suggests.

^ permalink raw reply

* Re: Command to list commits that point to a given tree.
From: Junio C Hamano @ 2006-05-08 20:34 UTC (permalink / raw)
  To: Carl Baldwin; +Cc: git
In-Reply-To: <20060508163437.GA17390@hpsvcnb.fc.hp.com>

Carl Baldwin <cnb@fc.hp.com> writes:

> ... In particular, I am
> looking for a command that will return a list of commits that point to a
> particular tree.
>
> Right now I plan to brute force it.

You might want to explain what problem you are trying to solve,
by knowing which commit contains a particular tree object.
There might be a solution to that unstated problem which does
not require the reverse lookup.

^ permalink raw reply

* [PATCH/RFC] Teach git-clean optional <paths>... parameters.
From: Junio C Hamano @ 2006-05-08 19:02 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git

When optional paths arguments are given, git-clean passes them
to underlying git-ls-files; with this, you can say:

	git clean 'temp-*'

to clean only the garbage files whose names begin with 'temp-'.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * I usually do not use clean myself, so I am not sure if this
   is the kind of thing people who do use 'clean' regularly
   would generally want, hence this RFC.

 Documentation/git-clean.txt |    5 ++++-
 git-clean.sh                |   13 +++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 36890c5..b95545f 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from 
 SYNOPSIS
 --------
 [verse]
-'git-clean' [-d] [-n] [-q] [-x | -X]
+'git-clean' [-d] [-n] [-q] [-x | -X] <paths>...
 
 DESCRIPTION
 -----------
@@ -16,6 +16,9 @@ Removes files unknown to git.  This allo
 from files that are not under version control.  If the '-x' option is
 specified, ignored files are also removed, allowing to remove all
 build products.
+When optional `<paths>...` arguments are given, the paths
+affected are further limited to those that match them.
+
 
 OPTIONS
 -------
diff --git a/git-clean.sh b/git-clean.sh
index b200868..6c818f4 100755
--- a/git-clean.sh
+++ b/git-clean.sh
@@ -3,13 +3,15 @@ #
 # Copyright (c) 2005-2006 Pavel Roskin
 #
 
-USAGE="[-d] [-n] [-q] [-x | -X]"
+USAGE="[-d] [-n] [-q] [-x | -X] <paths>..."
 LONG_USAGE='Clean untracked files from the working directory
 	-d	remove directories as well
 	-n 	don'\''t remove anything, just show what would be done
 	-q	be quiet, only report errors
 	-x	remove ignored files as well
-	-X	remove only ignored files as well'
+	-X	remove only ignored files as well
+When optional <paths>... arguments are given, the paths
+affected are further limited to those that match them.'
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 
@@ -44,8 +46,11 @@ do
 	-X)
 		ignoredonly=1
 		;;
-	*)
+	-*)
 		usage
+		;;
+	*)
+		break
 	esac
 	shift
 done
@@ -64,7 +69,7 @@ if [ -z "$ignored" ]; then
 	fi
 fi
 
-git-ls-files --others --directory $excl ${excl_info:+"$excl_info"} |
+git-ls-files --others --directory $excl ${excl_info:+"$excl_info"} "$@" |
 while read -r file; do
 	if [ -d "$file" -a ! -L "$file" ]; then
 		if [ -z "$cleandir" ]; then
-- 
1.3.2.gb012

^ permalink raw reply related

* (patch) calloc->xcalloc in read-cache.c
From: Yakov Lerner @ 2006-05-08 18:01 UTC (permalink / raw)
  To: git

--- read-cache.c.000    2006-05-08 15:13:57.000000000 +0000
+++ read-cache.c        2006-05-08 15:15:35.000000000 +0000
@@ -557,7 +557,7 @@

        active_nr = ntohl(hdr->hdr_entries);
        active_alloc = alloc_nr(active_nr);
-       active_cache = calloc(active_alloc, sizeof(struct cache_entry *));
+       active_cache = xcalloc(active_alloc, sizeof(struct cache_entry *));

        offset = sizeof(*hdr);
        for (i = 0; i < active_nr; i++) {

Yakov

^ permalink raw reply

* Fix "git diff --stat" with long filenames
From: Linus Torvalds @ 2006-05-08 16:46 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


When we cut off the front of a filename to make it fit on the line, we add 
a "..." in front. However, the way the "git diff" code was written, we 
will never reset the prefix back to the empty string, so every single 
filename afterwards will have the "..." prefix, whether appropriate or 
not.

You can see this with "git diff v2.6.16.." on the current kernel tree, 
since there are filenames with long names that changed there:

 [ snip snip ]
 Documentation/filesystems/vfs.txt                  |  229 
 .../firmware_class/firmware_sample_driver.c        |    3 
 .../firmware_sample_firmware_class.c               |    1 
 ...Documentation/fujitsu/frv/kernel-ABI.txt           |  192 
 ...Documentation/hwmon/w83627hf                       |    4 
 [ snip snip ]

notice how the two Documentation/firmware** filenames caused the "..." to 
be added, but then the later filenames don't want it, and it also screws 
up the alignment of the line numbering afterwards.

Trivially fixed by moving the declaration (and initial setting) of the 
"prefix" variable into the for-loop where it is used.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/diff.c b/diff.c
index c845c87..5315270 100644
--- a/diff.c
+++ b/diff.c
@@ -296,7 +296,6 @@ static const char minuses[]= "----------
 
 static void show_stats(struct diffstat_t* data)
 {
-	char *prefix = "";
 	int i, len, add, del, total, adds = 0, dels = 0;
 	int max, max_change = 0, max_len = 0;
 	int total_files = data->nr;
@@ -318,6 +317,7 @@ static void show_stats(struct diffstat_t
 	}
 
 	for (i = 0; i < data->nr; i++) {
+		char *prefix = "";
 		char *name = data->files[i]->name;
 		int added = data->files[i]->added;
 		int deleted = data->files[i]->deleted;

^ permalink raw reply related

* Command to list commits that point to a given tree.
From: Carl Baldwin @ 2006-05-08 16:34 UTC (permalink / raw)
  To: git

Hi,

Normally, the natural thing to do is to dereference objects in the
following directions:

commit -> tree
tag    -> commit
tag    -> other object

However, sometimes it is convenient to look in the other direction.  The
command git-name-rev satisfies this need partially by listing symbolic
names for given commits.

Is there a command that can do this more generally?  In particular, I am
looking for a command that will return a list of commits that point to a
particular tree.

Right now I plan to brute force it.  Basically, I will call git-rev-list
to list the commits and, for each commit, map it to a tree.  Then, I
will reverse the map in order to be able to look up a commit based on
the tree.

I was thinking, though, that it might be somewhat useful in general to,
at least, provide a command for the following:

- Given a blob, list the tree and/or tag objects that reference that
  blob.

- Given a tree, list the tree, commit and/or tag objects that reference
  it.

- Given a commit, list the commit and/or tag objects that reference it.

Carl

-- 
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 Carl Baldwin                        RADCAD (R&D CAD)
 Hewlett Packard Company
 MS 88                               work: 970 898-1523
 3404 E. Harmony Rd.                 work: Carl.N.Baldwin@hp.com
 Fort Collins, CO 80525              home: Carl@ecBaldwin.net
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

^ permalink raw reply

* Re: Unresolved issues #2 (shallow clone again)
From: Linus Torvalds @ 2006-05-08 15:32 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20060508042429.GA20249@coredump.intra.peff.net>



On Mon, 8 May 2006, Jeff King wrote:
>
> On Sun, May 07, 2006 at 08:27:02AM -0700, Linus Torvalds wrote:
> 
> > factor for a lot of things for many "common" filesystem setups. You 
> > probably didn't even account for the size of inodes in your "du" setup.
> 
> My numbers came from git-count-objects, which uses the st_blocks sum for
> all objects. The actual du numbers showing space wasted by block
> boundaries are:
>   du -c ??: 1429216
>   du -c --apparent-size ??: 792277
> So it's about 45% wasted space.

And that's actually ignoring inode sizes and directory sizes (well, it 
doesn't "ignore" directory sizes - it counts them - but if you compare it 
to a straight packed format, it's still overhead).

Anyway, looks like it's about 2:1, not 3:1 like I claimed, but the point 
being that blocking factors tend to be at least on the same order of 
magnitude as just plain compression (which also tends to be in the 2:1 
area for normal, fairly easily compressible, stuff).

The delta-packing obviously is much bigger for any project with real 
history. In traditional setups (where you always delta-pack within one 
thing, ie at the level of individual SCCS/RCS files), the delta packing 
obviously _also_ avoids blocking issues, since it means that a thousand 
revisions of the same file will all share the same inode.

So because git uses a whole-file model, it obviously makes the blocking 
issues with its unpacked format _much_ higher than for any traditional 
medium - no conglomeration of different versions of the file in the same 
filesystem object. On the other hand, the packed format also tends to be 
even _more_ efficient than a traditional one, so the end result of it all 
is apparently a pretty big net win even in space consumption).

Side note: I realize that some people think the packs are ugly and 
strange. They aren't linear versions of a file, and instead appear as a 
fairly random "jumble". And they can't be incrementally re-packed, and you 
have to generate a whole new pack-file (which can be incremental in 
_content_, of course). So people think they are ugly.

I'd argue that they are beautiful. They are beautiful because they _don't_ 
contain history in themselves (the objects they contain encode the history 
of course, but the pack-file itself does not).

And they are beautiful because we can use the exact same format for 
streaming data over the network as for the database itself (that, of 
course, was just about _the_ design consideration). Show me another system 
that has exactly the same (not "similar", not "same concepts": _same_) 
network protocol as it internal database.

And they are beautiful exactly because their lack of any internal 
structure allows you to pack things by criteria _you_ care about, ie the 
whole "sort things by recency" thing, so that commonly accessed data can 
be packed at the head of the pack-file - exactly because the pack-file 
doesn't have any internal structure of its own that you need to worry 
about and that constrains your sorting.

The same thing is what allows you to delta any blob against any other 
blob - without worrying about history or other random pack-file rules. You 
can do packign purely by how well you want to pack, not by any secondary 
constraints.

And the "no incremental updates" may sound like a huge downside, but it's 
all the same basic git logic: objects and filesystem contents are 
immutable, and that allows us to avoid a lot of locking overhead. Locking 
is _hard_. Locking is _inefficient_. And locking really really screws you 
when you miss it.

So I'll happily say that pack-files are strange, and that you have to get 
a bit used to the notion that they should be repacked "asynchronously". 
But it's really a matter of "getting used to it", because once you do, 
you'll see that it's actually an absolutely huge deal, and you'll learn to 
love the bomb^H^H^H^Hpack-file.

			Linus "pack-files rule" Torvalds

^ permalink raw reply

* [PATCH] improve base85 generated assembly code
From: Nicolas Pitre @ 2006-05-08 15:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This code is arguably pretty hot, if you use binary patches of course. 
This patch helps gcc generate both smaller and faster code especially in 
the error free path.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/base85.c b/base85.c
index b97f7f9..a9e97f8 100644
--- a/base85.c
+++ b/base85.c
@@ -44,34 +44,38 @@ int decode_85(char *dst, char *buffer, i
 	say2("decode 85 <%.*s>", len/4*5, buffer);
 	while (len) {
 		unsigned acc = 0;
-		int cnt;
-		for (cnt = 0; cnt < 5; cnt++, buffer++) {
-			int ch = *((unsigned char *)buffer);
-			int de = de85[ch];
-			if (!de)
+		int de, cnt = 4;
+		unsigned char ch;
+		do {
+			ch = *buffer++;
+			de = de85[ch];
+			if (--de < 0)
 				return error("invalid base85 alphabet %c", ch);
-			de--;
-			if (cnt == 4) {
-				/*
-				 * Detect overflow.  The largest
-				 * 5-letter possible is "|NsC0" to
-				 * encode 0xffffffff, and "|NsC" gives
-				 * 0x03030303 at this point (i.e.
-				 * 0xffffffff = 0x03030303 * 85).
-				 */
-				if (0x03030303 < acc ||
-				    (0x03030303 == acc && de))
-					error("invalid base85 sequence %.5s",
-					      buffer-3);
-			}
 			acc = acc * 85 + de;
-			say1(" <%08x>", acc);
-		}
+		} while (--cnt);
+		ch = *buffer++;
+		de = de85[ch];
+		if (--de < 0)
+			return error("invalid base85 alphabet %c", ch);
+		/*
+		 * Detect overflow.  The largest
+		 * 5-letter possible is "|NsC0" to
+		 * encode 0xffffffff, and "|NsC" gives
+		 * 0x03030303 at this point (i.e.
+		 * 0xffffffff = 0x03030303 * 85).
+		 */
+		if (0x03030303 < acc ||
+		    0xffffffff - de < (acc *= 85))
+			error("invalid base85 sequence %.5s", buffer-5);
+		acc += de;
 		say1(" %08x", acc);
-		for (cnt = 0; cnt < 4 && len; cnt++, len--) {
-			*dst++ = (acc >> 24) & 0xff;
-			acc = acc << 8;
-		}
+
+		cnt = (len < 4) ? len : 4;
+		len -= cnt;
+		do {
+			acc = (acc << 8) | (acc >> 24);
+			*dst++ = acc;
+		} while (--cnt);
 	}
 	say("\n");
 
@@ -86,15 +90,17 @@ void encode_85(char *buf, unsigned char 
 	while (bytes) {
 		unsigned acc = 0;
 		int cnt;
-		for (cnt = 0; cnt < 4 && bytes; cnt++, bytes--) {
+		for (cnt = 24; cnt >= 0; cnt -= 8) {
 			int ch = *data++;
-			acc |= ch << ((3-cnt)*8);
+			acc |= ch << cnt;
+			if (--bytes == 0)
+				break;
 		}
 		say1(" %08x", acc);
-		for (cnt = 0; cnt < 5; cnt++) {
+		for (cnt = 4; cnt >= 0; cnt--) {
 			int val = acc % 85;
 			acc /= 85;
-			buf[4-cnt] = en85[val];
+			buf[cnt] = en85[val];
 		}
 		buf += 5;
 	}

^ permalink raw reply related

* Re: Implementing branch attributes in git config
From: Pavel Roskin @ 2006-05-08 15:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vody8howu.fsf@assigned-by-dhcp.cox.net>

On Mon, 2006-05-08 at 02:00 -0700, Junio C Hamano wrote:
> Stating what you do not like about something is a good first
> step to improve that something.  It should not be too hard to
> extend the parser to grok:
> 
> 	repo-config --get branchdata.description '\(.*\) for netdev$'
> 
> and when the value_regex has a capture return what matches
> instead of the entire value.  I think that would do what you
> want.

OK, that would be acceptable.

I still don't like the "for" conversion because it masquerades syntax
(note that text to right of "for" must be unique) as plain text, but
it's a matter of taste, so it's hard for me to argue about it.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [patch] munmap-before-rename, cygwin need
From: Yakov Lerner @ 2006-05-08 14:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vslnlk04v.fsf@assigned-by-dhcp.cox.net>

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

On 5/7/06, Junio C Hamano <junkio@cox.net> wrote:
> "Yakov Lerner" <iler.ml@gmail.com> writes:
> > I found that mmap() works on cygwin, but needs a patch.
> > On Cygwin, rename() fails if target file has active mmap().
> > The patch below adds  munmap() before rename().
> This is interesting in three counts.
>
>  - I from time to time test Cygwin version on my day-job machine
>    (W2K) and my wife's machine (XP); on both machines I usually
>    have less than two weeks old Cygwin installation, and I have
>    not seen the breakage.  I wonder how reproducible this is.
>    Also previously people reported mmap() works for some and
>    fake mmap is needed for others.  Would this patch make things
>    work for everybody?
>
>  - The part you patched is commit_index_file().  This typically
>    is called just before program exit, but some callers, like
>    apply.c, may want to still look at the index after calling
>    it, fully aware that the changes after commit_index will not
>    be written out.  Although I haven't traced the codepath fully
>    in apply.c yet, unmapping would break the access to the index
>    (i.e. active_cache[]).  Does apply still work with your
>    patch?

You are right. Apply did not work when I gave it more than one patchfile on
commandline (and --index option). I fixed this by zeroing active_nr and freeing
active_cache in unmap_cache(). Then I got infinite loop in
remove_lock_file (after multiple calls to hold_index_file_for_update
with same cf, cache_file_list points to cf and cf->next points to
cf creating infinite loop.) The fix in index.c is easy.

The patch below works for me. However, it changes internal
working of apply.c in the scenario 'git-apply --index patch1 patch2 ...'.

(1) With the patch below, apply.c repeats mmap() on index after every patch
argument (because index gets unmapped after every patchfile argument).

(2) Current apply.c does single mmap() at the beginning. It modfies index
on disk and cache in memory and it does not repeat mmap. This mmap
is to original (now deleted) index, if i understand correctly (the
no-name inode).

But (2) this does not work in cygwin. The end results of (1) and
(2) are the same, I think. (2) looks to me bit faster (I didn't do
measurements).

It's up to you whether to make it under #ifdef MUNMAP_BEFORE_RENAME
of not. The changes are now bigger that in original patch.

The fix to index.c prevents circular list. I got infinite loop in
cache_file_list
every time I tried more than 1 patch on commandline of git-apply. I tried
other solution with the function below, but what I put in the atached patch
is shorter than the alternative below.

Yakov

P.S. I am attaching not inlining bacause
gmail totally removes leading tabs from inline text.

P.P.S.
Alternate fix for index.c:

static void clean_cache_file_list(struct cache_file *cf)
{
        struct cache_file *ppcf = &cache_file_list;
        cf->lockfile[0] = 0;
        while( *ppcf ) {
            if(*ppcf == cf ) {
                *ppcf = cf->next;
            } else
                ppcf = &(cf->next);
        }
}
-         cf->lockfile[0] = 0;
+        clean_cache_file_list(cf);
-         cf->lockfile[0] = 0;
+        clean_cache_file_list(cf);

[-- Attachment #2: patch2-unmap-before-rename --]
[-- Type: application/octet-stream, Size: 2712 bytes --]

--- cache.h.000	2006-05-08 16:59:09.000000000 +0000
+++ cache.h	2006-05-08 16:42:15.000000000 +0000
@@ -141,6 +141,7 @@
 
 /* Initialize and use the cache information */
 extern int read_cache(void);
+extern void unmap_cache(void);
 extern int write_cache(int newfd, struct cache_entry **cache, int entries);
 extern int cache_name_pos(const char *name, int namelen);
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
@@ -161,6 +162,7 @@
 struct cache_file {
 	struct cache_file *next;
 	char lockfile[PATH_MAX];
+	int enlisted;
 };
 extern int hold_index_file_for_update(struct cache_file *, const char *path);
 extern int commit_index_file(struct cache_file *);
--- index.c.000	2006-05-08 16:19:22.000000000 +0000
+++ index.c	2006-05-08 16:57:14.000000000 +0000
@@ -5,6 +5,7 @@
 #include "cache.h"
 
 static struct cache_file *cache_file_list;
+static int cleanup_installed = 0;
 
 static void remove_lock_file(void)
 {
@@ -27,11 +28,17 @@
 	int fd;
 	sprintf(cf->lockfile, "%s.lock", path);
 	fd = open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0666);
-	if (fd >=0 && !cf->next) {
-		cf->next = cache_file_list;
-		cache_file_list = cf;
-		signal(SIGINT, remove_lock_file_on_signal);
-		atexit(remove_lock_file);
+	if (fd >=0) {
+		if (!cf->enlisted) {
+			cf->next = cache_file_list;
+			cache_file_list = cf;
+			cf->enlisted = 1;
+		}
+		if (!cleanup_installed) {
+			signal(SIGINT, remove_lock_file_on_signal);
+			atexit(remove_lock_file);
+			cleanup_installed = 1;
+		}
 	}
 	return fd;
 }
@@ -43,6 +50,9 @@
 	strcpy(indexfile, cf->lockfile);
 	i = strlen(indexfile) - 5; /* .lock */
 	indexfile[i] = 0;
+//#ifdef MUMNAP_BEFORE_RENAME
+	unmap_cache();
+//#endif
 	i = rename(cf->lockfile, indexfile);
 	cf->lockfile[0] = 0;
 	return i;
--- read-cache.c.000	2006-05-08 17:01:28.000000000 +0000
+++ read-cache.c	2006-05-08 17:02:24.000000000 +0000
@@ -513,6 +513,9 @@
 	return 0;
 }
 
+static void *mapaddr = MAP_FAILED;
+static unsigned long mapsize;
+
 int read_cache(void)
 {
 	int fd, i;
@@ -541,6 +544,8 @@
 		errno = EINVAL;
 		if (size >= sizeof(struct cache_header) + 20)
 			map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+		mapaddr = map;
+		mapsize = size;
 	}
 	close(fd);
 	if (map == MAP_FAILED)
@@ -565,10 +570,23 @@
 
 unmap:
 	munmap(map, size);
+	mapaddr = MAP_FAILED;
 	errno = EINVAL;
 	die("index file corrupt");
 }
 
+void unmap_cache(void)
+{
+	if ( mapaddr != MAP_FAILED ) {
+		munmap(mapaddr, mapsize);
+		mapaddr = MAP_FAILED;
+	}
+	active_cache_changed = 0;
+	active_nr = 0;
+	free(active_cache);
+	active_cache = NULL;
+}
+
 #define WRITE_BUFFER_SIZE 8192
 static unsigned char write_buffer[WRITE_BUFFER_SIZE];
 static unsigned long write_buffer_len;








^ permalink raw reply

* Re: stgit bug: pulling file deletion patch leaves copy of file
From: Catalin Marinas @ 2006-05-08 14:26 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <20060508124127.GA30662@diana.vm.bytemark.co.uk>

On 08/05/06, Karl Hasselström <kha@treskal.com> wrote:
> When I pull (with -m) a branch that has accepted a patch of mine which
> deletes a file, stgit does the right thing wrt detecting the applied
> patch, etc. But it leaves an untracked copy of the deleted file in my
> working directory, which I get to delete.

Indeed, it should but it doesn't. The file is generated while
reverse-applying the patches but the git.reset() function (which calls
git.checkout()) doesn't remove the file. I'll fix it and push the
changes tonight.

Thanks.

--
Catalin

^ permalink raw reply

* stgit bug: pulling file deletion patch leaves copy of file
From: Karl Hasselström @ 2006-05-08 12:41 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

When I pull (with -m) a branch that has accepted a patch of mine which
deletes a file, stgit does the right thing wrt detecting the applied
patch, etc. But it leaves an untracked copy of the deleted file in my
working directory, which I get to delete.

I suspect this file should be deleted -- but isn't -- by the patch
merge detection code that undoes the application of the reverse of my
patch.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: Implementing branch attributes in git config
From: Johannes Schindelin @ 2006-05-08 12:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pavel Roskin, git
In-Reply-To: <7vody8howu.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 8 May 2006, Junio C Hamano wrote:

> 	repo-config --get branchdata.description '\(.*\) for netdev$'

POSIX regexps do not want the backslashes...

Something like this?

---
[PATCH] repo-config: allow one group in value regexp

The regular expression for the value can now contain one group. In case
there is one, the output will be just that group, not the whole value.
Now you can say

        git-repo-config --get branch.defaultremote '(.*) for junio'

and for a config like this:

        [branch]
                defaultRemote = sushi for junio

the output will be "sushi".

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

---

 repo-config.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/repo-config.c b/repo-config.c
index 63eda1b..9ac4c51 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -26,31 +26,41 @@ static int show_all_config(const char *k
 static int show_config(const char* key_, const char* value_)
 {
 	char value[256];
-	const char *vptr = value;
+	const char *vptr = value_;
 	int dup_error = 0;
 
 	if (value_ == NULL)
-		value_ = "";
+		vptr = value_ = "";
 
 	if (!use_key_regexp && strcmp(key_, key))
 		return 0;
 	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
 		return 0;
-	if (regexp != NULL &&
-			 (do_not_match ^
-			  regexec(regexp, value_, 0, NULL, 0)))
-		return 0;
+	if (regexp != NULL) {
+		regmatch_t matches[2];
+		int len;
+
+		if (do_not_match ^ regexec(regexp, value_, 2, matches, 0))
+			return 0;
+		len = matches[1].rm_eo - matches[1].rm_so;
+		if (!do_not_match && len > 0) {
+			if (len > 255)
+				len = 255;
+			strncpy(value, value_ + matches[1].rm_so, len);
+			value[len] = 0;
+			vptr = value;
+		}
+	}
 
 	if (show_keys)
 		printf("%s ", key_);
 	if (seen && !do_all)
 		dup_error = 1;
-	if (type == T_INT)
+	if (type == T_INT) {
 		sprintf(value, "%d", git_config_int(key_, value_));
-	else if (type == T_BOOL)
+		vptr = value;
+	} else if (type == T_BOOL)
 		vptr = git_config_bool(key_, value_) ? "true" : "false";
-	else
-		vptr = value_;
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
-- 
1.3.1.g297e8-dirty

^ permalink raw reply related

* Re: [PATCH] Release config lock if the regex is invalid
From: Johannes Schindelin @ 2006-05-08 11:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j4xi6lo.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sun, 7 May 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This is not enough. There are quite a few exit paths. Notice the "goto 
> > out_free"? That is where this must go.
> >
> > This patch is totally untested but obviously correct:
> 
> except that many places you already close(fd) and
> unlink(lock_file).

Yes, my bad. I forgot to look for (and remove) them. Your patch looks fine 
to me.

Ciao,
Dscho

^ permalink raw reply

* Re: [patch] munmap-before-rename, cygwin need
From: Yakov Lerner @ 2006-05-08 10:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vslnlk04v.fsf@assigned-by-dhcp.cox.net>

On 5/8/06, Junio C Hamano <junkio@cox.net> wrote:
> "Yakov Lerner" <iler.ml@gmail.com> writes:
>
> > I found that mmap() works on cygwin, but needs a patch.
> > On Cygwin, rename() fails if target file has active mmap().
> > The patch below adds  munmap() before rename().
>
> This is interesting in three counts.
>
>  - I from time to time test Cygwin version on my day-job machine
>   (W2K) and my wife's machine (XP); on both machines I usually
>   have less than two weeks old Cygwin installation, and I have
>   not seen the breakage.  I wonder how reproducible this is.
>   Also previously people reported mmap() works for some and
>   fake mmap is needed for others.  Would this patch make things
>   work for everybody?
>
>  - The part you patched is commit_index_file().  This typically
>   is called just before program exit, but some callers, like
>   apply.c, may want to still look at the index after calling
>   it, fully aware that the changes after commit_index will not
>   be written out.  Although I haven't traced the codepath fully
>   in apply.c yet, unmapping would break the access to the index
>   (i.e. active_cache[]).  Does apply still work with your
>   patch?

I am checking this. I am trying different options and different
scenarios.

Yakov

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox