* [PATCH 5/5] reset: update cache-tree data when appropriate
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
In the case of --mixed and --hard, we throw away the old index and
rebuild everything from the tree argument (or HEAD). So we have an
opportunity here to fill in the cache-tree data, just as read-tree
did.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
builtin/reset.c | 7 +++++++
t/t0090-cache-tree.sh | 4 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e2..8c2c1d5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -43,6 +43,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
int nr = 1;
int newfd;
struct tree_desc desc[2];
+ struct tree *tree;
struct unpack_trees_options opts;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
@@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
return error(_("Failed to find tree of %s."), sha1_to_hex(sha1));
if (unpack_trees(nr, desc, &opts))
return -1;
+
+ if (reset_type == MIXED || reset_type == HARD) {
+ tree = parse_tree_indirect(sha1);
+ prime_cache_tree(&active_cache_tree, tree);
+ }
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
return error(_("Could not write new index file."));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index a3527a5..f972562 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -75,13 +75,13 @@ test_expect_success 'second commit has cache-tree' '
test_shallow_cache_tree
'
-test_expect_failure 'reset --hard gives cache-tree' '
+test_expect_success 'reset --hard gives cache-tree' '
test-scrap-cache-tree &&
git reset --hard &&
test_shallow_cache_tree
'
-test_expect_failure 'reset --hard without index gives cache-tree' '
+test_expect_success 'reset --hard without index gives cache-tree' '
rm -f .git/index &&
git reset --hard &&
test_shallow_cache_tree
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 4/5] commit: write cache-tree data when writing index anyway
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
In prepare_index(), we refresh the index, and then write it to disk if
this changed the index data. After running hooks we re-read the index
and compute the root tree sha1 with the cache-tree machinery.
This gives us a mostly free opportunity to write up-to-date cache-tree
data: we can compute it in prepare_index() immediately before writing
the index to disk.
If we do this, we were going to write the index anyway, and the later
cache-tree update has no further work to do. If we don't do it, we
don't do any extra work, though we still don't have have cache-tree
data after the commit.
The only case that suffers badly is when the pre-commit hook changes
many trees in the index. I'm writing this off as highly unusual.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Don't ask me why git thinks the index has been changed only when
already building upon an earlier commit. I don't know. I suspect
it's some raciness issue though.
builtin/commit.c | 2 ++
t/t0090-cache-tree.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 0163086..57d028e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -394,6 +394,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
+ update_main_cache_tree(1);
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(&index_lock))
die(_("unable to write new_index file"));
@@ -414,6 +415,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
+ update_main_cache_tree(1);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
die(_("unable to write new_index file"));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 3d0702a..a3527a5 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -70,7 +70,7 @@ test_expect_success 'test-scrap-cache-tree works' '
test_no_cache_tree
'
-test_expect_failure 'second commit has cache-tree' '
+test_expect_success 'second commit has cache-tree' '
test_commit bar &&
test_shallow_cache_tree
'
--
1.7.8.425.ga639d3
^ permalink raw reply related
* Re: git svn rebase “out of memory” error after merging two tracking branches
From: Thomas Rast @ 2011-12-06 17:56 UTC (permalink / raw)
To: Onsager; +Cc: git
In-Reply-To: <4EDE4589.9030601@gmx.net>
Onsager wrote:
> @list
>
> ... more detailed explanation on stackoverflow:
>
> http://stackoverflow.com/questions/8398405/git-svn-rebase-out-of-memory-error-after-merging-two-tracking-branches
>
> Is there something, I can do about this issue as a common user?
In this age of 5-second attention spans, I'm not sure you can expect
us to follow a link to an external site when a simple cut&paste job
would have sufficed, like so:
I'm tracking two customer SVN branches with msysgit 1.7.7.1 (Win 7 64bit):
SVN Git
trunk --> master
release_x_y_z --> git-local-release_x_y_z
After a successful local merge of 'git-local-release_x_y_z' into
'master' I'm running out of memory, when trying to synchronize the
result with svn:
mb@MMPEPA23 /c/git/MySoft (master)
$ git svn rebase
First, rewinding head to replay your work on top of it...
Applying:
fatal: Out of memory, realloc failed
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001
When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To check out the original branch and stop rebasing run "git rebase --abort".
rebase refs/remotes/git-svn: command returned error: 1
The .git/rebase-apply directory contains a few files ('patch', '0001')
with more than 1GB size. What can I do in order to apply this patch?
First you should find out whether something went wrong with the patch
generation, or if that 1GB size is plausible. Did your merge bring in
blobs that were that big?
Second, you could try with the -m option. This will use a 3-way merge
to rebase, which avoids generating a full patch.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Shawn Pearce @ 2011-12-06 18:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Michael Haggerty, kusmabite, git, Junio C Hamano
In-Reply-To: <CACsJy8Btwn0=PGS+PJV-6DqYBmzOp7LTB2=R_kCx4SJHA2YDRw@mail.gmail.com>
On Tue, Dec 6, 2011 at 07:30, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> index-pack is called at server side as part of a push. So in theory
> the sending side can generate very long delta chains and bring down
> the server side.
It is also called at client side during fetch. So in theory the server
can produce very long delta chains and take down a client.
>> Because if not, then the "creator" had better never be configured
>> to use a chain depth that the "reader" cannot handle.
>
> Normal creators (i.e. C Git) use default depth 50 so we should be safe.
JGit is also a "normal creator", and it sometimes produces chains
deeper than 50. Junio identified a 255 deep chain a week or two ago.
Some people have repacked their repositories very aggressively with
deeper chains when they are trying to optimize for space and don't
access historical revisions very often. I doubt anyone has packed
deeper than 120ish intentionally... but we shouldn't assume that in
the code.
>> This in turn
>> imply that there should be a common limit that is supported by all git
>> clients and is a documented part of the protocol. (Or the code has to
>> be rewritten to use an explicit stack instead of recursion.)
>
> It's the implementation limitation, not the protocol. If the server
> propagates error messages from index-pack back to client (I'm not
> sure), then users can adjust --depth to be accepted by server. We
> could negotiate the limit over the protocol but not sure we would want
> to go that route.
What about clients fetching from a server? The client can't change the
depth the server sends it.
> The troubled code could be rewritten to avoid recursion. However long
> delta chains may degrade performance, I'd rather have support to split
> long chains (which can be done with --depth at client already) instead
> of just recursion elimination. This patch is simpler, so it would be
> easier to back port it if someone wants to do so.
JGit long ago changed its IndexPack routine to use a manually managed
heap based stack instead of recursion, making it immune from
overrunning the stack due to a long delta chain. That is probably the
cleaner route. But you also have to fix sha1_file.c and its recursion
based unpacking of a delta chain... again which JGit fixed a long time
ago.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 18:20 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0mpPoZJWviBesWgy2dZ4xJrNyhED2znFid8iGbSTirPhQ@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> 0. You can merge this into `master` resolving the conflicts: it's a
> fairly straightforward resolution. As soon as you publish the new
> `master`, I can continue working on `rr/sequencer`.
> 1. I can post the `rr/revert-cherry-pick` to the list, hoping that it
> will make it to `master` without more disruptions. I can rebase
> `rr/sequencer` on this new series and continue working. For your
> reference, this [1] is what I'll be posting to the list if we pick
> this option.
Merging
https://github.com/artagnon/git.git rr/revert-cherry-pick ;# 1b7c2632
and merging rr/revert-cherry-pick currently in 'pu' (6a156fd) to 'master'
results in identical trees, so I think these amount to the same thing.
A few comments and thoughts, all minor.
* On "revert: simplify getting commit subject in format_todo()"
The old code used to use get_message() on cur->item, checked its return
value (if cur->item is not parsed, or has already been used and its buffer
cleared, cur->item->buffer would be NULL and get_message() returns -1) and
issued an error. The new code uses find_commit_subject without first
checking if cur->item->buffer is NULL.
Does this mean the old code was overly defensive, or is the new code too
lax?
I understand that parse_insn_line() uses lookup_commit_reference() which
calls parse_object() on the object name (and if it is a tag, deref_tag()
will parse the tagged object until we see something that is not a tag), so
we know cur->item is parsed before we see it, so I suspect you merely were
being overly defensive, but I may be missing something.
* On "revert: make commit subjects in insn sheet optional"
After finding the verb and advancing the pointer "bol" at the beginning of
the object name, end_of_object_name variable points at the first SP or LF
using strcspn(bol, " \n"), but I wonder why we are not grabbing a run of
hexdigits instead, i.e. strspn(bol, "0123456789abcdef"). Is this so that
we can allow something like "pick rr/revert-cherry-pick~3"?
I also wonder if this should be (sorry for pcre) "(pick|revert)\s+(\S+)\s"
instead, i.e. allow users with fat fingers to use one or more SP or even HT
to separate the verb and the operand.
The last test you added to 3510 in this patch runs test_line_count
unconditionally, by the way.
* On "revert: allow mixed pick and revert instructions"
Reporting what we did not understand from parse_insn_line() is a good
idea, but I think the line number should be reported at the beginning
of the same line.
I'd say that I'd prefer queuing re-rolled patches posted on the list; I'll
discard the rr/revert-cherry-pick (6a156fd) from my tree.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 18:35 UTC (permalink / raw)
To: Jeff King; +Cc: Erik Faye-Lund, git
In-Reply-To: <20111206055239.GA20671@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Also, let's drop the top git_getpass bits from the topic for now (they
> will not be part of my rebase). They are a separate topic that can go on
> top, but I think there was some question from Erik of whether we should
> simply roll our own getpass().
Sounds sensible.
I suspect that there may be a codepath where we could ask both username
and password; instead of making two consecutive calls to getpass() or
git_prompt(), the series may want to give a higher level abstraction, so
that GUI can show a dialog with two input fields (single-line input and
password input) and interact only once with the user. Such an input widget
could _show_ the username, and optionally even let it edited (there may be
ramifications depending on how the codepath uses the username), while
asking for the corresponding password.
>> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit
>> - archive: don't let remote clients get unreachable commits
>> (this branch is used by jk/maint-upload-archive.)
>>
>> * jk/maint-upload-archive (2011-11-21) 1 commit
>> - Merge branch 'jk/maint-1.6.2-upload-archive' into jk/maint-upload-archive
>> (this branch uses jk/maint-1.6.2-upload-archive.)
>>
>> Will merge to 'next' after taking another look.
>
> Thanks. I also have some followup patches to re-loosen to at least
> trees reachable from refs. Do you want to leave the tightening to the
> maint track, and then consider the re-loosening for master?
I was planning to first have the really tight version graduate to 'master'
and ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we
hear complaints from real users in the meantime before or after such
releases, we could apply loosening patch on top of these topics and call
them "regression fix", but I have been assuming that nobody would have
been using this backdoor for anything that really matters.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Jeff King @ 2011-12-06 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git
In-Reply-To: <7vhb1dh7ki.fsf@alter.siamese.dyndns.org>
On Tue, Dec 06, 2011 at 10:35:25AM -0800, Junio C Hamano wrote:
> > Also, let's drop the top git_getpass bits from the topic for now (they
> > will not be part of my rebase). They are a separate topic that can go on
> > top, but I think there was some question from Erik of whether we should
> > simply roll our own getpass().
>
> Sounds sensible.
>
> I suspect that there may be a codepath where we could ask both username
> and password; instead of making two consecutive calls to getpass() or
> git_prompt(), the series may want to give a higher level abstraction, so
> that GUI can show a dialog with two input fields (single-line input and
> password input) and interact only once with the user. Such an input widget
> could _show_ the username, and optionally even let it edited (there may be
> ramifications depending on how the codepath uses the username), while
> asking for the corresponding password.
Yes, I've considered that, too. But I think the idea of a combined
username/password is part of the credential code, and the right
call chain is something like:
credential_fill
-> call helpers with "get"; return if it works
-> credential_getpass
-> call helpers with "ask" for combined GUI prompt
-> otherwise, use git_prompt
-> git_prompt("username")
-> git_prompt("password")
So the "switch getpass to a generic prompt" idea is separate from
providing that higher-level abstraction.
> >> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit
> >> - archive: don't let remote clients get unreachable commits
> >> (this branch is used by jk/maint-upload-archive.)
> [...]
> I was planning to first have the really tight version graduate to 'master'
> and ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we
> hear complaints from real users in the meantime before or after such
> releases, we could apply loosening patch on top of these topics and call
> them "regression fix", but I have been assuming that nobody would have
> been using this backdoor for anything that really matters.
OK. I'll hold back on the loosening then.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Jeff King @ 2011-12-06 18:52 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <CABPQNSbOReM71HaPmce3v_98NDu17fT3YnySR4pWzJEDa-RKnA@mail.gmail.com>
On Tue, Dec 06, 2011 at 12:22:06PM +0100, Erik Faye-Lund wrote:
> >> * jk/upload-archive-use-start-command (2011-11-21) 1 commit
> >> - upload-archive: use start_command instead of fork
> >>
> >> What's the status of this one?
> >
> > I think what you have in pu is good, but of course I didn't actually
> > test it on Windows. Erik?
> >
>
> I tried to check out ee27ca4 and build, and got hit by a wall of warnings:
I think you are on the wrong topic. ee27ca4 is the maint topic for
"don't let remote clients get unreachable commits", and is based on the
ancient 1.6.2. Which is why you are getting all of those warnings.
The topic in question is jk/upload-archive-use-start-command, which is
at 1bc01ef, and should be based on recent-ish master.
-Peff
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Jeff King @ 2011-12-06 18:56 UTC (permalink / raw)
To: Shawn Pearce
Cc: Nguyen Thai Ngoc Duy, Michael Haggerty, kusmabite, git,
Junio C Hamano
In-Reply-To: <CAJo=hJuy27Uagmubbv=RqAOMx03e6JBgZxObkjFLg9oG2x_UqA@mail.gmail.com>
On Tue, Dec 06, 2011 at 10:12:54AM -0800, Shawn O. Pearce wrote:
> > Normal creators (i.e. C Git) use default depth 50 so we should be safe.
>
> JGit is also a "normal creator", and it sometimes produces chains
> deeper than 50. Junio identified a 255 deep chain a week or two ago.
> Some people have repacked their repositories very aggressively with
> deeper chains when they are trying to optimize for space and don't
> access historical revisions very often. I doubt anyone has packed
> deeper than 120ish intentionally... but we shouldn't assume that in
> the code.
"git gc --aggressive" will set the depth to 250.
-Peff
^ permalink raw reply
* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Jeff King @ 2011-12-06 19:02 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, git
In-Reply-To: <a639d328e15bce3057de9238ee31097d15850de1.1323189110.git.trast@student.ethz.ch>
On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:
> The cpp pattern, used for C and C++, would not match the start of a
> declaration such as
>
> static char *prepare_index(int argc,
>
> because it did not allow for * anywhere between the various words that
> constitute the modifiers, type and function name. Fix it.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> This is a really sneaky one-character bug that I cannot believe went
> unnoticed for so long, seeing as there are plenty of instances within
> git itself where it matters.
Looks reasonable to me. You can see the difference, for instance, with:
git show -U1 3c73a1d
(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 19:10 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CZjrKpv53Ep8ietAHAeVW4bnYeXTa6x5FGncT1HXGWQg@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> What can I do to get "build in repack" [1] series in or moved forward?
Resubmit to re-ignite interest in reviews, perhaps?
As long as it is done right at the implementation level, I do not think
there is anything controversial in the desired end result, iow it is not
like we need three people who want to have repack rewritten in C for it to
happen.
It may be tricky to get the flushing of old and new packfiles right,
though.
Use of reprepare_packed_git() is prerequisite if you want to do it in a
single process, but once you start using that API, it may not gain much
performance benefit to link the whole pack-objects logic in to the process
over a much simpler and less error prone approach of just rewriting the
shell script straight to a small C program that spawns pack-objects
binary.
^ permalink raw reply
* Re: Query on git commit amend
From: Jeff King @ 2011-12-06 19:11 UTC (permalink / raw)
To: Vijay Lakshminarayanan; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <87fwgxwvn9.fsf@gmail.com>
On Tue, Dec 06, 2011 at 09:16:18PM +0530, Vijay Lakshminarayanan wrote:
> I've found
>
> $ GIT_EDITOR=cat git commit --amend
>
> useful.
>
> The benefit of this technique is that it even works for git-rebase -i.
I sometimes do a similar thing, but I don't use "cat". That will dump
all of the log message (including the generated template) to stdout
(i.e., the terminal), which is quite noisy. Instead, I use:
GIT_EDITOR=true git commit --amend
which silently leaves the file untouched.
-Peff
^ permalink raw reply
* Re: Query on git commit amend
From: Dirk Süsserott @ 2011-12-06 19:09 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <20111206130138.119db519.kostix@domain007.com>
Am 06.12.2011 10:01 schrieb Konstantin Khomoutov:
> On Tue, 6 Dec 2011 13:53:00 +0530
> Viresh Kumar <viresh.kumar@st.com> wrote:
>
>> Suppose i want to add few new changes to my last commit (HEAD).
>> The way i do it is
>> $ git add all_changed_files
>> $ git commit --amend
>>
>> OR
>> $ git commit --amend -a
>>
>> With both these ways, i get a screen to edit the message too.
>>
>> I want to know if there is a way to skip this screen.
>>
>> i.e.
>> $ git commit --amend -a -some_other_option
>>
>> which simply adds new changes to existing commit, without asking to
>> change message.
>>
>> If there is no such way, then can we add a patch for this, if it
>> looks a valid case.
> git commit --amend -C HEAD
$ git commit --amend -C HEAD
works fine but will keep the authorship (name _and_ date). To change the
date to the current timestamp, use
$ git commit --amend -C HEAD --reset-author
Note that this will also change the author's name to yours, so it
depends on your case. The commiter's name and timestamp are always
updated to "you/now", independently of that option. To change only the
author's date, use --date=<date>.
Cheers,
Dirk
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 19:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8AmwQxcKz9vhtvFJPPKpXeipufD_0OqoMv41SHQFZgqeQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> I thought this would be replaced by a new version [1] I posted a while
> ago (and forgot to address comments). Do you want me to rebase that on
> top of this or replace it?
I think what was merged to 'master' is already sane. If you have updates,
treat them just like other new topics.
Thanks.
^ permalink raw reply
* BUG: Confusing submodule error message
From: Seth Robertson @ 2011-12-06 19:30 UTC (permalink / raw)
To: git
Someone on #git just encountered a problem where `git init && git add . &&
git status` was failing with a message about a corrupted index.
error: bad index file sha1 signature
fatal: index file corrupt
fatal: git status --porcelain failed
This confused everyone for a while, until he provided access to the
directory to play with. I eventually tracked it down to a directory
in the tree which already had a .git directory in it. Unfortunately,
that .git repo was corrupted and was the one returning the message
about a corrupted index. The problem is that the error message we
were seeing did not provide any direct hints that submodules were
involved or that the problem was not at the top level (`git status
--porcelain` is admittedly an indirect hint to both). Here is a
recipe to reproduce a similar problem:
(mkdir -p z/foo; cd z/foo; git init; echo A>A; git add A; git commit -m A; cd ..; echo B>B; rm -f foo/.git/objects/*/*; git init; git add .; git status)
Providing an expanded error message which clarifies that this is
failing in a submodule directory makes everything clear.
----------------------------------------------------------------------
--- submodule.c~ 2011-12-02 14:25:08.000000000 -0500
+++ submodule.c 2011-12-06 14:13:00.554413432 -0500
@@ -714,7 +714,7 @@
close(cp.out);
if (finish_command(&cp))
- die("git status --porcelain failed");
+ die("git status --porcelain failed in submodule directory %s", path);
strbuf_release(&buf);
return dirty_submodule;
----------------------------------------------------------------------
Do more error messages in submodule.c need adjusting? It seems likely.
-Seth Robertson
^ permalink raw reply
* Re: Query on git commit amend
From: Junio C Hamano @ 2011-12-06 20:03 UTC (permalink / raw)
To: Vijay Lakshminarayanan; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <87fwgxwvn9.fsf@gmail.com>
Vijay Lakshminarayanan <laksvij@gmail.com> writes:
> I've found
>
> $ GIT_EDITOR=cat git commit --amend
>
> useful.
Are you sure it is a cat?
I almost always use
$ EDITOR=: git commit --amend
when rewriting the contents without updating the message, but I think
we should allow people to say:
$ git commit --amend --no-edit
which is accepted from the command line but is not honoured.
^ permalink raw reply
* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Thomas Rast @ 2011-12-06 20:17 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111206190217.GD9492@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:
>
> > The cpp pattern, used for C and C++, would not match the start of a
> > declaration such as
> >
> > static char *prepare_index(int argc,
> >
> > because it did not allow for * anywhere between the various words that
> > constitute the modifiers, type and function name. Fix it.
> >
> > Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> > ---
> >
> > This is a really sneaky one-character bug that I cannot believe went
> > unnoticed for so long, seeing as there are plenty of instances within
> > git itself where it matters.
>
> Looks reasonable to me. You can see the difference, for instance, with:
>
> git show -U1 3c73a1d
>
> (The -U1 is because of the annoying "we will start looking for the
> header at the top of context, not the top of changes" behavior I
> mentioned last week).
Actually (sadly) I'll have to revise it. It doesn't match much of C++
either, and I haven't yet come up with a reasonable regex that
matches, say,
foo::Bar<int>::t& Baz::operator<<(
which I would call ludicrous, but it's valid C++.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Jeff King @ 2011-12-06 20:19 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, git
In-Reply-To: <201112062117.57690.trast@student.ethz.ch>
On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:
> > Looks reasonable to me. You can see the difference, for instance, with:
> >
> > git show -U1 3c73a1d
> >
> > (The -U1 is because of the annoying "we will start looking for the
> > header at the top of context, not the top of changes" behavior I
> > mentioned last week).
>
> Actually (sadly) I'll have to revise it. It doesn't match much of C++
> either, and I haven't yet come up with a reasonable regex that
> matches, say,
>
> foo::Bar<int>::t& Baz::operator<<(
>
> which I would call ludicrous, but it's valid C++.
Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
worrying about advanced C++ stuff on top as another patch. AFAICT, your
original patch is a strict improvement.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Ramkumar Ramachandra @ 2011-12-06 20:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vliqph8a6.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano wrote:
> A few comments and thoughts, all minor.
>
> * On "revert: simplify getting commit subject in format_todo()"
>
> The old code used to use get_message() on cur->item, checked its return
> value (if cur->item is not parsed, or has already been used and its buffer
> cleared, cur->item->buffer would be NULL and get_message() returns -1) and
> issued an error. The new code uses find_commit_subject without first
> checking if cur->item->buffer is NULL.
>
> Does this mean the old code was overly defensive, or is the new code too
> lax?
>
> I understand that parse_insn_line() uses lookup_commit_reference() which
> calls parse_object() on the object name (and if it is a tag, deref_tag()
> will parse the tagged object until we see something that is not a tag), so
> we know cur->item is parsed before we see it, so I suspect you merely were
> being overly defensive, but I may be missing something.
Right. Actually, I was being overly defensive because I was being
lazy about having to deal the an empty-commit-subject case in the
parser. With "revert: make commit subjects in insn sheet optional",
that's no more a concern- I'll reorder these two patches, and explain
this detail in the commit message in the re-roll.
> * On "revert: make commit subjects in insn sheet optional"
>
> After finding the verb and advancing the pointer "bol" at the beginning of
> the object name, end_of_object_name variable points at the first SP or LF
> using strcspn(bol, " \n"), but I wonder why we are not grabbing a run of
> hexdigits instead, i.e. strspn(bol, "0123456789abcdef"). Is this so that
> we can allow something like "pick rr/revert-cherry-pick~3"?
Yes, it is exactly for that reason :)
> I also wonder if this should be (sorry for pcre) "(pick|revert)\s+(\S+)\s"
> instead, i.e. allow users with fat fingers to use one or more SP or even HT
> to separate the verb and the operand.
Hm, I'm not too enthusiastic about this change, because we don't
advertise hand-editing the instruction sheet yet: it's just some extra
parsing cruft along with guardian buffer-overflow code that buys us no
immediate benefits.
> The last test you added to 3510 in this patch runs test_line_count
> unconditionally, by the way.
Good catch. Missing the chaining '&&' seems to be quite a common
error: I vaguely remember seeing a patch to detect this sometime ago.
Jonathan?
> * On "revert: allow mixed pick and revert instructions"
>
> Reporting what we did not understand from parse_insn_line() is a good
> idea, but I think the line number should be reported at the beginning
> of the same line.
Makes sense. Do you like this?
diff --git a/builtin/revert.c b/builtin/revert.c
index e447449..cfa770e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -715,7 +715,8 @@ static int format_todo(struct strbuf *buf, struct
replay_insn_list *todo_list)
return 0;
}
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+static int parse_insn_line(char *bol, char *eol,
+ struct replay_insn_list *item, int lineno)
{
unsigned char commit_sha1[20];
char *end_of_object_name;
@@ -731,7 +732,8 @@ static int parse_insn_line(char *bol, char *eol,
struct replay_insn_list *item)
size_t len = strchrnul(bol, '\n') - bol;
if (len > 255)
len = 255;
- return error(_("Unrecognized action: %.*s"), (int)len, bol);
+ return error(_("Line %d: Unrecognized action: %.*s"),
+ lineno, (int)len, bol);
}
end_of_object_name = bol + strcspn(bol, " \n");
@@ -741,11 +743,11 @@ static int parse_insn_line(char *bol, char *eol,
struct replay_insn_list *item)
*end_of_object_name = saved;
if (status < 0)
- return error(_("Malformed object name: %s"), bol);
+ return error(_("Line %d: Malformed object name: %s"), lineno, bol);
item->operand = lookup_commit_reference(commit_sha1);
if (!item->operand)
- return error(_("Not a valid commit: %s"), bol);
+ return error(_("Line %d: Not a valid commit: %s"), lineno, bol);
item->next = NULL;
return 0;
@@ -760,8 +762,8 @@ static int parse_insn_buffer(char *buf, struct
replay_insn_list **todo_list)
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item) < 0)
- return error(_("on line %d."), i);
+ if (parse_insn_line(p, eol, &item, lineno) < 0)
+ return -1;
next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
--
> I'd say that I'd prefer queuing re-rolled patches posted on the list; I'll
> discard the rr/revert-cherry-pick (6a156fd) from my tree.
Thanks for letting me know -- will post shortly.
-- Ram
^ permalink raw reply related
* ANNOUNCE: Git for Windows 1.7.8
From: Pat Thoyts @ 2011-12-06 20:32 UTC (permalink / raw)
To: msysGit, Git Mailing List
This release brings the latest release of Git to Windows users.
Pre-built installers are available from
http://code.google.com/p/msysgit/downloads/list
^ permalink raw reply
* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Johannes Sixt @ 2011-12-06 20:52 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, Junio C Hamano, git
In-Reply-To: <20111206201944.GB27930@sigill.intra.peff.net>
Am 06.12.2011 21:19, schrieb Jeff King:
> On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:
>
>>> Looks reasonable to me. You can see the difference, for instance, with:
>>>
>>> git show -U1 3c73a1d
>>>
>>> (The -U1 is because of the annoying "we will start looking for the
>>> header at the top of context, not the top of changes" behavior I
>>> mentioned last week).
>>
>> Actually (sadly) I'll have to revise it. It doesn't match much of C++
>> either, and I haven't yet come up with a reasonable regex that
>> matches, say,
>>
>> foo::Bar<int>::t& Baz::operator<<(
>>
>> which I would call ludicrous, but it's valid C++.
>
> Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
> worrying about advanced C++ stuff on top as another patch. AFAICT, your
> original patch is a strict improvement.
Excuse me, where's the problem? The above example shows this
@@ -105,8 +105,8 @@ char *url_decode(const char *url)
struct strbuf out = STRBUF_INIT;
- const char *slash = strchr(url, '/');
+ const char *colon = strchr(url, ':');
...
with current 4cb5d10b. This looks quite correct, no?
-- Hannes
^ permalink raw reply
* Re: Query on git commit amend
From: Junio C Hamano @ 2011-12-06 21:00 UTC (permalink / raw)
To: git; +Cc: Vijay Lakshminarayanan, Viresh Kumar, Shiraz HASHIM
In-Reply-To: <7vobvlfowk.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I almost always use
>
> $ EDITOR=: git commit --amend
>
> when rewriting the contents without updating the message, but I think
> we should allow people to say:
>
> $ git commit --amend --no-edit
>
> which is accepted from the command line but is not honoured.
And this should fix it (only lightly tested).
-- >8 --
Subject: [PATCH] commit: honor --no-edit
After making fixes to the contents to be committed, it is not unusual to
update the current commit without rewording the message. Idioms to do
tell "commit --amend" that we do not need an editor have been:
$ EDITOR=: git commit --amend
$ git commit --amend -C HEAD
but that was only because a more natural
$ git commit --amend --no-edit
did not honour "--no-edit" option.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/commit.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 8f2bebe..48bea8f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -81,7 +81,8 @@ static const char *template_file;
static const char *author_message, *author_message_buffer;
static char *edit_message, *use_message;
static char *fixup_message, *squash_message;
-static int all, edit_flag, also, interactive, patch_interactive, only, amend, signoff;
+static int all, also, interactive, patch_interactive, only, amend, signoff;
+static int edit_flag = -1; /* unspecified */
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
@@ -141,7 +142,7 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
- OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
+ OPT_BOOL('e', "edit", &edit_flag, "force edit of commit"),
OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
/* end commit message options */
@@ -1020,8 +1021,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (logfile || message.len || use_message || fixup_message)
use_editor = 0;
- if (edit_flag)
- use_editor = 1;
+ if (0 <= edit_flag)
+ use_editor = edit_flag;
if (!use_editor)
setenv("GIT_EDITOR", ":", 1);
--
1.7.8.157.g03e55
^ permalink raw reply related
* Re: ANNOUNCE: Git for Windows 1.7.8
From: Erik Faye-Lund @ 2011-12-06 21:03 UTC (permalink / raw)
To: Pat Thoyts; +Cc: msysGit, Git Mailing List
In-Reply-To: <CABNJ2GJ_1Nf9rWev6BKE9zcqt5yrgq6PbaMLVRD705UapLHf0w@mail.gmail.com>
On Tue, Dec 6, 2011 at 9:32 PM, Pat Thoyts <patthoyts@gmail.com> wrote:
> This release brings the latest release of Git to Windows users.
>
> Pre-built installers are available from
> http://code.google.com/p/msysgit/downloads/list
Great work, thanks a lot!
^ permalink raw reply
* Re: [PATCH] userdiff: allow * between cpp funcname words
From: René Scharfe @ 2011-12-06 21:07 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Thomas Rast, Junio C Hamano, git
In-Reply-To: <4EDE8086.9080303@kdbg.org>
Am 06.12.2011 21:52, schrieb Johannes Sixt:
> Am 06.12.2011 21:19, schrieb Jeff King:
>> On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:
>>
>>>> Looks reasonable to me. You can see the difference, for instance, with:
>>>>
>>>> git show -U1 3c73a1d
>>>>
>>>> (The -U1 is because of the annoying "we will start looking for the
>>>> header at the top of context, not the top of changes" behavior I
>>>> mentioned last week).
>>>
>>> Actually (sadly) I'll have to revise it. It doesn't match much of C++
>>> either, and I haven't yet come up with a reasonable regex that
>>> matches, say,
>>>
>>> foo::Bar<int>::t& Baz::operator<<(
>>>
>>> which I would call ludicrous, but it's valid C++.
>>
>> Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
>> worrying about advanced C++ stuff on top as another patch. AFAICT, your
>> original patch is a strict improvement.
>
> Excuse me, where's the problem? The above example shows this
>
> @@ -105,8 +105,8 @@ char *url_decode(const char *url)
> struct strbuf out = STRBUF_INIT;
> - const char *slash = strchr(url, '/');
> + const char *colon = strchr(url, ':');
> ...
>
> with current 4cb5d10b. This looks quite correct, no?
That's with the default heuristic; try something like this first to turn
on userdiff:
$ echo url.c diff=cpp >>.gitattributes
René
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Luke Diamand @ 2011-12-06 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vmqi98f.fsf@alter.siamese.dyndns.org>
On 06/12/11 05:01, Junio C Hamano wrote:
> Here are the topics that have been cooking. Commits prefixed with '-' are
> only in 'pu' (proposed updates) while commits prefixed with '+' are in
> 'next'.
>
>
> Will merge to 'next'.
>
> * ld/p4-labels-branches (2011-11-30) 4 commits
> - git-p4: importing labels should cope with missing owner
> - git-p4: add test for p4 labels
> - git-p4: cope with labels with empty descriptions
> - git-p4: handle p4 branches and labels containing shell chars
>
> I understand this has been retracted---please correct me otherwise.
> Will discard, expecting a reroll.
Yes, discard this one and I'll re-roll it with fixes for the other label
issues.
Thanks
Luke
^ 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