* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Junio C Hamano @ 2016-10-26 16:08 UTC (permalink / raw)
To: Duy Nguyen
Cc: Jeff King, Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <CACsJy8DUqrsaqmrCHzzuS3Q7DXRAPkisOJbSmYPX8-AhmNUz6w@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> I don't object the alias.<name>.<property> approach though. It's
> definitely a cleaner one in my opinion. It just needs people who can
> spend time to follow up until the end. But if someone decides to do
> that now, I'll drop the "(properties)!command" and try to support
> him/her.
I don't object to either approach, but what I would love to see
people avoid is to end up with both.
Thanks.
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Duy Nguyen @ 2016-10-26 14:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kb7PaqnyXZ7u0z8Q__ahTLKX8RQwV=dw7vbD4C9LnjOtw@mail.gmail.com>
On Tue, Oct 25, 2016 at 2:18 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Oct 24, 2016 at 11:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>>
>> Make that a double-asterisk. The same problem appears in an updated
>> example in technical/api-gitattributes.txt doc, but the example in
>> the commit log message (below) is correct.
>
> The implementation is actually using a double pointer, see below,
> I forgot commit message and documentation
>
>>> GIT_ATTR_RESULT_INIT_FOR(myresult, 1);
>>
>> Are you sure about this? We've called attr_check_initl() already so
>> if this is declaring myresult, it would be decl-after-stmt.
>
> I forgot to update the commit message and Documentation.
> GIT_ATTR_RESULT_INIT_FOR is gone
I was asking whether this function/macro was not thread-safe and found
out it didn't exist as well, and it's bed time so I'm stopping. Will
continue my skimming on the next re-roll :)
--
Duy
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Duy Nguyen @ 2016-10-26 13:52 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Git Mailing List, bmwill
In-Reply-To: <20161022233225.8883-28-sbeller@google.com>
On Sun, Oct 23, 2016 at 6:32 AM, Stefan Beller <sbeller@google.com> wrote:
> This revamps the API of the attr subsystem to be thread safe.
> Before we had the question and its results in one struct type.
> The typical usage of the API was
>
> static struct git_attr_check *check;
>
> if (!check)
> check = git_attr_check_initl("text", NULL);
Two cents. I read the .txt first and my first thought was "is _initl a
typo, shouldn't it be jsut _init"? I know we have this 'l' variant at
least in argv-array, but there we have many ways of adding arguments.
And here it's just "initl", not "init" nor other "initX" variants,
which looks odd. I wonder if the name git_attr_check_init() would do
the job fine, since we don't have different init variants and the
naming convention is not strong enough to tell me "it's multiple
arguments ended with a NULL one" right away. If you're worried about
people forgetting NULL at the end, how about passing an array of
strings, terminated by NULL, instead?
Just thinking out loud. Maybe _initl _is_ a better name.
--
Duy
^ permalink raw reply
* Re: [PATCH 32/36] pathspec: allow querying for attributes
From: Duy Nguyen @ 2016-10-26 13:33 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Git Mailing List, bmwill
In-Reply-To: <20161022233225.8883-33-sbeller@google.com>
(sorry if this should have been answered if I went through the series
patch by patch, I wanted to do a proper review but finally have to
admit to myself I won't, so I just skim through a single giant diff
instead)
On Sun, Oct 23, 2016 at 6:32 AM, Stefan Beller <sbeller@google.com> wrote:
> +attr;;
> +After `attr:` comes a space separated list of "attribute
> +requirements", all of which must be met in order for the
> +path to be considered a match;
What about (attr=abc def,attr=ghi lkj)? Does it mean (abc && def) ||
(ghi && lkj), or abc && def && ghi && lkj? Or is it forbidden to have
multiple 'attr' attribute in the same pathspec?
--
Duy
^ permalink raw reply
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Duy Nguyen @ 2016-10-26 13:23 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git Mailing List
In-Reply-To: <20161011150118.7eb474yg5c3oe5mn@sigill.intra.peff.net>
On Tue, Oct 11, 2016 at 10:01 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 11, 2016 at 11:44:50AM +0200, Johannes Schindelin wrote:
>
>> > Yeah, that's reasonable, too. So:
>> >
>> > [alias]
>> > d2u = "!dos2unix"
>> >
>> > acts exactly as if:
>> >
>> > [alias "d2u"]
>> > command = dos2unix
>> > type = shell
>> >
>> > was specified at that point, which is easy to understand.
>>
>> It is easy to understand, and even easier to get wrong or out of sync. I
>> really liked the ease of *one* `git config` call to add new aliases. Not
>> sure that I like the need for more such calls just to add *one* alias (one
>> config call for "shell", one for "don't cd up", etc).
>
> Could we simply support alias.d2u indefinitely, and you could use
> whichever format you felt like (the shorter, more limited one if you
> wanted, or the more verbose but flexible one)?
Before this thread goes completely dead... Since there's a lot more
work involved with the new alias.<name>.<property> approach (short
term would be git completion support, longer term would be the ability
to manipulate a config group more conveniently), I'm going with the
"(properties)!command" approach. But even then a new series is not
going to pop up, like, in the next two months.
I don't object the alias.<name>.<property> approach though. It's
definitely a cleaner one in my opinion. It just needs people who can
spend time to follow up until the end. But if someone decides to do
that now, I'll drop the "(properties)!command" and try to support
him/her.
--
Duy
^ permalink raw reply
* Re: [Question] Git histrory after greybus merge
From: Dmitry Safonov @ 2016-10-26 13:17 UTC (permalink / raw)
To: open list
Cc: gregkh, Greg KH, Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <CAJwJo6apgP-pTUccB1Hs81rQbaVCnxeh355fgCKvXc630WTJNg@mail.gmail.com>
Adding Cc: git list, Junio.
2016-10-26 15:55 GMT+03:00 Dmitry Safonov <0x7f454c46@gmail.com>:
> Hi,
>
> Is there any way to specify git-log or git-rev-list which root tree to use?
> I mean, I got the following situation:
> I saw the commit a67dd266adf4 ("netfilter: xtables: prepare for
> on-demand hook register")
> by git-blame and want to see commits on top of that particular commit.
> Earlier I've used for that:
> $ git log --reverse a67dd266adf4^..HEAD
>
> But now after merging greybus it follows the greybus's tree and shows me:
> [linux]$ git log --reverse a67dd266adf4^..HEAD --oneline
> cd26f1bd6bf3 greybus: Initial commit
> c8a797a98cb6 greybus: Import most recent greybus code to new repo.
> 06823c3eb9c4 greybus: README and .gitignore updates
>
> Which quite sucks as this isn't a hash I'm referencing.
> Anyway, back to the question, is there any option to tell git which tree to use?
> I'm sure this was asked before (on btrfs merge?), but I didn't find
> the answer so far.
> I'm using git v2.10.1 if anything.
>
> Thanks,
> Dmitry
^ permalink raw reply
* Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Jeff King @ 2016-10-26 12:31 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AHMJ7OJFKyjYAsPuT0nG=e0RGn59PuX06PJ5pYjinsqA@mail.gmail.com>
On Wed, Oct 26, 2016 at 07:26:20PM +0700, Duy Nguyen wrote:
> > I'm not sure this is really any convenience over dumping a corefile
> > and using gdb to pull out the
> > symbols after the fact.
>
> So are we back to forcing core files? I'm ok with that! The only
> inconvenience I see is pointing out where the core file is, which
> should be where `pwd` originally is. On linux we can even peek into
> /proc/sys/kernel/core_pattern if we want to be precise. ulimit can get
> in the way, but I don't if the default out there is enable or disable
> core dumping. Once we got OS info and git version, our chances of
> cracking the core files should be reasonably high.
TBH, most of the time I expect the solution to be walking the person
through:
git clone git://kernel.org/pub/scm/git/git.git
cd git
make
gdb --args ./git whatever
break die
run
bt
which would cover most cases (reproducible breakage, and not in a
sub-program). It's relatively rare that even I resort to corefiles.
-Peff
^ permalink raw reply
* Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Duy Nguyen @ 2016-10-26 12:26 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161026121058.a2pmf57oc7p2mlsp@sigill.intra.peff.net>
On Wed, Oct 26, 2016 at 7:10 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Oct 26, 2016 at 05:29:21PM +0700, Duy Nguyen wrote:
>
>> > I think you could conditionally make git_path() and all of its
>> > counterparts macros, similar to the way the trace code works. It seems
>> > like a pretty maintenance-heavy solution, though. I'd prefer
>> > conditionally compiling backtrace(); that also doesn't hit 100% of
>> > cases, but at least it isn't too invasive.
>>
>> OK, a more polished patch is this. There are warnings about
>> -fomit-function-pointers in glibc man page, at least in my simple
>> tests it does not cause any issue.
>
> Yeah, I tried with -fno-omit-frame-pointer, but it didn't help. The
> glibc backtrace(3) manpage specifically says:
>
> The symbol names may be unavailable without the use of special linker
> options. For systems using the GNU linker, it is necessary to use the
> -rdynamic linker option. Note that names of "static" functions are not
> exposed, and won't be available in the backtrace.
>
> which matches the behavior I get.
>
> Gcc ships with a libbacktrace which does seem to give reliable results
> (patch below for reference). But that's still relying on gcc, and on
> having debug symbols available.
Yep. On an optimized build you can't get anywhere without debug info,
which has a giant database to describe "if your rip/pc register is
here, then you clue to find your caller is there" for basically every
instruction in your program. Dwarf3 at least is a crazy world.
> I'm not sure this is really any convenience over dumping a corefile and using gdb to pull out the
> symbols after the fact.
So are we back to forcing core files? I'm ok with that! The only
inconvenience I see is pointing out where the core file is, which
should be where `pwd` originally is. On linux we can even peek into
/proc/sys/kernel/core_pattern if we want to be precise. ulimit can get
in the way, but I don't if the default out there is enable or disable
core dumping. Once we got OS info and git version, our chances of
cracking the core files should be reasonably high.
--
Duy
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Jeff King @ 2016-10-26 12:15 UTC (permalink / raw)
To: Simon Ruderich
Cc: Johannes Schindelin, Stefan Beller, gitster, git, bmwill, pclouds
In-Reply-To: <20161026093558.drsdwn2hm7bbcktt@ruderich.org>
On Wed, Oct 26, 2016 at 11:35:58AM +0200, Simon Ruderich wrote:
> > static pthread_mutex_t attr_mutex;
> > -#define attr_lock()pthread_mutex_lock(&attr_mutex)
> > +static inline void attr_lock(void)
> > +{
> > + static int initialized;
> > +
> > + if (!initialized) {
> > + pthread_mutex_init(&attr_mutex, NULL);
> > + initialized = 1;
> > + }
> > + pthread_mutex_lock(&attr_mutex);
> > +}
>
> This may initialize the mutex multiple times during the first
> lock (which may happen in parallel).
>
> pthread provides static initializers. To quote the man page:
>
> Variables of type pthread_mutex_t can also be initialized
> statically, using the constants PTHREAD_MUTEX_INITIALIZER
> (for fast mutexes), PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
> (for recursive mutexes), and
> PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP (for error checking
> mutexes).
I seem to recall this does not work on Windows, where the pthread
functions are thin wrappers over CRITICAL_SECTION. Other threaded code
in git does an explicit setup step before entering threaded sections.
E.g., see start_threads() in builtin/grep.c.
-Peff
^ permalink raw reply
* Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Jeff King @ 2016-10-26 12:10 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <20161026102921.GA31311@ash>
On Wed, Oct 26, 2016 at 05:29:21PM +0700, Duy Nguyen wrote:
> > I think you could conditionally make git_path() and all of its
> > counterparts macros, similar to the way the trace code works. It seems
> > like a pretty maintenance-heavy solution, though. I'd prefer
> > conditionally compiling backtrace(); that also doesn't hit 100% of
> > cases, but at least it isn't too invasive.
>
> OK, a more polished patch is this. There are warnings about
> -fomit-function-pointers in glibc man page, at least in my simple
> tests it does not cause any issue.
Yeah, I tried with -fno-omit-frame-pointer, but it didn't help. The
glibc backtrace(3) manpage specifically says:
The symbol names may be unavailable without the use of special linker
options. For systems using the GNU linker, it is necessary to use the
-rdynamic linker option. Note that names of "static" functions are not
exposed, and won't be available in the backtrace.
which matches the behavior I get.
Gcc ships with a libbacktrace which does seem to give reliable results
(patch below for reference). But that's still relying on gcc, and on
having debug symbols available. I'm not sure this is really any
convenience over dumping a corefile and using gdb to pull out the
symbols after the fact.
---
diff --git a/config.mak.uname b/config.mak.uname
index 76f885281c..62a14f10d3 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -40,6 +40,8 @@ ifeq ($(uname_S),Linux)
NEEDS_LIBRT = YesPlease
HAVE_GETDELIM = YesPlease
SANE_TEXT_GREP=-a
+ BASIC_CFLAGS += -DHAVE_BACKTRACE
+ EXTLIBS += -lbacktrace
endif
ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
diff --git a/usage.c b/usage.c
index 17f52c1b5c..4b9762ae62 100644
--- a/usage.c
+++ b/usage.c
@@ -5,6 +5,9 @@
*/
#include "git-compat-util.h"
#include "cache.h"
+#ifdef HAVE_BACKTRACE
+#include <backtrace.h>
+#endif
static FILE *error_handle;
static int tweaked_error_buffering;
@@ -24,6 +27,44 @@ void vreportf(const char *prefix, const char *err, va_list params)
fputc('\n', fh);
}
+#ifdef HAVE_BACKTRACE
+static int full_callback(void *fh, uintptr_t pc,
+ const char *filename, int lineno,
+ const char *function)
+{
+ if (!function || !filename)
+ return 0;
+ fprintf(fh, "debug: %s() at %s:%d\n", function, filename, lineno);
+ return 0;
+}
+
+static void error_callback(void *fh, const char *msg, int errnum)
+{
+ fprintf(fh, "backtrace error: %s", msg);
+ if (errnum > 0)
+ fprintf(fh, ": %s", strerror(errnum));
+ fputc('\n', fh);
+}
+
+static void maybe_backtrace(void)
+{
+ FILE *fh = error_handle ? error_handle : stderr;
+ struct backtrace_state *bt;
+
+ if (!git_env_bool("GIT_BACKTRACE_ON_DIE", 0))
+ return;
+
+ /* XXX obviously unportable use of /proc */
+ bt = backtrace_create_state("/proc/self/exe", 0, error_callback, fh);
+ if (bt) {
+ fprintf(fh, "debug: die() called at:\n");
+ backtrace_full(bt, 3, full_callback, error_callback, fh);
+ }
+}
+#else
+#define maybe_backtrace()
+#endif
+
static NORETURN void usage_builtin(const char *err, va_list params)
{
vreportf("usage: ", err, params);
@@ -33,6 +74,7 @@ static NORETURN void usage_builtin(const char *err, va_list params)
static NORETURN void die_builtin(const char *err, va_list params)
{
vreportf("fatal: ", err, params);
+ maybe_backtrace();
exit(128);
}
^ permalink raw reply related
* Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Duy Nguyen @ 2016-10-26 10:29 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161025151524.y7wwtetohhqgcvob@sigill.intra.peff.net>
On Tue, Oct 25, 2016 at 11:15:25AM -0400, Jeff King wrote:
> > The "once we've identified" part could be tricky though. This message
> > alone will not give us any clue where it's called since it's buried
> > deep in git_path() usually, which is buried deep elsewhere. Without
> > falling back to core dumps (with debug info), glibc's backtrace
> > (platform specifc), the best we could do is turn git_path() into a
> > macro that takes __FILE__ and __LINE__ and somehow pass the info down
> > here, but "..." in macros is C99 specific, sigh..
> >
> > Is it too bad to turn git_path() into a macro when we know the
> > compiler is C99 ? Older compilers will have no source location info in
> > git_path(), Hopefully they are rare, which means chances of this fault
> > popping up are also reduced.
>
> I think you could conditionally make git_path() and all of its
> counterparts macros, similar to the way the trace code works. It seems
> like a pretty maintenance-heavy solution, though. I'd prefer
> conditionally compiling backtrace(); that also doesn't hit 100% of
> cases, but at least it isn't too invasive.
OK, a more polished patch is this. There are warnings about
-fomit-function-pointers in glibc man page, at least in my simple
tests it does not cause any issue.
> But I think I still prefer just letting the corefile and the debugger do
> their job. This error shouldn't happen much, and when it does, it should
> be easily reproducible. Getting the bug reporter to give either a
> reproduction recipe, or to run "gdb git" doesn't seem like that big a
> hurdle.
There are other places where backtrace() support may come handy
too. If -rdynamic was not needed, I would push for this patch. Too bad
backtrace() is not a perfect magic wand.
-- 8< --
diff --git a/config.mak.uname b/config.mak.uname
index b232908..b38f62a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -40,6 +40,8 @@ ifeq ($(uname_S),Linux)
NEEDS_LIBRT = YesPlease
HAVE_GETDELIM = YesPlease
SANE_TEXT_GREP=-a
+ # for backtrace() support with glibc >= 2.1
+ BASIC_LDFLAGS += -rdynamic
endif
ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
diff --git a/git-compat-util.h b/git-compat-util.h
index 43718da..3561ab9 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -408,6 +408,7 @@ extern NORETURN void usage(const char *err);
extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void BUG(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
@@ -709,6 +710,7 @@ extern int git_vsnprintf(char *str, size_t maxsize,
#ifdef __GLIBC_PREREQ
#if __GLIBC_PREREQ(2, 1)
#define HAVE_STRCHRNUL
+#define HAVE_BACKTRACE
#endif
#endif
@@ -722,6 +724,23 @@ static inline char *gitstrchrnul(const char *s, int c)
}
#endif
+#ifdef HAVE_BACKTRACE
+#include <execinfo.h>
+static inline void dump_backtrace(FILE *fp)
+{
+ void *buffer[32];
+ int nptrs;
+
+ nptrs = backtrace(buffer, sizeof(buffer) / sizeof(*buffer));
+ fflush(fp);
+ backtrace_symbols_fd(buffer, nptrs, fileno(fp));
+}
+#else
+static inline void dump_backtrace(FILE *fp)
+{
+}
+#endif
+
#ifdef NO_INET_PTON
int inet_pton(int af, const char *src, void *dst);
#endif
diff --git a/usage.c b/usage.c
index 17f52c1..b00603c 100644
--- a/usage.c
+++ b/usage.c
@@ -204,3 +204,16 @@ void warning(const char *warn, ...)
warn_routine(warn, params);
va_end(params);
}
+
+void NORETURN BUG(const char *fmt, ...)
+{
+ va_list params;
+
+ va_start(params, fmt);
+ vreportf("BUG: ", fmt, params);
+ va_end(params);
+
+ dump_backtrace(error_handle ? error_handle : stderr);
+
+ exit(128);
+}
-- 8< --
--
Duy
^ permalink raw reply related
* [PATCH] rebase: add --forget to cleanup rebase, leave HEAD untouched
From: Nguyễn Thái Ngọc Duy @ 2016-10-26 9:46 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
There are occasions when you decide to abort an in-progress rebase and
move on to do something else but you forget to do "git rebase --abort"
first. Or the rebase has been in progress for so long you forgot about
it. By the time you realize that (e.g. by starting another rebase)
it's already too late to retrace your steps. The solution is normally
rm -r .git/<some rebase dir>
and continue with your life. But there could be two different
directories for <some rebase dir> (and it obviously requires some
knowledge of how rebase works), and the ".git" part could be much
longer if you are not at top-dir, or in a linked worktree. And
"rm -r" is very dangerous to do in .git, a mistake in there could
destroy object database or other important data.
Provide "git rebase --forget" for this exact use case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-rebase.txt | 5 ++++-
contrib/completion/git-completion.bash | 4 ++--
git-rebase.sh | 6 +++++-
t/t3407-rebase-abort.sh | 24 ++++++++++++++++++++++++
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..5a58fb3 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[<upstream> [<branch>]]
'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
--root [<branch>]
-'git rebase' --continue | --skip | --abort | --edit-todo
+'git rebase' --continue | --skip | --abort | --forget | --edit-todo
DESCRIPTION
-----------
@@ -252,6 +252,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
will be reset to where it was when the rebase operation was
started.
+--forget::
+ Abort the rebase operation but leave HEAD where it is.
+
--keep-empty::
Keep the commits that do not change anything from its
parents in the result.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9c8f738..bb64fb4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1733,10 +1733,10 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/rebase-merge/interactive ]; then
- __gitcomp "--continue --skip --abort --edit-todo"
+ __gitcomp "--continue --skip --abort --forget --edit-todo"
return
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
+ __gitcomp "--continue --skip --abort --forget"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..de712b7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
+forget! abort but keep HEAD where it is
"
. git-sh-setup
set_reflog_action rebase
@@ -241,7 +242,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--edit-todo)
+ --continue|--skip|--abort|--forget|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -399,6 +400,9 @@ abort)
finish_rebase
exit
;;
+forget)
+ exec rm -rf "$state_dir"
+ ;;
edit-todo)
run_specific_rebase
;;
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index a6a6c40..6bc5e71 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,4 +99,28 @@ testrebase() {
testrebase "" .git/rebase-apply
testrebase " --merge" .git/rebase-merge
+test_expect_success 'rebase --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase master &&
+ test_path_is_dir .git/rebase-apply &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-apply
+'
+
+test_expect_success 'rebase --merge --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase --merge master &&
+ test_path_is_dir .git/rebase-merge &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-merge
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Simon Ruderich @ 2016-10-26 9:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stefan Beller, gitster, git, bmwill, pclouds
In-Reply-To: <alpine.DEB.2.20.1610261049470.3264@virtualbox>
[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]
On Wed, Oct 26, 2016 at 10:52:23AM +0200, Johannes Schindelin wrote:
> diff --git a/attr.c b/attr.c
> index d5a6aa9..6933504 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -50,7 +50,16 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);
> #ifndef NO_PTHREADS
>
> static pthread_mutex_t attr_mutex;
> -#define attr_lock()pthread_mutex_lock(&attr_mutex)
> +static inline void attr_lock(void)
> +{
> + static int initialized;
> +
> + if (!initialized) {
> + pthread_mutex_init(&attr_mutex, NULL);
> + initialized = 1;
> + }
> + pthread_mutex_lock(&attr_mutex);
> +}
This may initialize the mutex multiple times during the first
lock (which may happen in parallel).
pthread provides static initializers. To quote the man page:
Variables of type pthread_mutex_t can also be initialized
statically, using the constants PTHREAD_MUTEX_INITIALIZER
(for fast mutexes), PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
(for recursive mutexes), and
PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP (for error checking
mutexes).
Regards
Simon
--
+ Privatsphäre ist notwendig
+ Ich verwende GnuPG http://gnupg.org
+ Öffentlicher Schlüssel: 0x92FEFDB7E44C32F9
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] reset: --unmerge
From: Duy Nguyen @ 2016-10-26 9:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqmvhsc8kn.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 6:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Somebody with a bright idea decided that vc-git-resolve-conflicts
> variable should be on by default in Emacs 25.1 X-<,
Oh good, I have an excuse to stick to 24.5.1 for a while longer then.
> which causes
> "save" after resolving conflicts to automatically run "git add", to
> destroy this valuable tool. My knee-jerk reaction, of course, to
> such a default is "that's brain dead", but perhaps they did so for
> some good reason that I fail to fathom.
I was curious. The default is t since the variable's introduction [1].
Interestingly the thread/bug that resulted in that commit started with
"report this bug to git" [2]. Something about git-stash. I quote the
original mail here in case anyone wants to look into it (not sure if
it's actually reported here before, I don't pay much attention to
git-stash mails)
-- 8< --
Bad news, everyone!
When a stash contains changes for several files, and "stash pop"
encounters conflicts only in some of them, the rest of the files are
stages automatically.
At least, that happens with Git 2.1.0 on my machine, and some
commenters here: http://stackoverflow.com/a/1237337/615245
So then when we unstage the files which had conflicts after resolving
those, the result is mixed. Which doesn't look right.
What shall we do? Unstage the automatically-staged files? Revert the
changes from this bug? It seems Git really wants the changes staged
after the conflict resolution.
-- 8< --
[1] http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=45651154473c7d2f16230da765d034ecfde7968a
[2] https://lists.gnu.org/archive/html/bug-gnu-emacs/2015-05/msg00433.html
--
Duy
^ permalink raw reply
* Re: [PATCH v1 00/19] Add configuration options for split-index
From: Duy Nguyen @ 2016-10-26 9:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, Git Mailing List,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <xmqqk2cws5t6.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 12:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Timestamps allow us to say, ok this base index file has not been read
>> by anybody for N+ hours (or better, days), it's most likely not
>> referenced by any temporary index files (including
>> $GIT_DIR/index.lock) anymore because those files, by the definition of
>> "temporary", must be gone by now....
>
> and if we guessed wrong, users will have a "temporary index" that
> they meant to keep for longer term that is now broken here. I am
> not sure if that risk is worth taking.
Even if we ignore user index files (by forcing them all to be stored
in one piece), there is a problem with the special temporary file
index.lock, which must use split-index because it will become the new
index. Handling race conditions could be tricky with ref counting.
Timestamps help in this regard.
--
Duy
^ permalink raw reply
* [PATCH] Documentation/git-diff: document git diff with 3+ commits
From: Michael J Gruber @ 2016-10-26 9:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <CA+P7+xq1i8AtQ7i=1m_n9HTSL10kFUFBn8jvNcB_t_6Rh29u4w@mail.gmail.com>
That one is difficult to discover but super useful, so document it:
Specifying 3 or more commits makes git diff switch to combined diff.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Notes:
Note that we have the following now:
'git diff A B' displays 'B minus A'
'git diff A B C' displays 'A minus B' and 'A minus C'
While I know why, that (the implicit '-R' seems unfortunate).
(NB: One has to use '-c' if A is an actual merge commiti, it seems.)
If M is a merge base for A and B, we have:
'git diff A..B' equivalent to 'git diff A B'
in contrast to 'git log A..B' listing commits between M and B only
(without the commits between M and A unless they are "in" B).
I would expect 'git diff M B' here.
'git diff A...B' is equivalent to 'git diff M B'
in contrast to 'git log A...B' listing commits between M and A (marked left)
as well as commits between M and B (marked right).
I would expect 'git diff -c -R M A B' here.
Somehow the positive and negative ends of these ranges don't correspond well
with thinking about diffs as differences between these ends.
[I'm not exact with my use of "between" regarding boundary commits.]
Documentation/git-diff.txt | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index bbab35fcaf..2047318a27 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -12,6 +12,7 @@ SYNOPSIS
'git diff' [options] [<commit>] [--] [<path>...]
'git diff' [options] --cached [<commit>] [--] [<path>...]
'git diff' [options] <commit> <commit> [--] [<path>...]
+'git diff' [options] <commit> <commit> <commit> [<commit>...]
'git diff' [options] <blob> <blob>
'git diff' [options] [--no-index] [--] <path> <path>
@@ -75,9 +76,16 @@ two blob objects, or changes between two files on disk.
"git diff $(git-merge-base A B) B". You can omit any one
of <commit>, which has the same effect as using HEAD instead.
+'git diff' [options] <commit> <commit> <commit> [<commit>...]::
+
+ This is to view a combined diff between the first <commit>
+ and the remaining ones, just like viewing a combined diff
+ for a merge commit (see below) where the first <commit>
+ is the merge commit and the remaining ones are the parents.
+
Just in case if you are doing something exotic, it should be
noted that all of the <commit> in the above description, except
-in the last two forms that use ".." notations, can be any
+in the two forms that use ".." notations, can be any
<tree>.
For a more complete list of ways to spell <commit>, see
--
2.10.1.723.g0f00470
^ permalink raw reply related
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Johannes Schindelin @ 2016-10-26 8:52 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, bmwill, pclouds
In-Reply-To: <20161022233225.8883-28-sbeller@google.com>
Hi Stefan,
On Sat, 22 Oct 2016, Stefan Beller wrote:
> @@ -46,6 +47,19 @@ struct git_attr {
> static int attr_nr;
> static struct git_attr *(git_attr_hash[HASHSIZE]);
>
> +#ifndef NO_PTHREADS
> +
> +static pthread_mutex_t attr_mutex;
> +#define attr_lock() pthread_mutex_lock(&attr_mutex)
> +#define attr_unlock() pthread_mutex_unlock(&attr_mutex)
This mutex is never initialized. That may work on the system you tested,
but it is incorrect, and it does segfault on Windows. A lot.
I need *at least* something like this to make it stop crashing all over
the test suite:
-- snipsnap --
diff --git a/attr.c b/attr.c
index d5a6aa9..6933504 100644
--- a/attr.c
+++ b/attr.c
@@ -50,7 +50,16 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);
#ifndef NO_PTHREADS
static pthread_mutex_t attr_mutex;
-#define attr_lock()pthread_mutex_lock(&attr_mutex)
+static inline void attr_lock(void)
+{
+ static int initialized;
+
+ if (!initialized) {
+ pthread_mutex_init(&attr_mutex, NULL);
+ initialized = 1;
+ }
+ pthread_mutex_lock(&attr_mutex);
+}
#define attr_unlock()pthread_mutex_unlock(&attr_mutex)
#else
^ permalink raw reply related
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Jeff King @ 2016-10-26 4:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Lars Schneider, Eric Wong, Johannes Schindelin
In-Reply-To: <20161025181621.4201-3-gitster@pobox.com>
On Tue, Oct 25, 2016 at 11:16:20AM -0700, Junio C Hamano wrote:
> diff --git a/sha1_file.c b/sha1_file.c
> index 5d2bcd3ed1..09045df1dc 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1571,12 +1571,17 @@ int git_open(const char *name)
> if (fd >= 0)
> return fd;
>
> - /* Might the failure be due to O_NOATIME? */
> - if (errno != ENOENT && sha1_file_open_flag) {
> - sha1_file_open_flag = 0;
> + /* Try again w/o O_CLOEXEC: the kernel might not support it */
> + if ((sha1_file_open_flag & O_CLOEXEC) && errno == EINVAL) {
> + sha1_file_open_flag &= ~O_CLOEXEC;
> continue;
> }
So if we start with O_CLOEXEC|O_NOATIME, we drop CLOEXEC here and try
again with just O_NOATIME. And then if _that_ fails...
> + /* Might the failure be due to O_NOATIME? */
> + if (errno != ENOENT && (sha1_file_open_flag & O_NOATIME)) {
> + sha1_file_open_flag &= ~O_NOATIME;
> + continue;
> + }
We drop O_NOATIME, and end up with an empty flag field.
But we will never have tried just O_CLOEXEC, which might have worked.
Because of the order here, this would not be a regression (i.e., any
system that used to work will still eventually find a working comb), but
it does mean that systems without O_NOATIME do not get the benefit of
the new O_CLOEXEC protection.
Unfortunately, I think covering all of the cases would be 2^nr_flags.
That's only 4 right now, but it does not bode well as a pattern.
I'm not sure it's worth worrying about or not; I don't know which
systems are actually lacking either of the flags, or if they tend to
have both.
-Peff
^ permalink raw reply
* A bug with "git svn show-externals"
From: Tao Peng @ 2016-10-25 23:53 UTC (permalink / raw)
To: git
Hi there,
I met a bug of the "git svn show-externals” command. If a subdirectory item has a svn:externals property, and the format of the property is “URL first, then the local path”, running "git svn show-externals” command at the root level will result in an unusable output.
Example:
$ svn pg svn:externals svn+ssh://src.foo.com/svn/ref/English.lproj/
svn+ssh://src.foo.com/svn/orig/trunk/Resources/English.lproj/Localizable.strings Localizable.strings
$ git svn show-externals
# /English.lproj/
/English.lproj/svn+ssh://src.foo.com/svn/orig/trunk/Resources/English.lproj/Localizable.strings Localizable.strings
This bug is preventing my script from correctly finishing the svn-to-git repo migration work. Does anyone know a workaround to this bug?
Thanks,
Peng
^ permalink raw reply
* Re: [PATCH] Allow stashes to be referenced by index only
From: Ramsay Jones @ 2016-10-26 0:12 UTC (permalink / raw)
To: Junio C Hamano, Aaron M Watson
Cc: git, Jon Seymour, David Caldwell, Øystein Walle, Jeff King,
Ævar Arnfjörð Bjarmason, David Aguilar,
Alex Henrie
In-Reply-To: <xmqqshrkqf79.fsf@gitster.mtv.corp.google.com>
On 25/10/16 22:41, Junio C Hamano wrote:
> Aaron M Watson <watsona4@gmail.com> writes:
>
> Aaron M Watson <watsona4@gmail.com> writes:
>
>> Instead of referencing "stash@{n}" explicitly, it can simply be
>> referenced as "n".
>> Most users only reference stashes by their position
>> in the stash stask (what I refer to as the "index").
>
> It is unclear if the first sentence is a statement of the fact, an
> expression of desire, or something else. With the current codebase,
> it cannot simply be referenced as "n", and you either "wish it were
> possible", or "make it possible to do so", or perhaps little bit of
> both.
>
> This is why we tend to use imperative mood to give an order to the
> codebase to "be like so" to make it clear.
>
> Perhaps
>
> Instead of referencing "stash@{n}" explicitly, make it possible to
> simply reference as "n". Most users only reference stashes by their
> position in the stash stask (what I refer to as the "index" here).
s/stask/stack/
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] reset: --unmerge
From: Junio C Hamano @ 2016-10-25 23:28 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8D4cT2EV_t6=+XTHxLmjV+NtAE+KgiN3ZiP7JQzvu29oQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> BTW making git-add (and "git commit -a") refuse files with conflict
> markers present could prevent this mistake earlier and is probably a
> better option because the user won't have to discover 'reset
> --unmerge'.
That may help some users, but you are solving a different problem.
I do not say "save" unless I know the editor buffer contents is not
ready. "This is ready to be saved" however is different from "This
resolution is correct", and I need the unmerged states in the index
to verify, namely by looking at "git diff" (no other parameters)
output that shows only the paths with unmerged stages and in the
compact combined diff format.
Somebody with a bright idea decided that vc-git-resolve-conflicts
variable should be on by default in Emacs 25.1 X-<, which causes
"save" after resolving conflicts to automatically run "git add", to
destroy this valuable tool. My knee-jerk reaction, of course, to
such a default is "that's brain dead", but perhaps they did so for
some good reason that I fail to fathom.
^ permalink raw reply
* Re: [PATCH v3 0/3] quick reroll of Lars's git_open() w/ O_CLOEXEC
From: Junio C Hamano @ 2016-10-25 22:56 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, Eric Wong, Johannes Schindelin, Johannes Sixt
In-Reply-To: <A6C42931-835D-42FD-B4D9-7F84921909E2@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
> heavy CPU/network task in the background now (I can't/don't want to
> stop that ATM).
>
> Here is the relevant part of the log:
>
> ++ rm -f rot13-filter.log
> ++ git checkout --quiet --no-progress .
> + test_eval_ret_=255
> + want_trace
> + test t = t
> + test t = t
> + set +x
> error: last command exited with $?=255
>
> Looks like Git exists with 255. Any idea why that might happen?
Other than "the --quiet above may be hiding it?", I do not
immediately have a useful guess, sorry.
> @Dscho/Johannes: Can you try this on your Windows machines?
>
> Thanks,
> Lars
^ permalink raw reply
* Re: [PATCH v3 3/3] read-cache: make sure file handles are not inherited by child processes
From: Junio C Hamano @ 2016-10-25 22:54 UTC (permalink / raw)
To: Eric Wong; +Cc: git, Lars Schneider, Johannes Schindelin
In-Reply-To: <20161025213318.GB8683@starla>
Eric Wong <e@80x24.org> writes:
> But I have a _slight_ preference towards Dscho's version in
> <alpine.DEB.2.20.1610251230150.3264@virtualbox> in case we
> decide to start using another O_* flag in here.
Interesting. The reason why I have a slight preferene to separate
the fixed part and the toggle-able part is exactly because I want
the code to be prepared in case we decide to start using other O_*
flags in the future.
I guess different people have different tastes, as usual ;-)
^ permalink raw reply
* Re: [PATCH v3 0/3] quick reroll of Lars's git_open() w/ O_CLOEXEC
From: Lars Schneider @ 2016-10-25 21:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong, Johannes Schindelin, Johannes Sixt
In-Reply-To: <20161025181621.4201-1-gitster@pobox.com>
> On 25 Oct 2016, at 20:16, Junio C Hamano <gitster@pobox.com> wrote:
>
> Here is to make sure everybody is on the same page regarding the
> series. The patches are adjusted for comments from Eric and Dscho.
Thank you, Junio! Your v3 looks good to me and I compiled and tested
it on Windows.
There is one catch though:
I ran the tests multiple times on Windows and I noticed that the
following test seems flaky:
t0021 `required process filter should be used only for "clean" operation only'
This flaky-ness was not introduced by your v3. I was able to reproduce
the same behavior for v2. I wonder why I did not noticed that before.
The only difference to before is that my Windows machines does some
heavy CPU/network task in the background now (I can't/don't want to
stop that ATM).
Here is the relevant part of the log:
++ rm -f rot13-filter.log
++ git checkout --quiet --no-progress .
+ test_eval_ret_=255
+ want_trace
+ test t = t
+ test t = t
+ set +x
error: last command exited with $?=255
Looks like Git exists with 255. Any idea why that might happen?
@Dscho/Johannes: Can you try this on your Windows machines?
Thanks,
Lars
^ permalink raw reply
* Re: [PATCH v2 2/2] read-cache: make sure file handles are not inherited by child processes
From: Johannes Sixt @ 2016-10-25 21:39 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, Johannes.Schindelin, e, jnareb, gitster
In-Reply-To: <4C96209A-756F-45F1-B075-037FE32B3291@gmail.com>
Am 24.10.2016 um 21:53 schrieb Lars Schneider:
>
>> On 24 Oct 2016, at 21:22, Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> Am 24.10.2016 um 20:03 schrieb larsxschneider@gmail.com:
>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>
>>> This fixes "convert: add filter.<driver>.process option" (edcc8581) on
>>> Windows.
>>
>> Today's next falls flat on its face on Windows in t0021.15
>> "required process filter should filter data"; might it be the
>> failure meant here?(I haven't dug deeper, yet.)
>
> Yes, this is the failure meant here :-)
I trust your word and Dscho's that it fixes a failure on Windows. In my
case, however, it was an outdated perl (5.8.8) which left a message
along the lines of "lookup of member flush in IO::Handle failed" in one
of the *.log files. I came up with the following workaround.
Fix-method: shot-in-the-dark
Perl-foo: not-present
Not-signed-off-by: Me
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
index ae4c50f..bb88c5f 100755
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -22,6 +22,7 @@
use strict;
use warnings;
+use FileHandle;
my $MAX_PACKET_CONTENT_SIZE = 65516;
my @capabilities = @ARGV;
--
2.10.0.343.g37bc62b
^ permalink raw reply related
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