* Re: [PATCH v2] help: improve is_executable() on Windows
From: Junio C Hamano @ 2017-01-30 17:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Heiko Voigt
In-Reply-To: <4b93fe44ff9020ed80e4fd93a24a6ffa647e7678.1485780050.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> From: Heiko Voigt <hvoigt@hvoigt.net>
>
> On Windows, executables need to have the file extension `.exe`, or they
> are not executables. Hence, to support scripts, Git for Windows also
> looks for a she-bang line by opening the file in question, and executing
> it via the specified script interpreter.
>
> To figure out whether files in the `PATH` are executable, `git help` has
> code that imitates this behavior. With one exception: it *always* opens
> the files and looks for a she-bang line *or* an `MZ` tell-tale
> (nevermind that files with the magic `MZ` but without file extension
> `.exe` would still not be executable).
>
> Opening this many files leads to performance problems that are even more
> serious when a virus scanner is running. Therefore, let's change the
> code to look for the file extension `.exe` early, and avoid opening the
> file altogether if we already know that it is executable.
Much more readable than the initial round. Will queue; thanks.
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-30 16:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1701301643260.3469@virtualbox>
Am 30.01.2017 um 17:01 schrieb Johannes Schindelin:
> Hi René,
>
> On Sat, 28 Jan 2017, René Scharfe wrote:
>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 87237b092b..66cd466eea 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -527,6 +527,16 @@ static inline int ends_with(const char *str, const char *suffix)
>> return strip_suffix(str, suffix, &len);
>> }
>>
>> +#define SWAP(a, b) do { \
>> + void *_swap_a_ptr = &(a); \
>> + void *_swap_b_ptr = &(b); \
>> + unsigned char _swap_buffer[sizeof(a)]; \
>> + memcpy(_swap_buffer, _swap_a_ptr, sizeof(a)); \
>> + memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) + \
>> + BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \
>> + memcpy(_swap_b_ptr, _swap_buffer, sizeof(a)); \
>> +} while (0)
>> +
>> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
>
> It may seem as a matter of taste, or maybe not: I prefer this without the
> _swap_a_ptr (and I would also prefer not to use identifiers starting with
> an underscore, as section 7.1.3 Reserved Identifiers of the C99 standard
> says they are reserved):
>
> +#define SWAP(a, b) do { \
> + unsigned char swap_buffer_[sizeof(a)]; \
> + memcpy(swap_buffer_, &(a), sizeof(a)); \
> + memcpy(&(a), &(b), sizeof(a) + \
> + BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \
> + memcpy(&(b), swap_buffer_, sizeof(a)); \
> +} while (0)
We can move the underscore to the end, but using a and b directly will
give surprising results if the parameters have side effects. E.g. if
you want to swap the first two elements of two arrays you might want to
do this:
SWAP(*x++, *y++);
SWAP(*x++, *y++);
And that would increment twice as much as one would guess and access
unexpected elements.
> One idea to address the concern that not all C compilers people use to
> build Git may optimize away those memcpy()s: we could also introduce a
> SWAP_PRIMITIVE_TYPE (or SWAP2 or SIMPLE_SWAP or whatever) that accepts
> only primitive types. But since __typeof__() is not portable...
I wouldn't worry too much about such a solution before seeing that SWAP
(even with memcpy(3) -- this function is probably optimized quite
heavily on most platforms) causes an actual performance problem.
René
^ permalink raw reply
* Re: [PATCH v3] mingw: allow hooks to be .exe files
From: Junio C Hamano @ 2017-01-30 16:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jeff King, Stefan Beller
In-Reply-To: <78a73c9d0a8e38fcc61302d0495533dcc4fab076.1485779272.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Executable files in Windows need to have the extension '.exe', otherwise
> they do not work. Extend the hooks to not just look at the hard coded
> names, but also at the names extended by the custom STRIP_EXTENSION,
> which is defined as '.exe' in Windows.
Will replace, and looks good enough for 'next'. Let's stop
iterating and go incremental if/as needed.
Thanks.
^ permalink raw reply
* Re: git-daemon shallow checkout fail
From: Johannes Schindelin @ 2017-01-30 16:52 UTC (permalink / raw)
To: Bob Proulx; +Cc: git
In-Reply-To: <20170129002932.GA19359@dismay.proulx.com>
Hi Bob,
On Sat, 28 Jan 2017, Bob Proulx wrote:
> And the server side says:
>
> [26071] Request upload-pack for '/test-project.git'
> [26071] fatal: Unable to create temporary file '/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
> [26055] [26071] Disconnected (with error)
Assuming that you can rebuild your Git with debug symbols and without
optimization (simply remove the -O2 from CFLAGS in the Makefile, I never
had any luck with single-stepping in gdb when compiled with -O2), you
could attach gdb to the git-daemon and/or upload-pack process. Setting a
breakpoint on die_builtin in the failing process should give you a good
idea why things are failing, at least looking at the stacktrace.
A few more tidbits from a cursory look at the Git source code with `git
grep` and the likes:
- that error message comes from shallow.c's setup_temporary_shallow()
function
- that function is only called from fetch-pack and receive-pack, neither
of which should be called by upload-pack, so it is a puzzle
- adding a test case to t5570-git-daemon.sh that tests specifically your
described scenario seems *not* to fail:
-- snip --
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 225a022e8a..0256c9aded 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -186,5 +186,17 @@ test_expect_success 'hostname cannot break out of directory' '
git clone --bare "$GIT_DAEMON_URL/escape.git" tmp.git
'
+test_expect_success POSIXPERM 'shallow clone from read-only server' '
+ test_when_finished "rm -rf tmp.git" &&
+ repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/readonly.git" &&
+ git init --bare "$repo" &&
+ git push "$repo" HEAD &&
+ >"$repo"/git-daemon-export-ok &&
+ chmod a-w "$repo" &&
+ test_must_fail \
+ env GIT_OVERRIDE_VIRTUAL_HOST=.. \
+ git clone --depth 1 "$GIT_DAEMON_URL/readonly.git" tmp.git
+'
+
stop_git_daemon
test_done
-- snap --
- I even modified t/lib-git-daemon.sh to start the daemon as `nobody` and
kill it as `root`, and I won't share that patch because it is as
ugly, but *even then* the test succeeded.
So my suspicion is that the repository you try to serve may already be
shallow, or something else funky is going on that has not been included in
your report.
The most direct way to get to the bottom of this may be to do something
like this:
-- snip --
diff --git a/shallow.c b/shallow.c
index 11f7dde9d9..30f5c96d50 100644
--- a/shallow.c
+++ b/shallow.c
@@ -288,12 +288,18 @@ int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
static struct tempfile temporary_shallow;
+static int debug_me;
+
const char *setup_temporary_shallow(const struct sha1_array *extra)
{
struct strbuf sb = STRBUF_INIT;
int fd;
if (write_shallow_commits(&sb, 0, extra)) {
+error("About to create shallow_XXXXXX: pid = %d", getpid());
+while (!debug_me) {
+ sleep(1);
+}
fd = xmks_tempfile(&temporary_shallow, git_path("shallow_XXXXXX"));
if (write_in_full(fd, sb.buf, sb.len) != sb.len)
-- snap --
Then let it run, wait for the error message "About to create
shallow_XXXXXX" and then attach with a gdb started as nobody via `attach
<pid>` to see the stack trace.
That should give you an idea where that code path is hit (unexpectedly).
Ciao,
Johannes
^ permalink raw reply related
* Re: [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-30 16:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1701301637570.3469@virtualbox>
Am 30.01.2017 um 16:39 schrieb Johannes Schindelin:
> Hi René,
>
> On Sat, 28 Jan 2017, René Scharfe wrote:
>
>> Add a macro for exchanging the values of variables. It allows users to
>> avoid repetition and takes care of the temporary variable for them. It
>> also makes sure that the storage sizes of its two parameters are the
>> same. Its memcpy(1) calls are optimized away by current compilers.
>
> How certain are we that "current compilers" optimize that away? And about
> which "current compilers" are we talking? GCC? GCC 6? Clang? Clang 3?
> Clang 3.8.*? Visual C 2005+?
GCC 4.4.7 and clang 3.2 on x86-64 compile the macro to the same object
code as a manual swap ; see https://godbolt.org/g/F4b9M9. Both were
released 2012. That website doesn't offer Visual C(++), but since you
added the original macro in e5a94313c0 ("Teach git-apply about '-R'",
2006) I guess we have that part covered. ;)
René
^ permalink raw reply
* Re: [PATCH 5/5] graph: use SWAP macro
From: Johannes Schindelin @ 2017-01-30 16:16 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <af5a7205-7703-f5ad-4ea2-b20ab4c01c80@web.de>
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> Exchange the values of graph->columns and graph->new_columns using the
> macro SWAP instead of hand-rolled code. The result is shorter and
> easier to read.
>
> This transformation was not done by the semantic patch swap.cocci
> because there's an unrelated statement between the second and the last
> step of the exchange, so it didn't match the expected pattern.
Is it really true that Coccinelle cannot be told to look for a code block
that declares a variable that is then used *only* in the lines we want to
match and replace?
I never used the tool, and a quick web search did not clarify the picture,
either...
Ciao,
Dscho
^ permalink raw reply
* gitconfig get out of sync with submodule entries on branch switch
From: Benjamin Schindler @ 2017-01-30 16:21 UTC (permalink / raw)
To: git
Hi
Consider the following usecase: I have the master branch where I have a
submodule A. I create a branch where I rename the submodule to be in the
directory B. After doing all of this, everything looks good.
Now, I switch back to master. The first oddity is, that it fails to
remove the folder B because there are still files in there:
bschindler@metis ~/Projects/submodule_test (testbranch) $ git checkout
master
warning: unable to rmdir other_submodule: Directory not empty
Switched to branch 'master'
Git submodule deinit on B fails because the submodule is not known to
git anymore (after all, the folder B exists only in the other branch). I
can easily just remove the folder B from disk and initialize the
submodule A again, so all seems good.
However, what is not good is that the submodule b is still known in
.git/config. This is in particular a problem for us, because I know a
number of tools which use git config to retrieve the submodule list. Is
it therefore a bug that upon branch switch, the submodule gets
deregistered, but its entry in .git/config remains?
thanks a lot
Benjamin Schindler
P.s. I did not subscribe to the mailing list, please add me at least do
CC. Thanks
^ permalink raw reply
* Re: [PATCH 4/5] diff: use SWAP macro
From: Johannes Schindelin @ 2017-01-30 16:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <84944ecd-d14e-b5e9-7566-9ab2b68c02fb@web.de>
[-- Attachment #1: Type: text/plain, Size: 780 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> Use the macro SWAP to exchange the value of pairs of variables instead
> of swapping them manually with the help of a temporary variable. The
> resulting code is shorter and easier to read.
>
> The two cases were not transformed by the semantic patch swap.cocci
> because it's extra careful and handles only cases where the types of all
> variables are the same -- and here we swap two ints and use an unsigned
> temporary variable for that. Nevertheless the conversion is safe, as
> the value range is preserved with and without the patch.
One way to make this more obvious would be to change the type to signed
first, and then transform (which then would catch these cases too,
right?).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/5] use SWAP macro
From: Johannes Schindelin @ 2017-01-30 16:03 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <187c2b39-40cf-7e07-b489-d40cdf5f9145@web.de>
[-- Attachment #1: Type: text/plain, Size: 661 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> index 806dd7a885..8ce00480cd 100644
> --- a/builtin/diff-tree.c
> +++ b/builtin/diff-tree.c
> @@ -147,9 +147,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
> tree1 = opt->pending.objects[0].item;
> tree2 = opt->pending.objects[1].item;
> if (tree2->flags & UNINTERESTING) {
> - struct object *tmp = tree2;
> - tree2 = tree1;
> - tree1 = tmp;
> + SWAP(tree2, tree1);
> }
Is there a way to transform away the curly braces for blocks that become
single-line blocks, too?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Johannes Schindelin @ 2017-01-30 16:01 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <0bdb58a6-3a7f-2218-4b70-c591ae90e95e@web.de>
[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 87237b092b..66cd466eea 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -527,6 +527,16 @@ static inline int ends_with(const char *str, const char *suffix)
> return strip_suffix(str, suffix, &len);
> }
>
> +#define SWAP(a, b) do { \
> + void *_swap_a_ptr = &(a); \
> + void *_swap_b_ptr = &(b); \
> + unsigned char _swap_buffer[sizeof(a)]; \
> + memcpy(_swap_buffer, _swap_a_ptr, sizeof(a)); \
> + memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) + \
> + BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \
> + memcpy(_swap_b_ptr, _swap_buffer, sizeof(a)); \
> +} while (0)
> +
> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
It may seem as a matter of taste, or maybe not: I prefer this without the
_swap_a_ptr (and I would also prefer not to use identifiers starting with
an underscore, as section 7.1.3 Reserved Identifiers of the C99 standard
says they are reserved):
+#define SWAP(a, b) do { \
+ unsigned char swap_buffer_[sizeof(a)]; \
+ memcpy(swap_buffer_, &(a), sizeof(a)); \
+ memcpy(&(a), &(b), sizeof(a) + \
+ BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \
+ memcpy(&(b), swap_buffer_, sizeof(a)); \
+} while (0)
One idea to address the concern that not all C compilers people use to
build Git may optimize away those memcpy()s: we could also introduce a
SWAP_PRIMITIVE_TYPE (or SWAP2 or SIMPLE_SWAP or whatever) that accepts
only primitive types. But since __typeof__() is not portable...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Johannes Schindelin @ 2017-01-30 15:39 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <0bdb58a6-3a7f-2218-4b70-c591ae90e95e@web.de>
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> Add a macro for exchanging the values of variables. It allows users to
> avoid repetition and takes care of the temporary variable for them. It
> also makes sure that the storage sizes of its two parameters are the
> same. Its memcpy(1) calls are optimized away by current compilers.
How certain are we that "current compilers" optimize that away? And about
which "current compilers" are we talking? GCC? GCC 6? Clang? Clang 3?
Clang 3.8.*? Visual C 2005+?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-30 15:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <xmqq4m0kz65d.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This patch automates the process of determining which tests failed
> > previously and re-running them.
> > ...
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> I stored both versions in files and compared them, and it seems the
> single word change in the proposed commit log message is the only
> difference. I would have written "Automate the process...", though.
Yes, we have different styles. Thanks for letting my commit keep my commit
message this time ;-)
> If you are resending, touching up to cover all points raised by a
> reviewer and doing nothing else, having "Reviewed-by: Jeff King
> <peff@peff.net>" would have been nicer.
TBH I am not at all sure that I know when to add those footers and when
not. After having been asked to remove such a footer, I decided to *not*
include them by default.
Having gray zones about the footers strikes me as similar to having gray
zones in the coding style guidelines: it sure gives the contributors more
freedom, but it also creates uncertainty and as a consequence takes up a
lot of reviewing space and time (hence taking away space and time from
reviewing the code for bugs).
In other words: while I appreciate the idea of giving contributors such as
myself a lot of leeway, I would love even more to be able to automate away
tedious and boring tasks (such as adding Tested-by: or Reviewed-by:
footers, or for that matter, addressing code style issues before any
reviewer has to shed bikes so that they can focus on the parts of the
review that no machine can do for them).
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] help: correct behavior for is_executable on Windows
From: Johannes Schindelin @ 2017-01-30 12:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Heiko Voigt
In-Reply-To: <xmqqd1f8z6lt.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > From: Heiko Voigt <hvoigt@hvoigt.net>
> >
> > The previous implementation said that the filesystem information on
> > Windows is not reliable to determine whether a file is executable. To
> > gather this information it was peeking into the first two bytes of a
> > file to see whether it looks executable.
> >
> > Apart from the fact that on Windows executables are defined as such by
> > their extension (and Git has special code to support "executing"
> > scripts when they have a she-bang line) it leads to serious
> > performance problems: not only do we have to open many files now, it
> > gets even slower when a virus scanner is running.
>
> Heiko, around here (before going into the details of how severe the
> problem is and how wonderful the result applying of this change is) is
> the best place to summarize the solution. E.g.
>
> Because the definition of "executable" on Windows is based
> on the file extension, update the function to declare that a
> file with ".exe" extension without opening and reading the
> early bytes from it. This avoids serious performance issues.
>
> I paraphrased the rest only so that the description of the solution
> (i.e. "instead of opening and peeking, we trust .exe suffix") fits well
> in the surrounding text; the important part is to say what the change
> does clearly.
I adjusted the commit message. It was tweaked a little differently from
what you suggested, as I preferred to condense the information a bit more.
> I agree with the reasoning and the execution of the patch, except
> that
>
> - "correct behaviour" in the title makes it appear that this is a
> correctness thing, but this is primarily a performance fix.
Primarily. But not only. The magic `MZ` without the file extension `.exe`
is pretty useless, as the file could not be executed, still.
To avoid further turnaround, though, I also edited the contentious
"correct" to read "improve" now.
> - It is a bit strange that "MZ" is dropped in the same patch
> without any mention.
I fixed that in the commit message.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] checkout: convert post_checkout_hook() to struct object_id
From: Johannes Schindelin @ 2017-01-30 13:01 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano, brian m. carlson
In-Reply-To: <b30e5d34-436a-af5f-dbad-b1df464bf303@web.de>
[-- Attachment #1: Type: text/plain, Size: 180 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
These three SHA-1 -> OID patches all appear correct to me.
Ciao,
Dscho
^ permalink raw reply
* [PATCH v2] help: improve is_executable() on Windows
From: Johannes Schindelin @ 2017-01-30 12:40 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Junio C Hamano
In-Reply-To: <c1c6ccae4e60608259809914e8ff3d3d5e1ead5a.1485524999.git.johannes.schindelin@gmx.de>
From: Heiko Voigt <hvoigt@hvoigt.net>
On Windows, executables need to have the file extension `.exe`, or they
are not executables. Hence, to support scripts, Git for Windows also
looks for a she-bang line by opening the file in question, and executing
it via the specified script interpreter.
To figure out whether files in the `PATH` are executable, `git help` has
code that imitates this behavior. With one exception: it *always* opens
the files and looks for a she-bang line *or* an `MZ` tell-tale
(nevermind that files with the magic `MZ` but without file extension
`.exe` would still not be executable).
Opening this many files leads to performance problems that are even more
serious when a virus scanner is running. Therefore, let's change the
code to look for the file extension `.exe` early, and avoid opening the
file altogether if we already know that it is executable.
See the following measurements (in seconds) as an example, where we
execute a simple program that simply lists the directory contents and
calls open() on every listed file:
With virus scanner running (coldcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.412873
before open (git-annotate.exe): 0.000175
after open (git-annotate.exe): 0.397925
before open (git-apply.exe): 0.000243
after open (git-apply.exe): 0.399996
before open (git-archive.exe): 0.000147
after open (git-archive.exe): 0.397783
before open (git-bisect--helper.exe): 0.000160
after open (git-bisect--helper.exe): 0.397700
before open (git-blame.exe): 0.000160
after open (git-blame.exe): 0.399136
...
With virus scanner running (hotcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.000325
before open (git-annotate.exe): 0.000229
after open (git-annotate.exe): 0.000177
before open (git-apply.exe): 0.000167
after open (git-apply.exe): 0.000150
before open (git-archive.exe): 0.000154
after open (git-archive.exe): 0.000156
before open (git-bisect--helper.exe): 0.000132
after open (git-bisect--helper.exe): 0.000180
before open (git-blame.exe): 0.000718
after open (git-blame.exe): 0.000724
...
With this patch I get:
$ time git help git
Launching default browser to display HTML ...
real 0m8.723s
user 0m0.000s
sys 0m0.000s
and without
$ time git help git
Launching default browser to display HTML ...
real 1m37.734s
user 0m0.000s
sys 0m0.031s
both tests with cold cache and giving the machine some time to settle
down after restart.
[jes: adjusted the commit message]
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/help-is-exe-v2
Fetch-It-Via: git fetch https://github.com/dscho/git help-is-exe-v2
help.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/help.c b/help.c
index 53e2a67e00..bc6cd19cf3 100644
--- a/help.c
+++ b/help.c
@@ -105,7 +105,22 @@ static int is_executable(const char *name)
return 0;
#if defined(GIT_WINDOWS_NATIVE)
-{ /* cannot trust the executable bit, peek into the file instead */
+ /*
+ * On Windows there is no executable bit. The file extension
+ * indicates whether it can be run as an executable, and Git
+ * has special-handling to detect scripts and launch them
+ * through the indicated script interpreter. We test for the
+ * file extension first because virus scanners may make
+ * it quite expensive to open many files.
+ */
+ if (ends_with(name, ".exe"))
+ return S_IXUSR;
+
+{
+ /*
+ * Now that we know it does not have an executable extension,
+ * peek into the file instead.
+ */
char buf[3] = { 0 };
int n;
int fd = open(name, O_RDONLY);
@@ -113,8 +128,8 @@ static int is_executable(const char *name)
if (fd >= 0) {
n = read(fd, buf, 2);
if (n == 2)
- /* DOS executables start with "MZ" */
- if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
+ /* look for a she-bang */
+ if (!strcmp(buf, "#!"))
st.st_mode |= S_IXUSR;
close(fd);
}
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH] fixup! worktree move: new command
From: Johannes Schindelin @ 2017-01-30 12:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqwpdgz8zq.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> The tip of 'pu' (or anything beyond the tip of 'jch') is not always
> expected to pass test or even build, [...]
This makes `pu` a lot less useful than it could be.
And we could easily improve the situation simply by changing the rule ever
so slightly: when a build, or a test, fails in `pu` and there exists a
fix, this fix should go into `pu` ASAP.
As you point out later in your mail, the fixup! or SQUASH! commit is a
very convenient reminder that a particular branch is still "under
construction". That is, changing the rule as I proposed above will not
only help the Continuous Integration [*1*] to avoid reporting duplicates,
it will also help us improve the project faster.
Ciao,
Johannes
Footnote *1*: It appears that there may be the misconception floating
around that Continuous Integration is designed to annoy developers by
pointing out unportable or unbuildable code. Once you realize, though,
that it detects and reports code that is below our existing code's
quality, no doubt you will agree that it is a convenient tool to relieve
reviewers from tedious work that can be done by a machine as well.
^ permalink raw reply
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-30 12:29 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Jeff King, git@vger.kernel.org
In-Reply-To: <CAGZ79kaa5WJmZkyFROfkfNb3++t37qAuAebKJTon2iD2bh+sWw@mail.gmail.com>
Hi Stefan,
On Fri, 27 Jan 2017, Stefan Beller wrote:
> On Fri, Jan 27, 2017 at 2:29 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Junio,
> >
> > On Thu, 26 Jan 2017, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > On Wed, 25 Jan 2017, Jeff King wrote:
> >> >
> >> > v2 coming,
>
> The commit message, while technically correct, seems a bit off. This is
> because the commit message only talks about .exe extensions, but the
> change in code doesn't even have the string "exe" mentioned once.
>
> With this discussion here, it is easy for me to connect the dots, but it
> would be nice to have the full picture in the commit message.
I just sent out v3, using a slightly tweaked version of the commit message
you proposed.
Ciao,
Dscho
^ permalink raw reply
* [PATCH v3] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-30 12:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Stefan Beller
In-Reply-To: <9a27b90e771d4c97dc580d344e161d7cf8d632ce.1485433248.git.johannes.schindelin@gmx.de>
Executable files in Windows need to have the extension '.exe', otherwise
they do not work. Extend the hooks to not just look at the hard coded
names, but also at the names extended by the custom STRIP_EXTENSION,
which is defined as '.exe' in Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/exe-as-hook-v3
Fetch-It-Via: git fetch https://github.com/dscho/git exe-as-hook-v3
run-command.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index 73bfba7ef9..5227f78aea 100644
--- a/run-command.c
+++ b/run-command.c
@@ -871,8 +871,14 @@ const char *find_hook(const char *name)
strbuf_reset(&path);
strbuf_git_path(&path, "hooks/%s", name);
- if (access(path.buf, X_OK) < 0)
+ if (access(path.buf, X_OK) < 0) {
+#ifdef STRIP_EXTENSION
+ strbuf_addstr(&path, STRIP_EXTENSION);
+ if (access(path.buf, X_OK) >= 0)
+ return path.buf;
+#endif
return NULL;
+ }
return path.buf;
}
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Junio C Hamano @ 2017-01-30 1:00 UTC (permalink / raw)
To: Carlo Wood, Heiko Voigt, Stefan Beller; +Cc: git
In-Reply-To: <20170129203348.1a8c0722@hikaru>
Carlo Wood <carlo@alinoe.com> writes:
> there seems to be a problem with using 'git commit --amend' in
> git submodules when using 'git push --recurse-submodules=on-demand'
> in the parent.
>
> The latter fails, saying "The following submodule paths contain changes
> that can not be found on any remote:" for such submodule, even though
> the submodule is clean, pushed and reports 'Everything up-to-date'
> when trying to push it.
>
> I believe that the reason has to be that the parent repository thinks
> that the comment that was amended, but not pushed, must be on the remote
> too, while the whole point of amend is that this commit is not pushed.
I am not super familiar with the actualy implementation of the
codepaths involved in this, so CC'ed the folks who can help you
better.
I suspect the submodule folks would say it is working as intended,
if \
- you made a commit in the submodule;
- recorded the resulting commit in the superproject;
- you amended the commit in the submodule; and then
- you did "push, while pushing out in the submodule as needed" from
the superproject.
There are two commits in the submodule that are involved in the
above scenario, and the first one before amending is needed by the
other participants of the project in order for them to check out
what you are trying to push in the superproject, because that is
what the superproject's tree records. You somehow need to make that
commit available to them, but after you amended, the original commit
may no longer be reachable from any branch in your submodule, so
even if you (or the "on-demand" mechanism) pushed any and all
branches out, that would not make the needed commit available to
others. If you push your top-level superproject out in such a
situation, you would break others.
I think you have two options.
1. If the amend was done to improve things in submodule but is not
quite ready, then get rid of that amended commit and restore the
branch in the submodule to the state before you amended, i.e.
the tip of the branch will become the same commit as the one
that is recorded in the superproject. Then push the submodule
and the superproject out. After that, move the submodule branch
to point at the amended commit (or record the amended commit as
a child of the commit you pushed out).
2. If the amend is good and ready to go, "git add" to update the
superproject to make that amended result the one that is needed
in the submodule.
^ permalink raw reply
* git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Carlo Wood @ 2017-01-29 19:33 UTC (permalink / raw)
To: git
Hi,
there seems to be a problem with using 'git commit --amend' in
git submodules when using 'git push --recurse-submodules=on-demand'
in the parent.
The latter fails, saying "The following submodule paths contain changes
that can not be found on any remote:" for such submodule, even though
the submodule is clean, pushed and reports 'Everything up-to-date'
when trying to push it.
I believe that the reason has to be that the parent repository thinks
that the comment that was amended, but not pushed, must be on the remote
too, while the whole point of amend is that this commit is not pushed.
I wrote a little script that demonstrates the problem.
Please run in an empty directory.
START-OF-SCRIPT
#! /bin/bash
# This script demonstrates a bug in git where it reports
#
# The following submodule paths contain changes that can
# not be found on any remote:
#
# for a submodule that is clean and pushed.
#
# Create an empty directory, put this script in it
# and run the script.
#
# Carlo Wood, 2017/01/29
# Clean a possible previous run:
rm -rf parent remote.parent remote.subm
REMOTE_BASE="$(pwd)"
# Create a 'remote' for the submodule 'subm'.
mkdir remote.subm
pushd remote.subm
git init --bare
popd
# Create a 'remote' for the 'parent' repository.
mkdir remote.parent
pushd remote.parent
git init --bare
popd
# Create initial parent/subm directory structore.
mkdir -p parent/subm
# Create an initial subm git repository.
pushd parent/subm
git init
git remote add local "$REMOTE_BASE/remote.subm"
touch s ; git add s
git commit -m 'Initial commit.'
git push --set-upstream local master
popd
# Create an initial parent git repository with subm as submodule
# and push.recurseSubmodules = on-demand.
pushd parent
git init
git config push.recurseSubmodules on-demand
git remote add local "$REMOTE_BASE/remote.parent"
touch p ; git add p
git submodule add "$REMOTE_BASE/remote.subm" subm
git add .gitmodules subm
git commit -m 'Initial commit.'
git push --set-upstream local master
popd
# Do some commit in subm, but do not push it to the remote.
pushd parent/subm
echo "My frist commit." > s
git commit -a -m 'Change s'
popd
# Add the subm hash to the parent.
pushd parent
git add subm
git commit -m 'Updated subm.'
popd
# Amend the commit in subm (and optionally push it).
pushd parent/subm
echo "My first commit." > s
git commit -a --amend -m 'Change s'
popd
# Correct that in the parent too:
pushd parent
git add subm
git commit -m 'Updated subm.'
popd
# At this point nothing was published yes, so the
# amend shouldn't have caused a problem. But it did.
pushd parent
git push
popd
echo "THE ABOVE ERROR CAN NOW BE REPRODUCED INDEFINITELY,"
echo "FOR EXAMPLE, DO:"
echo
echo "cd parent/subm"
echo "git push"
echo "cd .."
echo "git push"
END-OF-SCRIPT
Tested with current master 4e59582ff70d299f5a88449891e78d15b4b3fabe
Regards,
Carlo
--
Carlo Wood <carlo@alinoe.com>
^ permalink raw reply
* [PATCH v2 4/4] stash: support filename argument
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
While working on a repository, it's often helpful to stash the changes
of a single or multiple files, and leave others alone. Unfortunately
git currently offers no such option. git stash -p can be used to work
around this, but it's often impractical when there are a lot of changes
over multiple files.
Add an optional filename argument to git stash push, which allows for
stashing a single (or multiple) files.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 9 +++++++++
git-stash.sh | 30 +++++++++++++++++++++++-------
t/t3903-stash.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0bce33e3fc..8306bac397 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,9 +15,13 @@ SYNOPSIS
'git stash' branch <branchname> [<stash>]
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+ [--] [<paths>...]
'git stash' clear
'git stash' create [<message>]
'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
+ [-- <paths>...]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
@@ -47,6 +51,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<paths>...]::
Save your local modifications to a new 'stash' and roll them
back both in the working tree and in the index.
@@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
only <message> does not trigger this action to prevent a misspelled
subcommand from making an unwanted stash.
+
+If the paths argument is given in 'git stash push', only these files
+are put in the new 'stash'. In addition only the indicated files are
+changed in the working tree to match the index.
++
If the `--keep-index` option is used, all changes already added to the
index are left intact.
+
diff --git a/git-stash.sh b/git-stash.sh
index 5f08b43967..0072a38b4c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -41,7 +41,7 @@ no_changes () {
untracked_files () {
excl_opt=--exclude-standard
test "$untracked" = "all" && excl_opt=
- git ls-files -o -z $excl_opt
+ git ls-files -o -z $excl_opt -- $1
}
clear_stash () {
@@ -59,6 +59,7 @@ create_stash () {
stash_msg=
untracked=
new_style=
+ files=
while test $# != 0
do
case "$1" in
@@ -72,6 +73,12 @@ create_stash () {
untracked="$1"
new_style=t
;;
+ --)
+ shift
+ files="$@"
+ new_style=t
+ break
+ ;;
*)
if test -n "$new_style"
then
@@ -134,7 +141,7 @@ create_stash () {
# Untracked files are stored by themselves in a parentless commit, for
# ease of unpacking later.
u_commit=$(
- untracked_files | (
+ untracked_files $files | (
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
rm -f "$TMPindex" &&
@@ -157,7 +164,7 @@ create_stash () {
git read-tree --index-output="$TMPindex" -m $i_tree &&
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
- git diff-index --name-only -z HEAD -- >"$TMP-stagenames" &&
+ git diff-index --name-only -z HEAD -- $files >"$TMP-stagenames" &&
git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
git write-tree &&
rm -f "$TMPindex"
@@ -171,7 +178,7 @@ create_stash () {
# find out what the user wants
GIT_INDEX_FILE="$TMP-index" \
- git add--interactive --patch=stash -- &&
+ git add--interactive --patch=stash -- $files &&
# state of the working tree
w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
@@ -293,6 +300,8 @@ push_stash () {
shift
done
+ files="$*"
+
if test -n "$patch_mode" && test -n "$untracked"
then
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
@@ -307,18 +316,25 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash -m "$stash_msg" -u "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- $files
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
if test -z "$patch_mode"
then
- git reset --hard ${GIT_QUIET:+-q}
+ if test -n "$files"
+ then
+ git ls-files -z -- "$@" | xargs -0 git reset --
+ git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
+ git ls-files -z --others -- "$@" | xargs -0 git clean --force --
+ else
+ git reset --hard ${GIT_QUIET:+-q}
+ fi
test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
if test -n "$untracked"
then
- git clean --force --quiet -d $CLEAN_X_OPTION
+ git clean --force --quiet -d $CLEAN_X_OPTION -- $files
fi
if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34e9610bb6..ca4c44aa9c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,46 @@ test_expect_success 'new style stash create stores correct message' '
test_cmp expect actual
'
+test_expect_success 'stash -- <filename> stashes and restores the file' '
+ >foo &&
+ >bar &&
+ git add foo bar &&
+ git stash push -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple filename arguments' '
+ >foo &&
+ >bar &&
+ >extra &&
+ git add foo bar extra &&
+ git stash push -- foo bar &&
+ test_path_is_missing bar &&
+ test_path_is_missing foo &&
+ test_path_is_file extra &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >untracked &&
+ git add foo* &&
+ git stash push -- foo* &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_missing foo &&
+ test_path_is_file untracked &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file untracked
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
Don't mention git reset --hard in the documentation for git stash save.
It's an implementation detail that doesn't matter to the end user and
thus shouldn't be exposed to them. In addition it's not quite true for
git stash -p, and will not be true when a filename argument to limit the
stash to a few files is introduced.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0fc23c25ee 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -47,8 +47,9 @@ OPTIONS
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
- Save your local modifications to a new 'stash', and run `git reset
- --hard` to revert them. The <message> part is optional and gives
+ Save your local modifications to a new 'stash' and roll them
+ back both in the working tree and in the index.
+ The <message> part is optional and gives
the description along with the stashed state. For quickly making
a snapshot, you can omit _both_ "save" and <message>, but giving
only <message> does not trigger this action to prevent a misspelled
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 2/4] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is specified after a -m
parameter instead of being a positional argument.
This allows introducing a new filename argument to stash single files.
Using that as a positional argument is much more consistent with the
rest of git, than using the positional argument for the message.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++
2 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8528708f61 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -189,10 +189,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +217,10 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +256,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +294,69 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ -k|--keep-index)
+ push_options="-k $push_options"
+ ;;
+ --no-keep-index)
+ push_options="--no-keep-index $push_options"
+ ;;
+ -p|--patch)
+ push_options="-p $push_options"
+ ;;
+ -q|--quiet)
+ push_options="-q $push_options"
+ ;;
+ -u|--include-untracked)
+ push_options="-u $push_options"
+ ;;
+ -a|--all)
+ push_options="-a $push_options"
+ ;;
+ --help)
+ show_help
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash save': \$option
+ To provide a message, use git stash save -- '\$option'"
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ stash_msg="$*"
+
+ if test -z "$stash_msg"
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +683,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..0171b824c9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list | head -n 1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 0/4] stash: create filename argument
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170121200804.19009-1-t.gummerer@gmail.com>
Previous round is at:
http://public-inbox.org/git/20170121200804.19009-1-t.gummerer@gmail.com/.
Thanks Junio, Peff, Øyvind, Jakub and Johannes for your feedback on
the previous round.
Changes since the previous round:
- Re-phrased the Documentation update.
- Added missing $ in 2/3
- Added an extra patch introducing a new syntax for git stash create,
where the message can be specified with the -m flag, instead of as a
positional argument
- Filenames with $IFS in their name are now supported. Added a test
for that as well.
Interdiff below:
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 871a3b246c..8306bac397 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -20,6 +20,8 @@ SYNOPSIS
[--] [<paths>...]
'git stash' clear
'git stash' create [<message>]
+'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
+ [-- <paths>...]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
@@ -51,8 +53,8 @@ OPTIONS
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<paths>...]::
- Save your local modifications to a new 'stash', and revert the
- the changes in the working tree to match the index.
+ Save your local modifications to a new 'stash' and roll them
+ back both in the working tree and in the index.
The <message> part is optional and gives
the description along with the stashed state. For quickly making
a snapshot, you can omit _both_ "save" and <message>, but giving
diff --git a/git-stash.sh b/git-stash.sh
index 7dcce629bd..0072a38b4c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -56,25 +56,57 @@ clear_stash () {
}
create_stash () {
+ stash_msg=
+ untracked=
+ new_style=
files=
while test $# != 0
do
case "$1" in
+ -m|--message)
+ shift
+ stash_msg="$1"
+ new_style=t
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked="$1"
+ new_style=t
+ ;;
--)
shift
+ files="$@"
+ new_style=t
break
;;
- --files)
- ;;
*)
- files="$1 $files"
+ if test -n "$new_style"
+ then
+ echo "invalid argument"
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash create': \$option"
+ usage
+ fi
+ break
;;
esac
shift
done
- stash_msg="$1"
- untracked="$2"
+ if test -z "$new_style"
+ then
+ stash_msg="$*"
+ fi
git update-index -q --refresh
if no_changes
@@ -284,7 +316,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash --files $files -- "$stash_msg" "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- $files
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -293,9 +325,9 @@ push_stash () {
then
if test -n "$files"
then
- git reset -- $files
- git checkout HEAD -- $(git ls-files --modified -- $files)
- git clean --force --quiet -- $(git ls-files --others -- $files)
+ git ls-files -z -- "$@" | xargs -0 git reset --
+ git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
+ git ls-files -z --others -- "$@" | xargs -0 git clean --force --
else
git reset --hard ${GIT_QUIET:+-q}
fi
@@ -373,14 +405,9 @@ save_stash () {
shift
done
- # if test -n "$patch_mode" && test -n "$untracked"
- # then
- # die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
- # fi
-
stash_msg="$*"
- if test -z stash_msg
+ if test -z "$stash_msg"
then
push_stash $push_options
else
@@ -728,7 +755,7 @@ clear)
;;
create)
shift
- create_stash -- "$*" && echo "$w_commit"
+ create_stash "$@" && echo "$w_commit"
;;
store)
shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3e763ff766..ca4c44aa9c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,6 +784,24 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'deprecated version of stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'new style stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create -m "create test message new style") &&
+ echo "On master: create test message new style" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'stash -- <filename> stashes and restores the file' '
>foo &&
>bar &&
@@ -811,4 +829,19 @@ test_expect_success 'stash with multiple filename arguments' '
test_path_is_file extra
'
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >untracked &&
+ git add foo* &&
+ git stash push -- foo* &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_missing foo &&
+ test_path_is_file untracked &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file untracked
+'
+
test_done
Thomas Gummerer (4):
Documentation/stash: remove mention of git reset --hard
stash: introduce push verb
introduce new format for git stash create
stash: support filename argument
Documentation/git-stash.txt | 14 +++-
git-stash.sh | 154 ++++++++++++++++++++++++++++++++++++++++----
t/t3903-stash.sh | 69 ++++++++++++++++++++
3 files changed, 222 insertions(+), 15 deletions(-)
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 3/4] introduce new format for git stash create
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
git stash create currently supports a positional argument for adding a
message. This is not quite in line with how git commands usually take
comments (using a -m flag).
Add a new syntax for adding a message to git stash create using a -m
flag. This is with the goal of deprecating the old style git stash
create with positional arguments.
This also adds a -u argument, for untracked files. This is already used
internally as another positional argument, but can now be used from the
command line.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 1 +
git-stash.sh | 50 +++++++++++++++++++++++++++++++++++++++++----
t/t3903-stash.sh | 18 ++++++++++++++++
3 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0fc23c25ee..0bce33e3fc 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' clear
'git stash' create [<message>]
+'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
diff --git a/git-stash.sh b/git-stash.sh
index 8528708f61..5f08b43967 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -56,8 +56,50 @@ clear_stash () {
}
create_stash () {
- stash_msg="$1"
- untracked="$2"
+ stash_msg=
+ untracked=
+ new_style=
+ while test $# != 0
+ do
+ case "$1" in
+ -m|--message)
+ shift
+ stash_msg="$1"
+ new_style=t
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked="$1"
+ new_style=t
+ ;;
+ *)
+ if test -n "$new_style"
+ then
+ echo "invalid argument"
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash create': \$option"
+ usage
+ fi
+ break
+ ;;
+ esac
+ shift
+ done
+
+ if test -z "$new_style"
+ then
+ stash_msg="$*"
+ fi
git update-index -q --refresh
if no_changes
@@ -265,7 +307,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash "$stash_msg" $untracked
+ create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -697,7 +739,7 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ create_stash "$@" && echo "$w_commit"
;;
store)
shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0171b824c9..34e9610bb6 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'deprecated version of stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'new style stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create -m "create test message new style") &&
+ echo "On master: create test message new style" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ 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