* 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: 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: [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
* [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 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
* 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: 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: 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] 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] 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] 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: [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: 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: [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: [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 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: 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
* [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: Regulator updates for 3.3
From: Linus Torvalds @ 2012-01-11 3:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <7vehv77xeq.fsf@alter.siamese.dyndns.org>
On Tue, Jan 10, 2012 at 7:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I dunno. You just scrapped the plan for 1.7.10; it may have to be called 2.0
> instead.
Btw, version numbers are cheap. I already argued for updating to 2.0
just because of the new signed tag pulling, which I think is a much
bigger issue.
I don't think a small change like "start the editor by default for
merge messages" is nearly as worthy of a version number. But I
wouldn't argue against it either, exactly because those major numbers
are cheap.
It took the kernel until 2.6.39 to learn that, I think git could learn
to use its major number more freely much earlier.
Linus
^ permalink raw reply
* Re: Regulator updates for 3.3
From: Linus Torvalds @ 2012-01-11 3:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <7vehv77xeq.fsf@alter.siamese.dyndns.org>
On Tue, Jan 10, 2012 at 7:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I really wonder. Because not being default will always lead to really
>> odd ways of saying "it should have been default, so we'll make up
>> these complex and arbitrary special rules" (like the ones you were
>> starting to outline).
>
> Hrm. Lack of any quoted line other than the first line from my message,
> together with (c) above, makes me suspect that you did not read beyond the
> first line before composing this message you are responding to.
No. See again. I did read your suggestion, and that's where the "we'll
make up these complex and arbitrary special rules" comment comes from.
Did I mis-understand it?
I think it's a *horrible* idea to go down the road or some
branch-specific configurations and then, and I quote:
"depending on the combination of what is being merged into what,
toggle the --edit option by default"
THAT is the kind of design that sounds crazy.
Instead, just make editing the default. No ifs, buts, or maybe. No
configuration, no complexities - just make it act the same way our
pager logic acts (ie redirecting stdin/stdout obviously shuts down the
pager, and equally obviously needs to shut down the editor).
Then, the --edit/--no-edit flags are for future users that want to
make it explicit. But they aren't about rules, they are about just
making very explicit statements of "I don't want the editor".
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".
But the notion of complex rules? That sounds really really bad. I'd
much rather get *rid* of the one complex rule we have (the "merging a
tag implies --edit"). That rule is already a hack.
Linus
^ permalink raw reply
* Re: Intriguing error with Git 1.6.3.1: Too many open files
From: Jonathan Nieder @ 2012-01-11 3:09 UTC (permalink / raw)
To: Soham Mehta; +Cc: git
In-Reply-To: <CALjyegUJ+ZY7g0Lpxqs=gvAyhtYW_a3SNWVybSG4EG3X=ROV9Q@mail.gmail.com>
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
^ permalink raw reply
* Re: Regulator updates for 3.3
From: Junio C Hamano @ 2012-01-11 3:03 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <CA+55aFx5NATrpLnkMiV2vAxSAJPK7wkY2vyHbyeZGgT9+jP06w@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, Jan 10, 2012 at 6:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> It is a non-starter to unconditionally start an editor.
>
> I really wonder. Because not being default will always lead to really
> odd ways of saying "it should have been default, so we'll make up
> these complex and arbitrary special rules" (like the ones you were
> starting to outline).
>
> So I really suspect it would be easier and more straightforward to
> instead just bite the bullet, and say:
>
> (a) start an editor by default if both stdin/stdout matched in fstat
> and were istty().
>
> (b) have some trivial way to disable that default behavior for people
> who really want the legacy behavior. And by "trivial" I mean "set the
> GIT_LEGACY_MERGE environment variable" or something.
>
> (c) have a "--no-editor" command line switch so that scripts and/or
> users that want to make it explicit (rather than rely on the hacky
> legacy workaround) can do so (and a explicit "--editor" switch to
> enable people to use a GUI editor even if they aren't on a terminal -
> think something IDE environment, whatever).
Hrm. Lack of any quoted line other than the first line from my message,
together with (c) above, makes me suspect that you did not read beyond the
first line before composing this message you are responding to.
> Yes, git has been very good about not breaking semantics. But it's
> happened before too when it needed to happen. We've had much bigger
> breaks (like the whole "git-xyz" to "git xyz" transition, for example,
> which broke a lot of scripts).
Yes, I am learning from the experience to be cautious ;-)
I dunno. You just scrapped the plan for 1.7.10; it may have to be called 2.0
instead.
^ permalink raw reply
* Re: [GIT PULL] use generic pci_iomap on all architectures
From: Linus Torvalds @ 2012-01-11 2:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stephen Rothwell, Git Mailing List, Michael S. Tsirkin,
linux-arch, linux-kernel, Arnd Bergmann, Jesse Barnes,
Andrew Morton
In-Reply-To: <7vipkj7ykd.fsf@alter.siamese.dyndns.org>
On Tue, Jan 10, 2012 at 6:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> The parameter to "git shortlog" that appears later should also be updated
> to match this, by the way, even though that should not affect the outcome
> in any way.
No, don't do that part.
Why?
Remember: there can be *multiple* merge bases. The expression
git shortlog ^$baserev $headrev
always works, but changing "baserev" to "merge_base" will suddenly
break for the multiple merge-bases case.
> I am however not sure what would happen when there are more than one merge
> bases. I guess those who throw pull requests are not supposed to be doing
> merges in reverse direction, so it should not matter ;-)
The other cases don't really care. For them, "show one merge-base" is
fine, and they are "end-point" operations (like "diff") that really
cannot handle a set of commits anyway.
But for "git shortlog", switching to using the merge base would
actually start showing commits that shouldn't be shown. It's
fundamentally a set operator, and does the right thing in the presense
of multiple merge-bases (which "diff" and "since commit XYZ" are
clearly not set operators, although arguably you could try to show all
merge bases for the "since" case).
Linus
^ permalink raw reply
* Re: Regulator updates for 3.3
From: Linus Torvalds @ 2012-01-11 2:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <7vmx9v7z1r.fsf@alter.siamese.dyndns.org>
On Tue, Jan 10, 2012 at 6:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> It is a non-starter to unconditionally start an editor.
I really wonder. Because not being default will always lead to really
odd ways of saying "it should have been default, so we'll make up
these complex and arbitrary special rules" (like the ones you were
starting to outline).
So I really suspect it would be easier and more straightforward to
instead just bite the bullet, and say:
(a) start an editor by default if both stdin/stdout matched in fstat
and were istty().
(b) have some trivial way to disable that default behavior for people
who really want the legacy behavior. And by "trivial" I mean "set the
GIT_LEGACY_MERGE environment variable" or something.
(c) have a "--no-editor" command line switch so that scripts and/or
users that want to make it explicit (rather than rely on the hacky
legacy workaround) can do so (and a explicit "--editor" switch to
enable people to use a GUI editor even if they aren't on a terminal -
think something IDE environment, whatever).
Where (a) is so that people will always get the editor if they aren't
aware of it, and (b) is so that existing scripting environments can
then *trivially* work around the fact that we changed semantics,
including on a site-wide basis. With (c) being for future users. Of
course, just a "git merge < /dev/null" would also do it, but sounds
ridiculously hacky (and doesn't allow the "--editor" version), so that
"--no-editor" flag sounds saner and much more powerful.
Of course, if you use "-m", no editor would fire up anyway, exactly
like with "git commit", so that's one way to avoid the issue forever
(and be backwards compatible). But if you actually *want* to get the
auto-generated message and no editor, that would need that new switch.
Yes, git has been very good about not breaking semantics. But it's
happened before too when it needed to happen. We've had much bigger
breaks (like the whole "git-xyz" to "git xyz" transition, for example,
which broke a lot of scripts).
Linus
^ permalink raw reply
* Intriguing error with Git 1.6.3.1: Too many open files
From: Soham Mehta @ 2012-01-11 2:44 UTC (permalink / raw)
To: git
We use Git (1.6.3.1) exclusively here at Box.
Starting this afternoon, we started getting errors when pushing to one of
the repos. It is a very active, bare shared repo, shared among
multiple developers. The
error is:
*"error: unable to open object pack directory: ./objects/pack: Too many
open files"
Things I tried/observed to resolve it:
1) ulimit -H = unlimited, when run as root (original error seems
misleading?). I also killed other proceses that had some open files, and
brought down the total count of open files (roughly lsof | wc -l) to <1000.
2) I can push to several other repos on the same machine just fine
3) git fsck, git gc, git-branch and git-status return the same error
4) I copied the repo (using scp -r, git-clone failed) to a different
machine, but got the same errors when pushing to it
5) I asked #git and the suggestion was that the repo is somehow corrupt and
I should re-push to it
One more thing worth mentioning: Gerrit (code review) uses the same repo
(lots of stuff in refs/changes)
If the repo is indeed not recoverable, I know how to restore it. But I'd
rather fix this if possible. Wondering if it is worth pursuing and what
might be wrong?
Thanks.
-Soham
^ 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