Git development
 help / color / mirror / Atom feed
* Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice
From: Johannes Schindelin @ 2009-01-18  1:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <7vmydp5tqj.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > That is correct.  But this is such a highly uncritical code path that 
> > I'd like to keep this simple rather than fast.
> 
> I actually not worried about "fast" at all; it was more about unbounded 
> memory consumption.

Let's just assume that I make a branch switch per minute, continuously, 
for 90 days (until the reflogs are expired), and let's say that all my 
branchnames have 20 characters.  Conservatively, the memory requirement 
would be 100 * 2000 * 30 = 3 megabyte.

Note: these are the memory requirements after some really unrealistically 
high activity, and the memory is free()d during parameter parsing.

A much more realistical expectation would be to switch branches maybe 20 
times a day, which would amount to something like 36 kilobyte.  And again, 
they are free()d before the action really starts.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Anders Melchiorsen @ 2009-01-18  1:24 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: SZEDER Gábor, Sverre Rabbelier, Junio C Hamano, git,
	Johannes.Schindelin
In-Reply-To: <20090116001139.GA26357@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> No, you don't have to do that. As long as you only want to "edit"
> the commit you marked as "edit", you only need to use "git add" and
> "git rebase --continue". rebase -i checks whether HEAD still
> resolves to the same commit and if so, it automatically does the
> soft reset for you.
>
> Maybe we should just advertise that in the message provided by
> rebase after it stops? I'm afraid I can't come up with a sane
> wording though, as there are still cases when you need to commit
> yourself, eg. when you use reset. And getting that into one simple
> sentence seems a bit hard (for me).

I was happy to learn that trick when looking at the source, so I agree
that it is a good idea to advertise it. You are right that it is hard
to describe well in few words, though. Does somebody feel like
repainting this?


--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -336,14 +336,13 @@ do_next () {
 		make_patch $sha1
 		git rev-parse --verify HEAD > "$DOTEST"/amend
 		warn "Stopped at $sha1... $rest"
-		warn "You can amend the commit now, with"
-		warn
-		warn "	git commit --amend"
-		warn
-		warn "Once you are satisfied with your changes, run"
+		warn "You can amend the commit now, by marking"
+		warn "paths with 'git add <paths>' and running"
 		warn
 		warn "	git rebase --continue"
 		warn
+		warn "If you want to create new commits, run"
+		warn "'git commit' yourself before continuing."
 		exit 0
 		;;
 	squash|s)

^ permalink raw reply

* Re: [PATCH] git svn clone -s was not prefixing 'branches/'
From: Eric Wong @ 2009-01-18  1:24 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Jon Lim, git
In-Reply-To: <20090117120723.GA24549@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
> On 2009.01.17 11:39:28 +0000, Jon Lim wrote:
> > Hi,
> > 
> > Maybe this patch doesn't fix the problem I was having. I will attempt
> > to describe it better here.
> > 
> > I understand that a standard subversion setup is as follows:
> > trunk
> > branches
> > tags
> > 
> > With the -s option, svn clone should expect this.
> > 
> > Using the example subversion repository:
> > trunk
> > branches/RB_1.0
> > branches/RB_2.0
> > tags/REL_1.0
> > tags/REL_2.0
> > 
> > Currently, using the -s option you get:
> > trunk
> > RB_1.0
> > RB_2.0
> > tags/REL_1.0
> > tags/REL_2.0
> > 
> > I think it makes sense to have:
> > trunk
> > branches/RB_1.0
> > branches/RB_2.0
> > tags/REL_1.0
> > tags/REL_2.0
> 
> Why? "trunk" is just a branch like any other branch, too. It's basically
> just a svn convention that it's not in branches/ but in its own
> "toplevel" directory. Once imported into git, it's just an ordinary
> remote tracking branch. It's already pretty well distiguishable from all
> the other branches due to its name.

Yup, you said it better than I could myself :)

> What _does_ make sense is to have a common prefix for all the stuff you
> got from svn, using for example --prefix=svn/. That way you get:
> svn/trunk
> svn/RB_1.0
> svn/RB_2.0
> svn/tags/REL_1.0
> svn/tags/REL_2.0
> 
> The important part is that those names aren't ambiguous if you have
> local branch heads called, for example:
> trunk
> RB_1.0
> RB_2.0
> 
> as the svn/ prefix is part of the shortname for the remote tracking
> branches. So "trunk" is the branch head and "svn/trunk" is the remote
> tracking branch.
> 
> Btw Eric, is there any reason why there's no prefix used by default?
> Using the name for the svn-remote as the prefix would make a lot of
> sense to me.

Backwards compatibility; and that I've been lazy :)

But I do agree that prefixing "svn/" is preferred for tracking new
repos.  I seem to recall that git-svn was actually the first user of the
"remotes/" namespace before it was adopted by the rest of git, and with
SVN, I didn't anticipate more than one remote.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] contrib/workdir: create logs/refs and rr-cache in the origin repository
From: Junio C Hamano @ 2009-01-18  1:31 UTC (permalink / raw)
  To: Adeodato Simó; +Cc: git, gitster
In-Reply-To: <1232208943-31756-1-git-send-email-dato@net.com.org.es>

Adeodato Simó <dato@net.com.org.es> writes:

> If logs/refs or rr-cache are dangling symlinks in the workdir, and reflogs
> and/or rerere are enabled, commit will die with "fatal: Could not create
> directory". (In the case of rr-cache, it will die after having created the
> commit.)
>
> This commit just creates logs/refs and rr-cache in the origin repository if
> they don't exist already.

Hmm, is that better than not creating the symlink of the borrowed
repository does not have them?

^ permalink raw reply

* Re: [PATCH] mergetool: put the cursor on the editable file for Vim
From: Junio C Hamano @ 2009-01-18  1:35 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: git
In-Reply-To: <200901172228.45450.markus.heidelberg@web.de>

