* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Alex Riesen @ 2007-09-08 23:25 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Johannes Schindelin, Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <a1bbc6950709071732s1f15e5ev28bdfc5c1ab5877b@mail.gmail.com>
Dmitry Kakurin, Sat, Sep 08, 2007 02:32:09 +0200:
> On 9/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Thu, 6 Sep 2007, Dmitry Kakurin wrote:
> >
> > > Anyway I don't mean to start a religious C vs. C++ war.
> >
> > You have a very strange way of not meaning to start a C vs. C++ war.
>
> I honestly didn't. I didn't even think it's possible. In the
> environment of mainstream commercial software development the last war
> on this subj was over 8-10 years ago.
It is because the "environment of mainstream commercial software
development" is stuck in "8-10" back from now.
> Even wars like "do we use exceptions/templates/stl" are pretty much
> over. Now days it's "do we use Boost", or "do we use template
> metaprogramming". But even more often it's Java/C# vs. C++.
Now that's a stupid argument to bring up. Commercial software
development is were the most stupid mistakes are done and repeated.
> That's why I was wondering how come C was chosen for Git.
"Just to annoy mainstream commercial software developers" would be a
good reason.
^ permalink raw reply
* [PATCH] git-svn: understand grafts when doing dcommit
From: Eric Wong @ 2007-09-08 23:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joakim Tjernlund, git
In-Reply-To: <7vfy1pyluy.fsf@gitster.siamese.dyndns.org>
Use the rev-list --parents functionality to read the parents
of the commit. cat-file only shows the raw object with the
original parents and doesn't take into account grafts; so
we'll rely on rev-list machinery for the smarts here.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
Junio C Hamano <gitster@pobox.com> wrote:
> "Joakim Tjernlund" <joakim.tjernlund@transmode.se> writes:
>
> > hmm, I think git-cat-file is to blame.
> > git-cat-file commit da783cce390ce013b19f1d308ea6813269c6a6b5 does
> > not list list any parent...
>
> The plumbing cat-file does not deal with grafts and this is
> deliberate. Otherwise you would not be able to find the true
> set of parents when you'd want to.
>
> So do not blame cat-file, but blame the Porcelain that uses
> cat-file to read a commit object, without annotating what it
> read with what is in grafts, in this case your command line
> experiment ;-).
>
> The log family of commands and rev-list plumbing while
> traversing commit ancestry chain do take grafts into account.
>
> One caveat is pretty=raw output format shows true parents
> without grafts on "parent " header line, while the "commit "
> fake header prepended in the output for each commit shows the
> parents that takes into account.
>
> To illustrate, if you forge the history and say the parent of
> 1ddea77 is 5da1606 (when the true parent is 820eca68) with
> grafts mechanism, here is what happens:
>
> $ echo '1ddea77e449ef28d8a7c74521af21121ab01abc0 5da1606d0bf5b970fadfa0ca91618a1e871f6755' >.git/info/grafts
> $ git show -s --pretty=raw --parents 1ddea77
> commit 1ddea77e449ef28d8a7c74521af21121ab01abc0 5da1606d0bf5b970fadfa0ca91618a1e871f6755
> tree e9e61bc801438062978ff47b0963c536ed1e51a9
> parent 820eca68c2577d7499d203d7f4f7ae479b577683
> author Nick Hengeveld <nickh@reactrix.com> 1127757131 -0700
> committer Junio C Hamano <junkio@cox.net> 1127805558 -0700
>
> [PATCH] Return CURL error message when object transfer fails
>
> Return CURL error message when object transfer fails
> ...
Interesting. I didn't know about the --parents option before.
git-svn.perl | 11 +++--------
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index fbd4691..f818160 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -841,14 +841,9 @@ sub working_head_info {
sub read_commit_parents {
my ($parents, $c) = @_;
- my ($fh, $ctx) = command_output_pipe(qw/cat-file commit/, $c);
- while (<$fh>) {
- chomp;
- last if '';
- /^parent ($sha1)/ or next;
- push @{$parents->{$c}}, $1;
- }
- close $fh; # break the pipe
+ chomp(my $p = command_oneline(qw/rev-list --parents -1/, $c));
+ $p =~ s/^($c)\s*// or die "rev-list --parents -1 $c failed!\n";
+ @{$parents->{$c}} = split(/ /, $p);
}
sub linearize_history {
--
Eric Wong
^ permalink raw reply related
* Re: [PATCH 1/3] Add strbuf_rtrim (to remove trailing spaces).
From: Pierre Habouzit @ 2007-09-08 22:53 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <46E2CB4C.8050209@lsrfire.ath.cx>
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
On sam, sep 08, 2007 at 04:18:20 +0000, René Scharfe wrote:
> Pierre Habouzit schrieb:
> > diff --git a/strbuf.c b/strbuf.c
> > index acc7fc8..565c343 100644
> > --- a/strbuf.c
> > +++ b/strbuf.c
> > @@ -28,6 +28,13 @@ void strbuf_grow(struct strbuf *sb, size_t extra) {
> > ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> > }
> >
> > +void strbuf_rtrim(struct strbuf *sb)
> > +{
> > + while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
> > + sb->len--;
> > + sb->buf[sb->len] = '\0';
> > +}
>
> Please use tabs instead of spaces to indent, just like you did in your
> other patches. :)
yeah, good catch.
I've added some more patches tonight, and have added another function
in strbufs, and thanks to the awsome git rebase -i, this patch
integrates this new function as well, so this patch series is obsolete
already. I wont repost it until I know if I should rebase it on next
rather than master though.
Cheers,
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Welcome to git
From: Majordomo @ 2007-09-14 16:51 UTC (permalink / raw)
To: gcvg-git-2
--
Welcome to the git mailing list!
Please save this message for future reference. Thank you.
If you ever want to remove yourself from this mailing list,
you can send mail to <Majordomo@vger.kernel.org> with the following
command in the body of your email message:
unsubscribe git
or from another account, besides gcvg-git-2@m.gmane.org:
unsubscribe git gcvg-git-2@m.gmane.org
If you ever need to get in contact with the owner of the list,
(if you have trouble unsubscribing, or have questions about the
list itself) send email to <owner-git@vger.kernel.org> .
This is the general rule for most mailing lists when you need
to contact a human.
Here's the general information for the list you've subscribed to,
in case you don't already have it:
Archives:
http://marc.info/?l=git (since list start in 2005)
http://www.spinics.net/lists/git/
^ permalink raw reply
* Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode
From: Linus Torvalds @ 2007-09-14 17:14 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: Junio C Hamano, Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0709132123570.16478@woody.linux-foundation.org>
On Thu, 13 Sep 2007, Linus Torvalds wrote:
>
> Ok, I'm downloading those tar-balls to reproduce and hopefully see what's
> going on, but I'm not going to be able to get at it today.
Ok, I'm seeing it, and there are serious problems in the diffcore-rename
code.
The problems include things like actual overflow in 31 bits (a signed
integer) from the multiplication of "num_create * num_src".
But you'll almost certainly never even get there, because you'd long since
have given up on the O(n^2) behaviour of the "cheap tests", that aren't
really cheap enough, if only because even just comparing the 20-byte SHA1
hashes will basically take forever since they will always miss in the
cache for lots of names.
So yes, we really *have* to have a rename limit, even if it's only to
avoid the technical bug of overflowing the multiplication.
Testing this actually showed another bug too: "git diff" would actually
never call "diff_setup_done() at all, if "diffopt.output_format" had been
set explicitly to something. So doing a "git diff --stat" would totally
ignore all the sanity checks (and, what caused me to find it, the
initialization of "diffopt.rename_limit") that diff_setup_done() is
supposed to do!
So I'm going to send out two patches - one to fix the "diff_setup_done()"
bug, and one that replaces the default rename_limit with something saner.
Both seem to be real bugs.
Linus
^ permalink raw reply
* [PATCH 1/2] Fix "git diff" setup code
From: Linus Torvalds @ 2007-09-14 17:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141002360.16478@woody.linux-foundation.org>
For some inexplicable reason, "git diff" would call "diff_setup_done()"
iff we hadn't given an explicit output format.
That makes no sense, since much of what diff_setup_done() does is exactly
about checking the output format!
This just moves the call to "diff_setup_done()" out of the conditional,
and to where we've actually done all of the diffopt changes.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
builtin-diff.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin-diff.c b/builtin-diff.c
index f77352b..cb4743b 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -252,13 +252,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
argc = 0;
else
argc = setup_revisions(argc, argv, &rev, NULL);
- if (!rev.diffopt.output_format) {
+ if (!rev.diffopt.output_format)
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
- if (diff_setup_done(&rev.diffopt) < 0)
- die("diff_setup_done failed");
- }
rev.diffopt.allow_external = 1;
rev.diffopt.recursive = 1;
+ if (diff_setup_done(&rev.diffopt) < 0)
+ die("diff_setup_done failed");
/* If the user asked for our exit code then don't start a
* pager or we would end up reporting its exit code instead.
^ permalink raw reply related
* Re: Track /etc directory using Git
From: Thomas Harning Jr. @ 2007-09-14 17:31 UTC (permalink / raw)
To: martin f krafft; +Cc: git, Francis Moreau
In-Reply-To: <20070914091545.GA26432@piper.oerlikon.madduck.net>
On 9/14/07, martin f krafft <madduck@madduck.net> wrote:
> also sprach Francis Moreau <francis.moro@gmail.com> [2007.09.14.1008 +0200]:
> > Did you find an alternative to git in this case ?
>
> No, and I did not look anywhere, but I know of no other VCS that can
> adequatly track permissions.
Has anyone checked out metastore? http://repo.or.cz/w/metastore.git
... there's an XML error in there somewhere, so its not loading the
'main' page, but http://repo.or.cz/w/metastore.git?a=shortlog should
work.
It looks like it could work.... any thoughts on this?
--
Thomas Harning Jr.
^ permalink raw reply
* Re: [ANNOUNCE] tig-0.9
From: Steven Grimm @ 2007-09-14 17:32 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Jonas Fonseca, git
In-Reply-To: <F6776054-00EC-49E8-B4C8-BB0EAB3939AB@silverinsanity.com>
Brian Gernhardt wrote:
> Complete build failure using autoconf here. Just using the Makefile
> like I always have works fine, but "autoconf ; ./configure" (from the
> git repo) fails with "configure: error: iconv() not found. Please
> install libiconv." This confuses me because I have
> /usr/lib/libiconv.dylib, and compiling with -liconv works. I fail to
> have the autoconf-foo to figure out what's wrong.
Try "aclocal ; autoconf ; ./configure".
-Steve
^ permalink raw reply
* [PATCH 2/2] Fix the rename detection limit checking
From: Linus Torvalds @ 2007-09-14 17:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141014130.16478@woody.linux-foundation.org>
This adds more proper rename detection limits. Instead of just checking
the limit against the number of potential rename destinations, we verify
that the rename matrix (which is what really matters) doesn't grow
ridiculously large, and we also make sure that we don't overflow when
doing the matrix size calculation.
This also changes the default limits from unlimited, to a rename matrix
that is limited to 100 entries on a side. You can raise it with the config
entry, or by using the "-l<n>" command line flag, but at least the default
is now a sane number that avoids spending lots of time (and memory) in
situations that likely don't merit it.
The choice of default value is of course very debatable. Limiting the
rename matrix to a 100x100 size will mean that even if you have just one
obvious rename, but you also create (or delete) 10,000 files, the rename
matrix will be so big that we disable the heuristics. Sounds reasonable to
me, but let's see if people hit this (and, perhaps more importantly,
actually *care*) in real life.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
Due to the overflow issue (and yes, Dmitry's test-case actually triggered
an overflow even on 64-bit machines, because the math was done on "int"
types), I really think this was a real bug.
Now, whether this is necessarily the right way to fix it, I dunno, but it
also *does* fix the old broken limit handling that was based just on the
number of target files, and didn't take the number of potential source
files into account.
But the fix to that logic also means that the meaning of "-l<n>" changes
subtly. For the better, I think, but changes nonetheless.
I'd also like to apologize to the change in wt-status.c, but that code
doesn't use the regular diffopt setup logic, so it really is a special
case and needs to be handled as such. Do we want to teach "git runstatus"
to actually honor command line flags for diff generation etc? That's a
separate question, this patch just makes it use the same rename default as
the normal diffs now do.
diff.c | 2 +-
diffcore-rename.c | 19 +++++++++++++++++--
wt-status.c | 1 +
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index 1aca5df..0ee9ea1 100644
--- a/diff.c
+++ b/diff.c
@@ -17,7 +17,7 @@
#endif
static int diff_detect_rename_default;
-static int diff_rename_limit_default = -1;
+static int diff_rename_limit_default = 100;
static int diff_use_color_default;
int diff_auto_refresh_index = 1;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 6bde439..41b35c3 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -298,10 +298,25 @@ void diffcore_rename(struct diff_options *options)
else if (detect_rename == DIFF_DETECT_COPY)
register_rename_src(p->one, 1, p->score);
}
- if (rename_dst_nr == 0 || rename_src_nr == 0 ||
- (0 < rename_limit && rename_limit < rename_dst_nr))
+ if (rename_dst_nr == 0 || rename_src_nr == 0)
goto cleanup; /* nothing to do */
+ /*
+ * This basically does a test for the rename matrix not
+ * growing larger than a "rename_limit" square matrix, ie:
+ *
+ * rename_dst_nr * rename_src_nr > rename_limit * rename_limit
+ *
+ * but handles the potential overflow case specially (and we
+ * assume at least 32-bit integers)
+ */
+ if (rename_limit <= 0 || rename_limit > 32767)
+ rename_limit = 32767;
+ if (rename_dst_nr > rename_limit && rename_src_nr > rename_limit)
+ goto cleanup;
+ if (rename_dst_nr * rename_src_nr > rename_limit * rename_limit)
+ goto cleanup;
+
/* We really want to cull the candidates list early
* with cheap tests in order to avoid doing deltas.
* The first round matches up the up-to-date entries,
diff --git a/wt-status.c b/wt-status.c
index 5205420..10ce6ee 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -227,6 +227,7 @@ static void wt_status_print_updated(struct wt_status *s)
rev.diffopt.format_callback = wt_status_print_updated_cb;
rev.diffopt.format_callback_data = s;
rev.diffopt.detect_rename = 1;
+ rev.diffopt.rename_limit = 100;
wt_read_cache(s);
run_diff_index(&rev, 1);
}
^ permalink raw reply related
* Re: [ANNOUNCE] tig-0.9
From: Brian Gernhardt @ 2007-09-14 17:44 UTC (permalink / raw)
To: Steven Grimm; +Cc: Jonas Fonseca, git
In-Reply-To: <46EAC5C4.6020403@midwinter.com>
On Sep 14, 2007, at 1:32 PM, Steven Grimm wrote:
> Brian Gernhardt wrote:
>> Complete build failure using autoconf here. Just using the
>> Makefile like I always have works fine, but "autoconf ; ./
>> configure" (from the git repo) fails with "configure: error: iconv
>> () not found. Please install libiconv." This confuses me because
>> I have /usr/lib/libiconv.dylib, and compiling with -liconv works.
>> I fail to have the autoconf-foo to figure out what's wrong.
>
> Try "aclocal ; autoconf ; ./configure".
Yes, remembering to regenerate the configure script would have been
smart. Works perfectly, thank you.
~~ Brian
^ permalink raw reply
* StGIT patch editing command
From: Catalin Marinas @ 2007-09-14 17:57 UTC (permalink / raw)
To: GIT list
Hi,
Since some people mentioned in the past that it would be nice to be
able to edit patches, including the diff, I added the 'edit' command
in today's snapshot (and StGIT repository).
The command moves the editing features from 'refresh' into a separate
(and improved) command. In addition, it allows one to modify the diff
directly with the '--diff' option. The command also parses the From:
and Date: fields in the patch description so that the author and date
information can be modified.
'stg help edit' displays more information.
Enjoy :-)
--
Catalin
^ permalink raw reply
* Re: git commit workflow question
From: Shawn O. Pearce @ 2007-09-14 18:14 UTC (permalink / raw)
To: Brian Swetland; +Cc: git
In-Reply-To: <20070914103348.GA22621@bulgaria>
Brian Swetland <swetland@google.com> wrote:
> With perforce I often have a bunch of files modified (having p4 add'd
> or p4 edit'd them) and then only commit a subset of them. I can do
> this interactively by doing a p4 submit and just removing the files
> I don't want to check in from the list in the changelist description
> when I'm writing up the change description in $EDITOR, invoked by
> p4 submit.
>
> It seems like my options with git are to invoke git commit with
> a specific list of things to commit, invoke git commit --interactive
> and use the interactive menu thing to shuffle stuff around, or
> manually unstage things until I have the index in a state where
> a git commit without other arguments will do what I want.
Or use git-citool/git-gui to make commits, in which case that
will help you to arrange the index with what you want to commit.
But yes, git-commit does not pay any attention to modifications
made to the template in the edit buffer.
I'm not sure how the Git community would react to being able to edit
the list of files being committed from within the commit message
buffer. I think most Git users run at least `git diff --cached`
before they commit to make sure they are happy with the difference.
I know a lot of users who do that. Most/all of those users also do
not stage something into the index until they are happy with the
change, which means there isn't any list of files to remove when
it comes time to make the commit as the contents of the index is
exactly what should be committed.
I used p4 for a while before Git was invented. I found the file
editing feature useful then because there was no concept of the
index. Now with Git I've embraced the index so much that I'm not
sure I can work without it, and I don't need to remove files from
my index during the actual commit itself.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/2] Fix "git diff" setup code
From: Junio C Hamano @ 2007-09-14 18:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141014130.16478@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> For some inexplicable reason, "git diff" would call "diff_setup_done()"
> iff we hadn't given an explicit output format.
>
> That makes no sense, since much of what diff_setup_done() does is exactly
> about checking the output format!
We did not even have _any_ setup_done() call in "git diff"
itself in the original, because setup_revisions() has its own
call to setup_done(), and it always called setup_revisions()
before you reached that part of the code you have in your patch.
Commit 047fbe906b375e8a3a7564ad0e4443f62dd528a2 ("builtin-diff:
turn recursive on when defaulting to --patch format.") added the
call to setup_done() only when we are defaulting to output
format, to re-validate the consistency of output format options.
The screw-up was commit fcfa33ec905fcde1c16e7cbbe00d7147b89f1f01
(diff: make more cases implicit --no-index) that made the call
to setup_revisions() conditional if setup_diff_no_index()
decides to take over, but it forgot that setup_diff_no_index()
does not call setup_done().
So I tend to think the attached is a better fix.
---
diff-lib.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index f5568c3..da55713 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -298,6 +298,8 @@ int setup_diff_no_index(struct rev_info *revs,
revs->diffopt.nr_paths = 2;
revs->diffopt.no_index = 1;
revs->max_count = -2;
+ if (diff_setup_done(&revs->diffopt) < 0)
+ die("diff_setup_done failed");
return 0;
}
^ permalink raw reply related
* Re: [PATCH] Added example hook script to save/restore permissions/ownership.
From: Junio C Hamano @ 2007-09-14 18:24 UTC (permalink / raw)
To: git; +Cc: Josh England
In-Reply-To: <118952994432-git-send-email-jjengla@sandia.gov>
Does nobody find these two patches useful?
I do not feel strong need myself to have them but on the other
hand I do not see anything wrong with them, so I am inclined to
apply it to 'master' soon, unless somebody objects.
^ permalink raw reply
* Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode
From: Junio C Hamano @ 2007-09-14 18:24 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Dmitry V. Levin, Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0709132215250.16478@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Yeah, we should probably:
> - default to something larger but still reasonably sane (ie not 100, but
> perhaps 1000)
> - special-case the "identical rename", and have a higher limit for that
> (we already handle the identicals as a separate pass before we even
> start doing the similarity analysis - and it's the similarity analysis
> that can be the really expensive part)
>
> There's really no point in trying to do rename analysis for tons and tons
> of files - even if we find perfect renames, the diff is going to be
> unreadable by a human, so realistically nobody is ever going to care! A
> machine won't care whether it was done as a create/delete or a rename, and
> a human won't be bothered to read about thousands of renames, so we're
> just wasting time trying to make it prettier.
>
> So quite arguably, the only case we really care about for renames is when
> the numbers are small enough to be human-readable.
I agree with that. At the same time we might want to revisit
the earlier "build a full matrix and pick the best ones"
approach commit 5c97558c9a813a0a775c438a79cfc438def00c22 (Detect
renames in diff family) introduced.
A tangent.
I've been thinking about updating the diffcore-rename for some
time to give bonus points to a filepair whose neighbors are
detected to be renames. E.g. if you have this pair of preimage
and postimage:
(preimage) (postimage)
arch/i386/foo.c arch/x86/foo-32.c
arch/i386/bar.c arch/x86/bar-32.c
arch/i386/baz.c arch/x86/baz-32.c
and if foo.c and bar.c are found to be very similar to foo-32.c
and bar-32.c while baz.c and baz-32.c are not that much, we may
want to take hints from the movement of neighbouring files and
boost the similarity score between baz.c and baz-32.c pair.
It would be a quite an interesting coding challenge for anybody
who wants to get his hands dirty. Would this be worth it in
practice? I dunno.
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Shawn O. Pearce @ 2007-09-14 18:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wd1d0le.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> * db/fetch-pack (Fri Sep 14 03:31:25 2007 -0400) 22 commits
...
> This is Daniel's fetch-pack in C plus fixups from Shawn.
> Unfortunately the fixups breaks t3200 ("*** glibc detected ***
> fetch: free(): invalid pointer xxx ***"), which I haven't looked
> into yet.
Doesn't crash out on my Mac OS X system but I am getting the
above failure on my amd64 Linux system. I'm debugging it now.
I'll have to quit in about an hour and pick it up later, so don't
expect a patch immediately. But I'll certainly send something soon.
Clearly I made a change in my fixups that I shouldn't have. ;-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/2] Fix "git diff" setup code
From: Linus Torvalds @ 2007-09-14 18:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <7v4phxaz3o.fsf@gitster.siamese.dyndns.org>
On Fri, 14 Sep 2007, Junio C Hamano wrote:
>
> So I tend to think the attached is a better fix.
Ahh, yes, that explains the conditional.
But whatever gets us to actually verify our options, and fill in the right
defaults is ok by me!
Linus
^ permalink raw reply
* [PATCH] Remove extra dots from doc of git-archive option --format
From: Jari Aalto @ 2007-09-14 18:38 UTC (permalink / raw)
To: git
The description of the option gave impression that there
were several formats available by using three dots. There are
no other formats than tar and gzip.
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
Documentation/git-archive.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index f2080eb..04771dc 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -31,7 +31,7 @@ OPTIONS
-------
--format=<fmt>::
- Format of the resulting archive: 'tar', 'zip'... The default
+ Format of the resulting archive: 'tar' or 'zip'. The default
is 'tar'.
--list, -l::
--
1.5.3
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related
* [PATCH] Add comment that git-archive writes to stdout
From: Jari Aalto @ 2007-09-14 18:41 UTC (permalink / raw)
To: git
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
Documentation/git-archive.txt | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index f2080eb..f172639 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -16,7 +16,8 @@ DESCRIPTION
-----------
Creates an archive of the specified format containing the tree
structure for the named tree. If <prefix> is specified it is
-prepended to the filenames in the archive.
+prepended to the filenames in the archive. The archive is
+written to stdout.
'git-archive' behaves differently when given a tree ID versus when
given a commit ID or tag ID. In the first case the current time is
--
1.5.3
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related
* Re: [PATCH 2/2] Fix the rename detection limit checking
From: Linus Torvalds @ 2007-09-14 18:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141017450.16478@woody.linux-foundation.org>
On Fri, 14 Sep 2007, Linus Torvalds wrote:
>
> ... and we also make sure that we don't overflow when doing the matrix
> size calculation.
Side note: by "make sure", I don't really mean a total guarantee.
We could be even more careful here. In particular:
- we later do end up allocating the matrix with
sizeof(*mx) * num_create * num_src
and I didn't actually fix the overflow that is possible due to the
"sizeof(*mx)" multiplication.
- even after we've checked that not *both* of the source and destination
counts are larger than the rename_limit, we could still overflow the
multiplication in just the limit check.
.. but with the rename_limit being set to 100, in practice neither of
these are really even close to realistic (ie you'd need to have less than
100 new files, and deleted over twenty million files to overflow, or vice
versa).
So with a rename_limit of 100, it's all good (I'm pretty sure you'd have
*other* issues long before you'd hit the integer overflows on renames ;)
But if somebody sets the rename_limit to something bigger, it gets
increasingly easier to screw it up.
If somebody wants to be *really* careful, they'd need to do something like
unsigned long max;
/* This isn't going to overflow, since we limited 'rename_limit' */
max = rename_limit * rename_limit;
/*
* But we should also check that multiplying by "sizeof(*mx)"
* won't make it overlof either..
*/
while ((sizeof(*mx) * max) / sizeof(*mx) != max)
max >>= 1;
/*
* And then avoid multiplying "rename_dst_nr" and "rename_src_nr"
* together by turning it into a division instead
*/
if (max / rename_dst_nr > rename_src_nr)
goto cleanup;
but the patch I sent out was the "obvious" first one that at least avoided
the overflow for the triggerable case that Dmitry had, and as per above
likely in all reasonable cases...
Linus
^ permalink raw reply
* Re: [PATCH 2/2] Fix the rename detection limit checking
From: Linus Torvalds @ 2007-09-14 18:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141132250.16478@woody.linux-foundation.org>
On Fri, 14 Sep 2007, Linus Torvalds wrote:
>
> but the patch I sent out was the "obvious" first one that at least avoided
> the overflow for the triggerable case that Dmitry had, and as per above
> likely in all reasonable cases...
Final note (I promise): the patch I sent out took "git runstatus" times on
the workload I replicated from Dmitry down from "so long you'd ^C it" to
about two seconds..
So I wanted to point out that this was not just the correctness issue of
the overflow, but that the rename limiting really does need to be done for
purely practical time reasons - doing the math in 64 bits would have
avoided the overflow, but wouldn't have avoided the real reason for not
wanting to do these kinds of things in the first place!
Linus
^ permalink raw reply
* Re: [PATCH 1/2] Fix "git diff" setup code
From: Junio C Hamano @ 2007-09-14 19:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <alpine.LFD.0.999.0709141129451.16478@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, 14 Sep 2007, Junio C Hamano wrote:
>>
>> So I tend to think the attached is a better fix.
>
> Ahh, yes, that explains the conditional.
>
> But whatever gets us to actually verify our options, and fill in the right
> defaults is ok by me!
Sorry, my explanation only explains about missing setup_done()
when --no-index is used, but does not explain _if_ you actually
found that setup_done() was not called for you when you did a
real life test. Was it only from code inspection, or did you
hit a case where setup_done() is not run? If the latter then
there is something else going on, as I cannot think of a way to
call setup_revisions() and not have it call setup_done()...
^ permalink raw reply
* tags "problem"
From: Rene Herman @ 2007-09-14 19:31 UTC (permalink / raw)
To: git
Hi people.
(please keep me in CC)
Was just busy constructing a tree and noticed a "problem" -- no doubt caused
by me, but help appreciated.
I clone linus' kernel tree into ./local, then do a
git checkout -b v22 v2.6.22
to get a 2.6.22 branch. I like to have a simple "git pull" while on this
branch pull from the upstream stable tree, so as advised earlier, I put:
[branch "v22"]
remote = linux-2.6.22.y
merge = refs/heads/master
[remote "linux-2.6.22.y"]
url =
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.22.y.git
fetch = refs/heads/master
in ./git/config. "git pull" now indeed works nicely.
I just noticed now though that this doesn't automatiically fetches tags as
well. That is, no v2.6.22.x tags after that. "git fetch --tags" (or git pull
--tags directly, I assume) gets them without problem, but I was expecting
they'd be fetched automatically.
Should they? Can they? Can I put anything in the config so they will?
Rene.
^ permalink raw reply
* Re: [PATCH 1/2] Fix "git diff" setup code
From: Linus Torvalds @ 2007-09-14 19:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry V. Levin, Shawn O. Pearce, Git Mailing List, Jeff King
In-Reply-To: <7vodg59i4x.fsf@gitster.siamese.dyndns.org>
On Fri, 14 Sep 2007, Junio C Hamano wrote:
>
> Sorry, my explanation only explains about missing setup_done()
> when --no-index is used, but does not explain _if_ you actually
> found that setup_done() was not called for you when you did a
> real life test. Was it only from code inspection, or did you
> hit a case where setup_done() is not run?
Hmm. Mea culpa. What seems to have happened is that I ran things under
gdb, and noticed that the default rename_limit hadn't been correctly set:
but now that I look more at it, that particular session was probably from
"git runstatus", not "git diff".
So yeah, ignore my 1/2. It was almost certainly based on a bogus debugging
session, before I noticed that wt-status.c doesn't use the normal diff
stuff at all..
Linus
^ permalink raw reply
* Re: tags "problem"
From: Junio C Hamano @ 2007-09-14 21:14 UTC (permalink / raw)
To: Rene Herman; +Cc: git
In-Reply-To: <46EAE189.5000804@gmail.com>
Rene Herman <rene.herman@gmail.com> writes:
> Hi people.
>
> (please keep me in CC)
>
> Was just busy constructing a tree and noticed a "problem" -- no doubt
> caused by me, but help appreciated.
>
> I clone linus' kernel tree into ./local, then do a
>
> git checkout -b v22 v2.6.22
>
> to get a 2.6.22 branch. I like to have a simple "git pull" while on
> this branch pull from the upstream stable tree, so as advised earlier,
> I put:
>
> [branch "v22"]
> remote = linux-2.6.22.y
> merge = refs/heads/master
> [remote "linux-2.6.22.y"]
> url =
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.22.y.git
> fetch = refs/heads/master
>
> in ./git/config. "git pull" now indeed works nicely.
Perhaps more canonical would be to make the latter fetch as:
fetch = refs/heads/*:refs/remotes/linux-2.6.22.y/*
> I just noticed now though that this doesn't automatiically fetches
> tags as well.
I suspect that fetch without remote tracking does not auto-fetch
the tags.
^ permalink raw reply
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