* [PATCH] t2203: fix wrong commit command
From: Nguyễn Thái Ngọc Duy @ 2012-01-11 3:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
Add commit message to avoid commit's aborting due to the lack of
commit message, not because there are INTENT_TO_ADD entries in index.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t2203-add-intent.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 58a3299..2543529 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,7 @@ test_expect_success 'cannot commit with i-t-a entry' '
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- test_must_fail git commit
+ test_must_fail git commit -m initial
'
test_expect_success 'can commit with an unrelated i-t-a entry in index' '
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* Re: leaky cherry-pick
From: Ramkumar Ramachandra @ 2012-01-11 3:30 UTC (permalink / raw)
To: Jeff King
Cc: Pete Wyckoff, git, Junio C Hamano,
Nguyễn Thái Ngọc
In-Reply-To: <20120110195017.GA19961@sigill.intra.peff.net>
Hi Jeff,
Jeff King wrote:
> On Tue, Jan 10, 2012 at 10:58:45AM +0530, Ramkumar Ramachandra wrote:
>> diff --git a/attr.c b/attr.c
>> index 76b079f..12e3824 100644
>> --- a/attr.c
>> +++ b/attr.c
>> @@ -745,6 +745,7 @@ int git_check_attr(const char *path, int num,
>> struct git_attr_check *check)
>> check[i].value = value;
>> }
>>
>> + drop_attr_stack();
>> return 0;
>> }
>
> I don't think this is right.
Yeah, I figured it wasn't right by running the testsuite. I was
struggling to figure out why.
> The attr_stack is intentionally kept in
> place after a lookup as a cache, because callers are very likely to
> lookup nearby filenames. The first thing we do is pop unrelated parts of
> the stack, keep the early bits, and then push any new needed
> directories.
>
> So if you do a lookup for "foo/bar/baz/file1", the stack afterwards would
> be:
>
> $GIT_DIR/info/attributes
> foo/bar/baz/.gitattributes
> foo/bar/.gitattributes
> foo/.gitattributes
> .gitattributes
> [builtins]
>
> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
> same stack without looking for or reparsing the attribute files. If you
> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
> "foo/bar/baz/.gitattributes", and pushes only the entry for
> "foo/bar/bleep/.gitattributes".
I see. Thanks for the excellent explanation- I'll try implementing
this scheme.
-- Ram
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11 4:02 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120110183857.GC22184@burratino>
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> [...]
>> -static const char *action_name(const struct replay_opts *opts)
>> +static const char *command_name(struct replay_opts *opts)
>
> This part does a similar renaming, and drops a const while at it for
> no intelligible reason.
Carried over from the previous iteration- sorry I forgot to fix this.
> [...]
>> @@ -142,7 +147,7 @@ static void verify_opt_mutually_compatible(const char *me, ...)
>> static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>> {
>> const char * const * usage_str = revert_or_cherry_pick_usage(opts);
>> - const char *me = action_name(opts);
>> + const char *me = command_name(opts);
>
> The rest is stuff like this, which follows from the first part.
>
> Stepping back, I think the idea is that "enum replay_action" is not a
> good way to identify the command name in error messages like
>
> fatal: cherry-pick: --abort cannot be used with --continue
>
> So you introduce a _new_ enum to represent the command name. Why not
> just use a string, so commands using the nice and generic sequencer
> library do not have to register themselves in a global callers list to
> use it?
Fine; I'm sold on the string idea. Also, I figured it would be
easier to explain the changes if I change this enum to a string -- I
should probably use "ease of explaining changes" as a stronger
criterion in the future when I have two competing implementations in
mind.
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11 4:17 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0k=44znLr2oYSx61Mk=qdAurona0f0H4i4=YXNSAeQhHQ@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Fine; I'm sold on the string idea. Also, I figured it would be
> easier to explain the changes if I change this enum to a string -- I
> should probably use "ease of explaining changes" as a stronger
> criterion in the future when I have two competing implementations in
> mind.
I wrote that too quickly. I can't stand seeing so many strcmp() calls
all over my codebase -- look at the number of instances of matching
opts->command to REPLAY_CMD_*. Why should I have to use strcmp() when
the data is semantic? It makes no sense: by spelling out the string
in so many places, I'm just making the code more error-prone, because
the compiler can't warn me if I make a spelling mistake in one place.
Why do you like the string so much? A new caller will have to
register new actions in the replay_actions enum and modify the
codebase to define a codepath for its specific case anyway: so I don't
mind it registering a new command in replay_command.
-- Ram
^ permalink raw reply
* Re: [PATCH] t2203: fix wrong commit command
From: Ramkumar Ramachandra @ 2012-01-11 4:21 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1326252098-2891-1-git-send-email-pclouds@gmail.com>
Hi Duy,
Nguyễn Thái Ngọc Duy wrote:
> Add commit message to avoid commit's aborting due to the lack of
> commit message, not because there are INTENT_TO_ADD entries in index.
Is there any way to differentiate between the two kinds of errors, so
that we can avoid this in the future? Can we grep the error message
for something, or inspect the exit status?
-- Ram
^ permalink raw reply
* Re: git svn clone terminating prematurely (I think)
From: Ramkumar Ramachandra @ 2012-01-11 4:34 UTC (permalink / raw)
To: Steven Line; +Cc: git
In-Reply-To: <CAJ1a7SrkDOyNRv8Spo4xvoKjP4zaXteim4h3JGcWU-nYDugx9Q@mail.gmail.com>
Hi Steven,
Steven Line wrote:
> First off I am a new user to git, I'm not a git developer or power
> user. Am I in the right mailing list? If not could somebody point me
> where I could get some help from experienced git people?
This is the right place. The Git community believes in maintaining
just one mailing list.
> I need some help getting my subversion repository cloned over to git.
> Our svn repository has about 12,000 commits, when I run
> git svn clone -s -A authors.txt
> svn+ssh://csvn <at> source.res.ourdomain.com/home/svn/sem sem
> It runs for about 2h 15m then completes with no error messages. I have
> also cloned starting at revision 6300, about the middle of the svn
> repository, and I get the same results as below.
> $ git branch -a # shows only about half the branches that should have
> been cloned
Interesting. From the git-svn-id of the most recent commit, can you
tell if there's anything especially fishy about the revision where
git-svn stops? Your Subversion repository is probably broken in some
way, but git-svn should not use that as an excuse for appearing to
finish successfully while failing in reality.
Cheers.
-- Ram
^ permalink raw reply
* Re: [BUG] gitattribute macro expansion oddity
From: Michael Haggerty @ 2012-01-11 4:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Henrik Grubbström, git-dev
In-Reply-To: <7v1ur7bhhe.fsf@alter.siamese.dyndns.org>
On 01/10/2012 06:22 PM, Junio C Hamano wrote:
> Regardless of this unrelated regression, after looking at what ec775c4
> wanted to do again, I am very much tempted to just revert it.
>
> It aimed to take these three
>
> * ident
> foo mybin
> bar mybin ident
>
> and wanted to omit 'ident' from "foo" when there is this macro definition
> elsewhere:
>
> [attr] mybin binary -ident
>
> But the real point of the macro was that the users do not have to know
> their internals, iow, if you explicitly specify a pattern that overrides
> the contents of the macro, that explicit pattern should win. When deciding
> the value of "ident" attribute for path "foo", "* ident" is stronger than
> "foo mybin" (the latter of which does not say anything about 'ident'
> explicitly).
I like the simplicity of the rule "apply attributes in the order found
in the .gitattributes files" better than the rule you are proposing,
which seems like it will become more complicated to explain.
For example, it would seem under your rule for the above example that
the "mybin" macro should bestow on file foo the "binary" attribute and
also the "mybin" attribute (given that macros are themselves
attributes), but not "-ident".
You would also have to decide and explain whether a macro that invokes a
macro that sets or clears attribute "foo" is "weaker" than a simple
macro that clears or sets attribute "foo".
I have one real-life use case that would become more difficult with your
rule:
# Marker for textlike files whose EOL characters haven't been
# normalized yet:
[attr]eol-fixme -text !eol
*.cc text eol=lf
# Then later, perhaps in some subdirectory's .gitattributes:
SomeParticularScrewedUpFile.cc eol-fixme
The point of the eol-fixme macro is (1) to prevent git from throwing a
tantrum and (2) to mark the file as needing cleanup sometime in the future.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] t2203: fix wrong commit command
From: Nguyen Thai Ngoc Duy @ 2012-01-11 4:43 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Junio C Hamano
In-Reply-To: <CALkWK0kDBFvssyX2ZPJ9WNzfNXD=wEJoXTRVpFWm1TDKJrvNzA@mail.gmail.com>
2012/1/11 Ramkumar Ramachandra <artagnon@gmail.com>:
> Hi Duy,
>
> Nguyễn Thái Ngọc Duy wrote:
>> Add commit message to avoid commit's aborting due to the lack of
>> commit message, not because there are INTENT_TO_ADD entries in index.
>
> Is there any way to differentiate between the two kinds of errors, so
> that we can avoid this in the future? Can we grep the error message
> for something, or inspect the exit status?
commit exits with 1 in both cases, so no.
--
Duy
^ permalink raw reply
* Re: [PATCH] t2203: fix wrong commit command
From: Nguyen Thai Ngoc Duy @ 2012-01-11 4:43 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8CjWBRtj3x3guyu-iJeTYxcRriXKbE=Be-rG=Rmesgt=g@mail.gmail.com>
On Wed, Jan 11, 2012 at 11:43 AM, Nguyen Thai Ngoc Duy
<pclouds@gmail.com> wrote:
> 2012/1/11 Ramkumar Ramachandra <artagnon@gmail.com>:
>> Hi Duy,
>>
>> Nguyễn Thái Ngọc Duy wrote:
>>> Add commit message to avoid commit's aborting due to the lack of
>>> commit message, not because there are INTENT_TO_ADD entries in index.
>>
>> Is there any way to differentiate between the two kinds of errors, so
>> that we can avoid this in the future? Can we grep the error message
>> for something, or inspect the exit status?
>
> commit exits with 1 in both cases, so no.
but grepping error message should work, sorry I hit send too fast
--
Duy
^ permalink raw reply
* Re: [PATCH] Fix incorrect ref namespace check
From: Michael Haggerty @ 2012-01-11 4:49 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8Af=PjXE4sHqSk1v8qoraivOnT2w495W1c1dVQBW1s-cQ@mail.gmail.com>
On 01/11/2012 02:54 AM, Nguyen Thai Ngoc Duy wrote:
> Any comments Mike?
Looks fine to me.
Theoretically, we could add an API for iterating over references
*excluding* a particular namespace, which (after hierarchical refs is
accepted) could be implemented a bit more efficiently than iterating
over all references and then excluding those in the unwanted namespace.
But I think it would be far more trouble than it's worth.
Michael
> 2012/1/5 Junio C Hamano <gitster@pobox.com>:
>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>>
>>> The reason why the trailing slash is needed is obvious. refs/stash and
>>> HEAD are not namespace, but complete refs. Do full string compare on them.
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>> I missed prefixcmp(..., "HEAD") right below prefixcmp(..., "refs/stash")
>>
>> As Michael has been actively showing interest in cleaning up the area, he
>> should have been CC'ed, I would think.
>>
>>>
>>> builtin/fetch.c | 2 +-
>>> builtin/remote.c | 2 +-
>>> log-tree.c | 4 ++--
>>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>>> index 33ad3aa..daa68d2 100644
>>> --- a/builtin/fetch.c
>>> +++ b/builtin/fetch.c
>>> @@ -573,7 +573,7 @@ static void find_non_local_tags(struct transport *transport,
>>>
>>> for_each_ref(add_existing, &existing_refs);
>>> for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
>>> - if (prefixcmp(ref->name, "refs/tags"))
>>> + if (prefixcmp(ref->name, "refs/tags/"))
>>> continue;
>>>
>>> /*
>>> diff --git a/builtin/remote.c b/builtin/remote.c
>>> index 583eec9..f54a89a 100644
>>> --- a/builtin/remote.c
>>> +++ b/builtin/remote.c
>>> @@ -534,7 +534,7 @@ static int add_branch_for_removal(const char *refname,
>>> }
>>>
>>> /* don't delete non-remote-tracking refs */
>>> - if (prefixcmp(refname, "refs/remotes")) {
>>> + if (prefixcmp(refname, "refs/remotes/")) {
>>> /* advise user how to delete local branches */
>>> if (!prefixcmp(refname, "refs/heads/"))
>>> string_list_append(branches->skipped,
>>> diff --git a/log-tree.c b/log-tree.c
>>> index 319bd31..535b905 100644
>>> --- a/log-tree.c
>>> +++ b/log-tree.c
>>> @@ -119,9 +119,9 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
>>> type = DECORATION_REF_REMOTE;
>>> else if (!prefixcmp(refname, "refs/tags/"))
>>> type = DECORATION_REF_TAG;
>>> - else if (!prefixcmp(refname, "refs/stash"))
>>> + else if (!strcmp(refname, "refs/stash"))
>>> type = DECORATION_REF_STASH;
>>> - else if (!prefixcmp(refname, "HEAD"))
>>> + else if (!strcmp(refname, "HEAD"))
>>> type = DECORATION_REF_HEAD;
>>>
>>> if (!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS)
>
>
>
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-11 5:04 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=bEPPv4rtPrMrQnk3MK=JY4-wwAByWPmzg86NBm_56iQ@mail.gmail.com>
Ramkumar Ramachandra wrote:
> I wrote that too quickly. I can't stand seeing so many strcmp() calls
> all over my codebase -- look at the number of instances of matching
> opts->command to REPLAY_CMD_*.
If the number of such in sequencer.c is greater than 0, it's probably
a bug. Why would the sequencer change behavior based on its caller's
name?
It's very easy for me to be wrong, though --- this is just a first
reaction, and I haven't looked at this version of sequencer.c yet,
since it doesn't exist. :)
Is this what you're talking about?
if (opts->action == CHERRY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
Now imagine "git rebase" wants to hook into the same stuff. What
should the message be?
Maybe:
error(_("your local changes would be overwritten - aborting"));
Or:
error(_(opts->error_msgs[REPLAY_WOULD_CLOBBER_LOCAL_CHANGES]));
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11 5:14 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120111050404.GA13507@burratino>
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> I wrote that too quickly. I can't stand seeing so many strcmp() calls
>> all over my codebase -- look at the number of instances of matching
>> opts->command to REPLAY_CMD_*.
>
> If the number of such in sequencer.c is greater than 0, it's probably
> a bug. Why would the sequencer change behavior based on its caller's
> name?
In the super-final version, yes- I agree. However, we're not there
yet -- this patch is more of a transition, to make the life of
"revert: allow mixing "pick" and "revert" actions" easier. Once the
painful move to sequencer.c is completed, we can think about all these
things. Right now, I'm only focusing on the move, and everything that
should logically precede it (in my opinion).
> Is this what you're talking about?
>
> if (opts->action == CHERRY_PICK)
> error(_("Your local changes would be overwritten by cherry-pick."));
> else
> error(_("Your local changes would be overwritten by revert."));
1. Yes, error messages like this.
2. Since I haven't modified function prototypes to include a
"replay_action" at the "revert: decouple sequencer actions from
builtin commands" stage, functions like do_pick_commit() have to rely
on opts->command! That's the entire point: I want to show how this
"opts->command" is changing to "action" in most places in "revert:
allow mixing "pick" and "revert" actions".
I'd rationalize that taking care of (1) is not urgent- we can do it
after we move to sequencer.c. As for (2), we have little choice at
this point- either make it a string like it's supposed to be in the
super-final version, and struggle with the ugliness of strcmp() now,
or make it an enum now; I choose the latter.
Thanks for making me explain this.
-- Ram
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11 5:26 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0kJpEXvBMV=D7h91sz7U2sLvXdW1UzomW0kG2bbM+byYA@mail.gmail.com>
Hi Jonathan,
I wrote a new commit message for this patch. Perhaps it can help clarify?
revert: decouple sequencer actions from commands
Currently, we have two actions "pick" and "revert" that directly
correspond to the action of the commands 'git cherry-pick' and 'git
revert' respectively. Since we would like to permit mixing different
actions in the same instruction sheet inspired by the way 'git rebase
--interactive' works, a future instruction sheet would look like:
pickle next~4
revert rr/moo^2~34
action3 b74fea
where the actions "pickle", "revert" and "action3" need not directly
correspond to the specific commands. So, instead of using the same
representation for both the command executed and the sequencer action
(the "replay_action" enum), store the command separately as a
"replay_command" enum.
Since we don't allow mixing actions yet, the entire operation of 'git
cherry-pick'/ 'git revert' relies on the command, with the
exception of the instruction sheet parser which converts a
"replay_action" to a "replay_command". As we support mixing actions
in future patches, more portions of the code will rely on the
action, demoting the command to a string used in error reporting.
-- Ram
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-11 5:49 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0muXXKu37_qQ8E+LEZiCxebWvWghkc8QjyfdBazjLOstw@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Hi Jonathan,
>
> I wrote a new commit message for this patch. Perhaps it can help clarify?
Thanks.
> revert: decouple sequencer actions from commands
>
> Currently, we have two actions "pick" and "revert" that directly
> correspond to the action of the commands 'git cherry-pick' and 'git
> revert' respectively.
Well, let's start here. The two insns "pick" and "revert" and the
ability to mix them doesn't have much to do with the picture, does it?
I think the actual problem being solved is that insn types, as described
by the replay_action enum, are being abused to refer to top-level git
commands "revert" and "cherry-pick". The sequencer isn't supposed to
care which top-level git command called it, except in some messages, so
we'd certainly like to stop pretending that has something to do with
insn types.
Based on what you've said, correcting this cleanly is complicated in
some places by the inconvenient fact that the sequencer _does_ care
which top-level git command called it. (I haven't checked this; I'm
just taking it on faith from you.) If we want to let other git
commands (like "git rebase" or "git sequence") call into the
sequencer, that sounds like a way bigger problem than any conflict of
terminology.
^ permalink raw reply
* [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Nguyễn Thái Ngọc Duy @ 2012-01-11 6:01 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This patch replaces the approach in 331fcb5 (git add --intent-to-add:
do not let an empty blob be committed by accident) regarding i-t-a
entries: instead of forbidding i-t-a entries at commit time, we can
simply ignore them.
We already ignore CE_REMOVE entries while updating cache-tree. Putting
CE_INTENT_TO_ADD ones in the same category should not cause any negative
effects regarding cache-tree.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
On the few chances I have to use "git add -N" it does not fit well
with "git add -p; git diff --cached; git commit -m foo" style. I
think this may be a good thing to do.
builtin/commit.c | 2 +-
builtin/write-tree.c | 2 +-
cache-tree.c | 14 +++++---------
t/t2203-add-intent.sh | 10 +++++++++-
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..767b78a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -871,7 +871,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
discard_cache();
read_cache_from(index_file);
if (update_main_cache_tree(0)) {
- error(_("Error building trees"));
+ error(_("Error building trees; the index is unmerged?"));
return 0;
}
diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index b223af4..68baa24 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -46,7 +46,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
die("%s: error reading the index", me);
break;
case WRITE_TREE_UNMERGED_INDEX:
- die("%s: error building trees", me);
+ die("%s: error building trees; the index is unmerged?", me);
break;
case WRITE_TREE_PREFIX_ERROR:
die("%s: prefix %s not found", me, prefix);
diff --git a/cache-tree.c b/cache-tree.c
index 8de3959..47defd1 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -158,19 +158,15 @@ static int verify_cache(struct cache_entry **cache,
funny = 0;
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
- if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+ if (ce_stage(ce)) {
if (silent)
return -1;
if (10 < ++funny) {
fprintf(stderr, "...\n");
break;
}
- if (ce_stage(ce))
- fprintf(stderr, "%s: unmerged (%s)\n",
- ce->name, sha1_to_hex(ce->sha1));
- else
- fprintf(stderr, "%s: not added yet\n",
- ce->name);
+ fprintf(stderr, "%s: unmerged (%s)\n",
+ ce->name, sha1_to_hex(ce->sha1));
}
}
if (funny)
@@ -338,8 +334,8 @@ static int update_one(struct cache_tree *it,
mode, sha1_to_hex(sha1), entlen+baselen, path);
}
- if (ce->ce_flags & CE_REMOVE)
- continue; /* entry being removed */
+ if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
+ continue; /* entry being removed or just placeholder */
strbuf_grow(&buffer, entlen + 100);
strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2543529..65430e4 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,15 @@ test_expect_success 'cannot commit with i-t-a entry' '
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- test_must_fail git commit -m initial
+ git commit -m initial &&
+ git ls-tree -r HEAD >actual &&
+ cat >expected <<EOF &&
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a elif
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a file
+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4 rezrov
+EOF
+ test_cmp expected actual &&
+ git reset HEAD^
'
test_expect_success 'can commit with an unrelated i-t-a entry in index' '
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery
From: Jonathan Nieder @ 2012-01-11 6:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds, Nguyen Thai Ngoc Duy
In-Reply-To: <1314653603-7533-4-git-send-email-gitster@pobox.com>
Hi,
Junio C Hamano wrote:
> And finally, pass the pathspec down through unpack_trees() to traverse_trees()
> callchain.
>
> Before and after applying this series, looking for changes in the kernel
> repository with a fairly narrow pathspec gets a moderate speeds up.
Check this out:
mkdir -p test-repo/subdir
cd test-repo
git init
echo hi >subdir/hello.h
git add subdir/hello.h
git commit -m 'say hi'
git diff-index --abbrev HEAD -- '*.h'
In git versions which include the patch described above, the unchanged
subdir/hello.h shows up as a newly added file. Reverting that patch
(v1.7.7.1~22^2, diff-index: pass pathspec down to unpack-trees
machinery, 2011-08-29) makes "git diff HEAD" with wildcards work
again.
It looks like the pruning of the preimage by pathspec is too
aggressive and is omiting whole directories that do not match the
pathspec without realizing that the path to a file contained within
might match.
We could just add a test for this to the testsuite and do that revert,
but I'd rather not yet, in case this is a symptom of some deeper
unpack_trees() confusion.
Hints?
Thanks,
Jonathan
> diff-lib.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index f8454dd..ebe751e 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -468,6 +468,7 @@ static int diff_cache(struct rev_info *revs,
> opts.unpack_data = revs;
> opts.src_index = &the_index;
> opts.dst_index = NULL;
> + opts.pathspec = &revs->diffopt.pathspec;
>
> init_tree_desc(&t, tree->buffer, tree->size);
> return unpack_trees(1, &t, &opts);
> --
> 1.7.7.rc0.70.g82660
>
>
^ permalink raw reply
* Re: Intriguing error with Git 1.6.3.1: Too many open files
From: Soham Mehta @ 2012-01-11 6:52 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20120111030931.GA12862@burratino>
On Tue, Jan 10, 2012 at 7:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Soham Mehta wrote:
>
>> *"error: unable to open object pack directory: ./objects/pack: Too many
>> open files"
>
> Do current git versions (v1.7.4.2 or later) behave the same way?
>
> Sorry for the trouble,
> Jonathan
I haven't played with versions yet. Are you alluding that #2 on this
list may have addressed it?
https://raw.github.com/gitster/git/master/Documentation/RelNotes/1.7.4.2.txt
* We used to keep one file descriptor open for each and every packfile
that we have a mmap window on it (read: "in use"), even when for very
tiny packfiles. We now close the file descriptor early when the entire
packfile fits inside one mmap window.
The corrupt repo does have a 1000+ pack files, anywhere from 650M to 2.5K.
^ permalink raw reply
* Re* Regulator updates for 3.3
From: Junio C Hamano @ 2012-01-11 6:59 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <CA+55aFzuGtJkQFDooSGWQ2_NiJVHN2E7S5dmOnWTYn8_s8Gg3g@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> The (b) thing I suggested was for "work around for people who have
> legacy cases that they don't want to make explicit". I guess you could
> count that as some rule, but I really think it's more of a "ok, we had
> bad legacy behavior, and now we have scripts that depended on that bad
> legacy".
I would think that would solve the issue for scripts like the one used to
rebuild the linux-next tree. I also have such a script to rebuild 'pu' I
use three or four times a day, but admittedly the standard input of "git
merge" in that script is connected to a here document that feeds the list
of branches to be merged to the loop that drives the "git merge", so your
heuristic (a) will kick in and I wouldn't need the GIT_MERGE_NO_EDIT
environment myself.
What makes me uneasy about the idea of running the editor by default is
that many people still use Git as a better CVS/SVN. Their workflow is to
build randomly on their 'master', attempt to push and get rejected, pull
only so that they can push out, and then push the merge result out. Such
merges are done without any consideration on the cohesiveness of the
branch (the segment of the history that records their work since they last
pulled from the central repository). Having to justify the backmerge is
nothing but a nuisance for them, as they do not have any justification
better than "I am done for the day, and my commuter shuttle will leave in
15 minutes, so I tried to push what I've done so far, but it was rejected
due to non fast-forward, and I am merging random things others did so that
I can push back". They won't be saying "This merges the great work I
completed and have been testing privately for a few days to the trunk", as
the direction of their merge is backwards.
With that caveat, the patch should look like this.
-- >8 --
Subject: [PATCH] merge: use editor by default in interactive sessions
Traditionally, a cleanly resolved merge was committed by "git merge" using
the auto-generated merge commit log message with invoking the editor.
After 5 years of use in the field, it turns out that many people perform
too many unjustified backmerges of the upstream history into their topic
branches. These merges are not just useless, but they are more often than
not explained and making the end result unreadable when it gets time for
merging their history back to their upstream.
Earlier we added the "--edit" option to the command, so that people can
edit the log message to explain and justify their merge commits. Let's
take it one step further and spawn the editor by default when we are in an
interactive session (i.e. the standard input and the standard output are
pointing at the same tty device).
There may be existing scripts that leave the standard input and the
standard output of the "git merge" connected to whatever environment the
scripts were started, and such invocation might trigger the above
"interactive session" heuristics. Such scripts can export GIT_MERGE_LEGACY
environment variable set to "yes" to force the traditional behaviour.
Suggested-by: Linus Torvalds
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge.c | 34 ++++++++++++++++++++++++++++++----
t/test-lib.sh | 3 ++-
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 99f1429..6a80e1e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {
static int show_diffstat = 1, shortlog_len, squash;
static int option_commit = 1, allow_fast_forward = 1;
-static int fast_forward_only, option_edit;
+static int fast_forward_only, option_edit = -1;
static int allow_trivial = 1, have_message;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
@@ -189,7 +189,7 @@ static struct option builtin_merge_options[] = {
"create a single commit instead of doing a merge"),
OPT_BOOLEAN(0, "commit", &option_commit,
"perform a commit if the merge succeeds (default)"),
- OPT_BOOLEAN('e', "edit", &option_edit,
+ OPT_BOOL('e', "edit", &option_edit,
"edit message before committing"),
OPT_BOOLEAN(0, "ff", &allow_fast_forward,
"allow fast-forward (default)"),
@@ -877,12 +877,12 @@ static void prepare_to_commit(void)
write_merge_msg(&msg);
run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL);
- if (option_edit) {
+ if (0 < option_edit) {
if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
abort_commit(NULL);
}
read_merge_msg(&msg);
- stripspace(&msg, option_edit);
+ stripspace(&msg, 0 < option_edit);
if (!msg.len)
abort_commit(_("Empty commit message."));
strbuf_release(&merge_msg);
@@ -1076,6 +1076,29 @@ static void write_merge_state(void)
close(fd);
}
+static int default_edit_option(void)
+{
+ static const char name[] = "GIT_MERGE_LEGACY";
+ const char *e = getenv(name);
+ struct stat st_stdin, st_stdout;
+
+ if (e) {
+ int v = git_config_maybe_bool(name, e);
+ if (v < 0)
+ die("Bad value '%s' in environment '%s'", e, name);
+ return !v;
+ }
+
+ /* Use editor if stdin and stdout are the same and is a tty */
+ return (!fstat(0, &st_stdin) &&
+ !fstat(1, &st_stdout) &&
+ isatty(0) &&
+ st_stdin.st_dev == st_stdout.st_dev &&
+ st_stdin.st_ino == st_stdout.st_ino &&
+ st_stdin.st_rdev == st_stdout.st_rdev);
+}
+
+
int cmd_merge(int argc, const char **argv, const char *prefix)
{
unsigned char result_tree[20];
@@ -1261,6 +1284,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
}
+ if (option_edit < 0)
+ option_edit = default_edit_option();
+
if (!use_strategies) {
if (!remoteheads->next)
add_strategies(pull_twohead, DEFAULT_TWOHEAD);
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..439f192 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -63,7 +63,8 @@ GIT_AUTHOR_NAME='A U Thor'
GIT_COMMITTER_EMAIL=committer@example.com
GIT_COMMITTER_NAME='C O Mitter'
GIT_MERGE_VERBOSITY=5
-export GIT_MERGE_VERBOSITY
+GIT_MERGE_LEGACY=yes
+export GIT_MERGE_VERBOSITY GIT_MERGE_LEGACY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
export EDITOR
--
1.7.9.rc0.39.ged86b
^ permalink raw reply related
* Re: Intriguing error with Git 1.6.3.1: Too many open files
From: Jonathan Nieder @ 2012-01-11 7:18 UTC (permalink / raw)
To: Soham Mehta; +Cc: git
In-Reply-To: <CALjyegVzchN==DEsARD65jEmz0BkkbYMQRhG=ujNiGCBHuB4SQ@mail.gmail.com>
Soham Mehta wrote:
> I haven't played with versions yet.
For almost all purposes, the latest stable version should work fine
(v1.7.8.3 at the moment). The people on this list tend to be very
keen to hear about cases where that's not true --- if people are stuck
using old versions because of regressions, that means we're failing.
> Are you alluding that #2 on this
> list may have addressed it?
> https://raw.github.com/gitster/git/master/Documentation/RelNotes/1.7.4.2.txt
Not exactly. I was thinking of commit c7934306 (Limit file
descriptors used by packs, 2011-02-28).
In git time, 1.6.3.1 is ancient history. There are many little fixes
and performance improvements you are missing out on.
Just my two cents,
Jonathan
^ permalink raw reply
* Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery
From: Junio C Hamano @ 2012-01-11 8:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Linus Torvalds, Nguyen Thai Ngoc Duy
In-Reply-To: <20120111063104.GA3153@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>
>> And finally, pass the pathspec down through unpack_trees() to traverse_trees()
>> callchain.
>
> In git versions which include the patch described above, the unchanged
> subdir/hello.h shows up as a newly added file. Reverting that patch
> (v1.7.7.1~22^2, diff-index: pass pathspec down to unpack-trees
> machinery, 2011-08-29) makes "git diff HEAD" with wildcards work
> again.
I suspect that the particular change on the side branch predates Nguyen's
effort to unify the pathspec semantics to teach the wildcard (i.e. not the
traditional "prefix match") to the tree traversal code, but it is fairly
late here, so I didn't check.
I think the right fix is to update the logic that still assumes that a
pathspec used for tree traversal is always prefix match when leaving the
traversal early, and instead use the proper matching logic that knows that
a wildcard pathspec needs to dig deeper into the tree regardless (I think
the pathspec implementation used in "git grep" got this right, but please
double check), so that we do not dig unnecessary subtrees when pathspecs
are all prefixes, but still keep digging when there is a wildcard match.
I suspect that it would lead to a fairly complete unification of the three
implementations of pathspec matching logic and would allow us to also do
something like "git log -- '*.h'" for free.
^ permalink raw reply
* Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Junio C Hamano @ 2012-01-11 8:08 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326261707-11484-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This patch replaces the approach in 331fcb5 (git add --intent-to-add:
> do not let an empty blob be committed by accident) regarding i-t-a
> entries: instead of forbidding i-t-a entries at commit time, we can
> simply ignore them.
I have a mild suspicion that in earlier incarnation of the patch we used
to let empty blobs committed, and then we used to instead not commit
anything at all for such a path, and the real users were bitten by either
of these approaches, forgetting to add the contents to the final commit.
So I am not sure if this is such a good idea.
^ permalink raw reply
* Re: rsync a *bunch* of git repos
From: Andreas T.Auer @ 2012-01-11 8:01 UTC (permalink / raw)
To: Jason; +Cc: git
In-Reply-To: <20120110211548.GD10255@titan.lakedaemon.net>
On 10.01.2012 22:15 Jason wrote:
> The nuts and bolts of this aren't difficult, the problem is I don't have
> a complete understanding of how git stores data. I've heard in the
> past that it uses a lot of hardlinks and softlinks. I need to make
> sure, that once I transfer the data, and reboot the machine with the new
> partition mounted under /home, that all my git repos will be okay.
>
>
As far as I know git uses hardlinks to save diskspace, but it doesn't
rely on hardlinks to work. It also works with filesystems that don't
support hardlinks.
But without hardlinks you will probably waste disk space so I recommend
the -H parameter to rsync, which preserve hardlinks, if possible. If you
have hundreds of git repos that are cloned from each other, than this
should make a difference.
^ permalink raw reply
* Re: [PATCH] t2203: fix wrong commit command
From: Junio C Hamano @ 2012-01-11 8:09 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326252098-2891-1-git-send-email-pclouds@gmail.com>
Thanks.
^ permalink raw reply
* Re: leaky cherry-pick
From: Junio C Hamano @ 2012-01-11 8:16 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Jeff King, Pete Wyckoff, git, Nguyễn Thái Ngọc
In-Reply-To: <CALkWK0kDnxjtQ+ihH_dif_7yivHLd=pibao4KPs_PDXfc2UMOA@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
>> same stack without looking for or reparsing the attribute files. If you
>> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
>> "foo/bar/baz/.gitattributes", and pushes only the entry for
>> "foo/bar/bleep/.gitattributes".
>
> I see. Thanks for the excellent explanation- I'll try implementing
> this scheme.
I somehow have a feeling that you did not read the conclusion in Peff's
message correctly. The code only keeps data from one active path of
per-directory .gitattributes files to the leaf of a working tree and
releases unneeded data (IOW, it "pops" the attr_stack elements) when it
goes on to look at the next path, so my understanding is that there is
nothing to "try implementing" here.
Unless attr.c::free_attr_elem() is leaky, that is. If that were the case,
such a leak will accumulate and would make a difference.
^ permalink raw reply
* Re: leaky cherry-pick
From: Ramkumar Ramachandra @ 2012-01-11 9:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Pete Wyckoff, git, Nguyễn Thái Ngọc
In-Reply-To: <7vipki7ix9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>>> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
>>> same stack without looking for or reparsing the attribute files. If you
>>> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
>>> "foo/bar/baz/.gitattributes", and pushes only the entry for
>>> "foo/bar/bleep/.gitattributes".
>>
>> I see. Thanks for the excellent explanation- I'll try implementing
>> this scheme.
>
> I somehow have a feeling that you did not read the conclusion in Peff's
> message correctly. The code only keeps data from one active path of
> per-directory .gitattributes files to the leaf of a working tree and
> releases unneeded data (IOW, it "pops" the attr_stack elements) when it
> goes on to look at the next path, so my understanding is that there is
> nothing to "try implementing" here.
My bad- I thought the current implementation doesn't release the
unneeded data. So, does the entire 7 KB of leaked data come from one
active path?
-- Ram
^ 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