Markus Heidelberg <markus.heidelberg@web.de> writes:

> When resolving conflicts, you only need to edit the $MERGED file. Put
> the cursor automatically into its window for vimdiff and gvimdiff to
> avoid doing <C-w>l every time.

I think this is sensible.

I do not use vim, and I do not know if the patch does what it claims to
do, though.

^ permalink raw reply

* [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it
From: Thomas Rast @ 2009-01-18  1:38 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901161213370.3586@pacific.mpi-cbg.de>

Add a tool 'git resurrect <branch>...' that tries to find traces of
each <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

So here's a slightly more polished version so gmane can keep it
forever.  Thanks for the sed trick!  I was too lazy to add more
options, but at least there's a "fast" and a "complete" mode.



 Makefile         |    1 +
 git-resurrect.sh |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 0 deletions(-)
 create mode 100755 git-resurrect.sh

diff --git a/Makefile b/Makefile
index 2b873fa..87cb539 100644
--- a/Makefile
+++ b/Makefile
@@ -260,6 +260,7 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
+SCRIPT_SH += git-resurrect.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase.sh
diff --git a/git-resurrect.sh b/git-resurrect.sh
new file mode 100755
index 0000000..6d5a0c7
--- /dev/null
+++ b/git-resurrect.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+
+USAGE="git resurrect [-m | --merges] [-n | --dry-run] <name>..."
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip called <name>,
+and tries to resurrect it.  Currently, the reflog is searched for
+checkout and merge messages.  With --merges, the history of all refs
+is scanned for merge commit subjects, which is rather slow but allows
+you to resurrect other people's topic branches."
+
+. git-sh-setup
+cd_to_toplevel
+
+OPTIONS_SPEC="\
+git resurrect [-m | --merges] [-n | --dry-run] <name>...
+--
+m,merges             also scan merges (slow)
+n,dry-run            don't recreate the branch"
+
+test "$#" = 0 && usage
+
+eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
+
+search_reflog () {
+        sed -n 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_reflog_merges () {
+        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_merges () {
+	git rev-list --pretty=tformat:"%h %p:%s" --all |
+	grep "Merge branch.*'$branch'.*into" |
+	while read sha rest; do
+		parents="$(echo "$rest" | cut -d: -f1)"
+		case "$parents" in
+		    *' '*' '*)
+			warn "$branch took part in octopus merge $sha"
+			warn "check manually!"
+			;;
+		    *' '*)
+			echo "$parents" | cut -d' ' -f2
+			;;
+		esac
+	done
+}
+
+search_merge_targets () {
+	git rev-list --pretty=tformat:"%h %s" --all |
+	grep "Merge branch '[^']*' into $branch$" |
+	cut -d' ' -f1
+}
+
+dry_run=
+scan_merges=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+for branch in "$@"; do
+	candidates="$(search_reflog $1; search_reflog_merges $1)"
+	if test ! -z "$scan_merges"; then
+		candidates="$candidates $(search_merges $1; search_merge_targets $1)"
+	fi
+
+	candidates="$(git rev-parse $candidates | sort -u)"
+
+	if test -z "$candidates"; then
+		echo "** No candidates for $branch found **"
+		test -z "$scan_merges" && echo "(maybe try again with -m)"
+	else
+		echo "** Candidates for $branch **"
+		for cmt in $candidates; do
+			git --no-pager log --pretty=oneline --abbrev-commit -1 $cmt
+		done
+
+		newest="$(git rev-list -1 $candidates)"
+		if test ! -z "$dry_run"; then
+			printf "Most recent: "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+		elif ! git rev-parse --verify --quiet $branch >/dev/null; then
+			printf "** Restoring $branch to "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+			git branch $branch $newest
+		else
+			printf "Most recent: "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+			echo "** $branch already exists, doing nothing"
+		fi
+	fi
+done
-- 
1.6.1.320.gd5dca.dirty

^ permalink raw reply related

* Re: [PATCH] bash: offer to show (un)staged changes
From: Junio C Hamano @ 2009-01-18  1:53 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Shawn O. Pearce
In-Reply-To: <1232240184-10906-1-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> +		if test ! -z "$GIT_PS1_EXPENSIVE"; then
> +			git update-index --refresh >/dev/null 2>&1 || w="*"

This makes the feature unavailable for people who care about the stat
dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?

^ permalink raw reply

* Re: [PATCH v2 1/5] checkout: don't crash on file checkout before running post-checkout hook
From: Junio C Hamano @ 2009-01-18  1:56 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: git, Johannes Schindelin, Paolo Bonzini, Miklos Vajna,
	Shawn O. Pearce, Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <1232133002-21725-1-git-send-email-s-beyer@gmx.net>

All looked good to me except one thing; you need to initialize argv to
NULL as ALLOC_GROW() calls xrealloc on it in the last one.

Will queue with an amend.

^ permalink raw reply

* Re: [PATCH v2 1/5] checkout: don't crash on file checkout before running post-checkout hook
From: Stephan Beyer @ 2009-01-18  2:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Paolo Bonzini, Miklos Vajna,
	Shawn O. Pearce, Daniel Barkalow, Christian Couder
In-Reply-To: <7vd4el2x7g.fsf@gitster.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:
> All looked good to me except one thing; you need to initialize argv to
> NULL as ALLOC_GROW() calls xrealloc on it in the last one.

This is true. Sorry.

> Will queue with an amend.

Thank you.

  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: [PATCH] bash: offer to show (un)staged changes
From: Thomas Rast @ 2009-01-18  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vwsct2xd1.fsf@gitster.siamese.dyndns.org>

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

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > +		if test ! -z "$GIT_PS1_EXPENSIVE"; then
> > +			git update-index --refresh >/dev/null 2>&1 || w="*"
> 
> This makes the feature unavailable for people who care about the stat
> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?

True, and I admit I didn't know there was an option to change that.
OTOH git-diff-files doesn't normally update the index even if the
option is set.  Should I ask 'git diff --exit-code --raw' or some such
instead?

(Why would people want to keep the stat info dirty even though there
may not have been any changes?)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch




[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] bash: offer to show (un)staged changes
From: Junio C Hamano @ 2009-01-18  2:13 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Shawn O. Pearce
In-Reply-To: <1232240184-10906-1-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> I came up with this after sending two incomplete patches on the same
> night, and really like it.  Perhaps others might find it useful.

Any patch worth discussing (on this list at least) would need a nontrivial
commit log message that you need to really think while writing.  It is
natural to assume people would be making them with their editor, not with
"commit -m".  These two incomplete patches could have been avoided if you
paid attention to the status output that is in the commit log message
buffer.  Perhaps we should make it even louder in some way?

^ permalink raw reply

* Re: [PATCH] bash: offer to show (un)staged changes
From: Thomas Rast @ 2009-01-18  2:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vvdsd1hur.fsf@gitster.siamese.dyndns.org>

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

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > I came up with this after sending two incomplete patches on the same
> > night, and really like it.  Perhaps others might find it useful.
> 
> Any patch worth discussing (on this list at least) would need a nontrivial
> commit log message that you need to really think while writing.  It is
> natural to assume people would be making them with their editor, not with
> "commit -m".  These two incomplete patches could have been avoided if you
> paid attention to the status output that is in the commit log message
> buffer.  Perhaps we should make it even louder in some way?

Actually I tend to write the commit (and message) sometime halfway
through, and then amend the commit with fixes, docs and such, possibly
tweaking the message if I need to.  That night I just forgot to amend
before format-patch, and there's no status message at that point which
could have reminded me.  So the *+ display is just what I needed; it
shows the status right before I get a chance to format-patch (or
whatever else command expects a commit).

[As a side note, this kind of workflow is what will probably prevent
me from working with any other SCM in the near future.  I simply
cannot imagine going back to a world without add -p, commit --amend
and rebase -i.]

That being said, I never look at that status message; so far I've been
too lazy to make my emacs add syntax highlighting there, and without
it, it's just a big chunk of text.  In fact, for most commits the 1-2
file names completely drown in the big chunk of surrounding,
invariant, instructions.  If it becomes even louder in the ASCII
dimension, that most likely means I'll just have to steer my eye away
from it even harder to see the commit message I'm typing.  Perhaps
some colours would help, I should really try that.

Of course the real solution would be to hack less and sleep more, but
who would want to do that?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
From: Ray Chuan @ 2009-01-18  3:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqod5thi.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, Jan 18, 2009 at 12:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Ray Chuan" <rctay89@gmail.com> writes:
>
>> Currently, git PUTs to
>>
>>  /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
>>
>> then MOVEs to
>>
>>  /repo.git/objects/1a/1a2b...9z
>>
>> This is needless. In fact, the only time MOVE requests are sent is for
>> this sole purpose (ie. of renaming an object).

this would be my "true" reason. i do believe PUTting to a "temporary"
file (ie with '__opaquelocktoken:1234-....' appended) to be a needless
step.

>> A concern raised was repository corruption in the event of failure
>> during PUT. "put && move" won't afford any more protection than using
>> simply "put", since info/refs is not updated if a PUT fails, so there
>> is no cause for concern.
>
> That's a completely bogus reasoning.  Normal operation inside the
> repository that was pushed into won't look at info/refs at all.
>
> The true reason you want to avoid put-then-move is...?

i mentioned this "repository corruption" issue as it was raised
previously by Johannes (towards the bottom):

  http://article.gmane.org/gmane.comp.version-control.git/106031

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.
From: Stephen Haberman @ 2009-01-18  3:57 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Michael J Gruber, Stephan Beyer, Sitaram Chamarty
In-Reply-To: <alpine.DEB.1.00.0901180108480.3586@pacific.mpi-cbg.de>


> > However, I have a strong feeling that just piling onto the current
> > code will not fix the underlying issues.
> 
> BTW just to clarify what I mean by "underlying issues": if you say
> "git rebase -i" in Sitaram's test case, you will see the two commits
> -- as expected.
> 
> However, if you add "-p", all of a sudden you will only see "noop".
> IMO there is no excuse that the code can hide them at all.  If the
> commits are reachable from HEAD but not from $UPSTREAM, they have to
> be in the list.  As simple as that.

Agreed--the rewritten-parent probing being rooted at the merge bases
was not good enough.

> Another thing that I find horribly wrong: there is a "touch
> $REWRITTEN/sha1".  There was a simple design in the beginning: the
> files in $REWRITTEN are actually a mapping from old SHA-1 (file name)
> to new SHA-1 (content).  This was broken, without any good
> explanation.

Perhaps it is not "good", but the explanation a blank REWRITTEN/sha1 is
used a marker during the probe phase that this commit will be rewritten.
So when looking at any of its children commits, they should be rewritten
if a REWRITTEN/parentSha1 exists. Then as the rewriting actually happens,
they get filled in with the new sha1. I cribbed this approach from
Stephan's sequencer rewrite of rebase-i-p.

If you want a different data structure, be it file based, or bash/list
based, or whatever, to track "this commit will eventually be rewritten
but we haven't gotten there yet" during the probe, then we could go back
to leaving REWRITTEN/sha1 alone until after the sha1 commit has been
rebased.

I'm open to suggestions.

Also, as you seem to realize, the current bug stems from not knowing how
to initialize the rewritten data structure. For Sitaram's case, the
first commit is behind any of the merge bases, so marking its parents
(if they exist) as rewritten to ONTO seems reasonable.

If there are no parents, as you point out, I added a "-o sha1 = FIRST"
that should also get the ball rolling. It's another hack, but does this
address your concern until a large refactoring happens?

-------------------------- git-rebase--interactive.sh --------------------------
index c8b0861..8740d9f 100755
@@ -604,11 +604,18 @@ first and then run 'git rebase --continue' again."
 				echo $ONTO > "$REWRITTEN"/$c ||
 					die "Could not init rewritten commits"
 			done
+			# Along with the merge bases, look at the first commit's
+			# parent (which may be before the merge base) and mark it
+			# as rewritten to ONTO
+			FIRST="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
+			for p in $(git rev-list --parents -1 $FIRST | cut -d' ' -f2)
+			do
+				echo $ONTO > "$REWRITTEN/$p"
+			done
 			# No cherry-pick because our first pass is to determine
 			# parents to rewrite and skipping dropped commits would
 			# prematurely end our probe
 			MERGES_OPTION=
-			first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
 		else
 			MERGES_OPTION="--no-merges --cherry-pick"
 		fi
@@ -629,12 +636,12 @@ first and then run 'git rebase --continue' again."
 				preserve=t
 				for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)
 				do
