* Re* [PATCH] commit: make default of "cleanup" option configurable
From: Junio C Hamano @ 2013-01-10 19:37 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Ralf Thielow
In-Reply-To: <7vhamq8i44.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I also wonder, as a longer term alternative (which would require a
> lot of code auditing and some refactoring), if it is useful to have
> an option and/or configuration that lets you configure the "comment
> in log message editor" character from the default "#" to something
> else. "git -c log.commentchar=% commit" may start the log message
> editor with something like this in it:
>
> % Please enter the commit message for your changes. Lines starting
> % with '%' will be ignored, and an empty message aborts the commit.
>
> Naturally, setting log.commentchar to "none" would disable stripping
> of any commented lines if such a feature existed, and stop issuing
> these helpful comments altogether, but still strip excess blank
> lines and trailing whitespaces.
>
> I wouldn't seriously suggest it as I am not sure if the usefulness
> of such a feature would outweigh the cost of coding it, though; at
> least not at this point yet.
A beginning of a patch to do would be like this, which does not look
too bad. There are some low hanging fruits I didn't bother to do in
this illustration (see NEEDSWORK comment in builtin/branch.c if some
of you are interested in pursuing it).
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 10 Jan 2013 11:17:21 -0800
Subject: [PATCH] Allow custom "comment char"
Some users do want to write a line that begin with a pound sign,
#, in their commit log message. Many tracking system recognise
a token of #<bugid> form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar="%" commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 6 ++++++
builtin/branch.c | 16 ++++++++++++----
builtin/commit.c | 15 ++++++++-------
builtin/fmt-merge-msg.c | 4 +++-
builtin/merge.c | 15 +++++++++++----
builtin/stripspace.c | 2 +-
cache.h | 6 ++++++
config.c | 8 ++++++++
environment.c | 6 ++++++
wt-status.c | 10 ++++++----
10 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..e99b9f2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -528,6 +528,12 @@ core.editor::
variable when it is set, and the environment variable
`GIT_EDITOR` is not set. See linkgit:git-var[1].
+core.commentchar::
+ Commands such as `commit` and `tag` that lets you edit
+ messages consider a line that begins with this character
+ commented, and removes them after the editor returns
+ (default '#').
+
sequence.editor::
Text editor used by `git rebase -i` for editing the rebase insn file.
The value is meant to be interpreted by the shell when it is used.
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..7f8865a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -706,11 +706,19 @@ static int edit_branch_description(const char *branch_name)
read_branch_desc(&buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(&buf, '\n');
+ /*
+ * NEEDSWORK: introduce a strbuf_commented_addf(), possibly
+ * sharing code with status_vprintf(), that makes each line
+ * commented with comment_line_char, and use it here and from
+ * other places (e.g. write_commented_object() and create_note()
+ * in builtin/notes.c and create_tag() in builtin/tag.c).
+ */
strbuf_addf(&buf,
- "# Please edit the description for the branch\n"
- "# %s\n"
- "# Lines starting with '#' will be stripped.\n",
- branch_name);
+ "%c Please edit the description for the branch\n"
+ "%c %s\n"
+ "%c Lines starting with '%c' will be stripped.\n",
+ comment_line_char, comment_line_char,
+ branch_name, comment_line_char, comment_line_char);
fp = fopen(git_path(edit_description), "w");
if ((fwrite(buf.buf, 1, buf.len, fp) < buf.len) || fclose(fp)) {
strbuf_release(&buf);
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..a946a13 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -733,15 +733,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (cleanup_mode == CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your changes."
- " Lines starting\nwith '#' will be ignored, and an empty"
- " message aborts the commit.\n"));
+ " Lines starting\nwith '%c' will be ignored, and an empty"
+ " message aborts the commit.\n"), comment_line_char);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
- _("Please enter the commit message for your changes."
- " Lines starting\n"
- "with '#' will be kept; you may remove them"
- " yourself if you want to.\n"
- "An empty message aborts the commit.\n"));
+ _("Please enter the commit message for your changes."
+ " Lines starting\n"
+ "with '%c' will be kept; you may remove them"
+ " yourself if you want to.\n"
+ "An empty message aborts the commit.\n"),
+ comment_line_char);
if (only_include_assumed)
status_printf_ln(s, GIT_COLOR_NORMAL,
"%s", only_include_assumed);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index e2e27b2..2261e1f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -463,8 +463,10 @@ static void fmt_tag_signature(struct strbuf *tagbuf,
}
strbuf_complete_line(tagbuf);
if (sig->len) {
+ char comment_head[3];
+ sprintf(comment_head, "%c ", comment_line_char);
strbuf_addch(tagbuf, '\n');
- strbuf_add_lines(tagbuf, "# ", sig->buf, sig->len);
+ strbuf_add_lines(tagbuf, comment_head, sig->buf, sig->len);
}
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 3a31c4b..632d860 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -788,17 +788,24 @@ static const char merge_editor_comment[] =
N_("Please enter a commit message to explain why this merge is necessary,\n"
"especially if it merges an updated upstream into a topic branch.\n"
"\n"
- "Lines starting with '#' will be ignored, and an empty message aborts\n"
+ "Lines starting with '%c' will be ignored, and an empty message aborts\n"
"the commit.\n");
static void prepare_to_commit(struct commit_list *remoteheads)
{
struct strbuf msg = STRBUF_INIT;
- const char *comment = _(merge_editor_comment);
+ const char *commentf = _(merge_editor_comment);
strbuf_addbuf(&msg, &merge_msg);
strbuf_addch(&msg, '\n');
- if (0 < option_edit)
- strbuf_add_lines(&msg, "# ", comment, strlen(comment));
+ if (0 < option_edit) {
+ struct strbuf hint = STRBUF_INIT;
+ char comment_head[3];
+
+ sprintf(comment_head, "%c ", comment_line_char);
+ strbuf_addf(&hint, commentf, comment_line_char);
+ strbuf_add_lines(&msg, comment_head, hint.buf, hint.len);
+ strbuf_release(&hint);
+ }
write_merge_msg(&msg);
if (run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL))
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index f16986c..600ca66 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -45,7 +45,7 @@ void stripspace(struct strbuf *sb, int skip_comments)
eol = memchr(sb->buf + i, '\n', sb->len - i);
len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
- if (skip_comments && len && sb->buf[i] == '#') {
+ if (skip_comments && len && sb->buf[i] == comment_line_char) {
newlen = 0;
continue;
}
diff --git a/cache.h b/cache.h
index c257953..0b435a4 100644
--- a/cache.h
+++ b/cache.h
@@ -562,6 +562,12 @@ extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+extern char comment_line_char;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index 7b444b6..d873c59 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,14 @@ static int git_default_core_config(const char *var, const char *value)
if (!strcmp(var, "core.editor"))
return git_config_string(&editor_program, var, value);
+ if (!strcmp(var, "core.commentchar")) {
+ const char *comment;
+ int ret = git_config_string(&comment, var, value);
+ if (!ret)
+ comment_line_char = comment[0];
+ return ret;
+ }
+
if (!strcmp(var, "core.askpass"))
return git_config_string(&askpass_program, var, value);
diff --git a/environment.c b/environment.c
index 85edd7f..a40c38b 100644
--- a/environment.c
+++ b/environment.c
@@ -62,6 +62,12 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+char comment_line_char = '#';
+
/* Parallel index stat data preload? */
int core_preload_index = 0;
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..f6f197e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -45,7 +45,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_vaddf(&sb, fmt, ap);
if (!sb.len) {
- strbuf_addch(&sb, '#');
+ strbuf_addch(&sb, comment_line_char);
if (!trail)
strbuf_addch(&sb, ' ');
color_print_strbuf(s->fp, color, &sb);
@@ -59,7 +59,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_reset(&linebuf);
if (at_bol) {
- strbuf_addch(&linebuf, '#');
+ strbuf_addch(&linebuf, comment_line_char);
if (*line != '\n' && *line != '\t')
strbuf_addch(&linebuf, ' ');
}
@@ -760,8 +760,10 @@ static void wt_status_print_tracking(struct wt_status *s)
for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s),
- "# %.*s", (int)(ep - cp), cp);
- color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
+ "%c %.*s", comment_line_char,
+ (int)(ep - cp), cp);
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
+ comment_line_char);
}
static int has_unmerged(struct wt_status *s)
--
1.8.1.351.g191b7b1
^ permalink raw reply related
* Re: t7400 broken on pu (Mac OS X)
From: Torsten Bögershausen @ 2013-01-10 19:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Torsten Bögershausen, git
In-Reply-To: <7v38y83ooo.fsf@alter.siamese.dyndns.org>
On 10.01.13 18:58, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote:
>>> The current pu fails on Mac OS, case insensitive FS.
>>>
>>>
>>> Bisecting points out
>>> commit 3f28e4fafc046284657945798d71c57608bee479
>>> [snip]
>>> Date: Sun Jan 6 13:21:07 2013 +0700
>>>
>>> Convert add_files_to_cache to take struct pathspec
>>>
>> I can reproduce it by setting core.ignorecase to true. There is a bug
>> that I overlooked. Can you verify if this throw-away patch fixes it
>> for you? A proper fix will be in the reroll later.
> I can see why it is wrong to let pathspec.raw be rewritten without
> making matching change to the containing pathspec, but I find it
> strange why it matters only on case-insensitive codepath.
>
> I agree with the "Hack" comment that the canonicalization should be
> done at a higher level upfront. Then ls-files does not need its own
> strip_trailing_slash_from_submodules(), and check_path_for_gitlink()
> can (and should---the callers of "check_anything" would not expect
> the function to change things) stop rewriting its parameter.
>
> Thanks for a quick response.
>
The patch fixes t7400.
Thanks from my side as well
/Torsten
^ permalink raw reply
* Re: [PATCH] git-commit-tree(1): correct description of defaults
From: Junio C Hamano @ 2013-01-10 18:33 UTC (permalink / raw)
To: Peter Eisentraut; +Cc: git
In-Reply-To: <1357820998.10754.6.camel@vanquo.pezone.net>
Peter Eisentraut <peter@eisentraut.org> writes:
> The old phrasing indicated that the EMAIL environment variable takes
> precedence over the user.email configuration setting, but it is the
> other way around.
>
> Signed-off-by: Peter Eisentraut <peter@eisentraut.org>
> ---
It could be argued that the observed behaviour is a bug, by the way.
If we followed the normal "command line options trump environment
variables that in turn trump config variables that in turn trump
whatever the default values we compute using cues from the system"
precedence order, EMAIL ought to come between the more specific
GIT_{AUTHOR,COMMITTER}_EMAIL environment variables and the
user.email configuration variable.
But reading the value of EMAIL can also be seen as part of the
"using cues from the system" (it often is set in equivalents of
"$HOME/.profile" by equivalents of "adduser") step, and the original
motivation to add user.email indeed was to allow users to override
EMAIL (or the name we grab from the system) without having to set
the GIT_COMMITTER_EMAIL environment variable.
So the current behaviour is correct, and the patch is a good
(belated ;-) update to the documentation.
Will apply. Thanks.
> Documentation/git-commit-tree.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
> index 6d5a04c..a221169 100644
> --- a/Documentation/git-commit-tree.txt
> +++ b/Documentation/git-commit-tree.txt
> @@ -72,13 +72,13 @@ if set:
> GIT_COMMITTER_NAME
> GIT_COMMITTER_EMAIL
> GIT_COMMITTER_DATE
> - EMAIL
>
> (nb "<", ">" and "\n"s are stripped)
>
> In case (some of) these environment variables are not set, the information
> is taken from the configuration items user.name and user.email, or, if not
> -present, system user name and the hostname used for outgoing mail (taken
> +present, the environment variable EMAIL, or, if that is not set,
> +system user name and the hostname used for outgoing mail (taken
> from `/etc/mailname` and falling back to the fully qualified hostname when
> that file does not exist).
^ permalink raw reply
* Re: [PATCH 04/19] reset: don't allow "git reset -- $pathspec" in bare repo
From: Junio C Hamano @ 2013-01-10 18:04 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <CANiSa6gK+RqovV+NKWgV57hz-p_O085HN7WCg9qvQAD-Ynpfjw@mail.gmail.com>
Martin von Zweigbergk <martinvonz@gmail.com> writes:
> ... Fix by moving
> the branching point after the check.
OK, that is what I missed. We have an existing check for mixed
reset, which was originally meant to handle case without any
pathspec but can use the same error condition (i.e. type is mixed
and repository is bare) and error message (i.e. no mixed reset in
a bare repository). "reset with pathspec" was done before that
check kicked in.
Thanks for clarification (and sorry for the noise).
^ permalink raw reply
* Re: t7400 broken on pu (Mac OS X)
From: Junio C Hamano @ 2013-01-10 17:58 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Torsten Bögershausen, git
In-Reply-To: <20130110062838.GA11634@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
> On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote:
>> The current pu fails on Mac OS, case insensitive FS.
>>
>>
>> Bisecting points out
>> commit 3f28e4fafc046284657945798d71c57608bee479
>> [snip]
>> Date: Sun Jan 6 13:21:07 2013 +0700
>>
>> Convert add_files_to_cache to take struct pathspec
>>
>
> I can reproduce it by setting core.ignorecase to true. There is a bug
> that I overlooked. Can you verify if this throw-away patch fixes it
> for you? A proper fix will be in the reroll later.
I can see why it is wrong to let pathspec.raw be rewritten without
making matching change to the containing pathspec, but I find it
strange why it matters only on case-insensitive codepath.
I agree with the "Hack" comment that the canonicalization should be
done at a higher level upfront. Then ls-files does not need its own
strip_trailing_slash_from_submodules(), and check_path_for_gitlink()
can (and should---the callers of "check_anything" would not expect
the function to change things) stop rewriting its parameter.
Thanks for a quick response.
> -- 8< --
> diff --git a/builtin/add.c b/builtin/add.c
> index 641037f..61cb8bd 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
> return seen;
> }
>
> -static void treat_gitlinks(const char **pathspec)
> +static int treat_gitlinks(const char **pathspec)
> {
> int i;
> + int modified = 0;
>
> if (!pathspec || !*pathspec)
> - return;
> + return modified;
>
> for (i = 0; i < active_nr; i++) {
> struct cache_entry *ce = active_cache[i];
> @@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec)
> if (len2 <= len || pathspec[j][len] != '/' ||
> memcmp(ce->name, pathspec[j], len))
> continue;
> - if (len2 == len + 1)
> + if (len2 == len + 1) {
> /* strip trailing slash */
> pathspec[j] = xstrndup(ce->name, len);
> - else
> + modified = 1;
> + } else
> die (_("Path '%s' is in submodule '%.*s'"),
> pathspec[j], len, ce->name);
> }
> }
> }
> + return modified;
> }
>
> static void refresh(int verbose, const struct pathspec *pathspec)
> @@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>
> if (read_cache() < 0)
> die(_("index file corrupt"));
> - treat_gitlinks(pathspec.raw);
> + if (treat_gitlinks(pathspec.raw))
> + /*
> + * HACK: treat_gitlinks strips the trailing slashes
> + * out of submodule entries but it only affects
> + * raw[]. Everything in pathspec.items is not touched.
> + * Re-init it to propagate the change. Long term, this
> + * function should be moved to pathspec.c and update
> + * everything in a consistent way.
> + */
> + init_pathspec(&pathspec, pathspec.raw);
>
> if (add_new_files) {
> int baselen;
> -- 8< --
^ permalink raw reply
* [PATCHv3] commit: make default of "cleanup" option configurable
From: Ralf Thielow @ 2013-01-10 17:45 UTC (permalink / raw)
To: gitster, jrnieder; +Cc: git, Ralf Thielow
In-Reply-To: <1357760209-3407-1-git-send-email-ralf.thielow@gmail.com>
The default of the "cleanup" option in "git commit"
is not configurable. Users who don't want to use the
default have to pass this option on every commit since
there's no way to configure it. This commit introduces
a new config option "commit.cleanup" which can be used
to change the default of the "cleanup" option in
"git commit".
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
changes in v3:
- remove extra blank line in builtin/commit.c
- add newline at end of t/t7500/add-content-and-comment
- improve documentation of commit.cleanup configuration variable
as Junio suggested (makes a lot of sense)
Thanks
Documentation/config.txt | 9 +++++
Documentation/git-commit.txt | 4 +-
builtin/commit.c | 4 +-
t/t7500/add-content-and-comment | 5 +++
t/t7502-commit.sh | 84 +++++++++++++++++++++++++++++++++++++----
5 files changed, 97 insertions(+), 9 deletions(-)
create mode 100755 t/t7500/add-content-and-comment
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 53c4ca1..c92a308 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -917,6 +917,15 @@ column.tag::
Specify whether to output tag listing in `git tag` in columns.
See `column.ui` for details.
+commit.cleanup::
+ This setting overrides the default of the `--cleanup` option in
+ `git commit`. See linkgit:git-commit[1] for details. Changing the
+ default can be useful when you always want to keep lines that begin
+ with comment character `#` in your log message, in which case you
+ would do `git config commit.cleanup whitespace` (note that you will
+ have to remove the help lines that begin with `#` in the commit log
+ template yourself, if you do this).
+
commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 7bdb039..41b27da 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -179,7 +179,9 @@ OPTIONS
only if the message is to be edited. Otherwise only whitespace
removed. The 'verbatim' mode does not change message at all,
'whitespace' removes just leading/trailing whitespace lines
- and 'strip' removes both whitespace and commentary.
+ and 'strip' removes both whitespace and commentary. The default
+ can be changed by the 'commit.cleanup' configuration variable
+ (see linkgit:git-config[1]).
-e::
--edit::
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..7c2a3d4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -103,7 +103,7 @@ static enum {
CLEANUP_NONE,
CLEANUP_ALL
} cleanup_mode;
-static char *cleanup_arg;
+static const char *cleanup_arg;
static enum commit_whence whence;
static int use_editor = 1, include_status = 1;
@@ -1320,6 +1320,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
+ if (!strcmp(k, "commit.cleanup"))
+ return git_config_string(&cleanup_arg, k, v);
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/t/t7500/add-content-and-comment b/t/t7500/add-content-and-comment
new file mode 100755
index 0000000..c4dccff
--- /dev/null
+++ b/t/t7500/add-content-and-comment
@@ -0,0 +1,5 @@
+#!/bin/sh
+echo "commit message" >> "$1"
+echo "# comment" >> "$1"
+exit 0
+
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 1a5cb69..b1c7648 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -4,6 +4,15 @@ test_description='git commit porcelain-ish'
. ./test-lib.sh
+commit_msg_is () {
+ expect=commit_msg_is.expect
+ actual=commit_msg_is.actual
+
+ printf "%s" "$(git log --pretty=format:%s%b -1)" >$actual &&
+ printf "%s" "$1" >$expect &&
+ test_i18ncmp $expect $actual
+}
+
# Arguments: [<prefix] [<commit message>] [<commit options>]
check_summary_oneline() {
test_tick &&
@@ -168,7 +177,7 @@ test_expect_success 'verbose respects diff config' '
git config --unset color.diff
'
-test_expect_success 'cleanup commit messages (verbatim,-t)' '
+test_expect_success 'cleanup commit messages (verbatim option,-t)' '
echo >>negative &&
{ echo;echo "# text";echo; } >expect &&
@@ -178,7 +187,7 @@ test_expect_success 'cleanup commit messages (verbatim,-t)' '
'
-test_expect_success 'cleanup commit messages (verbatim,-F)' '
+test_expect_success 'cleanup commit messages (verbatim option,-F)' '
echo >>negative &&
git commit --cleanup=verbatim -F expect -a &&
@@ -187,7 +196,7 @@ test_expect_success 'cleanup commit messages (verbatim,-F)' '
'
-test_expect_success 'cleanup commit messages (verbatim,-m)' '
+test_expect_success 'cleanup commit messages (verbatim option,-m)' '
echo >>negative &&
git commit --cleanup=verbatim -m "$(cat expect)" -a &&
@@ -196,7 +205,7 @@ test_expect_success 'cleanup commit messages (verbatim,-m)' '
'
-test_expect_success 'cleanup commit messages (whitespace,-F)' '
+test_expect_success 'cleanup commit messages (whitespace option,-F)' '
echo >>negative &&
{ echo;echo "# text";echo; } >text &&
@@ -207,7 +216,7 @@ test_expect_success 'cleanup commit messages (whitespace,-F)' '
'
-test_expect_success 'cleanup commit messages (strip,-F)' '
+test_expect_success 'cleanup commit messages (strip option,-F)' '
echo >>negative &&
{ echo;echo "# text";echo sample;echo; } >text &&
@@ -218,7 +227,7 @@ test_expect_success 'cleanup commit messages (strip,-F)' '
'
-test_expect_success 'cleanup commit messages (strip,-F,-e)' '
+test_expect_success 'cleanup commit messages (strip option,-F,-e)' '
echo >>negative &&
{ echo;echo sample;echo; } >text &&
@@ -231,10 +240,71 @@ echo "sample
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit." >expect
-test_expect_success 'cleanup commit messages (strip,-F,-e): output' '
+test_expect_success 'cleanup commit messages (strip option,-F,-e): output' '
test_i18ncmp expect actual
'
+test_expect_success 'cleanup commit message (fail on invalid cleanup mode option)' '
+ test_must_fail git commit --cleanup=non-existent
+'
+
+test_expect_success 'cleanup commit message (fail on invalid cleanup mode configuration)' '
+ test_must_fail git -c commit.cleanup=non-existent commit
+'
+
+test_expect_success 'cleanup commit message (no config and no option uses default)' '
+ echo content >>file &&
+ git add file &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+ git commit --no-status &&
+ commit_msg_is "commit message"
+'
+
+test_expect_success 'cleanup commit message (option overrides default)' '
+ echo content >>file &&
+ git add file &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+ git commit --cleanup=whitespace --no-status &&
+ commit_msg_is "commit message # comment"
+'
+
+test_expect_success 'cleanup commit message (config overrides default)' '
+ echo content >>file &&
+ git add file &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+ git -c commit.cleanup=whitespace commit --no-status &&
+ commit_msg_is "commit message # comment"
+'
+
+test_expect_success 'cleanup commit message (option overrides config)' '
+ echo content >>file &&
+ git add file &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+ git -c commit.cleanup=whitespace commit --cleanup=default &&
+ commit_msg_is "commit message"
+'
+
+test_expect_success 'cleanup commit message (default, -m)' '
+ echo content >>file &&
+ git add file &&
+ git commit -m "message #comment " &&
+ commit_msg_is "message #comment"
+'
+
+test_expect_success 'cleanup commit message (whitespace option, -m)' '
+ echo content >>file &&
+ git add file &&
+ git commit --cleanup=whitespace --no-status -m "message #comment " &&
+ commit_msg_is "message #comment"
+'
+
+test_expect_success 'cleanup commit message (whitespace config, -m)' '
+ echo content >>file &&
+ git add file &&
+ git -c commit.cleanup=whitespace commit --no-status -m "message #comment " &&
+ commit_msg_is "message #comment"
+'
+
test_expect_success 'message shows author when it is not equal to committer' '
echo >>negative &&
git commit -e -m "sample" -a &&
--
1.8.1.227.g44fe835
^ permalink raw reply related
* Re: [PATCH v4] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2013-01-10 17:45 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <CAKJhZwQ=RzLgCBUrx_QKY8Xzh-L8QC2UVcQJEpYxxGQz=8LwwQ@mail.gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
>> I think the code before this patch used to say "Would not remove"
>> and "Not removing" in certain cases to report the paths that the
>> command decided not to remove, but after this patch these two
>> messages no longer appear in the patch.
>>
>> Is it expected, are we losing information, or...?
>
> I do not think we are losing any information.
> Say, we have a repo like this:
> test.git
> |-- untracked_file
> |-- untracked_bar
> | |-- bar.txt
> |-- untracked_foo
> |-- foo.txt
>
> The original version prints out:
> $ git clean -fn
> Would remove untracked_file
> Would not remove untracked_bar/
> Would not remove untracked_foo/
>
> We never asked for any directories to be removed so IMHO the "Would
> not remove ..." messages are just noise.
>
> The new version prints out:
> $ git clean -fn
> Would remove untracked_file
Oh. I was blinded by the primary reason of your patch being "be
more careful and defer reporting a removal until we know everything
in a directory did get removed and managed to remove the directory",
and did not realize this "noise removal".
Perhaps add
Also do not mention that we are not removing directories
when the user did not ask us to do so with '-d'.
or something to the description?
Thanks for a clarification.
^ permalink raw reply
* Re: about vim contrib support
From: Jeff King @ 2013-01-10 13:36 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git@vger.kernel.org
In-Reply-To: <50EEAB36.6060508@gmail.com>
On Thu, Jan 10, 2013 at 12:51:18PM +0100, Manlio Perillo wrote:
> .patch files are handled by diff highlight.
> What I would like to do is to use gitcommit syntax highlight, in order
> to also enable commit subject message hightlight.
Using the regular gitcommit highlighter would not make sense, as it is
intended for the message templates seen when making a commit. Whereas
format-patch .patch files have the headers as email headers.
You can load the mail header highlighting on top of diff highlighting
like this (which only triggers for patches that look like emails):
au FileType diff
\ if getline(1) =~ '^From ' |
\ unlet b:current_syntax |
\ runtime! syntax/mail.vim |
\ endif
But maybe there is something else that you wanted to highlight. It's not
clear to me what you want from gitcommit's highlighting. Is it the
"complain about long lines" highlighting? I think that you'd have to
pull out of the gitcommit.vim and execute manually (and you'd have to
tweak the regex to take into account the "Subject: [PATCH] bits).
-Peff
^ permalink raw reply
* [PATCH] git-commit-tree(1): correct description of defaults
From: Peter Eisentraut @ 2013-01-10 12:29 UTC (permalink / raw)
To: git, gitster
The old phrasing indicated that the EMAIL environment variable takes
precedence over the user.email configuration setting, but it is the
other way around.
Signed-off-by: Peter Eisentraut <peter@eisentraut.org>
---
Documentation/git-commit-tree.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index 6d5a04c..a221169 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -72,13 +72,13 @@ if set:
GIT_COMMITTER_NAME
GIT_COMMITTER_EMAIL
GIT_COMMITTER_DATE
- EMAIL
(nb "<", ">" and "\n"s are stripped)
In case (some of) these environment variables are not set, the information
is taken from the configuration items user.name and user.email, or, if not
-present, system user name and the hostname used for outgoing mail (taken
+present, the environment variable EMAIL, or, if that is not set,
+system user name and the hostname used for outgoing mail (taken
from `/etc/mailname` and falling back to the fully qualified hostname when
that file does not exist).
--
1.7.10.4
^ permalink raw reply related
* [PATCH v2 2/2] wincred: improve compatibility with windows versions
From: Karsten Blees @ 2013-01-10 12:10 UTC (permalink / raw)
To: git; +Cc: kusmabite, msysgit, Jeff King
In-Reply-To: <CABPQNSb7MjTKgmeB9TcUV0+-FfjPZ1sgKPsfVDg6+uaw2f_azQ@mail.gmail.com>
On WinXP, the windows credential helper doesn't work at all (due to missing
Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used
by wincred is incompatible with native Windows tools (such as the control
panel applet or 'cmdkey.exe /generic'). These Windows tools only set the
TargetName, UserName and CredentialBlob members of the CREDENTIAL
structure (where CredentialBlob is the UTF-16-encoded password).
Remove the unnecessary packing / unpacking of the password, along with the
related API definitions, for compatibility with Windows XP.
Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility
with Windows credential manager tools. Parse the protocol, username, host
and path fields from the credential's target name instead.
Credentials created with an old wincred version will have mangled or empty
passwords after this change.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
.../credential/wincred/git-credential-wincred.c | 199 ++++++++-------------
1 file changed, 70 insertions(+), 129 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 94d7140..dac19ea 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -9,6 +9,8 @@
/* common helpers */
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+
static void die(const char *err, ...)
{
char msg[4096];
@@ -30,14 +32,6 @@ static void *xmalloc(size_t size)
return ret;
}
-static char *xstrdup(const char *str)
-{
- char *ret = strdup(str);
- if (!ret)
- die("Out of memory");
- return ret;
-}
-
/* MinGW doesn't have wincred.h, so we need to define stuff */
typedef struct _CREDENTIAL_ATTRIBUTEW {
@@ -67,20 +61,14 @@ typedef struct _CREDENTIALW {
#define CRED_MAX_ATTRIBUTES 64
typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
-typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
- LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
PCREDENTIALW **);
-typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
- PBYTE, DWORD *);
typedef VOID (WINAPI *CredFreeT)(PVOID);
typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
-static HMODULE advapi, credui;
+static HMODULE advapi;
static CredWriteWT CredWriteW;
-static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
static CredEnumerateWT CredEnumerateW;
-static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
static CredFreeT CredFree;
static CredDeleteWT CredDeleteW;
@@ -88,74 +76,84 @@ static void load_cred_funcs(void)
{
/* load DLLs */
advapi = LoadLibrary("advapi32.dll");
- credui = LoadLibrary("credui.dll");
- if (!advapi || !credui)
- die("failed to load DLLs");
+ if (!advapi)
+ die("failed to load advapi32.dll");
/* get function pointers */
CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
- CredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT)
- GetProcAddress(credui, "CredUnPackAuthenticationBufferW");
CredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi,
"CredEnumerateW");
- CredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT)
- GetProcAddress(credui, "CredPackAuthenticationBufferW");
CredFree = (CredFreeT)GetProcAddress(advapi, "CredFree");
CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
- if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
- !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
- !CredDeleteW)
+ if (!CredWriteW || !CredEnumerateW || !CredFree || !CredDeleteW)
die("failed to load functions");
}
-static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];
-static void write_item(const char *what, WCHAR *wbuf)
+static void write_item(const char *what, LPCWSTR wbuf, int wlen)
{
char *buf;
- int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
+ int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, NULL, 0, NULL,
FALSE);
buf = xmalloc(len);
- if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
+ if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, buf, len, NULL, FALSE))
die("WideCharToMultiByte failed!");
printf("%s=", what);
- fwrite(buf, 1, len - 1, stdout);
+ fwrite(buf, 1, len, stdout);
putchar('\n');
free(buf);
}
-static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
- const char *want)
+/*
+ * Match an (optional) expected string and a delimiter in the target string,
+ * consuming the matched text by updating the target pointer.
+ */
+static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
{
- int i;
- if (!want)
- return 1;
-
- for (i = 0; i < cred->AttributeCount; ++i)
- if (!wcscmp(cred->Attributes[i].Keyword, keyword))
- return !strcmp((const char *)cred->Attributes[i].Value,
- want);
-
- return 0; /* not found */
+ LPCWSTR delim_pos, start = *ptarget;
+ int len;
+
+ /* find start of delimiter (or end-of-string if delim is empty) */
+ if (*delim)
+ delim_pos = wcsstr(start, delim);
+ else
+ delim_pos = start + wcslen(start);
+
+ /*
+ * match text up to delimiter, or end of string (e.g. the '/' after
+ * host is optional if not followed by a path)
+ */
+ if (delim_pos)
+ len = delim_pos - start;
+ else
+ len = wcslen(start);
+
+ /* update ptarget if we either found a delimiter or need a match */
+ if (delim_pos || want)
+ *ptarget = delim_pos ? delim_pos + wcslen(delim) : start + len;
+
+ return !want || (!wcsncmp(want, start, len) && !want[len]);
}
static int match_cred(const CREDENTIALW *cred)
{
- return (!wusername || !wcscmp(wusername, cred->UserName)) &&
- match_attr(cred, L"git_protocol", protocol) &&
- match_attr(cred, L"git_host", host) &&
- match_attr(cred, L"git_path", path);
+ LPCWSTR target = cred->TargetName;
+ if (wusername && wcscmp(wusername, cred->UserName))
+ return 0;
+
+ return match_part(&target, L"git", L":") &&
+ match_part(&target, protocol, L"://") &&
+ match_part(&target, wusername, L"@") &&
+ match_part(&target, host, L"/") &&
+ match_part(&target, path, L"");
}
static void get_credential(void)
{
- WCHAR *user_buf, *pass_buf;
- DWORD user_buf_size = 0, pass_buf_size = 0;
- CREDENTIALW **creds, *cred = NULL;
+ CREDENTIALW **creds;
DWORD num_creds;
int i;
@@ -165,90 +163,36 @@ static void get_credential(void)
/* search for the first credential that matches username */
for (i = 0; i < num_creds; ++i)
if (match_cred(creds[i])) {
- cred = creds[i];
+ write_item("username", creds[i]->UserName,
+ wcslen(creds[i]->UserName));
+ write_item("password",
+ (LPCWSTR)creds[i]->CredentialBlob,
+ creds[i]->CredentialBlobSize / sizeof(WCHAR));
break;
}
- if (!cred)
- return;
-
- CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
- cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
- NULL, &pass_buf_size);
-
- user_buf = xmalloc(user_buf_size * sizeof(WCHAR));
- pass_buf = xmalloc(pass_buf_size * sizeof(WCHAR));
-
- if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
- cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
- pass_buf, &pass_buf_size))
- die("CredUnPackAuthenticationBuffer failed");
CredFree(creds);
-
- /* zero-terminate (sizes include zero-termination) */
- user_buf[user_buf_size - 1] = L'\0';
- pass_buf[pass_buf_size - 1] = L'\0';
-
- write_item("username", user_buf);
- write_item("password", pass_buf);
-
- free(user_buf);
- free(pass_buf);
-}
-
-static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
- const char *value)
-{
- attr->Keyword = (LPWSTR)keyword;
- attr->Flags = 0;
- attr->ValueSize = strlen(value) + 1; /* store zero-termination */
- attr->Value = (LPBYTE)value;
}
static void store_credential(void)
{
CREDENTIALW cred;
- BYTE *auth_buf;
- DWORD auth_buf_size = 0;
- CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
if (!wusername || !password)
return;
- /* query buffer size */
- CredPackAuthenticationBufferW(0, wusername, password,
- NULL, &auth_buf_size);
-
- auth_buf = xmalloc(auth_buf_size);
-
- if (!CredPackAuthenticationBufferW(0, wusername, password,
- auth_buf, &auth_buf_size))
- die("CredPackAuthenticationBuffer failed");
-
cred.Flags = 0;
cred.Type = CRED_TYPE_GENERIC;
cred.TargetName = target;
cred.Comment = L"saved by git-credential-wincred";
- cred.CredentialBlobSize = auth_buf_size;
- cred.CredentialBlob = auth_buf;
+ cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);
+ cred.CredentialBlob = (LPVOID)password;
cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
- cred.AttributeCount = 1;
- cred.Attributes = attrs;
+ cred.AttributeCount = 0;
+ cred.Attributes = NULL;
cred.TargetAlias = NULL;
cred.UserName = wusername;
- write_attr(attrs, L"git_protocol", protocol);
-
- if (host) {
- write_attr(attrs + cred.AttributeCount, L"git_host", host);
- cred.AttributeCount++;
- }
-
- if (path) {
- write_attr(attrs + cred.AttributeCount, L"git_path", path);
- cred.AttributeCount++;
- }
-
if (!CredWriteW(&cred, 0))
die("CredWrite failed");
}
@@ -298,13 +242,12 @@ static void read_credential(void)
*v++ = '\0';
if (!strcmp(buf, "protocol"))
- protocol = xstrdup(v);
+ protocol = utf8_to_utf16_dup(v);
else if (!strcmp(buf, "host"))
- host = xstrdup(v);
+ host = utf8_to_utf16_dup(v);
else if (!strcmp(buf, "path"))
- path = xstrdup(v);
+ path = utf8_to_utf16_dup(v);
else if (!strcmp(buf, "username")) {
- username = xstrdup(v);
wusername = utf8_to_utf16_dup(v);
} else if (!strcmp(buf, "password"))
password = utf8_to_utf16_dup(v);
@@ -333,22 +276,20 @@ int main(int argc, char *argv[])
return 0;
/* prepare 'target', the unique key for the credential */
- strncat(target_buf, "git:", sizeof(target_buf));
- strncat(target_buf, protocol, sizeof(target_buf));
- strncat(target_buf, "://", sizeof(target_buf));
- if (username) {
- strncat(target_buf, username, sizeof(target_buf));
- strncat(target_buf, "@", sizeof(target_buf));
+ wcscpy(target, L"git:");
+ wcsncat(target, protocol, ARRAY_SIZE(target));
+ wcsncat(target, L"://", ARRAY_SIZE(target));
+ if (wusername) {
+ wcsncat(target, wusername, ARRAY_SIZE(target));
+ wcsncat(target, L"@", ARRAY_SIZE(target));
}
if (host)
- strncat(target_buf, host, sizeof(target_buf));
+ wcsncat(target, host, ARRAY_SIZE(target));
if (path) {
- strncat(target_buf, "/", sizeof(target_buf));
- strncat(target_buf, path, sizeof(target_buf));
+ wcsncat(target, L"/", ARRAY_SIZE(target));
+ wcsncat(target, path, ARRAY_SIZE(target));
}
- target = utf8_to_utf16_dup(target_buf);
-
if (!strcmp(argv[1], "get"))
get_credential();
else if (!strcmp(argv[1], "store"))
--
1.8.0.msysgit.0.4.g4e40dea
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply related
* [PATCH v2 1/2] wincred: accept CRLF on stdin to simplify console usage
From: Karsten Blees @ 2013-01-10 12:10 UTC (permalink / raw)
To: git; +Cc: kusmabite, msysgit, Jeff King
In-Reply-To: <CABPQNSb7MjTKgmeB9TcUV0+-FfjPZ1sgKPsfVDg6+uaw2f_azQ@mail.gmail.com>
The windows credential helper currently only accepts LF on stdin, but bash
and cmd.exe both send CRLF. This prevents interactive use in the console.
Change the stdin parser to optionally accept CRLF.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
contrib/credential/wincred/git-credential-wincred.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index cbaec5f..94d7140 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -284,10 +284,13 @@ static void read_credential(void)
while (fgets(buf, sizeof(buf), stdin)) {
char *v;
+ int len = strlen(buf);
+ /* strip trailing CR / LF */
+ while (len && strchr("\r\n", buf[len - 1]))
+ buf[--len] = 0;
- if (!strcmp(buf, "\n"))
+ if (!*buf)
break;
- buf[strlen(buf)-1] = '\0';
v = strchr(buf, '=');
if (!v)
--
1.8.0.msysgit.0.4.g4e40dea
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply related
* [PATCH v2 0/2] improve-wincred-compatibility
From: Karsten Blees @ 2013-01-10 12:10 UTC (permalink / raw)
To: git; +Cc: kusmabite, msysgit, Jeff King
In-Reply-To: <CABPQNSb7MjTKgmeB9TcUV0+-FfjPZ1sgKPsfVDg6+uaw2f_azQ@mail.gmail.com>
Changes since initial version (see attached diff for details):
- split in two patches
- removed unused variables
- improved the dll error message
- changed ?: to if else
- added comments
Also available here:
https://github.com/kblees/git/tree/kb/improve-wincred-compatibility-v2
git pull git://github.com/kblees/git.git kb/improve-wincred-compatibility-v2
Karsten Blees (2):
wincred: accept CRLF on stdin to simplify console usage
wincred: improve compatibility with windows versions
.../credential/wincred/git-credential-wincred.c | 206 ++++++++-------------
1 file changed, 75 insertions(+), 131 deletions(-)
> git diff kb/improve-wincred-compatibility..kb/improve-wincred-compatibility-v2
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 3464080..dac19ea 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -66,7 +66,7 @@ typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
typedef VOID (WINAPI *CredFreeT)(PVOID);
typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
-static HMODULE advapi, credui;
+static HMODULE advapi;
static CredWriteWT CredWriteW;
static CredEnumerateWT CredEnumerateW;
static CredFreeT CredFree;
@@ -77,7 +77,7 @@ static void load_cred_funcs(void)
/* load DLLs */
advapi = LoadLibrary("advapi32.dll");
if (!advapi)
- die("failed to load DLLs");
+ die("failed to load advapi32.dll");
/* get function pointers */
CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
@@ -107,14 +107,34 @@ static void write_item(const char *what, LPCWSTR wbuf, int wlen)
free(buf);
}
+/*
+ * Match an (optional) expected string and a delimiter in the target string,
+ * consuming the matched text by updating the target pointer.
+ */
static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
{
- LPCWSTR start = *ptarget;
- LPCWSTR end = *delim ? wcsstr(start, delim) : start + wcslen(start);
- int len = end ? end - start : wcslen(start);
+ LPCWSTR delim_pos, start = *ptarget;
+ int len;
+
+ /* find start of delimiter (or end-of-string if delim is empty) */
+ if (*delim)
+ delim_pos = wcsstr(start, delim);
+ else
+ delim_pos = start + wcslen(start);
+
+ /*
+ * match text up to delimiter, or end of string (e.g. the '/' after
+ * host is optional if not followed by a path)
+ */
+ if (delim_pos)
+ len = delim_pos - start;
+ else
+ len = wcslen(start);
+
/* update ptarget if we either found a delimiter or need a match */
- if (end || want)
- *ptarget = end ? end + wcslen(delim) : start + len;
+ if (delim_pos || want)
+ *ptarget = delim_pos ? delim_pos + wcslen(delim) : start + len;
+
return !want || (!wcsncmp(want, start, len) && !want[len]);
}
@@ -157,9 +177,6 @@ static void get_credential(void)
static void store_credential(void)
{
CREDENTIALW cred;
- BYTE *auth_buf;
- DWORD auth_buf_size = 0;
- CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
if (!wusername || !password)
return;
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply related
* Re: about vim contrib support
From: Manlio Perillo @ 2013-01-10 11:51 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20130110113958.GA17137@sigill.intra.peff.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 10/01/2013 12:39, Jeff King ha scritto:
> On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote:
>
>> In the contrib/vim/README file there are instructions about how to setup
>> git support with Vim builtin git syntax files.
>>
>> However these instructions seems to be redundant, since the system
>> filetype.vim file already have the autocmd rules.
>
> What version of vim do you have? As the README says, version 7.2 and on
> come with the files already, and you do not need to do anything.
Ah, right.
I missed the first lines of the README file, sorry.
> [...]
>> The only issue I found is with:
>>
>> autocmd BufNewFile,BufRead .msg.[0-9]*
>> \ if getline(1) =~ '^From.*# This line is ignored.$' |
>> \ setf gitsendemail |
>> \ endif
>>
>> It should be:
>>
>> autocmd BufNewFile,BufRead [0-9]*.patch
>
> It looks like .msg.[0-9] was originally used for send-email cover
> letters,
Ok, thanks.
I was assuming it was used for the generated patched.
> and was changed to .gitsendemail.msg.* by commit eed6ca7. I
> think your [0-9]*.patch would match something else entirely (though it
> is still broken, of course, as .msg.* does not exist anymore).
>
> [...]
>> By the way: I don't understand the purpose of gitsendemail syntax.
>> On my system it does not highlight the diff.
>
> As far as I can tell, it is for cover letters, not for patches. Patches
> should already be handled by existing RFC822-message highlighting.
>
.patch files are handled by diff highlight.
What I would like to do is to use gitcommit syntax highlight, in order
to also enable commit subject message hightlight.
Thanks Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDuqzYACgkQscQJ24LbaUQ5TgCfQPeX53KOsQDF6WJF1AaSpiRd
NpMAn0GcffJwTA/etrnOnXAQctCKAY4W
=IDVf
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: about vim contrib support
From: Jeff King @ 2013-01-10 11:39 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git@vger.kernel.org
In-Reply-To: <50EEA34B.7070102@gmail.com>
On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote:
> In the contrib/vim/README file there are instructions about how to setup
> git support with Vim builtin git syntax files.
>
> However these instructions seems to be redundant, since the system
> filetype.vim file already have the autocmd rules.
What version of vim do you have? As the README says, version 7.2 and on
come with the files already, and you do not need to do anything. If you
have an older version that does not ship with them, and you are pulling
them down directly from the URLs provided, then your vim probably does
not already have them in its stock filetype.vim.
> The only issue I found is with:
>
> autocmd BufNewFile,BufRead .msg.[0-9]*
> \ if getline(1) =~ '^From.*# This line is ignored.$' |
> \ setf gitsendemail |
> \ endif
>
> It should be:
>
> autocmd BufNewFile,BufRead [0-9]*.patch
It looks like .msg.[0-9] was originally used for send-email cover
letters, and was changed to .gitsendemail.msg.* by commit eed6ca7. I
think your [0-9]*.patch would match something else entirely (though it
is still broken, of course, as .msg.* does not exist anymore).
I'd argue that we should just remove contrib/vim at this point. It has
no actual files in it, only pointers to vim.org for pre-7.2 vim users.
And that version was released in 2008, so the README is helping almost
nobody at this point (if you are on an ancient platform, and are an avid
enough vim user to download the syntax files, I suspect you would simply
install a newer version of vim).
> IMHO it should contain some other checks, to make sure it is a patch
> generated by git format-patch, and not, as an example, a plain patch or
> a Mercurial patch.
>
> By the way: I don't understand the purpose of gitsendemail syntax.
> On my system it does not highlight the diff.
As far as I can tell, it is for cover letters, not for patches. Patches
should already be handled by existing RFC822-message highlighting.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Jeff King @ 2013-01-10 11:26 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git, Bart Trojanowski
In-Reply-To: <20130110001844.GC21054@google.com>
On Wed, Jan 09, 2013 at 04:18:44PM -0800, Jonathan Nieder wrote:
> > Do we know if we are upstream of a pager that reads from us through
> > a pipe (I think we should, especially in a case where we are the one
> > who processed the "git -p $alias" option)? Is there any other case
> > where we would want to ignore child's death by SIGPIPE?
>
> When we die early by SIGPIPE because output was piped to "head", I
> still think the early end of output is not notable enough to complain
> about.
>
> I'm not sure whether there are SIGPIPE instances we really don't want
> to be silent about, though. I suspect not. ;-)
Some of our plumbing writes over pipes (e.g., pack-objects writing back
to send-pack, which is multiplexing over the network, or receive-pack
writing to index-pack). We _should_ be checking the value of every
write(), and your final close(), and making sure that sub-processes
reports success. But leaving SIGPIPE on is an extra safety measure; in
theory it can catch an unchecked write.
When one of those programs goes wrong, the message can be an extra
debugging aid. If the process died unexpectedly with no message (since
it died by signal), seeing "pack-objects died by signal 13" is much
better than not seeing anything at all. Usually it is accompanied by
other messages (like "remote end hung up unexpectedly" or similar), but
the extra output has helped me track down server-side issues in the
past.
> Compare <http://thread.gmane.org/gmane.comp.version-control.git/2062>,
> <http://thread.gmane.org/gmane.comp.version-control.git/48469/focus=48665>.
The argument there seems to be that bash is stupid for complaining about
SIGPIPE. And I would agree for the "alias" case, where we are running
commands from the command-line in a very shell-like manner. But
wait_or_whine is also used for connecting internal bits together.
Maybe the right rule is "if we are using the shell to execute, do not
mention SIGPIPE"? It seems a little iffy at first, but:
1. It tends to coincide with direct use of internal tools versus
external tools.
2. We do not reliably get SIGPIPE there, anyway, since most shells
will convert it into exit code 141 before we see it.
I.e., something like:
diff --git a/run-command.c b/run-command.c
index 24eaad5..8bd0b08 100644
--- a/run-command.c
+++ b/run-command.c
@@ -226,7 +226,7 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
}
-static int wait_or_whine(pid_t pid, const char *argv0)
+static int wait_or_whine(pid_t pid, const char *argv0, int ignore_sigpipe)
{
int status, code = -1;
pid_t waiting;
@@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0)
error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
code = WTERMSIG(status);
- if (code != SIGINT && code != SIGQUIT)
+ if (code != SIGINT && code != SIGQUIT &&
+ (!ignore_sigpipe || code != SIGPIPE))
error("%s died of signal %d", argv0, code);
/*
* This return value is chosen so that code & 0xff
@@ -433,7 +434,7 @@ fail_pipe:
* At this point we know that fork() succeeded, but execvp()
* failed. Errors have been reported to our stderr.
*/
- wait_or_whine(cmd->pid, cmd->argv[0]);
+ wait_or_whine(cmd->pid, cmd->argv[0], 0);
failed_errno = errno;
cmd->pid = -1;
}
@@ -538,7 +539,7 @@ int finish_command(struct child_process *cmd)
int finish_command(struct child_process *cmd)
{
- return wait_or_whine(cmd->pid, cmd->argv[0]);
+ return wait_or_whine(cmd->pid, cmd->argv[0], cmd->use_shell);
}
int run_command(struct child_process *cmd)
@@ -725,7 +726,7 @@ int finish_async(struct async *async)
int finish_async(struct async *async)
{
#ifdef NO_PTHREADS
- return wait_or_whine(async->pid, "child process");
+ return wait_or_whine(async->pid, "child process", 0);
#else
void *ret = (void *)(intptr_t)(-1);
^ permalink raw reply related
* about vim contrib support
From: Manlio Perillo @ 2013-01-10 11:17 UTC (permalink / raw)
To: git@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi.
In the contrib/vim/README file there are instructions about how to setup
git support with Vim builtin git syntax files.
However these instructions seems to be redundant, since the system
filetype.vim file already have the autocmd rules.
The only issue I found is with:
autocmd BufNewFile,BufRead .msg.[0-9]*
\ if getline(1) =~ '^From.*# This line is ignored.$' |
\ setf gitsendemail |
\ endif
It should be:
autocmd BufNewFile,BufRead [0-9]*.patch
IMHO it should contain some other checks, to make sure it is a patch
generated by git format-patch, and not, as an example, a plain patch or
a Mercurial patch.
By the way: I don't understand the purpose of gitsendemail syntax.
On my system it does not highlight the diff.
I have implemented an alternate gitpatch syntax file, attached.
What I would like to get, is to syntax highligth the commit subject
message, but I'm not a Vim expert.
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDuo0sACgkQscQJ24LbaUTZMQCgm7QRylhxc5v4i4tHBfUXCl8o
36IAn3t72o/+5R/x1TF7r9mu85z6wY25
=b2l0
-----END PGP SIGNATURE-----
[-- Attachment #2: gitpatch.vim --]
[-- Type: text/plain, Size: 609 bytes --]
" Vim syntax file
" Language: git format-patch message
" Maintainer: Manlio Perillo
" Filenames: [0-9]*.patch (first line is "From ... # This line is ignored.")
" Last Change: 2014 Gen 10
if exists("b:current_syntax")
finish
endif
syn case match
syn match gitsendemailComment "\%^From.*#.*"
syn match gitsendemailComment "^GIT:.*"
if has("spell")
syn spell toplevel
endif
syn include @gitcommitMessage syntax/gitcommit.vim
syn region gitcommitMessage start=/^Subject: \@=/ end=/^$|^#\@=/ contains=@gitcommitMessage
hi def link gitsendemailComment Comment
let b:current_syntax = "gitpatch"
^ permalink raw reply
* Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
From: Duy Nguyen @ 2013-01-10 11:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin von Zweigbergk, git
In-Reply-To: <7vy5g25f9b.fsf@alter.siamese.dyndns.org>
On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> We use the path arguments in two places in reset.c: in
>> interactive_reset() and read_from_tree(). Both of these call
>> get_pathspec(), so we pass the (prefix, arv) pair to both
>> functions. Move the call to get_pathspec() out of these methods, for
>> two reasons: 1) One argument is simpler than two. 2) It lets us use
>> the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
>> ---
>> If I understand correctly, this should be rebased on top of
>> nd/parse-pathspec. Please let me know.
>
> Yeah, this will conflict with the get_pathspec-to-parse_pathspec
> conversion Duy has been working on.
Or I could hold off nd/parse-pathspec if this series has a better
chance of graduation first. Decision?
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Jeff King @ 2013-01-10 10:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bart Trojanowski
In-Reply-To: <7vehhu3u2y.fsf@alter.siamese.dyndns.org>
On Wed, Jan 09, 2013 at 01:49:41PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > But we still say "error: ... died of signal 13", because that comes from
> > inside wait_or_whine. So it is a separate issue whether or not
> > wait_or_whine should be silent on SIGPIPE (we already are on SIGINT and
> > SIGQUIT, as of some recent patches).
> >
> > The upside is that it is noise in this case that we would no longer see.
> > The downside is that we may be losing a clue when debugging server
> > problems, which do not expect to die from SIGPIPE. Should it be an
> > optional run-command flag?
>
> Do we know if we are upstream of a pager that reads from us through
> a pipe (I think we should, especially in a case where we are the one
> who processed the "git -p $alias" option)? Is there any other case
> where we would want to ignore child's death by SIGPIPE? If the
> answers are yes and no, then perhaps we can ask pager_in_use() to
> decide this?
The answer to the first is unfortunately no. Consider an alias like
"[alias]foo = !git log" (which yes, you could implement as just "log",
but there are cases where you want to manipulate the environment and we
do not allow it).
Your process tree for running "git foo" looks like:
git foo (A)
git log (B)
less (C)
The user hits 'q', which kills process C. Process B then dies due to
SIGPIPE, and process A sees that the alias command died due to a signal.
But process A has no clue that a pager is in effect; only process B,
which spawned the pager, can know that. So A cannot see death-by-SIGPIPE
and make a decision on whether a pager was in use.
If anything, it is process B's responsibility to say "Oops, I was killed
by SIGPIPE. But that's OK, it's not a big deal to me". Which it could do
by installing a SIGPIPE handler that just calls exit(0).
-Peff
^ permalink raw reply
* Re: [PATCH v4] git-clean: Display more accurate delete messages
From: Zoltan Klinger @ 2013-01-10 10:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk3rl3fw5.fsf@alter.siamese.dyndns.org>
> I think the code before this patch used to say "Would not remove"
> and "Not removing" in certain cases to report the paths that the
> command decided not to remove, but after this patch these two
> messages no longer appear in the patch.
>
> Is it expected, are we losing information, or...?
>
I do not think we are losing any information.
Say, we have a repo like this:
test.git
|-- untracked_file
|-- untracked_bar
| |-- bar.txt
|-- untracked_foo
|-- foo.txt
The original version prints out:
$ git clean -fn
Would remove untracked_file
Would not remove untracked_bar/
Would not remove untracked_foo/
We never asked for any directories to be removed so IMHO the "Would
not remove ..." messages are just noise.
The new version prints out:
$ git clean -fn
Would remove untracked_file
^ permalink raw reply
* Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()
From: Martin von Zweigbergk @ 2013-01-10 8:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlic25e9d.fsf@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 11:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> Use a single condition to guard the call to die_if_unmerged_cache for
>> both --soft and --keep. This avoids the small distraction of the
>> precondition check from the logic following it.
>>
>> Also change an instance of
>>
>> if (e)
>> err = err || f();
>>
>> to the almost as short, but clearer
>>
>> if (e && !err)
>> err = f();
>>
>> (which is equivalent since we only care whether exit code is 0)
>
> It is not just equivalent, but should give us identical result, even
> if we cared the actual value.
If err is initially 0, and f() evaluates to 2, err would be 1 in the
first case, but 2 in the second case, right?
I think the two might be identical in e.g. JavaScript and Python, but
I don't use either much.
^ permalink raw reply
* Re: [PATCH 06/19] reset.c: remove unnecessary variable 'i'
From: Martin von Zweigbergk @ 2013-01-10 8:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpq1e5ent.fsf@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> Throughout most of parse_args(), the variable 'i' remains at 0. In the
>> remaining few cases, we can do pointer arithmentic on argv itself
>> instead.
>> ---
>> This is clearly mostly a matter of taste. The remainder of the series
>> does not depend on it in any way.
>
> I agree that it indeed is a matter of taste between
>
> (1) look at av[i], check with (i < ac) for the end, and increment i to
> iterate over the arguments; and
>
> (2) look at av[0], check with (0 < ac) for the end, and increment
> av and decrement ac at the same time to iterate over the
> arguments.
>
> When (ac, av) appear as a pair, however, adjusting only av without
> adjusting ac is asking for future trouble. It violates a common
> expectation that av[ac] points at the NULL at the end of the list.
Good points.
> If a code chooses to use !av[0] as the terminating condition and
> never looks at ac, then incrementing only av is fine, but in such a
> case, the function probably should lose ac altogether.
Makes sense. I've picked this style for now (i.e. dropped both 'i' and
'argc'). I was surprised by the style that referred to the variable in
many places where it was know to be 0, but I'm no experienced C
programmer, so if that's a common practice when it comes to argument
parsing, I'm also happy to drop the patch. Let me know what you
prefer.
^ permalink raw reply
* Re: [PATCH 04/19] reset: don't allow "git reset -- $pathspec" in bare repo
From: Martin von Zweigbergk @ 2013-01-10 8:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtxqq5f0g.fsf@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 11:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> ---
>> builtin/reset.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> With the patch that does not have any explicit check for bareness
> nor new error message to scold user with, it is rather hard to tell
> what is going on, without any description on what (if anything) is
> broken at the end user level and what remedy is done about that
> breakage...
Will include the following in a re-roll.
reset: don't allow "git reset -- $pathspec" in bare repo
Running e.g. "git reset ." in a bare repo results in an index file
being created from the HEAD commit. The differences compared to the
index are then printed as usual, but since there is no worktree, it
will appear as if all files are deleted. For example, in a bare clone
of git.git:
Unstaged changes after reset:
D .gitattributes
D .gitignore
D .mailmap
...
This happens because the check for is_bare_repository() happens after
we branch off into read_from_tree() to reset with paths. Fix by moving
the branching point after the check.
^ permalink raw reply
* Re: [PATCH 4/4] t5002: check if unzip supports symlinks
From: Jonathan Nieder @ 2013-01-10 7:36 UTC (permalink / raw)
To: René Scharfe; +Cc: git discussion list, Junio C Hamano
In-Reply-To: <50EAFCD7.9090008@lsrfire.ath.cx>
René Scharfe wrote:
> Am 07.01.2013 09:52, schrieb Jonathan Nieder:
>> Hm. Do some implementations of "unzip" not support symlinks, or is
>> the problem that some systems build Info-ZIP without the SYMLINKS
>> option?
>
> The unzip supplied with NetBSD 6.0.1, which is based on libarchive, doesn't
> support symlinks. It creates a file with the link target path as its only
> content for such entries.
Ok, that makes sense. A quick search finds
<https://code.google.com/p/libarchive/issues/detail?id=104>, which if
I understand correctly was fixed in libarchive 3.0.2. NetBSD 6 uses a
patched 2.8.4.
[...]
> For the test script there is no difference: If we don't have a tool to
> verify symlinks in archives, we better skip that part.
Yeah, I just wanted to see if there were other parts of the world that
needed fixing while at it. Thanks for explaining.
Ciao,
Jonathan
^ permalink raw reply
* Re: t7400 broken on pu (Mac OS X)
From: Duy Nguyen @ 2013-01-10 6:28 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <50EDBA37.30205@web.de>
On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote:
> The current pu fails on Mac OS, case insensitive FS.
>
>
> Bisecting points out
> commit 3f28e4fafc046284657945798d71c57608bee479
> [snip]
> Date: Sun Jan 6 13:21:07 2013 +0700
>
> Convert add_files_to_cache to take struct pathspec
>
I can reproduce it by setting core.ignorecase to true. There is a bug
that I overlooked. Can you verify if this throw-away patch fixes it
for you? A proper fix will be in the reroll later.
-- 8< --
diff --git a/builtin/add.c b/builtin/add.c
index 641037f..61cb8bd 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
return seen;
}
-static void treat_gitlinks(const char **pathspec)
+static int treat_gitlinks(const char **pathspec)
{
int i;
+ int modified = 0;
if (!pathspec || !*pathspec)
- return;
+ return modified;
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
@@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec)
if (len2 <= len || pathspec[j][len] != '/' ||
memcmp(ce->name, pathspec[j], len))
continue;
- if (len2 == len + 1)
+ if (len2 == len + 1) {
/* strip trailing slash */
pathspec[j] = xstrndup(ce->name, len);
- else
+ modified = 1;
+ } else
die (_("Path '%s' is in submodule '%.*s'"),
pathspec[j], len, ce->name);
}
}
}
+ return modified;
}
static void refresh(int verbose, const struct pathspec *pathspec)
@@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (read_cache() < 0)
die(_("index file corrupt"));
- treat_gitlinks(pathspec.raw);
+ if (treat_gitlinks(pathspec.raw))
+ /*
+ * HACK: treat_gitlinks strips the trailing slashes
+ * out of submodule entries but it only affects
+ * raw[]. Everything in pathspec.items is not touched.
+ * Re-init it to propagate the change. Long term, this
+ * function should be moved to pathspec.c and update
+ * everything in a consistent way.
+ */
+ init_pathspec(&pathspec, pathspec.raw);
if (add_new_files) {
int baselen;
-- 8< --
^ permalink raw reply related
* Re: [PATCH v4] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2013-01-10 2:56 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <1357514219-16102-1-git-send-email-zoltan.klinger@gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
> Consider the output of the improved version:
>
> $ git clean -fd
> Removing tracked_dir/some_untracked_file
> Removing untracked_file
> warning: ignoring untracked git repository untracked_foo/frotz.git
> Removing untracked_foo/bar
> Removing untracked_foo/emptydir
> warning: ignoring untracked git repository untracked_some.git/
>
> Now it displays only the file and directory names that got actually
> deleted and shows warnings about ignored untracked git repositories.
>
> Reported-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>
> Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>
> ---
I think the code before this patch used to say "Would not remove"
and "Not removing" in certain cases to report the paths that the
command decided not to remove, but after this patch these two
messages no longer appear in the patch.
Is it expected, are we losing information, or...?
> builtin/clean.c | 158 +++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 129 insertions(+), 29 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 69c1cda..1714546 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -10,6 +10,7 @@
> #include "cache.h"
> #include "dir.h"
> #include "parse-options.h"
> +#include "refs.h"
> #include "string-list.h"
> #include "quote.h"
>
> @@ -20,6 +21,12 @@ static const char *const builtin_clean_usage[] = {
> NULL
> };
>
> +static const char *msg_remove = N_("Removing %s\n");
> +static const char *msg_would_remove = N_("Would remove %s\n");
> +static const char *msg_would_ignore_git_dir = N_("Would ignore untracked git repository %s\n");
> +static const char *msg_warn_ignore_git_dir = N_("ignoring untracked git repository %s");
> +static const char *msg_warn_remove_failed = N_("failed to remove %s");
> +
> static int git_clean_config(const char *var, const char *value, void *cb)
> {
> if (!strcmp(var, "clean.requireforce"))
> @@ -34,11 +41,116 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
> return 0;
> }
>
> +static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
> + int dry_run, int quiet, int *dir_gone)
> +{
> + DIR *dir;
> + struct strbuf quoted = STRBUF_INIT;
> + struct dirent *e;
> + int res = 0, ret = 0, gone = 1, original_len = path->len, len, i;
> + unsigned char submodule_head[20];
> + struct string_list dels = STRING_LIST_INIT_DUP;
> +
> + *dir_gone = 1;
> +
> + if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> + !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
> + if (dry_run && !quiet) {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + printf(_(msg_would_ignore_git_dir), quoted.buf);
> + } else if (!dry_run) {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + warning(_(msg_warn_ignore_git_dir), quoted.buf);
> + }
> +
> + *dir_gone = 0;
> + return 0;
> + }
> +
> + dir = opendir(path->buf);
> + if (!dir) {
> + /* an empty dir could be removed even if it is unreadble */
> + res = dry_run ? 0 : rmdir(path->buf);
> + if (res) {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + warning(_(msg_warn_remove_failed), quoted.buf);
> + *dir_gone = 0;
> + }
> + return res;
> + }
> +
> + if (path->buf[original_len - 1] != '/')
> + strbuf_addch(path, '/');
> +
> + len = path->len;
> + while ((e = readdir(dir)) != NULL) {
> + struct stat st;
> + if (is_dot_or_dotdot(e->d_name))
> + continue;
> +
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, e->d_name);
> + if (lstat(path->buf, &st))
> + ; /* fall thru */
> + else if (S_ISDIR(st.st_mode)) {
> + if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
> + ret = 1;
> + if (gone) {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + string_list_append(&dels, quoted.buf);
> + }
> + else
> + *dir_gone = 0;
> + continue;
> + } else {
> + res = dry_run ? 0 : unlink(path->buf);
> + if (!res) {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + string_list_append(&dels, quoted.buf);
> + }
> + else {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + warning(_(msg_warn_remove_failed), quoted.buf);
> + *dir_gone = 0;
> + ret = 1;
> + }
> + continue;
> + }
> +
> + /* path too long, stat fails, or non-directory still exists */
> + *dir_gone = 0;
> + ret = 1;
> + break;
> + }
> + closedir(dir);
> +
> + strbuf_setlen(path, original_len);
> +
> + if (*dir_gone) {
> + res = dry_run ? 0 : rmdir(path->buf);
> + if (!res)
> + *dir_gone = 1;
> + else {
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + warning(_(msg_warn_remove_failed), quoted.buf);
> + *dir_gone = 0;
> + ret = 1;
> + }
> + }
> +
> + if (!*dir_gone && !quiet) {
> + for (i = 0; i < dels.nr; i++)
> + printf(dry_run ? _(msg_would_remove) : _(msg_remove), dels.items[i].string);
> + }
> + string_list_clear(&dels, 0);
> + return ret;
> +}
> +
> int cmd_clean(int argc, const char **argv, const char *prefix)
> {
> - int i;
> - int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
> - int ignored_only = 0, config_set = 0, errors = 0;
> + int i, res;
> + int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
> + int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
> int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
> struct strbuf directory = STRBUF_INIT;
> struct dir_struct dir;
> @@ -49,7 +161,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> char *seen = NULL;
> struct option options[] = {
> OPT__QUIET(&quiet, N_("do not print names of files removed")),
> - OPT__DRY_RUN(&show_only, N_("dry run")),
> + OPT__DRY_RUN(&dry_run, N_("dry run")),
> OPT__FORCE(&force, N_("force")),
> OPT_BOOLEAN('d', NULL, &remove_directories,
> N_("remove whole directories")),
> @@ -77,7 +189,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> if (ignored && ignored_only)
> die(_("-x and -X cannot be used together"));
>
> - if (!show_only && !force) {
> + if (!dry_run && !force) {
> if (config_set)
> die(_("clean.requireForce set to true and neither -n nor -f given; "
> "refusing to clean"));
> @@ -149,38 +261,26 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>
> if (S_ISDIR(st.st_mode)) {
> strbuf_addstr(&directory, ent->name);
> - qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> - if (show_only && (remove_directories ||
> - (matches == MATCHED_EXACTLY))) {
> - printf(_("Would remove %s\n"), qname);
> - } else if (remove_directories ||
> - (matches == MATCHED_EXACTLY)) {
> - if (!quiet)
> - printf(_("Removing %s\n"), qname);
> - if (remove_dir_recursively(&directory,
> - rm_flags) != 0) {
> - warning(_("failed to remove %s"), qname);
> + if (remove_directories || (matches == MATCHED_EXACTLY)) {
> + if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))
> errors++;
> + if (gone && !quiet) {
> + qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> + printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
> }
> - } else if (show_only) {
> - printf(_("Would not remove %s\n"), qname);
> - } else {
> - printf(_("Not removing %s\n"), qname);
> }
> strbuf_reset(&directory);
> } else {
> if (pathspec && !matches)
> continue;
> - qname = quote_path_relative(ent->name, -1, &buf, prefix);
> - if (show_only) {
> - printf(_("Would remove %s\n"), qname);
> - continue;
> - } else if (!quiet) {
> - printf(_("Removing %s\n"), qname);
> - }
> - if (unlink(ent->name) != 0) {
> - warning(_("failed to remove %s"), qname);
> + res = dry_run ? 0 : unlink(ent->name);
> + if (res) {
> + qname = quote_path_relative(ent->name, -1, &buf, prefix);
> + warning(_(msg_warn_remove_failed), qname);
> errors++;
> + } else if (!quiet) {
> + qname = quote_path_relative(ent->name, -1, &buf, prefix);
> + printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
> }
> }
> }
^ 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