* Re: difftool / mergetool waiting
From: Andreas Schwab @ 2012-01-14 8:38 UTC (permalink / raw)
To: Jonathan Seng; +Cc: git
In-Reply-To: <CAG=s6FnG=3hO5jykc8s40SrCPfvJSvtEMVNBSihX5Y7T3b9SMg@mail.gmail.com>
Jonathan Seng <nekenyu@gmail.com> writes:
> If wait is false, git would fire off the tool command and proceed to
> the next then exit cleanly.
That doesn't work for git mergetool, it wouldn't be able to postprocess
the result of calling the tool command.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH v2 1/2] Document limited recursion pathspec matching with wildcards
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 9:23 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Linus Torvalds,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326341371-16628-1-git-send-email-pclouds@gmail.com>
It's actually unlimited recursion if wildcards are active regardless
--max-depth
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Regarding Junio's question earlier:
> - Shouldn't "onelevel_only" be the same as limiting to a single depth
> with "max_depth"?
Doing that would change the behavior of "git grep --max-depth=0 -- 'a*'"
from unlimited recursion currently to limited. We did not come to agree
how --max-depth should behave with wildcards last time it was discussed,
so it's best separating two flags (in the next patch) for now.
Documentation/git-grep.txt | 3 +++
tree-walk.c | 3 +++
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 15d6711..6a8b1e3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -79,6 +79,9 @@ OPTIONS
--max-depth <depth>::
For each <pathspec> given on command line, descend at most <depth>
levels of directories. A negative value means no limit.
+ This option is ignored if <pathspec> contains active wildcards.
+ In other words if "a*" matches a directory named "a*",
+ "*" is matched literally so --max-depth is still effective.
-w::
--word-regexp::
diff --git a/tree-walk.c b/tree-walk.c
index f82dba6..492c7cd 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -661,6 +661,9 @@ match_wildcards:
/*
* Match all directories. We'll try to match files
* later on.
+ * max_depth is ignored but we may consider support it
+ * in future, see
+ * http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
*/
if (ps->recursive && S_ISDIR(entry->mode))
return entry_interesting;
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v2 2/2] tree_entry_interesting: make recursive mode default
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 9:23 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Linus Torvalds,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326533003-19686-1-git-send-email-pclouds@gmail.com>
There is a bit of history behind this. Some time ago, t_e_i() only
supported prefix matching. diff-tree supported recursive and
non-recursive mode but it did not make any difference to prefix
matching.
Later on, t_e_i() gained limited recursion support to unify a similar
matching code used by git-grep. It introduced two new fields in struct
pathspec: max_depth and recursive. "recursive" field functions as a
feature switch so that this feature is off by default.
Some time after that, t_e_i() further gained wildcard support. With
wildcard matching, recursive and non-recursive diff-tree
mattered. "recursive" field was reused to distinguish recursion in
diff-tree.
This choice has a side effect that by default wildcard matching is in
non-recursive mode, which is not true. All current call sites except
"diff-tree without -r" (grep, traverse_commit_list, tree-walk and
general tree diff) prefer recursive mode.
This patch decouples the use of recursive field. The max_depth feature
switch is now controlled by max_depth_valid field. diff-tree recursion
is controlled by onelevel_only, which makes it recursive by default.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
The ugly name "nonrecursive_diff_tree" is changed to "onelevel_only".
builtin/grep.c | 2 +-
cache.h | 3 ++-
dir.c | 4 ++--
t/t4010-diff-pathspec.sh | 8 ++++++++
tree-diff.c | 3 +--
tree-walk.c | 8 ++++----
6 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..c081bf4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1051,7 +1051,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
paths = get_pathspec(prefix, argv + i);
init_pathspec(&pathspec, paths);
pathspec.max_depth = opt.max_depth;
- pathspec.recursive = 1;
+ pathspec.max_depth_valid = 1;
if (show_in_pager && (cached || list.nr))
die(_("--open-files-in-pager only works on the worktree"));
diff --git a/cache.h b/cache.h
index 10afd71..0c4067d 100644
--- a/cache.h
+++ b/cache.h
@@ -526,7 +526,8 @@ struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int nr;
unsigned int has_wildcard:1;
- unsigned int recursive:1;
+ unsigned int max_depth_valid:1;
+ unsigned int onelevel_only:1;
int max_depth;
struct pathspec_item {
const char *match;
diff --git a/dir.c b/dir.c
index 0a78d00..5af3567 100644
--- a/dir.c
+++ b/dir.c
@@ -258,7 +258,7 @@ int match_pathspec_depth(const struct pathspec *ps,
int i, retval = 0;
if (!ps->nr) {
- if (!ps->recursive || ps->max_depth == -1)
+ if (!ps->max_depth_valid || ps->max_depth == -1)
return MATCHED_RECURSIVELY;
if (within_depth(name, namelen, 0, ps->max_depth))
@@ -275,7 +275,7 @@ int match_pathspec_depth(const struct pathspec *ps,
if (seen && seen[i] == MATCHED_EXACTLY)
continue;
how = match_pathspec_item(ps->items+i, prefix, name, namelen);
- if (ps->recursive && ps->max_depth != -1 &&
+ if (ps->max_depth_valid && ps->max_depth != -1 &&
how && how != MATCHED_FNMATCH) {
int len = ps->items[i].len;
if (name[len] == '/')
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index fbc8cd8..af5134b 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -48,6 +48,14 @@ test_expect_success \
compare_diff_raw current expected'
cat >expected <<\EOF
+:100644 100644 766498d93a4b06057a8e49d23f4068f1170ff38f 0a41e115ab61be0328a19b29f18cdcb49338d516 M path1/file1
+EOF
+test_expect_success \
+ '"*file1" should show path1/file1' \
+ 'git diff-index --cached $tree -- "*file1" >current &&
+ compare_diff_raw current expected'
+
+cat >expected <<\EOF
:100644 100644 766498d93a4b06057a8e49d23f4068f1170ff38f 0a41e115ab61be0328a19b29f18cdcb49338d516 M file0
EOF
test_expect_success \
diff --git a/tree-diff.c b/tree-diff.c
index 28ad6db..fbc683c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -137,8 +137,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
enum interesting t2_match = entry_not_interesting;
/* Enable recursion indefinitely */
- opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
- opt->pathspec.max_depth = -1;
+ opt->pathspec.onelevel_only = !DIFF_OPT_TST(opt, RECURSIVE);
strbuf_init(&base, PATH_MAX);
strbuf_add(&base, base_str, baselen);
diff --git a/tree-walk.c b/tree-walk.c
index 492c7cd..fdecacc 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -588,7 +588,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
entry_not_interesting : all_entries_not_interesting;
if (!ps->nr) {
- if (!ps->recursive || ps->max_depth == -1)
+ if (!ps->max_depth_valid || ps->max_depth == -1)
return all_entries_interesting;
return within_depth(base->buf + base_offset, baselen,
!!S_ISDIR(entry->mode),
@@ -609,7 +609,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
if (!match_dir_prefix(base_str, match, matchlen))
goto match_wildcards;
- if (!ps->recursive || ps->max_depth == -1)
+ if (!ps->max_depth_valid || ps->max_depth == -1)
return all_entries_interesting;
return within_depth(base_str + matchlen + 1,
@@ -634,7 +634,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
* Match all directories. We'll try to
* match files later on.
*/
- if (ps->recursive && S_ISDIR(entry->mode))
+ if (!ps->onelevel_only && S_ISDIR(entry->mode))
return entry_interesting;
}
@@ -665,7 +665,7 @@ match_wildcards:
* in future, see
* http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
*/
- if (ps->recursive && S_ISDIR(entry->mode))
+ if (!ps->onelevel_only && S_ISDIR(entry->mode))
return entry_interesting;
}
return never_interesting; /* No matches */
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: git grep doesn't follow symbolic link
From: Nguyen Thai Ngoc Duy @ 2012-01-14 9:50 UTC (permalink / raw)
To: Junio C Hamano, Pang Yan Han
Cc: Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT, git
In-Reply-To: <7vwr8za04q.fsf@alter.siamese.dyndns.org>
(Pang's patch [1] caught my attention so I returned to the original discussion)
[1] http://thread.gmane.org/gmane.comp.version-control.git/188552
On Wed, Jan 11, 2012 at 1:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>>> I'd imagine so: symbolic links are not portable across different file
>>> systems; Git's internal representation of a symbolic link is a file
>>> containing the path of the file to be linked to.
>>
>> I'd actually welcome a fix to this general area,...
>
> Even though some platforms may lack symbolic links, where they are
> supported, they have a clear and defined meaning and that is what Git
> tracks as contents: where the link points at.
>
> So we would want our "git diff" to tell us, even if you moved without
> content modification the symbolic link target that lives somewhere on your
> filesystem but is outside the control of Git, and updated a symbolic link
> that is tracked by Git to point to a new location, that you updated the
> link. On the other hand, if you did not update a tracked symbolic link,
> even if the location the link points at that may or may not be under the
> control of Git, we do not want "git diff" to show anything. As far as that
> link is concerned, nothing has changed.
>
> Changing this would not be a fix; it would be butchering.
That's a good default. But git should allow me to say "diff the files
that symlinks point to". Link target is content from git perspective,
not from user perspective.
So instead changing the default behavior specifically for git-grep as
Pang did, I think adding --follow-symlinks option, that could be
passed to grep or any of diff family, would be a better approach.
--
Duy
^ permalink raw reply
* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14 11:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pat Thoyts, git
In-Reply-To: <7vaa5qydj7.fsf@alter.siamese.dyndns.org>
On Sat, Jan 14, 2012 at 08:00, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>>> Selecting also the trailing newline of a line for staging/unstaging would
>>> have resulted in also staging/unstaging of the next line.
>>
>> same here, could you please consider pushing this into the 1.7.9
>> release. I see no point in waiting for the next release.
>
> I do not use git-gui myself, so I wasn't paying much attention to these
> two patches.
>
> If these two fixes are for a new feature that was not present in v1.7.8
> but has already been merged before v1.7.8-rc1, then do please make sure to
> push them forward.
>
> On the other hand, if they are fixes for an old feature that was already
> in v1.7.8, then it is a bit too late for the next release.
>
Thanks, considering this, than these two patches have to wait for the
next release.
Bert
> Thanks.
^ permalink raw reply
* Re: Commit changes to remote repository
From: Carlos Martín Nieto @ 2012-01-14 11:31 UTC (permalink / raw)
To: ruperty; +Cc: git
In-Reply-To: <1326486589088-7185551.post@n2.nabble.com>
[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]
On Fri, Jan 13, 2012 at 12:29:49PM -0800, ruperty wrote:
> Being new to git I am probably not doing things correctly so pointers in the
> right direction would be useful.
>
> What I want to do make changes on my laptop and commit them to a remote
> repository. Here is what I have done,
>
> 1. Created a repository on my remote linux host, in a folder of cource code,
> by,
>
> git init
> git add *
> git commit
>
> 2. On my laptop I did a git clone pointing by ssh to the remote repo which
> downloaded all the files to my local system.
>
> 3. I changed a file locally and did a commit.
>
> 4. I then wanted to update the remote repo with my change, which I did with
> a git push, but that didn't work, getting this error,
>
> remote: error: refusing to update checked out branch:
> refs/heads/master^[[K
> remote: error: By default, updating the current branch in a non-bare
> repository^[[K.......
>
>
> What am I doing wrong?
You're trying to push to a non-bare repository and change the
currently active branch, which can cause problems, so git isn't
letting you. There's an explanation of bare and non-bare at
http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
you should init the repo you want to use as the central point with
--bare and do modifications locally and then push there.
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] git-gui: fix selection regression introduced in a8ca786991
From: Bert Wesarg @ 2012-01-14 11:58 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Bert Wesarg, Shawn O. Pearce
In-Reply-To: <CAKPyHN0xRD7qYyLYaCB9u7mhCZYFObuTdJGHq-rRST-cEhTtXA@mail.gmail.com>
Hi Pat,
On Mon, Jan 9, 2012 at 10:28, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Hi,
>
> On Sat, Jan 7, 2012 at 20:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> While fixing the problem from a8ca786991, it introduces a regression
>> regarding what happen after the multi selected file operation (ie.
>> one of Ctrl-{T,U,J}) because the next selected file could not be handled
>> by such a subsequent file operation.
>>
>> The right way is to move the fix from this commit down into the show_diff
>> function. So that all code path add the current diff path to the list of
>> selections.
>>
>> This also simplifies helper functions for these operatione which needed
>> to handle the case whether there is only the current diff path or also
>> a selction.
>
> I think we need to think this more through, especially with input from
> Shawn, please.
>
> I have now find out, that git-gui has two selections in the file
> lists. The first is that for the current path for what we show the
> diff (the tag for this is called 'in_diff') and the the second is that
> for the current list of paths which are selected ('in_sel'). The file
> list operations 'staging', 'reverting', 'unstaging', work either on
> 'in_sel'; if that is not empty, or on 'in_diff'. The problem I've now
> realized is, that these two selections share the same visual hints,
> ie. a lightgray background.
>
> The problem I tried to solve in a8ca786991 was, that adding paths to
> the selection with Ctrl-Button-1 or Shift-cutton-1, didn't included
> the current diff path in the subsequent file list operation. But I
> would have expected it, because it was visual in the 'selection'.
>
> My current 'workaround' is to make the two selections visually
> distinguishable (and reverting a8ca786991), by using a different
> background color for the 'in_sel' tag and also the italic font, so
> that it is still possible to see whether the current diff path is in
> the selection or not:
>
> @@ -717,11 +717,11 @@ proc tk_optionMenu {w varName args} {
> proc rmsel_tag {text} {
> $text tag conf sel \
> -background [$text cget -background] \
> -foreground [$text cget -foreground] \
> -borderwidth 0
> - $text tag conf in_sel -background lightgray
> + $text tag conf in_sel -background SlateGray1 -font font_diffitalic
> bind $text <Motion> break
> return $text
> }
>
> wm withdraw .
> @@ -3557,11 +3557,11 @@ if {$use_ttk} {
> .vpane.files add .vpane.files.index -sticky news
> }
>
> foreach i [list $ui_index $ui_workdir] {
> rmsel_tag $i
> - $i tag conf in_diff -background [$i tag cget in_sel -background]
> + $i tag conf in_diff -background lightgray
> }
> unset i
>
> set files_ctxm .vpane.files.ctxm
> menu $files_ctxm -tearoff 0
>
> I'm not very pleased with this, but at least it is now possible to
> visual recognize what files will be handled by a subsequent file list
> operation.
>
> Any input is more than welcome.
>
I think we don't resolve this now, because a8ca786991 introduced a
regression introduced in 0.16, I propose to revert it for upcoming
1.7.9 release.
Thanks.
Bert
> Regards,
> Bert
^ permalink raw reply
* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14 12:08 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Bert Wesarg
In-Reply-To: <37339be035746797fcec7634e3560ffcd5b26cf3.1326116492.git.bert.wesarg@googlemail.com>
On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Selecting also the trailing newline of a line for staging/unstaging would
> have resulted in also staging/unstaging of the next line.
The fix is not complete, this logic should only be applied if we have
actually a range. I will send a replacement patch in the coming days.
Bert
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
> lib/diff.tcl | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 63f8742..a750ea7 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -632,7 +632,13 @@ proc apply_range_or_line {x y} {
> }
>
> set first_l [$ui_diff index "$first linestart"]
> - set last_l [$ui_diff index "$last lineend"]
> + # don't include the next line if $last points to the start of a line
> + # ie. <lno>.0
> + if {[lindex [split $last .] 1] == 0} {
> + set last_l [$ui_diff index "$last -1 line lineend"]
> + } else {
> + set last_l [$ui_diff index "$last lineend"]
> + }
>
> if {$current_diff_path eq {} || $current_diff_header eq {}} return
> if {![lock_index apply_hunk]} return
> --
> 1.7.8.1.873.gfea665
>
^ permalink raw reply
* [PATCH v3 0/3] nd/index-pack-no-recurse
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326081546-29320-1-git-send-email-pclouds@gmail.com>
This round adds more explanation in commit message of 2/3 and comment
before get_base_data() in 3/3. Changes in 3/3 does not really address
Junio's concern regarding maintainability though.
It also fixes a regression in 3/3. In current code, get_base_data()
goes up as far as the first deflated parent. v2 of this series always
goes up to top parent. v3 fixes this.
Junio raised a point about depth-first vs breadth-first search in 1/3. I
have not addressed that either, but it makes me wonder if we may
benefit from using bfs in find_unresolved_deltas(), 2/3. If the delta
chains form a fork-like figure (e.g. long delta chains sharing common
base), then we may run out of cache doing dfs on one chain, by the
time we get back on the common base, we would need to deflate them
again.
Another observation is recursion in get_base_data() is unlikely to be
called in real life. With 16M default delta base cache, git.git does
not trigger it at all. Perhaps repos with large blobs have better chance..
Nguyễn Thái Ngọc Duy (3):
Eliminate recursion in setting/clearing marks in commit list
index-pack: eliminate recursion in find_unresolved_deltas
index-pack: eliminate unlimited recursion in get_base_data()
builtin/index-pack.c | 164 ++++++++++++++++++++++++++++++++++---------------
commit.c | 13 ++++-
revision.c | 45 +++++++++-----
3 files changed, 154 insertions(+), 68 deletions(-)
--
1.7.8.36.g69ee2
^ permalink raw reply
* [PATCH v3 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Current find_unresolved_deltas() links all bases together in a form of
tree, using struct base_data, with prev_base pointer to point to
parent node. Then it traverses down from parent to children in
recursive manner with all base_data allocated on stack.
To eliminate recursion, we simply need to put all on heap
(parse_pack_objects and fix_unresolved_deltas). After that, it's
simple non-recursive depth-first traversal loop. Each node also
maintains its own state (ofs and ref indices) to iterate over all
children nodes.
So we process one node:
- if it returns a new (child) node (a parent base), we link it to our
tree, then process the new node.
- if it returns nothing, the node is done, free it. We go back to
parent node and resume whatever it's doing.
and do it until we have no nodes to process.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 111 +++++++++++++++++++++++++++++++------------------
1 files changed, 70 insertions(+), 41 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index af7dc37..38ff03a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -34,6 +34,8 @@ struct base_data {
struct object_entry *obj;
void *data;
unsigned long size;
+ int ref_first, ref_last;
+ int ofs_first, ofs_last;
};
/*
@@ -221,6 +223,15 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)
die("pack has bad object at offset %lu: %s", offset, buf);
}
+static struct base_data *alloc_base_data(void)
+{
+ struct base_data *base = xmalloc(sizeof(struct base_data));
+ memset(base, 0, sizeof(*base));
+ base->ref_last = -1;
+ base->ofs_last = -1;
+ return base;
+}
+
static void free_base_data(struct base_data *c)
{
if (c->data) {
@@ -553,58 +564,76 @@ static void resolve_delta(struct object_entry *delta_obj,
nr_resolved_deltas++;
}
-static void find_unresolved_deltas(struct base_data *base,
- struct base_data *prev_base)
+static struct base_data *find_unresolved_deltas_1(struct base_data *base,
+ struct base_data *prev_base)
{
- int i, ref_first, ref_last, ofs_first, ofs_last;
-
- /*
- * This is a recursive function. Those brackets should help reducing
- * stack usage by limiting the scope of the delta_base union.
- */
- {
+ if (base->ref_last == -1 && base->ofs_last == -1) {
union delta_base base_spec;
hashcpy(base_spec.sha1, base->obj->idx.sha1);
find_delta_children(&base_spec,
- &ref_first, &ref_last, OBJ_REF_DELTA);
+ &base->ref_first, &base->ref_last, OBJ_REF_DELTA);
memset(&base_spec, 0, sizeof(base_spec));
base_spec.offset = base->obj->idx.offset;
find_delta_children(&base_spec,
- &ofs_first, &ofs_last, OBJ_OFS_DELTA);
- }
+ &base->ofs_first, &base->ofs_last, OBJ_OFS_DELTA);
- if (ref_last == -1 && ofs_last == -1) {
- free(base->data);
- return;
- }
+ if (base->ref_last == -1 && base->ofs_last == -1) {
+ free(base->data);
+ return NULL;
+ }
- link_base_data(prev_base, base);
+ link_base_data(prev_base, base);
+ }
- for (i = ref_first; i <= ref_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ref_first <= base->ref_last) {
+ struct object_entry *child = objects + deltas[base->ref_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_REF_DELTA);
- resolve_delta(child, base, &result);
- if (i == ref_last && ofs_last == -1)
+ resolve_delta(child, base, result);
+ if (base->ref_first == base->ref_last && base->ofs_last == -1)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ref_first++;
+ return result;
}
- for (i = ofs_first; i <= ofs_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ofs_first <= base->ofs_last) {
+ struct object_entry *child = objects + deltas[base->ofs_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_OFS_DELTA);
- resolve_delta(child, base, &result);
- if (i == ofs_last)
+ resolve_delta(child, base, result);
+ if (base->ofs_first == base->ofs_last)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ofs_first++;
+ return result;
}
unlink_base_data(base);
+ return NULL;
+}
+
+static void find_unresolved_deltas(struct base_data *base)
+{
+ struct base_data *new_base, *prev_base = NULL;
+ for (;;) {
+ new_base = find_unresolved_deltas_1(base, prev_base);
+
+ if (new_base) {
+ prev_base = base;
+ base = new_base;
+ } else {
+ free(base);
+ base = prev_base;
+ if (!base)
+ return;
+ prev_base = base->base;
+ }
+ }
}
static int compare_delta_entry(const void *a, const void *b)
@@ -684,13 +713,13 @@ static void parse_pack_objects(unsigned char *sha1)
progress = start_progress("Resolving deltas", nr_deltas);
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = &objects[i];
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (is_delta_type(obj->type))
continue;
- base_obj.obj = obj;
- base_obj.data = NULL;
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = obj;
+ base_obj->data = NULL;
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
}
@@ -783,20 +812,20 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
for (i = 0; i < n; i++) {
struct delta_entry *d = sorted_by_pos[i];
enum object_type type;
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
continue;
- base_obj.data = read_sha1_file(d->base.sha1, &type, &base_obj.size);
- if (!base_obj.data)
+ base_obj->data = read_sha1_file(d->base.sha1, &type, &base_obj->size);
+ if (!base_obj->data)
continue;
- if (check_sha1_signature(d->base.sha1, base_obj.data,
- base_obj.size, typename(type)))
+ if (check_sha1_signature(d->base.sha1, base_obj->data,
+ base_obj->size, typename(type)))
die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
- base_obj.obj = append_obj_to_pack(f, d->base.sha1,
- base_obj.data, base_obj.size, type);
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = append_obj_to_pack(f, d->base.sha1,
+ base_obj->data, base_obj->size, type);
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
free(sorted_by_pos);
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v3 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Recursion in a DAG is generally a bad idea because it could be very
deep. Be defensive and avoid recursion in mark_parents_uninteresting()
and clear_commit_marks().
mark_parents_uninteresting() learns a trick from clear_commit_marks()
to avoid malloc() in (dorminant) single-parent case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
commit.c | 13 +++++++++++--
revision.c | 45 +++++++++++++++++++++++++++++----------------
2 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/commit.c b/commit.c
index 44bc96d..c7aefbf 100644
--- a/commit.c
+++ b/commit.c
@@ -421,7 +421,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
return ret;
}
-void clear_commit_marks(struct commit *commit, unsigned int mark)
+static void clear_commit_marks_1(struct commit_list **plist,
+ struct commit *commit, unsigned int mark)
{
while (commit) {
struct commit_list *parents;
@@ -436,12 +437,20 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
return;
while ((parents = parents->next))
- clear_commit_marks(parents->item, mark);
+ commit_list_insert(parents->item, plist);
commit = commit->parents->item;
}
}
+void clear_commit_marks(struct commit *commit, unsigned int mark)
+{
+ struct commit_list *list = NULL;
+ commit_list_insert(commit, &list);
+ while (list)
+ clear_commit_marks_1(&list, pop_commit(&list), mark);
+}
+
void clear_commit_marks_for_object_array(struct object_array *a, unsigned mark)
{
struct object *object;
diff --git a/revision.c b/revision.c
index 8764dde..7cc72fc 100644
--- a/revision.c
+++ b/revision.c
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
void mark_parents_uninteresting(struct commit *commit)
{
- struct commit_list *parents = commit->parents;
+ struct commit_list *parents = NULL, *l;
+
+ for (l = commit->parents; l; l = l->next)
+ commit_list_insert(l->item, &parents);
while (parents) {
struct commit *commit = parents->item;
- if (!(commit->object.flags & UNINTERESTING)) {
+ l = parents;
+ parents = parents->next;
+ free(l);
+
+ while (commit) {
+ /*
+ * A missing commit is ok iff its parent is marked
+ * uninteresting.
+ *
+ * We just mark such a thing parsed, so that when
+ * it is popped next time around, we won't be trying
+ * to parse it and get an error.
+ */
+ if (!has_sha1_file(commit->object.sha1))
+ commit->object.parsed = 1;
+
+ if (commit->object.flags & UNINTERESTING)
+ break;
+
commit->object.flags |= UNINTERESTING;
/*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
* wasn't uninteresting), in which case we need
* to mark its parents recursively too..
*/
- if (commit->parents)
- mark_parents_uninteresting(commit);
- }
+ if (!commit->parents)
+ break;
- /*
- * A missing commit is ok iff its parent is marked
- * uninteresting.
- *
- * We just mark such a thing parsed, so that when
- * it is popped next time around, we won't be trying
- * to parse it and get an error.
- */
- if (!has_sha1_file(commit->object.sha1))
- commit->object.parsed = 1;
- parents = parents->next;
+ for (l = commit->parents->next; l; l = l->next)
+ commit_list_insert(l->item, &parents);
+ commit = commit->parents->item;
+ }
}
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v3 3/3] index-pack: eliminate unlimited recursion in get_base_data()
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Revese the order of delta applying so that by the time a delta is
applied, its base is either non-delta or already inflated.
get_base_data() is still recursive, but because base's data is always
ready, the inner get_base_data() call never has any chance to call
itself again.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 53 +++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 38ff03a..dc6a584 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -515,14 +515,52 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
}
+/*
+ * This function is part of find_unresolved_deltas(). There are two
+ * walkers going in the opposite ways.
+ *
+ * The first one in find_unresolved_deltas() traverses down from
+ * parent node to children, deflating nodes along the way. However,
+ * memory for deflated nodes is limited by delta_base_cache_limit, so
+ * at some point parent node's deflated content may be freed.
+ *
+ * The second walker is this function, which goes from current node up
+ * to top parent if necessary to deflate the node. In normal
+ * situation, its parent node would be already deflated, so it just
+ * needs to apply delta.
+ *
+ * In worst case scenario, parent node is no longer deflated because
+ * we're running out of delta_base_cache_limit, then we need to
+ * re-deflate parents, possibly up to the top base.
+ *
+ * All deflated objecsts here are subject to be freed if we exceed
+ * delta_base_cache_limit, just like in find_unresolved_deltas(), we
+ * just need to make sure the last node is not freed.
+ */
static void *get_base_data(struct base_data *c)
{
if (!c->data) {
struct object_entry *obj = c->obj;
+ struct base_data **delta = NULL;
+ int delta_nr = 0, delta_alloc = 0;
- if (is_delta_type(obj->type)) {
- void *base = get_base_data(c->base);
- void *raw = get_data_from_pack(obj);
+ while (is_delta_type(c->obj->type) && !c->data) {
+ ALLOC_GROW(delta, delta_nr + 1, delta_alloc);
+ delta[delta_nr++] = c;
+ c = c->base;
+ }
+ if (!delta_nr) {
+ c->data = get_data_from_pack(obj);
+ c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
+ }
+ for (; delta_nr > 0; delta_nr--) {
+ void *base, *raw;
+ c = delta[delta_nr - 1];
+ obj = c->obj;
+ base = get_base_data(c->base);
+ raw = get_data_from_pack(obj);
c->data = patch_delta(
base, c->base->size,
raw, obj->size,
@@ -530,13 +568,10 @@ static void *get_base_data(struct base_data *c)
free(raw);
if (!c->data)
bad_object(obj->idx.offset, "failed to apply delta");
- } else {
- c->data = get_data_from_pack(obj);
- c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
}
-
- base_cache_used += c->size;
- prune_base_data(c);
+ free(delta);
}
return c->data;
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Zsh completion regression
From: SZEDER Gábor @ 2012-01-14 13:23 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Stefan Haller, git
In-Reply-To: <vpq62ghj7fn.fsf@bauges.imag.fr>
Hi,
On Thu, Jan 12, 2012 at 03:56:28PM +0100, Matthieu Moy wrote:
> lists@haller-berlin.de (Stefan Haller) writes:
>
> > I'm using zsh 4.3.11.
> >
> > When I type "git log mas<TAB>", it completes to "git log master\ " (a
> > backslash, a space, and then the cursor).
Stefan, thanks for reporting and bisecting.
> The following patch makes the situation better, but is not really a fix:
>
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -525,7 +525,7 @@ __gitcomp ()
> __gitcomp_nl ()
> {
> local s=$'\n' IFS=' '$'\t'$'\n'
> - local cur_="$cur" suffix=" "
> + local cur_="$cur" suffix=""
>
> if [ $# -gt 2 ]; then
> cur_="$3"
>
> With this, the trailing space isn't added,
Yeah, this would be a regression for Bash users, because then they
will need to manually append that space.
> but e.g. "git checkout
> master<TAB>" does not add the trailing space, at all.
I'm not sure what you mean; did you got a trailing space after
'master<TAB>' before a31e6262 (completion: optimize refs completion,
2011-10-15)?
I'm not a zsh user myself, but just tried it and found that in
a31e626^ 'git checkout master<TAB>' doesn't add the trailing space,
and neither does 'git checkout mas<TAB>', which is in sync with what
Stefan reported ("Before this commit, I get "git log master" (with no
space at the end).").
> The problem is a little bit below:
>
> IFS=$s
> COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
>
> The -S "$suffix" adds a space to the completion, but ZSH escapes the
> space (which sounds sensible in general, but is not at all what we
> expect).
So it seems we have two issues here with zsh:
- Spaces appended with 'compgen -S " "' in __gitcomp_nl() are quoted.
This is the regression caused by a31e6262.
- Spaces appended in __gitcomp_1() (by echo "$word ") are stripped.
This is a long-standing issue; if those spaces weren't stripped, the
quoting issue would have been noted earlier, I suspect.
We could fix the regression by not appending a space suffix to
completion words in __gitcomp_nl(), but only when the completion
script is running under zsh to avoid hurting bash users, like this:
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2d02a7f3..49393243 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -601,6 +601,9 @@ __gitcomp_nl ()
suffix="$4"
fi
fi
+ if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+ suffix=""
+ fi
IFS=$s
COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
Although it wouldn't append a space at all for zsh users, that's
nothing new, it's just restoring old behavior. And then later someone
more knowledgable about the quirks of zsh's completion and in
particular zsh's bash completion emulation can come up with a proper
fix to the issue of quoted spaces and stripped trailing spaces.
However.
While playing around with zsh, I noticed that git completion works
without even sourcing git's bash completion script. As it turned out,
zsh ships its own git completion script[1], and from my cursory tests
it seems to be quite capable: it supports commands, aliases, options,
refs, ref1..ref2, config variables, ... and I also saw a __git_ps1()
equivalent for zsh.
So, is there any reason why you are still using git's bash completion
under zsh (which has some quirks and lacks some features) instead of
zsh's own? Perhaps it would make sense to point zsh users to zsh's
git completion and drop zsh compatibility from git's bash completion.
We did similar with vim config files: git included a vim syntax
highlight config file for commit messages under contrib/vim/, but
eventually we dropped it after vim started shipping more capable
git-specific config files (for git config files, rebase instruction
sheets, etc.).
[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Completion/Unix/Command/_git;hb=HEAD
Best,
Gábor
^ permalink raw reply related
* git-upload-archive help was not shown correctly
From: devendra @ 2012-01-14 13:40 UTC (permalink / raw)
To: git
Hi git folks,
the command git-upload-archive is not properly showing usage info when
ran barely with out any args.
it shows some kind of unwanted garbage instead of showing a nice help
message.
The output is pasted here.
root@devendra-Linux:/home/devendra/git/Documentation#
git-upload-archive
0008ACK
00000026 usage: git upload-archive <repo>
0031 git upload-archive: archiver died with errorfatal: sent error to
the client: git upload-archive: archiver died with error
my git latest version shows commit sha1 number
6db5c6e43dccb380ca6e9947777985eb11248c31.
Devendra.
^ permalink raw reply
* Re: Commit changes to remote repository
From: Matthieu Moy @ 2012-01-14 14:27 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: ruperty, git
In-Reply-To: <20120114113141.GG2850@centaur.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> You're trying to push to a non-bare repository and change the
> currently active branch, which can cause problems, so git isn't
> letting you. There's an explanation of bare and non-bare at
> http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
> you should init the repo you want to use as the central point with
> --bare and do modifications locally and then push there.
An alternative is to push to a temporary, non-checked-out branch.
I sometimes do
laptop$ git push desktop HEAD:incomming
and then
desktop$ git merge incomming
The push does not disturb the worktree on the desktop, and the merge is
done manually on the receiving machine.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Zsh completion regression
From: Matthieu Moy @ 2012-01-14 14:32 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Stefan Haller, git
In-Reply-To: <20120114132343.GW30469@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
>> but e.g. "git checkout
>> master<TAB>" does not add the trailing space, at all.
>
> I'm not sure what you mean; did you got a trailing space after
> 'master<TAB>' before a31e6262 (completion: optimize refs completion,
> 2011-10-15)?
No. My above sentence should read "... not add the trailing space, at
all, even for bash users". IOW, your understanding is correct.
> We could fix the regression by not appending a space suffix to
> completion words in __gitcomp_nl(), but only when the completion
> script is running under zsh to avoid hurting bash users, like this:
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2d02a7f3..49393243 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -601,6 +601,9 @@ __gitcomp_nl ()
> suffix="$4"
> fi
> fi
> + if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
> + suffix=""
> + fi
>
> IFS=$s
> COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
I hate to see special case for different shells, but if no one finds a
better solution, then yes, this is the way to go. Not having the space
may be irritating, but having the quoted space hurts really much more (I
have to delete the space and the backslash manually to continue).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Zsh completion regression
From: SZEDER Gábor @ 2012-01-14 15:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Stefan Haller, git
In-Reply-To: <vpq7h0ufj87.fsf@bauges.imag.fr>
Hi,
On Sat, Jan 14, 2012 at 03:32:08PM +0100, Matthieu Moy wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> > We could fix the regression by not appending a space suffix to
> > completion words in __gitcomp_nl(), but only when the completion
> > script is running under zsh to avoid hurting bash users, like this:
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 2d02a7f3..49393243 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -601,6 +601,9 @@ __gitcomp_nl ()
> > suffix="$4"
> > fi
> > fi
> > + if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
> > + suffix=""
> > + fi
> >
> > IFS=$s
> > COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
>
> I hate to see special case for different shells,
I agree and thought about that, too. We are dealing with zsh quirks
in the completion script either by
- having whole functions with different definitions in zsh and
in bash (currently__git_shopt() and _get_comp_words_by_ref()), or
- having similar shell-specific cases inside functions (currently
_git() and _gitk()).
We use the former when the implementations for the two shells differ
significantly, and the latter when the shell-specific parts are small
and most of the function's implementation can be used in both shells.
I think this regression fix fits into the latter category.
> but if no one finds a
> better solution, then yes, this is the way to go. Not having the space
> may be irritating, but having the quoted space hurts really much more (I
> have to delete the space and the backslash manually to continue).
I have no better idea for fixing this regression, and no idea at all
for a proper fix (i.e. to make zsh behave the same way in this respect
as bash). I can imagine that it's irritating, that's why I aimed for
this minimal regression fix, which, maybe with a Tested-by from you or
Stefan, can even go into maint, so affected users can get it faster.
Best,
Gábor
^ permalink raw reply
* Re: [PATCH v3 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Peter Baumann @ 2012-01-14 15:23 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1326543595-28300-2-git-send-email-pclouds@gmail.com>
On Sat, Jan 14, 2012 at 07:19:53PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Recursion in a DAG is generally a bad idea because it could be very
> deep. Be defensive and avoid recursion in mark_parents_uninteresting()
> and clear_commit_marks().
>
> mark_parents_uninteresting() learns a trick from clear_commit_marks()
> to avoid malloc() in (dorminant) single-parent case.
^^^^^^^^^
I think you ment dominant here.
^ permalink raw reply
* Re: git-upload-archive help was not shown correctly
From: Carlos Martín Nieto @ 2012-01-14 15:46 UTC (permalink / raw)
To: devendra; +Cc: git
In-Reply-To: <1326548416.5992.1.camel@devendra-Linux>
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> Hi git folks,
>
> the command git-upload-archive is not properly showing usage info when
> ran barely with out any args.
git-upload-package is not for human use. It's what gets run on the
remote end when you run e.g. 'git archive --remote=origin HEAD'
>
> it shows some kind of unwanted garbage instead of showing a nice help
> message.
It's trying to talk to git. What you see is the "Git Smart
Protocol". What were you trying to do?
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: Sending patches with KMail
From: Jonathan Nieder @ 2012-01-14 18:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Rüdiger Sonderfeld, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <7vlipbxfne.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> The hints at [1] might also be useful, in case you would like to try
>> and consider improving the manpage to document them if they work.
>
> Don't you need similar updates to sections for other MUAs and procedures?
Thunderbird approach 3, yes[*]. The others, no.
[...]
> Perhaps rephrasing the early part of the Discussion section, with an
> illustration that is designed to be more visible, would be a better
> approach?
I understand what you mean, but I don't think so. The Discussion
section already seems clear to me, so I would prefer to wait to hear
from someone confused by it to find what exactly in it needs tweaking.
Adding additional paragraphs for each potential misunderstanding by
people who have not necessarily read the section has the potential to
backfire and lead even more people not to read the section...
My favorite approach would be to introduce a new option
--format=plain|mbox, with the default being mbox, allowing
format-patch --format=plain to produce a nice patch that does _not_
include a "From " line or q-encode its header lines, ready for use
without much tweaking in an email body as an attachment. Then we can
just say "If you are not importing your patch as an mbox file, use the
--format=plain option".
Sane?
Jonathan
[*] Though I'd rather just remove it, since "how to use an external
editor" seems orthogonal to "how to teach Thunderbird not to mangle my
patches".
^ permalink raw reply
* Re: Sending patches with KMail
From: Jonathan Nieder @ 2012-01-14 18:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Rüdiger Sonderfeld, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <20120114183111.GC27850@burratino>
Jonathan Nieder wrote:
> My favorite approach would be to introduce a new option
> --format=plain|mbox, with the default being mbox, allowing
> format-patch --format=plain to produce a nice patch that does _not_
> include a "From " line or q-encode its header lines, ready for use
> without much tweaking in an email body as an attachment.
This should have said "ready for use in an email body or as an
attachment" (missing "or"). Sorry for the confusion.
^ permalink raw reply
* [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Matthieu Moy @ 2012-01-14 18:55 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <20120114152030.GX30469@goldbirke>
Commit a31e626 (completion: optimize refs completion) introduced a
regression for ZSH users: ref names were completed with a quoted trailing
space (i.e. "git checkout ma" completes to "git checkout master\ "). The
space is convenient for bash users since we use "-o nospace", but a
quoted space is worse than nothing. The absence of trailing space for ZSH
is a long-standing issue, that this patch is not fixing. We just fix the
regression by not appending a space when the shell is ZSH.
Original-patch-by: SZEDER Gábor <szeder@ira.uka.de>
Reported-by: Stefan Haller <lists@haller-berlin.de>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
contrib/completion/git-completion.bash | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..488e1f4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -534,6 +534,12 @@ __gitcomp_nl ()
fi
fi
+ # ZSH would quote the trailing space added with -S. bash users
+ # will appreciate the extra space to compensate the use of -o nospace.
+ if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+ suffix=""
+ fi
+
IFS=$s
COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
}
--
1.7.9.rc0.25.gb9a1f.dirty
^ permalink raw reply related
* Re: Sending patches with KMail (Re: [PATCH] git-blame.el: Fix compilation warnings.)
From: Rüdiger Sonderfeld @ 2012-01-14 19:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <7vlipbxfne.fsf@alter.siamese.dyndns.org>
On Friday 13 January 2012 16:59:49 Junio C Hamano wrote:
> Perhaps rephrasing the early part of the Discussion section, with an
> illustration that is designed to be more visible, would be a better
> approach?
Why not do both?
I think you are right, that it is currently not very visible in the Discussion
section. But on the other hand if you have a step by step guide it should
probably mention that as well. It has nothing to do with laziness. But most
people follow a step by step guide because they expect that it illustrates the
correct procedure. Jonathan's addition is short but effective.
Regards,
Rüdiger
^ permalink raw reply
* Re: git-upload-archive help was not shown correctly
From: Jeff King @ 2012-01-14 20:56 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: devendra, git
In-Reply-To: <20120114154633.GA3444@beez.lab.cmartin.tk>
On Sat, Jan 14, 2012 at 04:46:33PM +0100, Carlos Martín Nieto wrote:
> On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> > Hi git folks,
> >
> > the command git-upload-archive is not properly showing usage info when
> > ran barely with out any args.
>
> git-upload-package is not for human use. It's what gets run on the
> remote end when you run e.g. 'git archive --remote=origin HEAD'
Yeah, but notice that it does detect the error condition; it just wraps
it in a bunch of cruft. More explanation (and a patch) are below, though
I'm lukewarm on the patch.
> > it shows some kind of unwanted garbage instead of showing a nice help
> > message.
>
> It's trying to talk to git. What you see is the "Git Smart
> Protocol". What were you trying to do?
Yeah, this is the more important question. You shouldn't generally ever
be running git-upload-* yourself.
-- >8 --
Subject: upload-archive: detect bad command-line arguments early
The upload-archive command spawns a helper process to
actually generate the archive (passing along all of its
command-line arguments), and the parent process then
multiplexes the stdout and stderr of the helper back to the
client.
This means an incorrect invocation (which in this case means
failing to provide a single <repo> argument) will cause the
helper to complain, and the error will be sent to the client
over the multiplexed channel.
This is not ideal for two reasons:
1. An invocation error in upload-pack is almost certainly
not something the client can do anything about. It
generally implies a bug in either git-daemon or
git-archive. The one exception is something like:
git archive --remote=ssh_host:repo.git \
--exec="git upload-archive bogus" \
2. For local users who are experimenting with git
commands, running "upload-archive" appears to generate
a bunch of barely-readable garbage (which is in fact
the git protocol intermingled with stderr). It's nicer
if we provide them with a readable error message.
Instead, we can detect a bad command-line before we spawn
the helper and give a more human-readable complaint (i.e.,
helping reason (2) above). This doesn't hurt case (1) as
much as you might think, because in the ssh case, the user
should be receiving stderr from the parent process anyway.
So before they saw:
$ git archive --remote=... --exec='git upload-archive bogus'
fatal: sent error to the client: git upload-archive: archiver died with error
remote: usage: git upload-archive <repo>
remote: git upload-archive: archiver died with error
and now they see:
usage: git upload-archive <repo>
fatal: The remote end hung up unexpectedly
Signed-off-by: Jeff King <peff@peff.net>
---
I'm lukewarm on this patch. I actually think the original output is
slightly nicer, so you are hurting case (1) for people in case (2). But
case (2), running random programs to see what happens, is not something
we probably need to be encouraging.
builtin/upload-archive.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index b928beb..0733775 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -10,6 +10,8 @@
static const char upload_archive_usage[] =
"git upload-archive <repo>";
+static const char upload_archive_writer_usage[] =
+ "git upload-archive--writer <repo>";
static const char deadchild[] =
"git upload-archive: archiver died with error";
@@ -25,7 +27,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
int len;
if (argc != 2)
- usage(upload_archive_usage);
+ usage(upload_archive_writer_usage);
if (strlen(argv[1]) + 1 > sizeof(buf))
die("insanely long repository name");
@@ -96,6 +98,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
struct child_process writer = { argv };
+ if (argc != 2)
+ usage(upload_archive_usage);
+
/*
* Set up sideband subprocess.
*
--
1.7.9.rc0.33.gd3c17
^ permalink raw reply related
* The shared Git repo used by git-new-workdir
From: Hilco Wijbenga @ 2012-01-14 20:59 UTC (permalink / raw)
To: Git Users
Hi all,
First off, I use git-new-workdir a lot and it's working great. Kudos
to its developers!
I have been looking at the Git clone that is at the root of
git-new-workdir (i.e. the repository that is reused by all my
git-new-workdir created directories). This repo shows a lot of
activity when I run "git status" there.
So now I'm wondering. Should I simply ignore this completely? Or is
there some "clean up" I can do so that "git status" shows nothing? Or
would I destroy my git-new-workdir directories doing that? So far I've
only used this repo to create branches (i.e. to run git-new-workdir).
I would like to understand a bit better how I should treat this repo.
Whether it's basically a "do-not-touch" environment or whether I can
safely treat it as a normal Git repo.
Cheers,
Hilco
^ 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