* Re: [PATCH] git-gui: Allow users to set commit.signoff from options.
From: Shawn O. Pearce @ 2007-07-08 21:48 UTC (permalink / raw)
To: Gerrit Pape; +Cc: git
In-Reply-To: <20070706144654.12095.qmail@c8e1b1dc78de94.315fe32.mid.smarden.org>
Gerrit Pape <pape@smarden.org> wrote:
> Users may want to automatically sign-off any commit for a specific
> repository. If they are mostly a git-gui user they should be able to
> view/set this option from within the git-gui environment, rather than
> needing to edit a raw text file on their local filesystem.
Sure. But your patch to git-gui actually just lets the user set
the flag, but doesn't make git-gui honor it. So the user can set
"Automatically Sign-Off" through git-gui but it will have no effect
within git-gui (git-gui doesn't use git-commit.sh, it has its own
pure-Tcl implementation).
I'm not applying this to git-gui, for the very same reason that
Junio already gave as to why he won't apply the git-commit.sh patch.
Within git-gui adding a signoff is either one mouse click (the
button on the toolbar), a single keystroke (Ctrl-S) or a menu action
(Commit->Signoff). Three easy ways to insert the signoff line.
But it still needs to be a choice from the user, every time they
make a commit.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Junio C Hamano @ 2007-07-08 21:54 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1183053733.6108.0.camel@mattlaptop2>
Matt McCutchen <hashproduct@gmail.com> writes:
> -sub gitweb_have_snapshot {
> - my ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');
> - my $have_snapshot = (defined $ctype && defined $suffix);
> -
> - return $have_snapshot;
Although you are removing this function, you still have a couple
of callers left in the code.
^ permalink raw reply
* Re: [PATCH] t7004: Skip tests for signed tags in an old version of gpg.
From: Junio C Hamano @ 2007-07-08 21:56 UTC (permalink / raw)
To: skimo; +Cc: Carlos Rica, git, Johannes Schindelin
In-Reply-To: <20070708202201.GN1528MdfPADPa@greensroom.kotnet.org>
Thanks.
^ permalink raw reply
* Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add
From: Johannes Schindelin @ 2007-07-08 21:49 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jan Hudec, Yann Dirson, Christian Jaeger
In-Reply-To: <vpq1wfi8wjl.fsf@bauges.imag.fr>
Hi,
On Sun, 8 Jul 2007, Matthieu Moy wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> This patch proposes a saner behavior. When there are no difference at
> >> all between file, index and HEAD, the file is removed both from the
> >> index and the tree, as before.
> >>
> >> Otherwise, if the index matches either the file on disk or the HEAD,
> >> the file is removed from the index, but the file is kept on disk, it
> >> may contain important data.
> >
> > However, if some of the files are of the first kind, and some are of
> > the second kind, you happily apply with mixed strategies. IMO that is
> > wrong.
>
> I'm not sure whether this is really wrong. The things git should
> really care about are the index and the repository itself, and the
> proposed behavior is consistant regarding that (either remove all
> files from the index, or remove none).
Well, I think it is wrong for the same reason as it is wrong to apply the
changes to _any_ file when one would fail. And since "git apply" shares
my understanding, I think "git rm" should, too.
> >> static struct {
> >> int nr, alloc;
> >> - const char **name;
> >> + struct file_info * files;
> >> } list;
> >>
> >> static void add_list(const char *name)
> >> {
> >> if (list.nr >= list.alloc) {
> >> list.alloc = alloc_nr(list.alloc);
> >> - list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
> >> + list.files = xrealloc(list.files, list.alloc * sizeof(const char *));
> >
> > This is wrong, too. Yes, it works. But it really should be
> > "sizeof(struct file_info *)". Remember, code is also documentation.
>
> You don't need to argue, that was a typo. My code is definitely wrong,
> but you're wrong too ;-). That's actually sizeof(struct file_info).
Heh, right.
> >> + if (!quiet)
> >> + fprintf(stderr,
> >> + "note: file '%s' not removed "
> >> + "(doesn't match %s).\n",
> >> + path,
> >> + fi.local_changes?"the index":"HEAD");
> >> + return 0;
> >> + }
> >> +}
> >
> > I suspect that this case does never fail. 0 means success for
> > remove_file(). Not good. You should at least have a way to ensure that
> > it removed the files from the working tree from a script. Otherwise there
> > is not much point in returning a value to begin with.
>
> I've changed it to have exit_status = 1 if git-rm aborted before
> starting, and 2 if git-rm skiped some file removals (and of course, 0
> if everything is done as expected).
Oh, so you do not take the return value of this function to determine if
it has or has not done something with the files? That's a bit confusing.
Besides, it would be all the more a reason for a test case, so that I can
see that I am actually wrong.
> > Additionally, since this changes semantics, you better provide test
> > cases to show what is expected to work, and _ensure_ that it actually
> > works.
>
> Sure. I forgot to mention it in my message, but I wanted to have
> feedback before getting into the testsuite stuff.
I think it should be the other way. If you change semantics with the
patch, but another revision changes semantics _differently_, it is really
easy to get lost. In order to demonstrate what should be true, you have
to provide examples. And if you are already providing examples, just wrap
them into
test_description <description>
. ./test-lib.sh
...
test_done
and prefix each test with "test_expect_success", and you're done. It is
really not something requiring a wizard.
> I'm posting the updated patch for info, but it should anyway not be
> merged until
>
> * We agree on the behavior when different files have different kinds
> of changes
I'd understand better what you wish to accomplish with the...
> * I add a testcase.
... testcase. So those are not two distinct points.
> >From f39ae646049b95b055e34da378ea470ef3f3caef Mon Sep 17 00:00:00 2001
> From: Matthieu Moy <Matthieu.Moy@imag.fr>
> Date: Sun, 8 Jul 2007 19:27:44 +0200
> Subject: [PATCH] Change the behavior of git-rm to let it obey in more circumstances without -f.
Please do not do this.
I meant to complain about your OP, but this time it is even worse. The
best way to guarantee that a patch gets lost in a thread is to move it _at
the end_ of a reply.
Please follow the form that you change the subject, still reply, but but
the quoted mail with your answers to that text between the "---" and the
diffstat.
If that text is too long, you should use a separate email for the patch.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Enable "git rerere" by the config variable rerere.enabled
From: Shawn O. Pearce @ 2007-07-08 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vejjkdaqe.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> wrote:
> I'll exclude git-gui part and commit with a minor tweaks; having
> extra "does the directory exist" check in git-gui would not hurt
> people who are used to how rerere works in the short term, and I
> think I read somewhere that I should expect git-gui updates over
> the weekend anyway.
There is a slew of git-gui updates already made but not yet pulled
in by you, and a pile yet still coming.
I'm going to be hacking on git-gui pretty much the rest of today,
and probably a good deal tomorrow too. I'm trying to simplify
the workflow for users in my 400+ branch production repository.
They are actively using git-gui now and really need the tool to
handle things for them. Especially since all but 1 of the users
is _not_ command-line friendly.
So you can feel free to pull maint and master over if you want,
but you might just want to wait another day or two.
Are you thinking of doing a git 1.5.2.4? I'd like to get the bug
fixes accumulating in git-gui's maint into 1.5.2.4 as gitgui-0.7.5.
If you are doing a 1.5.2.4 release that is...
--
Shawn.
^ permalink raw reply
* stuff
From: J. Bruce Fields @ 2007-07-08 22:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The following changes are available in the "maint" branch at:
git://linux-nfs.org/~bfields/git.git maint
Andy Parkins (1):
user-manual: grammar and style fixes
J. Bruce Fields (2):
tutorial: Fix typo
user-manual: more explanation of push and pull usage
William Pursell (1):
user-manual: fix directory name in git-archive example
Documentation/tutorial.txt | 2 +-
Documentation/user-manual.txt | 27 ++++++++++++++++++++++-----
2 files changed, 23 insertions(+), 6 deletions(-)
^ permalink raw reply
* Re: [PATCH] Make fetch-pack a builtin with an internal API
From: Junio C Hamano @ 2007-07-08 22:38 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0707081551120.6977@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> In addition to making fetch-pack a builtin, this allows it to be called
> directly from other built-in code without generating and parsing argument
> lists, which will be useful for builtin-fetch.
>
> Incidently, it makes git-fetch-pack not output lists of what it fetched
> when it fails.
That sounds sensible.
Hmmm. incidentally it seems to do a bit more than that.
> @@ -663,15 +659,42 @@ static int fetch_pack_config(const char *var, const char *value)
>
> static struct lock_file lock;
>
> -int main(int argc, char **argv)
> +void setup_fetch_pack(const char *_uploadpack,
> ...
> +#if 0
> + git_config(fetch_pack_config);
> +
> + if (0 <= transfer_unpack_limit)
> + unpack_limit = transfer_unpack_limit;
> + else if (0 <= fetch_unpack_limit)
> + unpack_limit = fetch_unpack_limit;
> +#endif
> +}
This has been moved elsewhere? I do not see how the code now
honors the configuration.
^ permalink raw reply
* Re: stuff
From: Junio C Hamano @ 2007-07-08 23:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <20070708222956.GB11356@fieldses.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] Make fetch-pack a builtin with an internal API
From: Daniel Barkalow @ 2007-07-08 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl7yzfkg.fsf@assigned-by-dhcp.cox.net>
On Sun, 8 Jul 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > In addition to making fetch-pack a builtin, this allows it to be called
> > directly from other built-in code without generating and parsing argument
> > lists, which will be useful for builtin-fetch.
> >
> > Incidently, it makes git-fetch-pack not output lists of what it fetched
> > when it fails.
>
> That sounds sensible.
>
> Hmmm. incidentally it seems to do a bit more than that.
>
> > @@ -663,15 +659,42 @@ static int fetch_pack_config(const char *var, const char *value)
> >
> > static struct lock_file lock;
> >
> > -int main(int argc, char **argv)
> > +void setup_fetch_pack(const char *_uploadpack,
> > ...
> > +#if 0
> > + git_config(fetch_pack_config);
> > +
> > + if (0 <= transfer_unpack_limit)
> > + unpack_limit = transfer_unpack_limit;
> > + else if (0 <= fetch_unpack_limit)
> > + unpack_limit = fetch_unpack_limit;
> > +#endif
> > +}
>
> This has been moved elsewhere? I do not see how the code now
> honors the configuration.
It's at the beginning of cmd_fetch_pack() for the case of running it from
the command line, but it should probably also be an argument to
setup_fetch_pack(). Got it added to my revisions, and I'll send out a new
version once people have looked over this further.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH 4/4] Add git-rewrite-commits
From: Johannes Schindelin @ 2007-07-08 23:56 UTC (permalink / raw)
To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <1183911808787-git-send-email-skimo@liacs.nl>
Hi,
I am way too tired to comment in detail, but here are some preliminary
findings:
- your command line interface is very nice and elegant.
- you use a lot of what was in cg-admin-rewritehist (including
adjustments I made in the documentation for filter-branch), but you also
make it more confusing for people used to that tool:
- instead of leaving the original branches as they are, you
overwrite them. That's okay. But then you put the originals into
refs/rewritten. Without cg-admin-rewritehist using that name
for the _result_, you could explain your way out of confusion.
As it is, you cannot.
- in spite of doing the same as cg-admin-rewritehist with filters,
you call them maps. But they are no maps. They are manipulators,
you can call them mutators or filters, too. Given what people
know of cg-admin-rewritehist, you really should keep the name
"filter".
- the name "map" itself is used in cg-admin-rewritehist, to map
commit names from old to new. By using that name differently,
again you contribute to confusion, for no good reason.
- neat idea to abuse the decorator for rewriting purposes.
- get_one_line() is a misnomer. It wants to be named get_linelen().
- instead of spawning read-tree, you could use unpack_trees() to boost
performance even more. But I guess it is probably left for later, to
make it easier to review the patch.
- your memspn() and memcspn() functions are very inefficient. Better walk
the memory, introduce a GIT_HEXCHAR into ctype.c, ishexchar() into
git-compat-util.h, and use that.
- The example you give with "git update-index --remove" can fail, right?
Tell the user about that.
- The commit filter again deviates from the usage in cg-admin-rewritehist.
I can see that you wanted to make it more versatile. However, it makes
the tool potentially also a bit more cumbersome to use. Besides, you use
a temporary file where there is no need to.
- "map" is missing. This is a function that you can use in all filters in
cg-admin-rewritehist, except (unfortunately) in the commit filter (for
technical reasons). Alas, these technical reasons also prevent such a
function to be usable by any of the filters ("maps", as you call them).
- the more fundamental problem with the missing "map", I do not see a
reasonable way to get the same functionality from any of the code
snippets passed to rewrite-commits. Indeed, even the workaround of
cg-admin-rewritehist, to read $TEMP/map/$sha1, does not work, since you
are keeping it all in memory. On IRC, gitster suggested to use a
bidirectional pipe (such as stdin/stdout) to get at the new commit
names, but because of buffering, I guess this is no joy.
As commented on IRC, the env/tree/parent/msg filters of
cg-admin-rewritehist could be all emulated by commit filters. However,
that would be really inconvenient. So at a later stage, these would have
to be integrated into rewrite-commits (even if it would be possible to
drive rewrite-commits by a shell porcelain, but I guess you are opposed to
that idea, since you want to do everything else in C, too).
However, the biggest and very real problem is that your filters do not
have a "map" function to get the rewritten sha1 for a given sha1. That is
what makes the filters so versatile, though, since you can skip revisions
by much more complex rules than just greps on the commit message or
header.
But hey, maybe it _is_ time to rethink the whole filter business, and
introduce some kind of regular expression based action language. Something
like
git rewrite-commits -e '/^author Darl McBribe/skip-commit' \
-e 'substitute/^author Joahnnes/author Johannes/header' \
-e 'substitute/poreclain/porcelain/body' \
-e 'rewrite-commit-names'
Hmm?
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Document custom hunk header selection
From: Junio C Hamano @ 2007-07-09 0:14 UTC (permalink / raw)
To: git
Since the external interface seems to have stabilized for this
new feature, let's document it properly.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/gitattributes.txt | 55 ++++++++++++++++++++++++++++++++++++---
1 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index d3ac9c7..810df07 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -72,8 +72,8 @@ EFFECTS
-------
Certain operations by git can be influenced by assigning
-particular attributes to a path. Currently, three operations
-are attributes-aware.
+particular attributes to a path. Currently, the following
+operations are attributes-aware.
Checking-out and checking-in
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -199,7 +199,9 @@ Generating diff text
~~~~~~~~~~~~~~~~~~~~
The attribute `diff` affects if `git diff` generates textual
-patch for the path or just says `Binary files differ`.
+patch for the path or just says `Binary files differ`. It also
+can affect what line is shown on the hunk header `@@ -k,l +n,m @@`
+line.
Set::
@@ -224,7 +226,8 @@ String::
Diff is shown using the specified custom diff driver.
The driver program is given its input using the same
calling convention as used for GIT_EXTERNAL_DIFF
- program.
+ program. This name is also used for custom hunk header
+ selection.
Defining a custom diff driver
@@ -249,6 +252,50 @@ parameters, just like `GIT_EXTERNAL_DIFF` program is called.
See gitlink:git[7] for details.
+Defining a custom hunk-header
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Each group of changes (called "hunk") in the textual diff output
+is prefixed with a line of the form:
+
+ @@ -k,l +n,m @@ TEXT
+
+The text is called 'hunk header', and by default a line that
+begins with an alphabet, an underscore or a dollar sign is used,
+which matches what GNU `diff -p` output uses. This default
+selection however is not suited for some contents, and you can
+use customized pattern to make a selection.
+
+First in .gitattributes, you would assign the `diff` attribute
+for paths.
+
+------------------------
+*.tex diff=tex
+------------------------
+
+Then, you would define "diff.tex.funcname" configuration to
+specify a regular expression that matches a line that you would
+want to appear as the hunk header, like this:
+
+------------------------
+[diff "tex"]
+ funcname = "^\\(\\\\\\(sub\\)*section{.*\\)$"
+------------------------
+
+Note. A single level of backslashes are eaten by the
+configuration file parser, so you would need to double the
+backslashes; the pattern above picks a line that begins with a
+backslash, and zero or more occurences of `sub` followed by
+`section` followed by open brace, to the end of line.
+
+There are a few built-in patterns to make this easier, and `tex`
+is one of them, so you do not have to write the above in your
+configuration file (you still need to enable this with the
+attribute mechanism, via `.gitattributes`). Another built-in
+pattern is defined for `java` that defines a pattern suitable
+for program text in Java language.
+
+
Performing a three-way merge
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
^ permalink raw reply related
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-09 0:37 UTC (permalink / raw)
To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550707080737j56c161bdrf0c079b61877085b@mail.gmail.com>
This is whitespace damaged in a funny way (some lines do not
have leading whitespace while some do).
log_tree_diff() hence log_tree_commit() is caled by codepaths
other than "git log -p", and I am not convinced that this patch
would not break them. Worrysome.
I wonder why we did not actually remove the duplicate parents
from "commit->parents" list after history simplification when we
originally did 88494239, instead of filtering only on the output
path. Anybody remember the reason?
^ permalink raw reply
* Re: [PATCH] Document custom hunk header selection
From: Johannes Schindelin @ 2007-07-09 1:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vir8uzb50.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 8 Jul 2007, Junio C Hamano wrote:
> Since the external interface seems to have stabilized for this
> new feature, let's document it properly.
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-09 1:20 UTC (permalink / raw)
To: Marco Costalba; +Cc: Git Mailing List, Linus Torvalds
In-Reply-To: <7vd4z2za2g.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> writes:
> This is whitespace damaged in a funny way (some lines do not
> have leading whitespace while some do).
>
> log_tree_diff() hence log_tree_commit() is caled by codepaths
> other than "git log -p", and I am not convinced that this patch
> would not break them. Worrysome.
>
> I wonder why we did not actually remove the duplicate parents
> from "commit->parents" list after history simplification when we
> originally did 88494423, instead of filtering only on the output
> path. Anybody remember the reason?
In any case, I think this patch would let us revert 88494239 and
the "matching" patch for git-log I sent out earlier, and also
makes your patch unnecessary.
Seems to pass the test suite, and your original example of
checking addafaf with path limiter of "diff.h".
I am a bit worried about the possible fallout, though. There
might be some callers that expect to find the same number of
parents in commit->parents list and what is recorded in the
commit object itself, although I do not think of any, and I tend
to think it is broken if there is one.
-- >8 --
[PATCH] remove duplicated parents after history simplification
When we simplify history due to path limits, the parents list
for a rewritten commit can end up having duplicates. Instead of
filtering them out in the output codepath like earlier commit
88494423 did, remove them much earlier, when the parent
rewriting actually happens.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
revision.c | 22 +++++++++++++++++++++-
1 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/revision.c b/revision.c
index 5184716..b8a7571 100644
--- a/revision.c
+++ b/revision.c
@@ -1308,6 +1308,26 @@ static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp
}
}
+static int remove_duplicate_parents(struct commit *commit)
+{
+ struct commit_list *p;
+ struct commit_list **pp = &commit->parents;
+
+ /* Examine existing parents while marking ones we have seen... */
+ for (p = commit->parents; p; p = p->next) {
+ struct commit *parent = p->item;
+ if (parent->object.flags & TMP_MARK)
+ continue;
+ parent->object.flags |= TMP_MARK;
+ *pp = p;
+ pp = &p->next;
+ }
+ /* ... and clear the temporary mark */
+ for (p = commit->parents; p; p = p->next)
+ p->item->object.flags &= ~TMP_MARK;
+ return 0;
+}
+
static int rewrite_parents(struct rev_info *revs, struct commit *commit)
{
struct commit_list **pp = &commit->parents;
@@ -1324,7 +1344,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
}
pp = &parent->next;
}
- return 0;
+ return remove_duplicate_parents(commit);
}
static int commit_match(struct commit *commit, struct rev_info *opt)
^ permalink raw reply related
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Johannes Schindelin @ 2007-07-09 1:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <7vd4z2za2g.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 8 Jul 2007, Junio C Hamano wrote:
> I wonder why we did not actually remove the duplicate parents
> >from "commit->parents" list after history simplification when we
> originally did 88494239, instead of filtering only on the output
> path. Anybody remember the reason?
I do not have that commit here.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-09 1:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0707090214280.4248@racer.site>
Sorry, not 88494239 but this one.
commit 884944239f2ab673cedfaa5e7999d31fd6a46331
Author: Junio C Hamano <junkio@cox.net>
Date: Sun Jan 29 15:24:42 2006 -0800
rev-list: omit duplicated parents.
Showing the same parent more than once for a commit does not
make much sense downstream, so stop it.
This can happen with an incorrectly made merge commit that
merges the same parent twice, but can happen in an otherwise
sane development history while squishing the history by taking
into account only commits that touch specified paths.
For example,
$ git rev-list --max-count=1 --parents addafaf -- rev-list.c
would have to show this commit ancestry graph:
.---o---.
/ \
.---*---o---.
/ 93b74bc \
---*---o---o-----o---o-----o addafaf
d8f6b34 \ /
.---o---o---.
\ /
.---*---.
3815f42
where 5 independent development tracks, only two of which have
changes in the specified paths since they forked. The last
change for the other three development tracks was done by the
same commit before they forked, and we were showing that three
times.
Signed-off-by: Junio C Hamano <junkio@cox.net>
^ permalink raw reply
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Johannes Schindelin @ 2007-07-09 1:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <7vd4z2za2g.fsf@assigned-by-dhcp.cox.net>
Hi,
I only found this:
http://thread.gmane.org/gmane.comp.version-control.git/29222
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-09 1:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0707090220281.4248@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I only found this:
>
> http://thread.gmane.org/gmane.comp.version-control.git/29222
Thanks. As it happens, the patch I just sent out is a moral
equivalent of your patch in that thread ;-)
We still may need to think about being careful upon --full-history
though.
Second try, this time not doing the simplification when --full-history
is given.
-- >8 --
[PATCH] remove duplicated parents after history simplification
When we simplify history due to path limits, the parents list
for a rewritten commit can end up having duplicates. Instead of
filtering them out in the output codepath like earlier commit
88494423 did, remove them much earlier, when the parent
information actually gets rewritten.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
revision.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/revision.c b/revision.c
index 5184716..740e6a8 100644
--- a/revision.c
+++ b/revision.c
@@ -1308,6 +1308,25 @@ static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp
}
}
+static void remove_duplicate_parents(struct commit *commit)
+{
+ struct commit_list *p;
+ struct commit_list **pp = &commit->parents;
+
+ /* Examine existing parents while marking ones we have seen... */
+ for (p = commit->parents; p; p = p->next) {
+ struct commit *parent = p->item;
+ if (parent->object.flags & TMP_MARK)
+ continue;
+ parent->object.flags |= TMP_MARK;
+ *pp = p;
+ pp = &p->next;
+ }
+ /* ... and clear the temporary mark */
+ for (p = commit->parents; p; p = p->next)
+ p->item->object.flags &= ~TMP_MARK;
+}
+
static int rewrite_parents(struct rev_info *revs, struct commit *commit)
{
struct commit_list **pp = &commit->parents;
@@ -1324,6 +1343,8 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
}
pp = &parent->next;
}
+ if (revs->simplify_history)
+ remove_duplicate_parents(commit);
return 0;
}
^ permalink raw reply related
* Re: [PATCH] branch.autosetupmerge: allow boolean values, or "all"
From: Paolo Bonzini @ 2007-07-09 1:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vzm2620wp.fsf@assigned-by-dhcp.cox.net>
> Having prepared the patch below, I do not think if the original
> patch even wanted to have 'all' semantics. The surrounding text
> only talks about "off a remote branch" and I strongly suspect
> that nobody wanted to do this for a local branch case at all.
If I remember correctly, the problem was that you are not sure that
remote branches are in refs/remotes.
Paolo
^ permalink raw reply
* Re: [PATCH] branch.autosetupmerge: allow boolean values, or "all"
From: Junio C Hamano @ 2007-07-09 2:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Johannes Schindelin, git
In-Reply-To: <46919692.5020708@gnu.org>
Paolo Bonzini <bonzini@gnu.org> writes:
> > Having prepared the patch below, I do not think if the original
>> patch even wanted to have 'all' semantics. The surrounding text
>> only talks about "off a remote branch" and I strongly suspect
>> that nobody wanted to do this for a local branch case at all.
>
> If I remember correctly, the problem was that you are not sure that
> remote branches are in refs/remotes.
Yes, the user can use traditional layout (e.g. refs/heads/origin
is used as a remote tracking branch).
So the check with refs/remotes/ is not technically correct, but
it should probably look-up the configuration to check the
tracking, if we really want to be strict about it.
I personally do not care too much about it, though.
^ permalink raw reply
* Re: [PATCH] Per-path attribute based hunk header selection.
From: Nicolas Pitre @ 2007-07-09 3:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Johannes Schindelin, git
In-Reply-To: <7v8x9tdlbv.fsf@assigned-by-dhcp.cox.net>
On Fri, 6 Jul 2007, Junio C Hamano wrote:
> About the comment from Johannes regarding hunk_header vs
> funcname, I would actually prefer hunk_header, since that is
> what this is about ("funcname" and "find_func" were misnomer
> from the beginning), but I'd rename hunk_header to funcname for
> the sake of consistency and minimizing the diff.
I think "minimizing the diff" in this case is a bad reason. Using
hunk_header is so much better than funcname IMHO.
Nicolas
^ permalink raw reply
* Re: git-gc "--aggressive" somewhat broken
From: Nicolas Pitre @ 2007-07-09 3:46 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Theodore Tso, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0707061512550.8278@woody.linux-foundation.org>
On Fri, 6 Jul 2007, Linus Torvalds wrote:
>
>
> On Fri, 6 Jul 2007, Linus Torvalds wrote:
> >
> > If we want to be really aggressive, we migth decide to pass a new flag to
> > pack-objects that does something closer to what "aggressive" was meant to
> > do: it would use existing delta's if they exist, but _despite_ existing it
> > could look if there are even better choices.
>
> This is a totally untested patch that may or may not work.
>
> The reason I say "may not work" is not just that I haven't really tested
> it, it's also because I haven't thought it through very well.
>
> In particular, does this possibly cause infinite loops of delta chains?
> Probably. It would need code to explicitly make sure that we don't do
> that, but I couldn't even convince myself as to why we might not hit that
> case _already_ with delta re-use, so maybe there's something going that
> protects us against it.
There is.
> Anyway, consider this a starting point for somebody else who wants to
> really try to look into this.
This is a real teaser. But I have real work to do if I want to leave on
vacation this summer, and therefore I'll then be on vacation. So if I
end up looking at it myself it will be in September.
Nicolas
^ permalink raw reply
* Re: git cvsimport branches not consistent with CVS branches
From: Josh Triplett @ 2007-07-09 4:31 UTC (permalink / raw)
To: Steffen Prohaska
Cc: Robin Rosenberg, Brian Downing, Gordon Heydon, Git Mailing List
In-Reply-To: <3BA4D1AE-D5C9-42CB-81B4-38DC3946A966@zib.de>
Steffen Prohaska wrote:
> On Jul 8, 2007, at 12:53 PM, Robin Rosenberg wrote:
>> Corecode's fromcvs is pretty fast and incremental and AFAIK
>> accurate. I had
>> plenty problems with cvsimport, but fromcvs keeps in sync with the
>> CVS repo.
>> Get it at http://ww2.fs.ei.tum.de/~corecode/hg/fromcvs/ .
>>
>> It does not convert regular tags, only branches, however so there
>> is something to
>> do for those that want a complete cvs import.
>
> Did anyone compare
> * git-cvsimport with cvsps patches from [1]
> * parsecvs [2]
> * fromcvs
> and can give a recommendation?
>
> My experience with plain git-cvsimport (without cvsps patches from [1])
> is that it has a lot of problems. I'd recommend not to use it for
> incremental import and be very suspicious about the git repository
> created by git-cvsimport. You need to carefully validate the repository.
> It's likely that you need to fix imported branches. The trunk seems to
> be ok.
I've used both parsecvs and git-cvsimport, and I can definitely say from
experience that cvsimport badly munges history, and parsecvs seems to handle
it correctly. parsecvs does have the limitations that it requires the ,v
files and doesn't work incrementally, making it great for project repository
conversions and painful for just wanting to use git on a CVS-(mis)managed
project.
I've never used fromcvs.
One idea to mitigate one of the limitations: how about modifying the CVS
server to add an extension that supports downloading full ,v files? CVS has
had server extensions in the past, such as for ls/rls. Once support for this
extension makes its way out to most CVS servers, this problem goes away.
- Josh Triplett
^ permalink raw reply
* Preferring shallower deltas on repack
From: Brian Downing @ 2007-07-09 4:43 UTC (permalink / raw)
To: git
SBCL, a Lisp implementation, has a file in their (CVS) repository called
"version.lisp-expr" which is updated every commit. This file looks like:
------------------------------------------------------------------------
;;; This is the master value for LISP-IMPLEMENTATION-VERSION. It's
;;; separated into its own file here so that it's easy for
;;; text-munging make-ish or cvs-ish scripts to find and tweak it. For
;;; the convenience of such scripts, only a simple subset of Lisp
;;; reader syntax should be used here: semicolon-delimited comments,
;;; possible blank lines or other whitespace, and a single
;;; double-quoted string value alone on its own line.
;;;
;;; ANSI says LISP-IMPLEMENTATION-VERSION can be NIL "if no
;;; appropriate and relevant result can be produced", but as long as
;;; we control the build, we can always assign an appropriate and
;;; relevant result, so this must be a string, not NIL.
;;;
;;; Conventionally a string like "0.6.6", with three numeric fields,
;;; is used for released versions, and a string like "0.6.5.xyzzy",
;;; with something arbitrary in the fourth field, is used for CVS
;;; checkins which aren't released. (And occasionally for internal
;;; versions, especially for internal versions off the main CVS
;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".)
"1.0.7.10"
------------------------------------------------------------------------
Only very rarely does anything but the last line change.
The current repack implementation, when given values like "--window=100
--depth=1000", happily generates an incredibly deep tree for this file,
maxing out the depth. (I'm only using such a high depth to demonstrate
the pessimal behavior.) I noticed that it just takes the first delta
it finds that is the smallest; it then only looks for deltas with a
max_size of the old size - 1, so it can never find better matches with
regard to depth.
I modified this to prefer shallower deltas of the same size. This made
the deltas for this file a very wide tree with a maximum depth of about
65. Other (much smaller) improvements were seen elsewhere in the pack.
Runtime does not seem to have been affected, as most of the work had
already been done when it was tossing deltas before.
Some simple statistics:
SBCL, standard pack-objects, window 100, depth 1000:
Max depth: 980
Mean depth: 100.223622114502
Median depth: 12
Standard deviation: 188.214331919176
SBCL, patched pack-objects, window 100, depth 1000:
Max depth: 787
Mean depth: 61.5669990817656
Median depth: 11
Standard deviation: 127.644652607399
git, standard pack-objects, window 100, depth 1000:
Max depth: 925
Mean depth: 77.184264479754
Median depth: 8
Standard deviation: 150.112998198182
git, patched pack-objects, window 100, depth 1000:
Max depth: 913
Mean depth: 74.9981877425496
Median depth: 7
Standard deviation: 147.900721785959
The only negative effect I could see from this patch might be pack
locality. Unfortunately I don't know enough (read: anything) about pack
access patterns to determine if this is the case.
Patch to follow.
-bcd
^ permalink raw reply
* [PATCH] pack-objects: Prefer shallower deltas if the size is equal
From: Brian Downing @ 2007-07-09 4:45 UTC (permalink / raw)
To: git
In-Reply-To: <20070709044326.GH4087@lavos.net>
Change "try_delta" so that if it finds a delta that has the same size
but shallower depth than the existing delta, it will prefer the
shallower one. This makes certain delta trees vastly less deep.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
builtin-pack-objects.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 3d396ca..54b9d26 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1337,7 +1337,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
if (max_size == 0)
return 0;
if (trg_entry->delta && trg_entry->delta_size <= max_size)
- max_size = trg_entry->delta_size-1;
+ max_size = trg_entry->delta_size;
src_size = src_entry->size;
sizediff = src_size < trg_size ? trg_size - src_size : 0;
if (sizediff >= max_size)
@@ -1371,6 +1371,12 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
return 0;
if (trg_entry->delta_data) {
+ /* Prefer only shallower same-sized deltas. */
+ if (delta_size == trg_entry->delta_size &&
+ src_entry->depth + 1 >= trg_entry->depth) {
+ free(delta_buf);
+ return 0;
+ }
delta_cache_size -= trg_entry->delta_size;
free(trg_entry->delta_data);
}
--
1.5.2.GIT
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox