* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
From: Junio C Hamano @ 2007-12-22 6:49 UTC (permalink / raw)
To: David Symonds; +Cc: Junio C Hamano, Gustaf Hendeby, luciano, git
In-Reply-To: <ee77f5c20712211718g230802b6jb70e5db1f6a43973@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Don't you have $repo (an instance of Git) at that point? You
>> should be able to ask repo_path() about it, shouldn't you?
>
> Isn't git-send-email still useful outside a Git repo?
Then why does it run "rev-parse --git-dir"?
^ permalink raw reply
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
From: David Symonds @ 2007-12-22 7:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Gustaf Hendeby, luciano, git
In-Reply-To: <7vhcib2phi.fsf@gitster.siamese.dyndns.org>
On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> "David Symonds" <dsymonds@gmail.com> writes:
>
> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Don't you have $repo (an instance of Git) at that point? You
> >> should be able to ask repo_path() about it, shouldn't you?
> >
> > Isn't git-send-email still useful outside a Git repo?
>
> Then why does it run "rev-parse --git-dir"?
I'm suggesting that it should still function just fine without being
inside a repo, so it should adequately handle "rev-parse --git-dir"
returning 128.
Dave.
^ permalink raw reply
* Re: [PATCH] Allow selection of different cleanup modes for commit messages
From: Junio C Hamano @ 2007-12-22 7:59 UTC (permalink / raw)
To: Alex Riesen; +Cc: Linus Torvalds, git
In-Reply-To: <20071221230851.GA3260@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
> Junio C Hamano, Fri, Dec 21, 2007 22:43:49 +0100:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>
>> > The patch is on top of my previos patches. Junio, if you wish, I can
>> > squash and resend.
>>
>> That would be a sane thing to do.
>
> Done
Thanks. I am afraid I have to ask you to either refute my
misunderstanding or a second round.
> +--cleanup=<mode>::
> + This option sets how the commit message is cleaned up.
> + The '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
> + and 'default'. The 'default' mode will strip leading and
> + trailing empty lines and #commentary from the commit message
> + only if the message is to be edited. Otherwise only whitespace
> + removed. The 'verbatim' mode wont change message at all,
> + 'whitespace' removes just leading/trailing whitespace lines
> + and 'strip' removes both whitespace and commentary.
(minor) s/wont/does not/
> @@ -394,20 +410,24 @@ static int prepare_log_message(const char *index_file, const char *prefix)
> return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
> }
>
> + if (!no_edit) {
> + if (in_merge)
> + fprintf(fp,
> + "#\n"
> + "# It looks like you may be committing a MERGE.\n"
> + "# If this is not correct, please remove the file\n"
> + "# %s\n"
> + "# and try again.\n"
> + "#\n",
> + git_path("MERGE_HEAD"));
> + if (cleanup_mode != CLEANUP_NONE)
> + fprintf(fp,
> + "\n"
> + "# Please enter the commit message for your changes.\n");
> + if (cleanup_mode == CLEANUP_DEFAULT)
> + fprintf(fp,
> + "# (Comment lines starting with '#' will not be included)\n");
Can't cleanup_mode be CLEANUP_ALL at this point, and if so
shouldn't this insn be given as well?
In addition, if:
- we are talking with editor, and
- the mode is verbatim, and
- we added any "#" line ourselves (e.g. in_merge adds the insn
shown above, or perhaps we added "git status" output),
then perhaps we would want to say that "#" lines need to be
stripped by the user; otherwise it will be left in the commit.
> @@ -431,10 +451,13 @@ static int message_is_empty(struct strbuf *sb, int start)
> const char *nl;
> int eol, i;
>
> + if (cleanup_mode == CLEANUP_NONE && sb->len)
> + return 0;
> +
> /* See if the template is just a prefix of the message. */
> strbuf_init(&tmpl, 0);
> if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
> - stripspace(&tmpl, 1);
> + stripspace(&tmpl, !no_edit);
We at this point know that some stripping would happen. The
template needs to be cleaned the same way as the commit message
was stripped. Is checking no_edit the right thing to do? What
if cleanup_mode was CLEANUP_ALL and there is no editing going
on?
> @@ -813,7 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
> if (p != NULL)
> strbuf_setlen(&sb, p - sb.buf + 1);
>
> - stripspace(&sb, 1);
> + if (cleanup_mode != CLEANUP_NONE)
> + stripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?
> + !no_edit: cleanup_mode == CLEANUP_ALL);
When writing such a long ? : expression, laying it out like:
condition
? if-true
: if-false
would be easier to read. You tilt your head sideways 90
degrees, just like you read a smiley ;-), and you will see the
parse tree.
But I suspect, assuming that the two issues around CLEANUP_DEFAULT vs
CLEANUP_ALL I mentioned above are indeed bugs, it may be cleaner
to:
- parse the options; you get one of NONE/DEFAULT/SPACE/ALL
- convert DEFAULT to SPACE or ALL if editor is in effect.
- do not worry about no_editor or DEFAULT in the rest of the
code when calling stripspace(). The only values you will
see are NONE, SPACE or ALL.
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index 21ac785..6219378 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -89,4 +89,44 @@ test_expect_success 'verbose' '
>
> '
>
> +test_expect_success 'verbatim commit messages' '
> +
These tests check too many things at once, and it would be very
hard to diagnose when breakage does happen. Please split them
perhaps into three separate tests like this:
> + echo >>negative &&
> + { echo;echo "# text";echo; } >expect &&
> + git commit --cleanup=verbatim -t expect -a &&
> + git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
> + diff -u expect actual &&
This is one test --- making sure the commit body match the
unedited template verbatim, except the status part to be shown
in the editor. But we are not using editor in this test and I
thought the point of this --cleanup=verbatim exercise was that
the "status" part is not even have to be added to the commit log
message when editor is not in effect... Isn't the "head -n 3"
just sweeping a bug under the rug?
> + echo >>negative &&
> + git commit --cleanup=verbatim -F expect -a &&
> + git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
> + diff -u expect actual &&
And this is another test.
> + echo >>negative &&
> + git commit --cleanup=verbatim -m "$(cat expect)" -a &&
> + git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
> + diff -u expect actual
And this is yet another.
> +'
Also, the tests do not check "-F file -e", "-m msg -e" etc. We
would want to make sure the right insn are shown to the user in
the editor, wouldn't we?
^ permalink raw reply
* Re: [PATCH] git-tag: fix -l switch handling regression.
From: Junio C Hamano @ 2007-12-22 8:01 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071221211833.GA10318@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> On Fri, Dec 21, 2007 at 04:32:43PM +0000, Junio C Hamano wrote:
> ...
>> I thought this depended on some other changes. As is, doesn't
>> it break the t7004 test?
>
> Well there are tests that test:
>
> git tag -n xxx -l ...
>
> or
>
> git tag -n "" -l ...
>
> but I think we agreed those test nothing legitimate, and that the
> tests have to be removed. SO yes it hides another patch to cleanse t7004
> from the broken tests.
Thanks for a clarification; I'll resurect parts of my test
fix-ups and squash that in.
^ permalink raw reply
* Re: unexpected git-cherry-pick conflict
From: Junio C Hamano @ 2007-12-22 8:20 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Johannes Schindelin, git
In-Reply-To: <20071221103743.2210.qmail@80f7ec9d6d380d.315fe32.mid.smarden.org>
Gerrit Pape <pape@smarden.org> writes:
> On Sat, Jul 07, 2007 at 09:58:08PM +0100, Johannes Schindelin wrote:
>> On Mon, 25 Jun 2007, Gerrit Pape wrote:
>> > Hi, did you get to this yet?, not to stress you, just to make sure we
>> > don't forget about it.
>>
>> Okay. Since now both you and Junio asked for it, and I made today a Git
>> day for me, I looked into this.
>
> Hi, the discussion on this topic unfortunately didn't result in a patch.
I know. Unfortunately the code structure in merge-recursive
around directory-file conflict handling needs major surgery to
fix this, and it won't be until somebody really sits down and
rethink the issue. I've been quietly working on some ideas and a
small nascent part of it is in 'pu' (or 'offcuts', perhaps, I do
not remember offhand), but it will take time.
^ permalink raw reply
* Re: Serious bug with pretty format strings & empty bodies?
From: Junio C Hamano @ 2007-12-22 8:39 UTC (permalink / raw)
To: Jonathan del Strother; +Cc: git
In-Reply-To: <57518fd10712210247s2fcbbf61ke67bbdc0f5ffe70b@mail.gmail.com>
"Jonathan del Strother" <maillist@steelskies.com> writes:
> Has anyone actually managed to reproduce my problem? I've got
> multiple repos here that show the problem in several commits, made by
> different people. However, I can't actually come up with a way to
> reproduce it at will...
Marking an issue as a "serious bug" without giving enough
material for reproduction nor diagnosis tends to discourage
people from looking into it seriously, as the issue cannot be
even judged if it is really serious. For example:
On Dec 19, 2007 6:44 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
> Could you try
>
> git cat-file 18d2480ab689b483ef1ebbdb3f7420904049ba0b
>
> (or any other problematic commit) and post its output here?
You mean git cat-file commit ... ?
I get the normal output, but the problematic commits don't show a
newline character at the end of the cat-file output.
"I get the normal output" is not what Alex asked you to supply,
nor would be sufficient information. There may be some
abnormality in the commit object that you probably did not spot,
but Alex or other people may have been able to if you were
actually posted its output here.
I have been suspecting that there is a NUL in the middle of the
message somehow (I know the lowest-level plumbing commit-tree
allows any byte sequence in the log message if you worked hard
enough), which the parser is not prepared to cope with, but we
haven't seen enough evidence to support nor refute that theory.
We do not have much to work from.
One thing I noticed funny in your original message was "-1-".
Is it essential that the number is spelled incorrectly to
reproduce this problem?
$ git rev-list -1- --pretty=format:"%s%n%b"
18d2480ab689b483ef1ebbdb3f7420904049ba0b
commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b
Try to flush log files before terminating the app
tree 57bc7cf30a10aee96251852125cf30fd2c81d7aa
parent 04c833865828538315fcdf6e187da077869ce444
author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000
committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000
Check that ThreadWorker's work method actually returns a value with
method signatures
^ permalink raw reply
* Re: [PATCH v3] Make git send-email accept $EDITOR with arguments
From: Gustaf Hendeby @ 2007-12-22 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: peff, luciano, git
In-Reply-To: <7vr6hf35fh.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
>
>> Currently git send-email does not accept $EDITOR with arguments, eg,
>> emacs -nw, when starting an editor to produce a cover letter. This
>> patch changes this by letting the shell handle the option parsing.
>>
>> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
>> ---
>>
>> This is based on Junio's suggestion on most readable and compatible
>> solution. I'm not sure if it is identical to the C solution for git
>> tag, but it seems to be a reasonable solution.
>>
>> git-send-email.perl | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 248d035..e47994a 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -400,7 +400,7 @@ EOT
>> close(C);
>>
>> my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
>> - system($editor, $compose_filename);
>> + system('sh', '-c', '$0 $@', $editor, $compose_filename);
>>
>> open(C2,">",$compose_filename . ".final")
>> or die "Failed to open $compose_filename.final : " . $!;
>> --
>> 1.5.4.rc1.16.gc817f
>
> Thanks. Has this been tested? IOW, did you compose this
> message with this patch?
Yes, I sent out the patch with git send-email, with git including the
patch itself and EDITOR=emacsclient -a emacs. I have also played around
with it a bit trying to make sure it is valid, but no more rigorous
testing that that. I'm not really sure how I would do that.
Thanks for all comments from everyone, I have learned a lot!
/Gustaf
^ permalink raw reply
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
From: Junio C Hamano @ 2007-12-22 8:52 UTC (permalink / raw)
To: David Symonds; +Cc: Junio C Hamano, Gustaf Hendeby, luciano, git
In-Reply-To: <ee77f5c20712212304s598d344dg41d03f58084d794e@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "David Symonds" <dsymonds@gmail.com> writes:
>>
>> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> Don't you have $repo (an instance of Git) at that point? You
>> >> should be able to ask repo_path() about it, shouldn't you?
>> >
>> > Isn't git-send-email still useful outside a Git repo?
>>
>> Then why does it run "rev-parse --git-dir"?
>
> I'm suggesting that it should still function just fine without being
> inside a repo, so it should adequately handle "rev-parse --git-dir"
> returning 128.
Ah, true. Then the current behaviour to use the $(pwd) for
temporary file area would be Ok for now.
^ permalink raw reply
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
From: Gustaf Hendeby @ 2007-12-22 9:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Symonds, luciano, git
In-Reply-To: <7vd4szyuva.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> "David Symonds" <dsymonds@gmail.com> writes:
>
>> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> "David Symonds" <dsymonds@gmail.com> writes:
>>>
>>>> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Don't you have $repo (an instance of Git) at that point? You
>>>>> should be able to ask repo_path() about it, shouldn't you?
>>>> Isn't git-send-email still useful outside a Git repo?
>>> Then why does it run "rev-parse --git-dir"?
>> I'm suggesting that it should still function just fine without being
>> inside a repo, so it should adequately handle "rev-parse --git-dir"
>> returning 128.
>
> Ah, true. Then the current behaviour to use the $(pwd) for
> temporary file area would be Ok for now.
Ok, just drop the patch I don't feel strongly about it.
However, the code today demands that git send-email is run from within a
git repository - it seems that $repo = Git->repository() assumes that.
I'd suggest changing this behavior so that git send-email becomes
runnable from outside a git repository. Unfortunately, I'm starting to
get in above my head here, for one I really don't know the helper
functions in Git.pm. Is there any good place to read up on what is in
Git.pm, except for the code itself?
/Gustaf
^ permalink raw reply
* Re: git-svn: pulling from local mirror but committing via svn+ssh
From: Sam Vilain @ 2007-12-22 9:16 UTC (permalink / raw)
To: Eric Wong; +Cc: Mike Frysinger, git
In-Reply-To: <20071222043528.GB18812@soma>
Eric Wong wrote:
>> i have local (fast) mirrors of the svn trees i use, so i normally keep
>> everything pointing at those. when i need to commit, i use `svn
>> switch --relocate ...` to flip to the svn+ssh master, and then flop
>> back to the local mirror. it actually works out nicely.
> Can useSvmProps handle this? I honestly forgot how that stuff
> was supposed to work with SVK/svn-mirror.
It's precisely this use case. If you are syncing with SVN::Mirror, part
of SVK, or via svnsync, these tools leave breadcrumbs as svn properties
that point to where the repository was copied from. So, in your commit
log the message will be the URL of the immediately[*] upstream
repository. SVN didn't ever really support clone per se, what did you
use to copy the repository?
One thing that isn't documented to my knowledge is that you can use 'git
commit --amend' to alter the URL in the 'git-svn-id:' in the commit
message of the top commit as a crude form of grafting. If the master
repository changes URL or UUID that's probably what you'd have to do.
But in your case, you probably want to figure out which switches you're
missing.
Sam.
* - Usually these tools don't allow for more than one level of indirection
>
>
> Mike:
>
> On the other hand, there's little point in keeping local mirrors around
> when using git-svn since git already does that for you :)
>
^ permalink raw reply
* Re: [PATCH] Don't dereference NULL upon lookup_tree failure.
From: Junio C Hamano @ 2007-12-22 9:25 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list, Matthew Farrellee
In-Reply-To: <8763yrvb3g.fsf@rho.meyering.net>
Jim Meyering <jim@meyering.net> writes:
>>> Junio C Hamano <gitster@pobox.com> wrote:
>>> ...
>> Independent of who creates a "invalid repository", our tools
>> should be prepared to be fed bad data and not get upset.
>> Somebody in our code read a non-commit (or it did not read
>> anything) and told the object layer it is a commit. That
>> codepath needs to be tightened up, no?
>
> Yes, of course.
> Unfortunately, I won't have time to investigate for a while.
Although I haven't seen the problematic repository, I think what
is going on is that the repository has objects with pointers to
other objects labelled with wrong types.
For example, a tag can tag any type of object.
$ git cat-file tag v1.5.4-rc1
object 74f6b03c5c8fceef416de9f9a18e5d64712b6d96
type commit
tag v1.5.4-rc1
tagger Junio C Hamano <gitster@pobox.com> 1198114975 -0800
GIT 1.5.4-rc1
...
When parsing such a tag object, it instantiates an in-core tag
object that has a pointer to an in-core commit object, because
the tag says that it points at commit 74f6b0, without reading
the tagged object itself. This is to allow the user of that
in-core tag object to ask "what do you point at" and get an
in-core object that represents the pointee. If (and only if)
the user choose to follow that pointer and look at the contents
of the pointee, it can then call parse_object() on that in-core
object. In other words, the object layer is designed to work
lazily.
So if you earlier parsed such a tag but if the object 74f6b0
were an object of different type, we would hit the codepath you
identified that leads to a NULL dereference. And "lying caller"
cannot really do much better (except perhaps actually validating
the presense and the type of the object, which would incur
additional overhead). The situation is the same if:
- a tree object records an entry with:
- mode bits 040000 pointing at an object that is not a tree; or
- mode bits 100(644|755) pointing at an object that is not a blob; or
- mode bits 160000 pointing at an object that is not a commit; or
- a commit object has a "parent" line that is not a commit; or
- a commit object has a "tree" line that is not a tree.
Also we cannot just say "then we would take safety over
overhead" and make the lying callers validate the pointee;
subproject commits pointed by gitlinks (i.e. entry with 160000
mode bits in a tree object) are not even required to be
available.
Your fix is all we can sensibly do. I however think you would
need similar fix to the same function for other object types, as
they dereference a potentially NULL pointer the same way.
^ permalink raw reply
* Re: git-svn: pulling from local mirror but committing via svn+ssh
From: Mike Frysinger @ 2007-12-22 9:32 UTC (permalink / raw)
To: Sam Vilain; +Cc: Eric Wong, git
In-Reply-To: <476CD5E1.9020204@vilain.net>
On Dec 22, 2007 4:16 AM, Sam Vilain <sam@vilain.net> wrote:
> Eric Wong wrote:
> > > i have local (fast) mirrors of the svn trees i use, so i normally keep
> > > everything pointing at those. when i need to commit, i use `svn
> > > switch --relocate ...` to flip to the svn+ssh master, and then flop
> > > back to the local mirror. it actually works out nicely.
> >
> > Can useSvmProps handle this? I honestly forgot how that stuff
> > was supposed to work with SVK/svn-mirror.
>
> It's precisely this use case. If you are syncing with SVN::Mirror, part
> of SVK, or via svnsync, these tools leave breadcrumbs as svn properties
> that point to where the repository was copied from. So, in your commit
> log the message will be the URL of the immediately[*] upstream
> repository. SVN didn't ever really support clone per se, what did you
> use to copy the repository?
we publish the raw svn tree via rsync so people can create their own svn mirrors
sounds like it's just easier to mirror the original and since this
isnt a terribly large repo (~50 meg w/out any scm stuff), that's ok
-mike
^ permalink raw reply
* Re: [PATCH] Don't dereference NULL upon lookup_tree failure.
From: Jim Meyering @ 2007-12-22 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Matthew Farrellee
In-Reply-To: <7vzlw3xeqy.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
...
> Your fix is all we can sensibly do. I however think you would
> need similar fix to the same function for other object types, as
> they dereference a potentially NULL pointer the same way.
Good point. Patch below. I wondered if these lookup functions have
always been able to return NULL, since there are many remaining uses
where the possibility of a NULL return value is not handled.
Here are a few that stood out in the output of a quick search:
git-grep -E -A3 'lookup_(commit|tag|blob|tree) *\('
Note: I did not look at cases where the return value is an argument
to some other function.
------------------
builtin-fmt-merge-msg.c:
head = lookup_commit(head_sha1);
...
for (i = 0; i < origins.nr; i++)
shortlog(origins.list[i], origins.payload[i],
head, &rev, limit);
------------------
builtin-read-tree.c: struct tree *subtree = lookup_tree(entry.
sha1);
builtin-read-tree.c- if (!subtree->object.parsed)
------------------
combine-diff.c: struct commit *commit = lookup_commit(sha1);
combine-diff.c- struct commit_list *parents;
------------------
http-push.c: struct commit *head = lookup_commit(head_sha1);
http-push.c: struct commit *branch = lookup_commit(branch_sha1);
http-push.c- struct commit_list *merge_bases = get_merge_bases(head, branch, 1);
------------------
reachable.c: struct tree *tree = lookup_tree(sha1);
reachable.c- add_pending_object(revs, &tree->object, "");
------------------
tag.c: item->tagged = &lookup_blob(sha1)->object;
tag.c- } else if (!strcmp(type, tree_type)) {
tag.c: item->tagged = &lookup_tree(sha1)->object;
tag.c- } else if (!strcmp(type, commit_type)) {
tag.c: item->tagged = &lookup_commit(sha1)->object;
tag.c- } else if (!strcmp(type, tag_type)) {
tag.c: item->tagged = &lookup_tag(sha1)->object;
tag.c- } else {
--------------------------
unpack-trees.c: struct tree *tree = lookup_tree(posns[i]->sha1);
...
unpack-trees.c- parse_tree(tree);
Here's the revised patch:
==================================================================
From 94152217e8e57d3932b4ba6f7ee014da1f4346d3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Fri, 21 Dec 2007 11:56:32 +0100
Subject: [PATCH] Don't dereference NULL upon lookup failure.
Signed-off-by: Jim Meyering <meyering@redhat.com>
---
object.c | 42 +++++++++++++++++++++++++++++-------------
1 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/object.c b/object.c
index 16793d9..9945b25 100644
--- a/object.c
+++ b/object.c
@@ -138,27 +138,43 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t
if (type == OBJ_BLOB) {
struct blob *blob = lookup_blob(sha1);
- parse_blob_buffer(blob, buffer, size);
- obj = &blob->object;
+ if (!blob)
+ obj = NULL;
+ else {
+ parse_blob_buffer(blob, buffer, size);
+ obj = &blob->object;
+ }
} else if (type == OBJ_TREE) {
struct tree *tree = lookup_tree(sha1);
- obj = &tree->object;
- if (!tree->object.parsed) {
- parse_tree_buffer(tree, buffer, size);
- eaten = 1;
+ if (!tree)
+ obj = NULL;
+ else {
+ obj = &tree->object;
+ if (!tree->object.parsed) {
+ parse_tree_buffer(tree, buffer, size);
+ eaten = 1;
+ }
}
} else if (type == OBJ_COMMIT) {
struct commit *commit = lookup_commit(sha1);
- parse_commit_buffer(commit, buffer, size);
- if (!commit->buffer) {
- commit->buffer = buffer;
- eaten = 1;
+ if (!commit)
+ obj = NULL;
+ else {
+ parse_commit_buffer(commit, buffer, size);
+ if (!commit->buffer) {
+ commit->buffer = buffer;
+ eaten = 1;
+ }
+ obj = &commit->object;
}
- obj = &commit->object;
} else if (type == OBJ_TAG) {
struct tag *tag = lookup_tag(sha1);
- parse_tag_buffer(tag, buffer, size);
- obj = &tag->object;
+ if (!tag)
+ obj = NULL;
+ else {
+ parse_tag_buffer(tag, buffer, size);
+ obj = &tag->object;
+ }
} else {
warning("object %s has unknown type id %d\n", sha1_to_hex(sha1), type);
obj = NULL;
--
1.5.4.rc1.16.g60f3b
^ permalink raw reply related
* [PATCH] Emit helpful status for accidental "git stash" save
From: Wincent Colaiuta @ 2007-12-22 14:22 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <7vmys36d7n.fsf@gitster.siamese.dyndns.org>
El 21/12/2007, a las 20:48, Junio C Hamano escribió:
> (4) A tool should support safety for clueless people when it is
> reasonable.
>
> Even though "I did not know what command foo does, so I
> tried running it and it did something unexpected" is a
> silly excuse to rob quickie "git stash" from people who
> know better, we cannot avoid the fact that there are
> clueless people. I think checking stash.iknowwhatitdoes to
> detect first-time users, and explaining what it does to
> them, may make sense. And we can take hints from the patch
> that started this thread how to do this.
>
> The decision here is that I am open to a change that
> implements the one-time safety instruction.
Here's a less invasive change that I think addresses this point (4)
that you make.
-- 8< --
Emit helpful status for accidental "git stash" save
If the user types "git stash" mistakenly thinking that this will list
their stashes he/she may be surprised to see that it actually saved
a new stash and reset their working tree and index.
In the worst case they might not know how to recover the state. So
help them by telling them exactly what was saved and also how to
restore it immediately.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
git-stash.sh | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..a2f3723 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -99,7 +99,8 @@ save_stash () {
git update-ref -m "$stash_msg" $ref_stash $w_commit ||
die "Cannot save the current status"
- printf >&2 'Saved "%s"\n' "$stash_msg"
+ printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
+ echo >&2 '(To restore them type "git stash apply")'
}
have_stash () {
--
1.5.4.rc0.68.g15eb8-dirty
^ permalink raw reply related
* Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>
From: Johannes Schindelin @ 2007-12-22 14:33 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <fcaeb9bf0712210617x2bafa33cp15815a59fc631f45@mail.gmail.com>
Hi,
On Fri, 21 Dec 2007, Nguyen Thai Ngoc Duy wrote:
> [...] from my user perspective, the right approach is to make
> <treeish>:path always be relative to current directory.
As said by Junio, this would be a bad decision.
BTW please do not quote parts of the email that you do not comment on; it
takes half a minute of _everybody_ who tries to read your mail, only to
realise that it was time wasted.
Ciao,
Dscho
^ permalink raw reply
* Re: Problem with git-svn
From: Pascal Obry @ 2007-12-22 14:38 UTC (permalink / raw)
To: Eric Wong; +Cc: Pascal Obry, git list
In-Reply-To: <20071222042924.GA18812@soma>
Eric,
> Since r48468 was where /importfromcvs/trunk got renamed into /trunk/PROJ
> (from your previous message http://mid.gmane.org/4764FE2C.1010103@obry.net)
>
> /importfromcvs/trunk exists at r45775, but /trunk/PROJ does not; and
> git-svn at least follows that (which is what I suppose everybody wants).
Right.
> However...
>
> Did /importfromcvs/trunk exist all the way between r9458 and r48468? Or
> was that directory replaced entirely by something else along the way?
It was replaced along the way. This very same tree /importfromcvs/trunk
was used to import completely unrelated projects. Then moved, so it did
not exist all the way.
In my case the previous version on this tree was 45546. There is nothing
in between 45546 and 45775 for example. So looking at any revision in
between return an error:
$ svn log -v svn+ssh://myserver/importfromcvs/trunk@45760
svn: File not found: revision 45760, path '/importfromcvs/trunk'
> git-svn may be following copy history too aggressively, in this case.
Looks like it is the case. Hence the reported errors:
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 100, path '/trunk/PROJ'
and
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 101, path
'/importfromcvs/trunk'
For sure there was no revision 101 on /importfromcvs/trunk:
$ svn log -v svn+ssh://myserver/importfromcvs/trunk@101
svn: File not found: revision 101, path '/importfromcvs/trunk'
> On the other hand, this was somewhat intended because it could also
> be a way to track merges as "moving" tags[1].
>
>> $ git svn clone svn+ssh://myserver/trunk/PROJ --revision=45775:HEAD
>>
>> But it would be lot cleaner to have git-svn handling this properly I think.
>
> Does --no-follow-parent work in your case? Or does it go too far
> in stopping at r48468 (probably).
No it works on my case. That's what I'm using to import the directory in
two steps. I just want to go to the bottom of this to see if we can
somehow improve git-svn.
> Maybe another switch should be added (--merge-foster-parent?)
I'm not sure to understand what you are proposing here. To look only at
parents that are directly "related" to the current branch ?
Thanks,
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
^ permalink raw reply
* Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR
From: Johannes Schindelin @ 2007-12-22 14:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Luciano Rocha, git
In-Reply-To: <7vhcidovxt.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 20 Dec 2007, Junio C Hamano wrote:
> I think something like this patch is probably more appropriate.
Looks obviously fine, especially thinking about this:
> GIT_EDITOR='emacs -l $HOME/my-customization.el'
Ciao,
Dscho
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Johannes Schindelin @ 2007-12-22 15:04 UTC (permalink / raw)
To: Mike Frysinger; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <8bd0f97a0712210109q7805d967sc9b4cd13d4131360@mail.gmail.com>
Hi,
On Fri, 21 Dec 2007, Mike Frysinger wrote:
> ive started to transition from using svn everywhere to trying out git,
> and saw reference to this "stash" command on another list. i wanted to
> learn more about it, so i started off with `git-stash` to get some info,
> and wondered what just happened. then i typoed the --help option and
> wondered even more what just happened :).
I'm very sorry for you, but I, for one, refuse to let decisions be
influenced by people who did not have so much as a glimpse in the
documentation.
It may be okay for a certain nation state to award people spilling that
hot coffee that they ordered over there laps, but the rest of the world
laughs about such a behaviour.
If you got rope, stuck your neck through the noose, and jumped, without
reading the manual first, well, that's not my problem.
But I guess that we'll get that no-default-action behaviour, and I will
have to change my ways. Sigh.
Ciao,
Dscho
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Johannes Schindelin @ 2007-12-22 15:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 20 Dec 2007, Junio C Hamano wrote:
> * Introduction of "<tree>:./path" (Dscho). I could be talked into
> accepting the patch if it is useful to people who live deep within
> subdirectories.
IMHO this can safely await post-1.5.4.
Ciao,
Dscho
^ permalink raw reply
* Re: Serious bug with pretty format strings & empty bodies?
From: Jonathan del Strother @ 2007-12-22 15:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcibyvh0.fsf@gitster.siamese.dyndns.org>
On Dec 22, 2007 8:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> On Dec 19, 2007 6:44 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
> > Could you try
> >
> > git cat-file 18d2480ab689b483ef1ebbdb3f7420904049ba0b
> >
> > (or any other problematic commit) and post its output here?
>
> You mean git cat-file commit ... ?
> I get the normal output, but the problematic commits don't show a
> newline character at the end of the cat-file output.
>
> "I get the normal output" is not what Alex asked you to supply,
> nor would be sufficient information. There may be some
> abnormality in the commit object that you probably did not spot,
> but Alex or other people may have been able to if you were
> actually posted its output here.
========
$ git cat-file commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b
tree 2416c620c6f7c1864065a6e778588b71b3e0bd5d
parent 9fc80d0a05835c68885f253844ab586b38d09202
author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000
committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000
Try to flush log files before terminating the app%
========
The '%' suffix is zsh showing that there's no newline there. Under
bash I get no '%' and the next prompt starts immediately following
'app'.
> One thing I noticed funny in your original message was "-1-".
> Is it essential that the number is spelled incorrectly to
> reproduce this problem?
No, sorry - I don't know how that got there. I get identical output
whether it's "rev-list -1" or "rev-list -1-"
I've been trying to reproduce this on my home repo, and have only been
partially successful. I haven't been able to make git rev-list -1
show a malformed message body. I can only get the problem to occur
when printing multiple commits (eg git rev-list -20 ...) Bizarrely,
the first 2 commits shown in rev-list never show a problem. After the
first 2, commits with the broken message bodies will always appear
broken, regardless of their position in the list. However, the
content that appears instead of the body will change depending on the
commits that appear prior to that one.
I realise this doesn't appear to mesh with my observations the other
day, where I was seeing the broken bodies even when just using
rev-list -1 - I'm at a loss to describe it. Both machines are intel
macs (one a MacPro, one a MacBook Pro), running OS X 10.5.1, with the
same git version (1.5.4.rc0.1162.g3bfea). Unfortunately I've switched
my work machine off for christmas, or I'd ssh in and try to re-verify
things.
I do appreciate people's help in trying to reproduce & fix this - I
realise I've not given people much to go on.
^ permalink raw reply
* Re: [PATCH] Emit helpful status for accidental "git stash" save
From: Junio C Hamano @ 2007-12-22 16:22 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <1198333369-17788-1-git-send-email-win@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> diff --git a/git-stash.sh b/git-stash.sh
> index f16fd9c..a2f3723 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -99,7 +99,8 @@ save_stash () {
>
> git update-ref -m "$stash_msg" $ref_stash $w_commit ||
> die "Cannot save the current status"
> - printf >&2 'Saved "%s"\n' "$stash_msg"
> + printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
> + echo >&2 '(To restore them type "git stash apply")'
> }
>
> have_stash () {
I like that "To restore them..." insn, and I like that this is
much less invasive than anything we have seen during the
discussion. But can we do this only for an accidental "git
stash" not for a deliberate "git stash save"?
^ permalink raw reply
* Re: cvs -> git tools?
From: David Soria Parra @ 2007-12-22 16:00 UTC (permalink / raw)
To: git
In-Reply-To: <m3zlw3oged.fsf@roke.D-201>
> There are many CVS -> git tools: git-cvsimport (uses cvsps), parsecvs
> (accesses ,v files directly), fromcvs (in Ruby), cvs2svn development
> branch. But I think only git-cvsimport allows incremental import.
there is also gc-utils (http://sf.net/projects/gcutils) which wraps
git-cvsimport and git-cvsexportcommit to make commiting back into CVS a
little bit easier.
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Pierre Habouzit @ 2007-12-22 17:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]
On ven, déc 21, 2007 at 12:32:28 +0000, Junio C Hamano wrote:
> I've tagged -rc1 last night. The changes are mostly fixes. There are
> some remaining issues I'd like to see fixed/decided before 1.5.4.
>
> One important issue is to identify and fix regressions since 1.5.3
> series. No "rewrite scripted git-foo completely in C" can be regression
> free, and we had quite a few internal changes during 1.5.4 cycle (not
> just rewrite to C, but C level uses new and improved API such as strbuf
> and parse-options). Currently I am aware of these regressions:
>
> * handling of options, "--abbrev 10 HEAD", "--abbrev=10 HEAD" and
> "--abbrev HEAD". The last one does not work for commands that use
> parse-options. Pierre is on top of this, I hope.
About that, I know we talked about the -default thing and so on, I'm
not sure we should hurry that for 1.5.4 for the following reasons:
* I grepped through the source and the _sole_ instance of
parse-options enabled option arguments are --abbrev ones for now
(even in the shell scripts migrated to git rev-parse --parseopt).
* Not adding *-default and lax parsing for optional arguments is _not_
a regression for the migrated commands.
* I don't want to urge that because well, I still have the hope we
could come up with something even better.
So I'd argue in favor of that:
+ push the patch that forces the stuck forms (that I already posted)
for 1.5.4.
+ prepare a series in pu with *-default post 1.5.4 to evaluate this
and see what people think.
Most of the function with optional arguments are the git diff ones,
and we'll see about them post 1.5.4 anyways.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-22 17:16 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071222170315.GB23262@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> So I'd argue in favor of that:
> + push the patch that forces the stuck forms (that I already posted)
> for 1.5.4.
Ok, sounds sane. You posted multiple serieses; which ones?
^ permalink raw reply
* cvsimport: trying to convert freebsd cvs to git
From: Miklos Vajna @ 2007-12-22 17:18 UTC (permalink / raw)
To: git; +Cc: Stefan Sperling
[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]
hi,
recently Stefan reported that he tried to convert the freebsd cvs' src
module to git and he failed. i tried to help him, but i failed, too.
here are my efforts:
he made the cvs available on a (relatively) fast rsync mirror at:
rsync://ftp.spline.de/FreeBSD-CVS
(so you don't have to use cvsup if it's a problem for you)
so after mirroring it, i had:
$ ls cvs
CVSROOT/ CVSROOT-ports/ CVSROOT-src/ ports/ src/
then i tried:
$ time git cvsimport -d `pwd`/cvs -C src.git src
Initialized empty Git repository in /home/vmiklos/git/freebsd/src.git/.git/
malformed revision
fatal: refs/heads/origin: not a valid SHA1
fatal: master: not a valid SHA1
warning: You appear to be on a branch yet to be born.
warning: Forcing checkout of HEAD.
fatal: just how do you expect me to merge 0 trees?
checkout failed: 256
real 15m11.529s
user 0m46.212s
sys 0m6.680s
my questions:
1) does cvsimport supports the case when the source if on the local
filesystem, and not in not on a cvs server?
first i wanted to make sure that cvsimport itself works properly here:
$ git cvsimport -d :pserver:anonymous@tcpflow.cvs.sourceforge.net:/cvsroot/tcpflow -C tcpflow tcpflow
and it converted this small repo fine
2) if it supports, then i think the real error message is 'malformed
revision'. what is the proper way to see where is that revision?
of course if cvsimport is not the right tool to incrementally convert
such a big repo, then i would be interested in other advices, too.
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ 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