-					if test -f "$REWRITTEN"/$p -a \( $p != $UPSTREAM -o $sha1 = $first_after_upstream \)
+					if test -f "$REWRITTEN"/$p -a $p != $UPSTREAM
 					then
 						preserve=f
 					fi
 				done
-				if test f = "$preserve"
+				if test f = "$preserve" -o $sha1 = $FIRST
 				then
 					touch "$REWRITTEN"/$sha1
 					echo "pick $shortsha1 $rest" >> "$TODO"

(I'm adding the other 3 cc's back after my failed patch attempt
stripped them out--sorry, guys.)

- Stephen

^ permalink raw reply

* Re: [PATCH/RFC] git-svn: Add --convert-timezone option
From: Pete Harlan @ 2009-01-18  3:57 UTC (permalink / raw)
  To: Eric Wong; +Cc: Git mailing list
In-Reply-To: <20090117103711.GB29598@dcvr.yhbt.net>

Eric Wong wrote:
> Pete Harlan <pgit@pcharlan.com> wrote:
>> By default git svn stores timestamps of fetched commits in
>> Subversion's UTC format, to facilitate interoperating with a
>> Subversion repository.
>>
>> If you're using git svn to convert a repository to Git and aren't
>> interested in pushing Git commits back to Subversion, you can use this
>> option to store timestamps of fetched commits as though they were made
>> in the local timezone of the host on which git svn is run.  This makes
>> the times and timezones of a resulting "git log" agree with what "svn
>> log" shows for the same repository.
>>
>> Signed-off-by: Pete Harlan <pgit@pcharlan.com>
>> ---
>>
>> This is a patch I've had floating around for a while.  I haven't
>> submitted it before because I find the solution ungainly.  There has
>> to be a better way to convert from one timezone to the other, but I
>> didn't run across it and now that I've converted away from Subversion
>> I'm sort of done thinking about it.  I'm submitting it now because
>> even in its current state it would have saved me some headache.
>>
>> Also, I'm not sure I'm correct when asserting that converting
>> timezones like this will break Subversion interoperability.  Eric, if
>> that isn't true then I can remove that claim and resubmit.  If
>> converting timezones breaks nothing, then maybe it could even be the
>> default.
> 
> Hi,
> 
> It'll break interoperability between multiple users of git-svn
> tracking the same repo.  But several options already allow for
> this (authors file, noMetdata, ...), so I'm fine with it as long
> as it's optional.

