* 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
* Re: [PATCH v4] git-clean: Display more accurate delete messages
From: Zoltan Klinger @ 2013-01-10 1:01 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Soren Brinkmann, Jens Lehmann, Peter Collingbourne
In-Reply-To: <20130106234022.GB3823@elie.Belkin>
> I wonder whether it's possible to make the output more consistent,
> as in:
>
> Removing tracked_dir/some_untracked_file
> Removing untracked_file
> Skipping repository untracked_foo/frotz.git
> Removing untracked_foo/bar
> Removing untracked_foo/emptydir
> Skipping repository untracked_some.git
>
> or similar. What do you think?
Agree, the output looks much neater printed like that. I am going to
update the patch unless someone feels strongly against the proposed
output.
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Junio C Hamano @ 2013-01-10 0:39 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jeff King, git, Bart Trojanowski
In-Reply-To: <20130110001844.GC21054@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> I'm not sure whether there are SIGPIPE instances we really don't want
> to be silent about, though. I suspect not. ;-)
>
> Compare <http://thread.gmane.org/gmane.comp.version-control.git/2062>,
> <http://thread.gmane.org/gmane.comp.version-control.git/48469/focus=48665>.
Yeah, thanks for the pointer to 48665. Quoting from there:
So EPIPE really _is_ special: because when you write to a pipe,
there's no guarantee that you'll get it at all, so whenever you get
an EPIPE you should ask yourself:
- what would I have done if the data had fit in the 64kB kernel
buffer?
- should I really return a different error message or complain just
because I just happened to notice that the reader went away
_this_
time, even if I might not notice it next time?
In other words, the "exit(0)" is actually _more_ consistent than
"exit(1)", because exiting with an error message or with an error
return is going to depend on luck and timing.
and I think I still agree with the analysis and conclusion:
So what _should_ you do for EPIPE?
Here's what EPIPE _really_ means:
- you might as well consider the write a success, but the
reader isn't actually interested, so rather than go on, you
might as well stop early.
Notice how I very carefull avoided the word "error" anywhere.
Because it's really not an error. The reader already got
everything it wanted. So EPIPE should generally be seen as an
"early success" rather than as a "failure".
^ permalink raw reply
* Re: [PATCH] t0008: avoid brace expansion
From: Adam Spiers @ 2013-01-10 0:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, git discussion list
In-Reply-To: <7vsj693n6o.fsf@alter.siamese.dyndns.org>
On Thu, Jan 10, 2013 at 12:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
>> <rene.scharfe@lsrfire.ath.cx> wrote:
>>> Brace expansion is not required by POSIX and not supported by dash nor
>>> NetBSD's sh. Explicitly list all combinations instead.
>>
>> Good catch, thanks!
>
> Yeah; thanks.
>
> It would also be nice to avoid touch while we are at it, by the way.
Noted.
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Jonathan Nieder @ 2013-01-10 0:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Bart Trojanowski
In-Reply-To: <7vehhu3u2y.fsf@alter.siamese.dyndns.org>
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?
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. ;-)
Compare <http://thread.gmane.org/gmane.comp.version-control.git/2062>,
<http://thread.gmane.org/gmane.comp.version-control.git/48469/focus=48665>.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH] t0008: avoid brace expansion
From: Junio C Hamano @ 2013-01-10 0:18 UTC (permalink / raw)
To: Adam Spiers; +Cc: René Scharfe, git discussion list
In-Reply-To: <CAOkDyE_EuuV04KxkkLuHMV+VbDWsDMN1q3YShLtKaimaXH40Sg@mail.gmail.com>
Adam Spiers <git@adamspiers.org> writes:
> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
> <rene.scharfe@lsrfire.ath.cx> wrote:
>> Brace expansion is not required by POSIX and not supported by dash nor
>> NetBSD's sh. Explicitly list all combinations instead.
>
> Good catch, thanks!
Yeah; thanks.
It would also be nice to avoid touch while we are at it, by the way.
^ permalink raw reply
* Re: [PATCHv2] commit: make default of "cleanup" option configurable
From: Junio C Hamano @ 2013-01-10 0:17 UTC (permalink / raw)
To: Ralf Thielow; +Cc: jrnieder, git
In-Reply-To: <1357760209-3407-1-git-send-email-ralf.thielow@gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> 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>
> ---
Thanks.
> Changes in v2:
> - simplify implementation
> - mention configuration variable in documentation of "git commit --cleanup"
> - add an example usecase to documention of commit.cleanup configuration variable
> - add tests
>
> Documentation/config.txt | 7 ++++
> Documentation/git-commit.txt | 4 +-
> builtin/commit.c | 5 ++-
> t/t7500/add-content-and-comment | 4 ++
> t/t7502-commit.sh | 84 +++++++++++++++++++++++++++++++++++++----
> 5 files changed, 95 insertions(+), 9 deletions(-)
> create mode 100755 t/t7500/add-content-and-comment
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 53c4ca1..0452d56 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -917,6 +917,13 @@ 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 if you want to use the comment character (#)
> + consistently within your commit messages, in which case you would
> + like to change the default to 'whitespace'.
When the documentation suggests to use 'whitespace', it would be
helpful to warn the readers that hints Git produces in '#'-commented
section need to be removed, if they are not ment to be kept (which
is 99.99% of the case). Perhaps:
This setting overrides the default of the `--cleanup` option
in `git commit`. 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).
or something?
> 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]).
Nicely written.
> --- 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;
> @@ -966,6 +966,7 @@ static const char *read_commit_message(const char *name)
> return out;
> }
>
> +
> static int parse_and_validate_options(int argc, const char *argv[],
> const struct option *options,
> const char * const usage[],
Don't add an extra blank line, please.
> @@ -1320,6 +1321,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);
Nice.
> diff --git a/t/t7500/add-content-and-comment b/t/t7500/add-content-and-comment
> new file mode 100755
> index 0000000..988f5e9
> --- /dev/null
> +++ b/t/t7500/add-content-and-comment
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +echo "commit message" >> "$1"
> +echo "# comment" >> "$1"
> +exit 0
> \ No newline at end of file
Have newline at end of file, please.
> 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'
> +...
> +'
> +
Nicely done.
Thanks.
^ permalink raw reply
* Re: [PATCH] t0008: avoid brace expansion
From: Adam Spiers @ 2013-01-09 23:56 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git discussion list
In-Reply-To: <50EE01F8.1070109@lsrfire.ath.cx>
On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Brace expansion is not required by POSIX and not supported by dash nor
> NetBSD's sh. Explicitly list all combinations instead.
Good catch, thanks!
^ permalink raw reply
* [PATCH] t0008: avoid brace expansion
From: René Scharfe @ 2013-01-09 23:49 UTC (permalink / raw)
To: Junio C Hamano, git discussion list; +Cc: Adam Spiers
In-Reply-To: <7vboczcq5a.fsf@alter.siamese.dyndns.org>
Brace expansion is not required by POSIX and not supported by dash nor
NetBSD's sh. Explicitly list all combinations instead.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t0008-ignores.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9b0fcd6..0273680 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -129,8 +129,9 @@ test_expect_success 'setup' '
one
ignored-*
EOF
- touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} &&
- git add -f {,a/}ignored-but-in-index
+ touch not-ignored ignored-and-untracked ignored-but-in-index &&
+ touch a/not-ignored a/ignored-and-untracked a/ignored-but-in-index &&
+ git add -f ignored-but-in-index a/ignored-but-in-index &&
cat <<-\EOF >a/.gitignore &&
two*
*three
--
1.8.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] git-fast-import(1): reorganise options
From: Junio C Hamano @ 2013-01-09 22:21 UTC (permalink / raw)
To: John Keeping
Cc: Jonathan Nieder, Eric S. Raymond, git, David Michael Barr,
Pete Wyckoff, Thomas Rast
In-Reply-To: <e228dd4522dbf7ebfe0e5207b1dcc9591faa4d15.1357760256.git.john@keeping.me.uk>
Thanks.
^ permalink raw reply
* Re: [PATCH] git-shortlog(1): document behaviour of zero-width wrap
From: Junio C Hamano @ 2013-01-09 22:09 UTC (permalink / raw)
To: John Keeping; +Cc: git
In-Reply-To: <20130109201645.GB4574@serenity.lan>
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Junio C Hamano @ 2013-01-09 21:49 UTC (permalink / raw)
To: Jeff King; +Cc: git, Bart Trojanowski
In-Reply-To: <20130109205116.GA24605@sigill.intra.peff.net>
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?
^ permalink raw reply
* Re: [PATCH v2 1/2] git-fast-import(1): combine documentation of --[no-]relative-marks
From: Jonathan Nieder @ 2013-01-09 21:42 UTC (permalink / raw)
To: John Keeping
Cc: Junio C Hamano, Eric S. Raymond, git, David Michael Barr,
Pete Wyckoff, Thomas Rast
In-Reply-To: <cf4a4a10c1de73491048e8283582bdbf8c79c75e.1357760256.git.john@keeping.me.uk>
John Keeping wrote:
> The descriptions of '--relative-marks' and '--no-relative-marks' make
> more sense when read together instead of as two independent options.
> Combine them into a single description block.
Yep, this is easier to read. Thanks.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: On --depth=funny value
From: Stefan Beller @ 2013-01-09 21:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vy5g383sy.fsf_-_@alter.siamese.dyndns.org>
On 01/09/2013 03:53 AM, Junio C Hamano wrote:
> Can people sanity check the reasoning outlined here? Anything I
> missed?
>
> The above outline identifies three concrete tasks that different
> people can tackle more or less independently, each with updated
> code, documentation and test:
>
> 1. "git fetch --unshallow" that gives a pretty surface on Duy's
> "--depth=inf";
>
> 2. Making "git fetch" and "git clone" die on "--depth=0" or
> "--depth=-4";
>
> 3 Updating "upload-pack" to count correctly.
>
> I'll refrain from saying "Any takers?" for now.
Sorry for answering with delay, I am just contributing to git in my
spare time.
So if I understood Duy correctly, he is going to solve 1. and 3 by his
patches.
I'll try to come up with a solution for 2. within the next days.
^ permalink raw reply
* Re: GIT get corrupted on lustre
From: Eric Chamberland @ 2013-01-09 21:20 UTC (permalink / raw)
To: Brian J. Murrell, git
In-Reply-To: <50EC453A.2060306@giref.ulaval.ca>
Hi Brian,
On 01/08/2013 11:11 AM, Eric Chamberland wrote:
> On 12/24/2012 10:11 AM, Brian J. Murrell wrote:
>> Have you tried adding a "-q" to the git command line to quiet down git's
>> "feedback" messages?
>>
>
I moved to git 1.8.1 and added the "-q" to the command "git gc" but it
occured to return an error, so the "-q" option is not avoiding the
problem here... :-/
command in crontab:
cd /rap/jsf-051-aa/ericc/tests_git_clones/GIREF && for i in seq 10; do
/software/apps/git/1.8.1/bin/git gc -q || true;done
results:
error: index file
.git/objects/pack/pack-1f09879c88cd71a15dcc891713cf038d249830ad.idx is
too small
error: refs/remotes/origin/BIB_Branche_1_4_x does not point to a valid
object!
and this clone was a "clean" clone in which only "git qc -q" has been
run on....
I still have a doubt on threads....
Eric
^ 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