Thank you for your review.  I'll comment here and then followup with a
replacement patch.

>> One improvement that I didn't bother to make would be to convert to
>> different local timezones based on author.  This change uses the
>> timezone of the machine running git-svn, which in my case was fine.
>> Using per-author timezones would be nice, but since parse_svn_date()
>> doesn't already know which author the date is associated with it would
>> be a more intrusive change.
> 
> Could be an interesting idea, but on the other hand I doubt many people
> would bother configuring the authors-file for it.
> 
> On a side note, for the total conversions I've done, I've found it
> easier/faster/more bandwidth efficient to just forgo authors-file
> entirely and use git-filter-branch after-the-fact.

Interesting.  When I was using git-svn I was new enough to Git that I
wasn't aware of filter-branch.

>> My primary motivation in this was to reduce transition shock among our
>> development team.  The fewer ways "git log" looks unhelpfully
>> different than the old "svn log" the better; converting all commit
>> times into GMT wasn't going to look friendly.
>>
>> Comments welcome.
> 
> My usual coding style nits apply (pretty much git (and Linux kernel)
> standard coding style things, too).
> 
> No space between "function(arguments)".
> 
> lower_snake_case, especially for local variables.  mixedCase requires
> more effort to read IMO.

Thanks; I'm sorry you had to point these things out.  Hopefully fixed
in the followup patch.

> More comments inline...
> 
>> --Pete
>>
>>  Documentation/git-svn.txt              |    8 ++++
>>  contrib/completion/git-completion.bash |    2 +-
>>  git-svn.perl                           |   56 ++++++++++++++++++++++++++++++-
>>  3 files changed, 63 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
>> index 8d0c421..8811bf0 100644
>> --- a/Documentation/git-svn.txt
>> +++ b/Documentation/git-svn.txt
>> @@ -92,6 +92,14 @@ COMMANDS
>>  	.git/config file may be specified as an optional command-line
>>  	argument.
>>
>> +--convert-timezones;;
> 
> Is it "timezone" or "timezones" here?  I think "convert-timezone" is
> more correct since we only use the local timezone, not different
> ones for each author.  On the other hand, maybe "--localtime" is
> an even better name for this option...
> 
> Any other opinions out there?

I liked your "--localtime" suggestion, and used that in v2.  I
originally chose plural for "timzeones" because it converts between
two timezones, and there are two timestamps associated with each
commit (even if they're the same, in git-svn).  But singular would
have made just as much sense, and at one point it was singular.

>> +	Store Git commit times in the local timezone instead of UTC.  This
>> +	makes 'git-log' (even without --date=local) show the same times
>> +	that `svn log` would in the local timezone.
>> +
>> +This breaks interoperability with SVN, but may be cosmetically
>> +desirable when converting a repository from SVN to Git.
> 
> Again, this only breaks interoperability with other users of git-svn
> using the default configuration on the same repo.

Thanks, fixed in v2.

>>  'clone'::
>>  	Runs 'init' and 'fetch'.  It will automatically create a
>>  	directory based on the basename of the URL passed to it;
>> diff --git a/git-svn.perl b/git-svn.perl
>> index ad01e18..c2f600d 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -66,7 +66,7 @@ my ($_stdin, $_help, $_edit,
>>  	$_version, $_fetch_all, $_no_rebase,
>>  	$_merge, $_strategy, $_dry_run, $_local,
>>  	$_prefix, $_no_checkout, $_url, $_verbose,
>> -	$_git_format, $_commit_url, $_tag);
>> +	$_git_format, $_commit_url, $_tag, $_convert_timezones);
> 
> Not easy to tell (and apologies for that) but this new variable probably
> belongs in the Git::SVN namespace.  I really need to find some time to
> reorganize and split out the source to git-svn.

I moved it into Git::SVN for v2, though I admit to not having a good
grasp of the big picture about what belongs where.

>>  $Git::SVN::_follow_parent = 1;
>>  my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
>>                      'config-dir=s' => \$Git::SVN::Ra::config_dir,
>> @@ -84,6 +84,7 @@ my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
>>  		   \$Git::SVN::_repack_flags,
>>  		'use-log-author' => \$Git::SVN::_use_log_author,
>>  		'add-author-from' => \$Git::SVN::_add_author_from,
>> +		'convert-timezones' => \$_convert_timezones,
>>  		%remote_opts );
>>
>>  my ($_trunk, $_tags, $_branches, $_stdlayout);
>> @@ -2526,12 +2527,63 @@ sub get_untracked {
>>  	\@out;
>>  }
>>
>> +# parse_svn_date(DATE)
>> +# --------------------
>> +# Given a date (in UTC) from Subversion, return a string in the format
>> +# "<TZ Offset> <local date/time>" that Git will use.
>> +#
>> +# By default the parsed date will be in UTC for interoperating with
>> +# Subversion, but if $_convert_timezones is true we'll convert it to
>> +# the local timezone instead.
>>  sub parse_svn_date {
>>  	my $date = shift || return '+0000 1970-01-01 00:00:00';
>>  	my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
>>  	                                    (\d\d)\:(\d\d)\:(\d\d).\d+Z$/x) or
>>  	                                 croak "Unable to parse date: $date\n";
>> -	"+0000 $Y-$m-$d $H:$M:$S";
>> +	my $parsed_date;    # Set next.
>> +
>> +	if ($_convert_timezones) {
>> +		# Translate the Subversion datetime to an epoch time.
>> +		# We need to switch ourselves to $date's timezone,
>> +		# UTC, for this.
>> +		my $oldEnvTZ = $ENV{TZ};
>> +		$ENV{TZ} = 'UTC';
>> +
>> +		my $epochUTC =
>> +		    POSIX::strftime ('%s', $S, $M, $H, $d, $m - 1, $Y - 1900);
>> +
>> +		# Determine our local timezone (including DST) at the
>> +		# time of $epochUTC.  $Git::SVN::Log::TZ stored the
>> +		# value of TZ, if any, at the time we were run.
>> +		if (defined $Git::SVN::Log::TZ) {
>> +			$ENV{TZ} = $Git::SVN::Log::TZ;
>> +		} else {
>> +			delete $ENV{TZ};
>> +		}
>> +
>> +		my $ourTZ =
>> +		    POSIX::strftime ('%Z', $S, $M, $H, $d, $m - 1, $Y - 1900);
>> +
>> +		# This converts $epochUTC into our local timezone.
>> +		my ($sec, $min, $hour, $mday, $mon, $year,
>> +		    $wday, $yday, $isdst) = localtime ($epochUTC);
>> +
>> +		$parsed_date = sprintf ('%s %04d-%02d-%02d %02d:%02d:%02d',
>> +					$ourTZ, $year + 1900, $mon + 1,
>> +					$mday, $hour, $min, $sec);
> 
> There's probably a reason you didn't use strftime here, or is there?

If you mean why couldn't I have saved an sprintf by using the previous
strftime call to format this data at the same time I got the timzeone,
it's because strftime doesn't convert the time into the local
timezone.  It just passes back the same values that you pass in.

> The stock Perl time/date handling functions have always frightened me,
> so I'll just trust the (+|-) (1|1900) things are correct :)

:) I tested it pretty well...

--
Pete Harlan
pgit@pcharlan.com

^ permalink raw reply

* Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.
From: Stephen Haberman @ 2009-01-18  4:02 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Michael J Gruber, Stephan Beyer, Sitaram Chamarty
In-Reply-To: <20090117215751.60ade90a.stephen@exigencecorp.com>


> Perhaps it is not "good", but the explanation a blank REWRITTEN/sha1 is
> used a marker during the probe phase that this commit will be rewritten.
> So when looking at any of its children commits, they should be rewritten
> if a REWRITTEN/parentSha1 exists.

Ugh, fixing several typos:

Perhaps it is not "good", but the explanation /is that/ a blank
REWRITTEN/sha1 is used /as/ a marker during the probe phase that this
commit will be rewritten. So when looking at any of its children
commits, /the children/ should be rewritten if a REWRITTEN/parentSha1
exists.

Sorry about that.

- Stephen

^ permalink raw reply

* [PATCH v2] git-svn: Add --localtime option to "fetch"
From: Pete Harlan @ 2009-01-18  4:10 UTC (permalink / raw)
  To: Eric Wong; +Cc: Git mailing list
In-Reply-To: <4972A896.5050205@pcharlan.com>

By default git-svn stores timestamps of fetched commits in
Subversion's UTC format.  Passing --localtime to fetch will convert
them to the timezone of the server on which git-svn is run.

This makes the timestamps of a resulting "git log" agree with what
"svn log" shows for the same repository.

Signed-off-by: Pete Harlan <pgit@pcharlan.com>
---

Changes to v2 after feedback from Eric Wong:

1. "--convert-timezones" renamed to "--localtime".

2. Removed warnings about breaking interoperability with Subversion,
   because the option doesn't do that.  Instead warn about
   interoperability with other git-svn users cloning from the same
   repository if they don't all use or not use --localtime.

3. Move config variable into Git::SVN namespace.

4. Better conformance to coding guidelines.

 Documentation/git-svn.txt              |   11 ++++++
 contrib/completion/git-completion.bash |    2 +-
 git-svn.perl                           |   54 ++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 8d0c421..63d2f5e 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -92,6 +92,17 @@ COMMANDS
 	.git/config file may be specified as an optional command-line
 	argument.

+--localtime;;
+	Store Git commit times in the local timezone instead of UTC.  This
+	makes 'git-log' (even without --date=local) show the same times
+	that `svn log` would in the local timezone.
+
+This doesn't interfere with interoperating with the Subversion
+repository you cloned from, but if you wish for your local Git
+repository to be able to interoperate with someone else's local Git
+repository, either don't use this option or you should both use it in
+the same local timezone.
+
 'clone'::
 	Runs 'init' and 'fetch'.  It will automatically create a
 	directory based on the basename of the URL passed to it;
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..c9d2c02 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1575,7 +1575,7 @@ _git_svn ()
 			--follow-parent --authors-file= --repack=
 			--no-metadata --use-svm-props --use-svnsync-props
 			--log-window-size= --no-checkout --quiet
-			--repack-flags --user-log-author $remote_opts
+			--repack-flags --user-log-author --localtime $remote_opts
 			"
 		local init_opts="
 			--template= --shared= --trunk= --tags=
diff --git a/git-svn.perl b/git-svn.perl
index ad01e18..0adc8db 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -84,6 +84,7 @@ my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
 		   \$Git::SVN::_repack_flags,
 		'use-log-author' => \$Git::SVN::_use_log_author,
 		'add-author-from' => \$Git::SVN::_add_author_from,
+		'localtime' => \$Git::SVN::_localtime,
 		%remote_opts );

 my ($_trunk, $_tags, $_branches, $_stdlayout);
@@ -1364,7 +1365,7 @@ use constant rev_map_fmt => 'NH40';
 use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
             $_repack $_repack_flags $_use_svm_props $_head
             $_use_svnsync_props $no_reuse_existing $_minimize_url
-	    $_use_log_author $_add_author_from/;
+	    $_use_log_author $_add_author_from $_localtime/;
 use Carp qw/croak/;
 use File::Path qw/mkpath/;
 use File::Copy qw/copy/;
@@ -2526,12 +2527,61 @@ sub get_untracked {
 	\@out;
 }

+# parse_svn_date(DATE)
+# --------------------
+# Given a date (in UTC) from Subversion, return a string in the format
+# "<TZ Offset> <local date/time>" that Git will use.
+#
+# By default the parsed date will be in UTC; if $Git::SVN::_localtime
+# is true we'll convert it to the local timezone instead.
 sub parse_svn_date {
 	my $date = shift || return '+0000 1970-01-01 00:00:00';
 	my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
 	                                    (\d\d)\:(\d\d)\:(\d\d).\d+Z$/x) or
 	                                 croak "Unable to parse date: $date\n";
-	"+0000 $Y-$m-$d $H:$M:$S";
+	my $parsed_date;    # Set next.
+
+	if ($Git::SVN::_localtime) {
+		# Translate the Subversion datetime to an epoch time.
+		# Begin by switching ourselves to $date's timezone, UTC.
+		my $old_env_TZ = $ENV{TZ};
+		$ENV{TZ} = 'UTC';
+
+		my $epoch_in_UTC =
+		    POSIX::strftime('%s', $S, $M, $H, $d, $m - 1, $Y - 1900);
+
+		# Determine our local timezone (including DST) at the
+		# time of $epoch_in_UTC.  $Git::SVN::Log::TZ stored the
+		# value of TZ, if any, at the time we were run.
+		if (defined $Git::SVN::Log::TZ) {
+			$ENV{TZ} = $Git::SVN::Log::TZ;
+		} else {
+			delete $ENV{TZ};
+		}
+
+		my $our_TZ =
+		    POSIX::strftime('%Z', $S, $M, $H, $d, $m - 1, $Y - 1900);
+
+		# This converts $epoch_in_UTC into our local timezone.
+		my ($sec, $min, $hour, $mday, $mon, $year,
+		    $wday, $yday, $isdst) = localtime($epoch_in_UTC);
+
+		$parsed_date = sprintf('%s %04d-%02d-%02d %02d:%02d:%02d',
+				       $our_TZ, $year + 1900, $mon + 1,
+				       $mday, $hour, $min, $sec);
+
+		# Reset us to the timezone in effect when we entered
+		# this routine.
+		if (defined $old_env_TZ) {
+			$ENV{TZ} = $old_env_TZ;
+		} else {
+			delete $ENV{TZ};
+		}
+	} else {
+		$parsed_date = "+0000 $Y-$m-$d $H:$M:$S";
+	}
+
+	return $parsed_date;
 }

 sub check_author {
-- 
1.6.1.77.g953e7

^ permalink raw reply related

* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Junio C Hamano @ 2009-01-18  5:42 UTC (permalink / raw)
  To: Jonas Flodén; +Cc: git, Johannes Schindelin
In-Reply-To: <636ecac0901160634r586c72a0r9bb63c6f019f5bff@mail.gmail.com>

"Jonas Flodén" <jonas@floden.nu> writes:

> When git-am fails it's not always easy to see which patch failed,
> since it's often hidden by a lot of error messages.
> Add an extra line which prints the name of the failed patch just
> before the resolve message to make it easier to find.
>
> Signed-off-by: Jonas Flodén <jonas@floden.nu>
> ---
> Johannes Schindelin wrote:
>> Maybe
>>
>> -               echo Patch failed at $msgnum.
>> +               echo Patch failed at $msgnum($FIRSTLINE).
>
> How about this instead. Though the line could get very long.
> This makes the line stand out a little more.
>
>  git-am.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 4b157fe..09c2f9c 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -501,7 +501,7 @@ do
>  	fi
>  	if test $apply_status != 0
>  	then
> -		echo Patch failed at $msgnum.
> +		printf '\nPatch failed at %s (%s)\n' "$msgnum" "$FIRSTLINE"
>  		stop_here_user_resolve $this
>  	fi

Looks sane except that I do not think you need printf nor the leading
blank line, i.e.

	echo "Patch failed at $msgnum ($FIRSTLINE)"

^ permalink raw reply

* Re: [PATCH] revision walker: include a detached HEAD in --all
From: Junio C Hamano @ 2009-01-18  6:01 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, Johannes Sixt, git
In-Reply-To: <alpine.DEB.1.00.0901161351460.3586@pacific.mpi-cbg.de>

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

> When HEAD is detached, --all should list it, too, logically, as a
> detached HEAD is by definition a temporary, unnamed branch.
>
> It is especially necessary to list it when garbage collecting, as
> the detached HEAD would be trashed.
>
> Noticed by Thomas Rast.
>
> Note that this affects creating bundles with --all; I contend that it
> is a good change to add the HEAD, so that cloning from such a bundle
> will give you a current branch.  However, I had to fix t5701 as it
> assumed that --all does not imply HEAD.

Sorry, but I do not understand.

> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
> index 8dfaaa4..14413f8 100755
> --- a/t/t5701-clone-local.sh
> +++ b/t/t5701-clone-local.sh
> @@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
>  	git clone --bare . x &&
>  	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
>  	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
> -	git bundle create b1.bundle --all HEAD &&
> -	git bundle create b2.bundle --all &&
> +	git bundle create b1.bundle master HEAD &&
> +	git bundle create b2.bundle master &&

Because --all did not imply HEAD, "--all HEAD" used to be the way to say
"everything and HEAD".  Now --all does imply HEAD, but it should still be
a valid way to say "everything, by the way, do not forget HEAD".

Does the first one need to be changed to "master HEAD"?  If "--all HEAD"
makes the rest of the test unhappy because HEAD is listed twice, perhaps
that is an independent bug that needs to be fixed?

For that matter, what does "git bundle create x HEAD HEAD" do?  Does it
list HEAD twice?

^ permalink raw reply

* Re: [PATCH] revision walker: include a detached HEAD in --all
From: Junio C Hamano @ 2009-01-18  6:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, Johannes Sixt, git
In-Reply-To: <7v8wp917c3.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> ...
>> Note that this affects creating bundles with --all; I contend that it
>> is a good change to add the HEAD, so that cloning from such a bundle
>> will give you a current branch.  However, I had to fix t5701 as it
>> assumed that --all does not imply HEAD.
>
> Sorry, but I do not understand.
>
>> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
>> index 8dfaaa4..14413f8 100755
>> --- a/t/t5701-clone-local.sh
>> +++ b/t/t5701-clone-local.sh
>> @@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
>>  	git clone --bare . x &&
>>  	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
>>  	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
>> -	git bundle create b1.bundle --all HEAD &&
>> -	git bundle create b2.bundle --all &&
>> +	git bundle create b1.bundle master HEAD &&
>> +	git bundle create b2.bundle master &&
>
> Because --all did not imply HEAD, "--all HEAD" used to be the way to say
> "everything and HEAD".  Now --all does imply HEAD, but it should still be
> a valid way to say "everything, by the way, do not forget HEAD".
>
> Does the first one need to be changed to "master HEAD"?  If "--all HEAD"
> makes the rest of the test unhappy because HEAD is listed twice, perhaps
> that is an independent bug that needs to be fixed?
>
> For that matter, what does "git bundle create x HEAD HEAD" do?  Does it
> list HEAD twice?

With a patch like this, I think b1.bundle can be created with "--all HEAD"
as before.

Of course, to advertise that --all now includes HEAD and it is a _good_
thing, we may want to even say "git bundle create b1.bundle --all" in the
above test sequence.

Creation of b2.bundle should say "master" explicitly as in your patch,
because the point of that bundle is to test a use of such HEAD-less bundle
in the later parts of the script.

-- >8 --
Subject: [PATCH] bundle: allow the same ref to be given more than once

"git bundle create x master master" used to create a bundle that lists
the same branch (master) twice.  Cloning from such a bundle resulted in
a needless warning "warning: Duplicated ref: refs/remotes/origin/master".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 bundle.c |    2 ++
 object.c |   19 +++++++++++++++++++
 object.h |    1 +
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/bundle.c b/bundle.c
index daecd8e..b20f210 100644
--- a/bundle.c
+++ b/bundle.c
@@ -240,6 +240,8 @@ int create_bundle(struct bundle_header *header, const char *path,
 		return error("unrecognized argument: %s'", argv[i]);
 	}
 
+	object_array_remove_duplicates(&revs.pending);
+
 	for (i = 0; i < revs.pending.nr; i++) {
 		struct object_array_entry *e = revs.pending.objects + i;
 		unsigned char sha1[20];
diff --git a/object.c b/object.c
index 50b6528..7e6a92c 100644
--- a/object.c
+++ b/object.c
@@ -268,3 +268,22 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
 	objects[nr].mode = mode;
 	array->nr = ++nr;
 }
+
+void object_array_remove_duplicates(struct object_array *array)
+{
+	int ref, src, dst;
+	struct object_array_entry *objects = array->objects;
+
+	for (ref = 0; ref < array->nr - 1; ref++) {
+		for (src = ref + 1, dst = src;
+		     src < array->nr;
+		     src++) {
+			if (!strcmp(objects[ref].name, objects[src].name))
+				continue;
+			if (src != dst)
+				objects[dst] = objects[src];
+			dst++;
+		}
+		array->nr = dst;
+	}
+}
diff --git a/object.h b/object.h
index 036bd66..3193916 100644
--- a/object.h
+++ b/object.h
@@ -71,5 +71,6 @@ int object_list_contains(struct object_list *list, struct object *obj);
 /* Object array handling .. */
 void add_object_array(struct object *obj, const char *name, struct object_array *array);
 void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
+void object_array_remove_duplicates(struct object_array *);
 
 #endif /* OBJECT_H */
-- 
1.6.1.208.g58df

^ permalink raw reply related

* Re: [PATCH next resend] bash completion: refactor diff options
From: Junio C Hamano @ 2009-01-18  7:18 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Shawn O. Pearce
In-Reply-To: <1232240603-11729-1-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> diff, log and show all take the same diff options.  Refactor them from
> __git_diff and __git_log into a variable, and complete them in
> __git_show too.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>
> ---
>
> Any news on this?

> +__git_diff_common_options="--stat --numstat --shortstat --summary
>  			--patch-with-stat --name-only --name-status --color
>  			--no-color --color-words --no-renames --check
>  			--full-index --binary --abbrev --diff-filter=
> -			--find-copies-harder --pickaxe-all --pickaxe-regex
> +			--find-copies-harder

The changes around pickaxe made me "Huh?".  For log pickaxe makes very
good sense but for a single diff it doesn't, yet the original seems to
have had them only on "git diff" and not on "git log", which feels wrong.

Other than that, I think the patch tries to achieve a great thing in the
longer term---we do not have to worry about common parts going out of sync
between diff and log family of commands.

^ permalink raw reply

* Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice
From: Junio C Hamano @ 2009-01-18  7:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901180201070.3586@pacific.mpi-cbg.de>

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

> Note: these are the memory requirements after some really unrealistically 
> high activity, and the memory is free()d during parameter parsing.
>
> A much more realistical expectation would be to switch branches maybe 20 
> times a day, which would amount to something like 36 kilobyte.  And again, 
> they are free()d before the action really starts.

My HEAD reflog is 7MB long with 39000 entries, and among them, 13100
entries have "checkout: moving ".

I know I will never want to switch back to the 10000th from the last
branch.  I am quite sure that I would forget which branch I was on after
switching branches three or four times (hence my original hardcoded
limitation of 10 which "should be plenty").  When I know I only have to
keep track of 10 entries, having to keep track of 13100 entries, even if
it is 36kB (it would actually be 260kB in my case) feels there is
something wrong in the design.

^ permalink raw reply

* Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests
From: Junio C Hamano @ 2009-01-18  7:45 UTC (permalink / raw)
  To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901141633030.9831@kiwi.cs.ucla.edu>

Thanks, applied.

^ permalink raw reply

* Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests
From: Junio C Hamano @ 2009-01-18  7:47 UTC (permalink / raw)
  To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901141633030.9831@kiwi.cs.ucla.edu>

Keith Cascio <keith@CS.UCLA.EDU> writes:

>  +5 cases (4 fail), diff whitespace tests
>  There are 2^3 = eight possible combinations of the three flags:
>  -w -b --ignore-space-at-eol
>  Three of those combinations were already being tested:
>  [none]
>  -w
>  -b
>  Add tests of the other five combinations,

Hmm.  Are these three supposed to be orthogonal?

^ permalink raw reply

* Re: [PATCH] http-push: fix off-by-path_len
From: Mike Hommey @ 2009-01-18  7:49 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Kirill A. Korinskiy, git, gitster, Ray Chuan, Nick Hengeveld
In-Reply-To: <alpine.DEB.1.00.0901171632330.3586@pacific.mpi-cbg.de>

On Sat, Jan 17, 2009 at 04:36:26PM +0100, Johannes Schindelin wrote:
> 
> When getting the result of remote_ls(), we were advancing the variable
> "path" to the relative path inside the repository.
> 
> However, then we went on to malloc a bogus amount of memory: we were
> subtracting the prefix length _again_, quite possibly getting something
> negative, which xmalloc() interprets as really, really much.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	Note that the push in t5540 is still broken, as http-push does
> 	not handle packed-refs (when looking what branches are on the 
> 	remote side).
> 
> 	It should not even try to access the directory structure under
> 	refs/ to begin with, but read info/refs instead.

It would actually need to do both, because nothing guarantees info/refs
is up-to-date.

> 	However, that is just one example of the ugliness that is 
> 	http-push.c; it also seems to be a perfect example of a copy-pasting 
> 	hell; just look at the output of "git grep
> 	curl_easy_setopt http-push.c".

Likewise for http.c.

> 	There _has_ to be lot of room for improvement.

And I realize I have had a partial improvement on that sitting on my
harddrive, without me having time (nor motivation) to go further.

Maybe it's time I let it go and post the work in progress for someone
else to take over.

Mike

^ 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