* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Johannes Schindelin @ 2009-08-18 21:42 UTC (permalink / raw)
To: Jeff King; +Cc: Matthieu Moy, git
In-Reply-To: <20090818174509.GA27518@coredump.intra.peff.net>
Hi,
On Tue, 18 Aug 2009, Jeff King wrote:
> On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:
>
> > Hmm, googling a bit, I just noticed that there's already something in
> > pu:
> > ea41cfc4f (Make 'git stash -k' a short form for 'git stash save --keep-index')
> > which also does the trick, while adding a -k alias for --keep-index.
> >
> > [...]
> >
> > Mine has at least two advantages:
> >
> > * It won't require changing the code again when new options are added
> > to 'git stash save'.
> >
> > * It works with 'git stash -k -q' for example, while the other
> > proposal checks that $# == 1, which won't work if there are more
> > than one option.
>
> I think yours is nicer, especially as we have just added the
> '-p|--patch' option, as well. With what is there now, you can do "git
> stash -p", but not "git stash -p -k".
But it is sloppy, in that it blindly accepts options that might be valid
for several subcommands, not just "save".
That was the reason I did not implement it this way.
But we do not have such ambiguous options yet.
Or do we? Look at what "list" accepts!
So please register my objection.
Ciao,
Dscho
^ permalink raw reply
* [PATCH 3/3] stash: reject stash name starting with a dash.
From: Matthieu Moy @ 2009-08-18 21:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1250631523-10524-3-git-send-email-Matthieu.Moy@imag.fr>
This avoids typos like 'git stash save --invalid-option', particularly
nasty since one can omit the 'save' subcommand. The syntax
'git stash save -- "-name starting with dash" still allows such stash name.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Jeff King <peff@peff.net> writes:
>> But I may have missed its drawbacks ;-)
>
> The only I can think of is that bogus input will provoke 'save'. So
> something like:
>
> git stash --apply
>
> will invoke "git stash save --apply", which doesn't even complain. It
> just tries to make a stash with message --apply. Now of course this
> input is obviously bogus, but probably the right thing to do is
> complain.
>
> OTOH, I think it is the fault of "git stash save --apply" for not doing
> the complaining, so your patch really isn't making it worse. Probably it
> should barf on anything unrecognized starting with a '-', and allow '--'
> to separate the message from the rest of the options (in the rare case
> that you want a message starting with '-').
>
> -Peff
So, here it is!
Documentation/git-stash.txt | 2 +-
git-stash.sh | 10 +++++++++-
t/t3903-stash.sh | 10 ++++++++++
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 7e515ce..ded62e0 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,7 +13,7 @@ SYNOPSIS
'git stash' drop [-q|--quiet] [<stash>]
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
'git stash' branch <branchname> [<stash>]
-'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]
+'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [--] [<message>]
'git stash' clear
'git stash' create
diff --git a/git-stash.sh b/git-stash.sh
index bb36bc7..642e265 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE="list [<options>]
or: $dashless drop [-q|--quiet] [<stash>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
- or: $dashless [save] [-k|--keep-index] [-q|--quiet] [<message>]
+ or: $dashless [save] [-k|--keep-index] [-q|--quiet] [--] [<message>]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -145,6 +145,14 @@ save_stash () {
-q|--quiet)
GIT_QUIET=t
;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ echo "error: unknown option for 'stash save': $1"
+ usage
+ ;;
*)
break
;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0e831e0..87e5a14 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -208,4 +208,14 @@ test_expect_success 'stash -k' '
test bar,bar4 = $(cat file),$(cat file2)
'
+test_expect_success 'stash --invalid-option' '
+ echo bar5 > file &&
+ echo bar6 > file2 &&
+ git add file2 &&
+ ! git stash --invalid-option &&
+ test bar5,bar6 = $(cat file),$(cat file2) &&
+ git stash -- -message-starting-with-dash &&
+ test bar,bar2 = $(cat file),$(cat file2)
+'
+
test_done
--
1.6.4.rc2.31.g2d7d7
^ permalink raw reply related
* Re: x86 SHA1: Faster than OpenSSL
From: Andy Polyakov @ 2009-08-18 21:50 UTC (permalink / raw)
To: Linus Torvalds; +Cc: George Spelvin, git
In-Reply-To: <alpine.LFD.2.01.0908031938280.3270@localhost.localdomain>
> On Mon, 3 Aug 2009, Linus Torvalds wrote:
>> The thing that I'd prefer is simply
>>
>> git fsck --full
>>
>> on the Linux kernel archive. For me (with a fast machine), it takes about
>> 4m30s with the OpenSSL SHA1, and takes 6m40s with the Mozilla SHA1 (ie
>> using a NO_OPENSSL=1 build).
>>
>> So that's an example of a load that is actually very sensitive to SHA1
>> performance (more so than _most_ git loads, I suspect), and at the same
>> time is a real git load rather than some SHA1-only microbenchmark.
I couldn't agree more that real-life benchmarks are of greater value
than specific algorithm micro-benchmark. And given the provided
profiling data one can argue that +17% (or my +12%) improvement on
micro-benchmark aren't really worth bothering about. But it's kind of
sport [at least for me], so don't judge too harshly:-)
>> It also
>> shows very clearly why we default to the OpenSSL version over the Mozilla
>> one.
As George implicitly mentioned most OpenSSL assembler modules are
available under more permissive license and if there is interest I'm
ready to assist...
> "perf report --sort comm,dso,symbol" profiling shows the following for
> 'git fsck --full' on the kernel repo, using the Mozilla SHA1:
>
> 47.69% git /home/torvalds/git/git [.] moz_SHA1_Update
> 22.98% git /lib64/libz.so.1.2.3 [.] inflate_fast
> 7.32% git /lib64/libc-2.10.1.so [.] __GI_memcpy
> 4.66% git /lib64/libz.so.1.2.3 [.] inflate
> 3.76% git /lib64/libz.so.1.2.3 [.] adler32
> 2.86% git /lib64/libz.so.1.2.3 [.] inflate_table
> 2.41% git /home/torvalds/git/git [.] lookup_object
> 1.31% git /lib64/libc-2.10.1.so [.] _int_malloc
> 0.84% git /home/torvalds/git/git [.] patch_delta
> 0.78% git [kernel] [k] hpet_next_event
>
> so yeah, SHA1 performance matters. Judging by the OpenSSL numbers, the
> OpenSSL SHA1 implementation must be about twice as fast as the C version
> we use.
And given /lib64 path this is 64-bit C compiler-generated code compared
to 32-bit assembler? Either way in this context I have extra comment
addressing previous subscriber, Mark Lodato, who effectively wondered
how would 64-bit assembler compare to 32-bit one. First of all there
*is* even 64-bit assembler version. But as SHA1 is essentially 32-bit
algorithm, 64-bit implementation is only nominally faster, +20% at most.
Faster thanks to larger register bank facilitating more efficient
instruction scheduling.
Cheers. A.
^ permalink raw reply
* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Matthieu Moy @ 2009-08-18 21:52 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0908182337200.8306@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> But it is sloppy, in that it blindly accepts options that might be valid
> for several subcommands, not just "save".
I wouldn't call that sloppy. 'save' is the default command, if you
don't provide any command, then 'save' will be used.
So, "git stash -p" means "git stash save -p" regardless of the fact
that there exists somewhere else a "git stash list -p". Actually, in
the current form of pu, it is already the case since
f300fab (Thomas Rast, DWIM 'git stash save -p' for 'git stash -p')
which came right after your patch.
> So please register my objection.
I will if you register my objection to yours ;-).
Jokes aside, if you insist in rejecting 'git stash -p', then my patch
can be slightly modified to say something like
+case "$1" in
+ -k|--keep-index|--patch)
+ set "save" "$@"
+ ;;
+esac
instead, which also allows multiple arguments (unlike your initial
proposal), and can control more precisely the list of options for
which 'save' is a default.
--
Matthieu
^ permalink raw reply
* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Johannes Schindelin @ 2009-08-18 21:53 UTC (permalink / raw)
To: Marius Storm-Olsen
Cc: Johan 't Hart, Junio C Hamano, msysgit, git, lznuaa, bonzini,
kusmabite
In-Reply-To: <4A8AED8B.9080604@gmail.com>
Hi,
On Tue, 18 Aug 2009, Marius Storm-Olsen wrote:
> Johan 't Hart said the following on 18.08.2009 19:41:
> > Junio C Hamano schreef:
> > > Since use of make implies use of shell, this makes me wonder if it
> > > would make sense to go one step further by giving msvc users a thin
> > > shell wrapper mcvc-cc that turns bog-standard cc command line into
> > > whatever cl uses.
> >
> > Just using the msvc toolchain for building git misses the whole
> > purpose of what VC is used for. MSVC would be used because of the
> > IDE, not for the compiler IMO.
>
> Bull. MSVC produces superior code on Windows compared to MinGW, I'm afraid.
> Add the /LTGC (Link Time Code Generation), and MSVC generates very good cross
> compile-unit optimized code. (I know gcc has the option, but it's not as good,
> by far)
> Coupled now with built-in static code analysis, these are only two reasons why
> *I* would want to build it directly from the command line without worrying
> that my .vcproj is out-of-sync with the main development.
> You can still debug with the MSVC debugger if you'd like, and the MSVC IDE
> allows you to wrap Makefile project too, so you can *still* use the IDE..
Well, "you can *still* use the IDE" is a bit exaggerated, no? At least
unless you misunderstand "IDE" to mean "Integrated Debugging environment".
You'll lose not only the ability to follow definitions/declarations in the
code, you'll also lose the ability to compile incrementally _while_
debuggin, and Visual Studio's feature to move to the next/previous compile
error.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Johannes Schindelin @ 2009-08-18 21:57 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Marius Storm-Olsen, msysgit, git, lznuaa, bonzini, kusmabite
In-Reply-To: <alpine.LNX.2.00.0908181420330.7195@iabervon.org>
Hi,
On Tue, 18 Aug 2009, Daniel Barkalow wrote:
> On Tue, 18 Aug 2009, Marius Storm-Olsen wrote:
>
> > Johannes Schindelin said the following on 18.08.2009 16:11:
> > > On Tue, 18 Aug 2009, Marius Storm-Olsen wrote:
> > > > So, instead of rely on these vcproj files which *will* go stale,
> > > > we can simply use the same Makefile system which everyone else is
> > > > using. :) After all, we're just compiling with a different
> > > > compiler. The end result will still rely on the *msysGit
> > > > environment* to function, so we already require it. Thus, GNU Make
> > > > is present, and we can use it.
> > >
> > > We can also use sed or perl to generate/modify the .vcproj files, or
> > > run CMake (once Pau got it to build), and package the stuff using
> > > zip (once I got that to build).
> >
> > Really? That would be some script being able to parse the Makefile,
> > and create something reasonable as a vcproj script :) Keeping all the
> > options in sync, conditional files/libs, all the various end
> > executables (a separate .vcproj for each of them, and a solution file
> > (.sln)to tie them all together into a .. "solution", a complete
> > product blah blah blah) etc.
>
> I think it wouldn't be impossible to split the Makefile into an
> easy-to-parse part and an irrelevant-to-vcproj part.
What? That is pretty fragile.
It would be _much_ better to just add another Makefile target that outputs
the files needed for the targets.
We do not need to keep the Makefile as-is! If the change is not
intrusive, I am sure our friendly maintainer will not see a problem
either.
No need for fragile parsing at all.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Erik Faye-Lund @ 2009-08-18 22:02 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Marius Storm-Olsen, Johan 't Hart, Junio C Hamano, msysgit,
git, lznuaa, bonzini
In-Reply-To: <alpine.DEB.1.00.0908182349220.8306@pacific.mpi-cbg.de>
On Tue, Aug 18, 2009 at 11:53 PM, Johannes
Schindelin<Johannes.Schindelin@gmx.de> wrote:
> debuggin, and Visual Studio's feature to move to the next/previous compile
> error.
Nope, you're still able to skip through the erros (with F4) as ususal.
I've been using a sed-script to translate gcc-style errors to
msvc-style errors for makefile projects before with great success. In
this case, the errors are already in msvc-style, so that part should
really not be any issue.
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: Continue git clone after interruption
From: Johannes Schindelin @ 2009-08-18 22:28 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jakub Narebski, Tomasz Kontusz, git
In-Reply-To: <alpine.LFD.2.00.0908181537360.6044@xanadu.home>
Hi,
On Tue, 18 Aug 2009, Nicolas Pitre wrote:
> On Tue, 18 Aug 2009, Jakub Narebski wrote:
>
> > There is another way which we can go to implement resumable clone.
> > Let's git first try to clone whole repository (single pack; BTW what
> > happens if this pack is larger than file size limit for given
> > filesystem?).
>
> We currently fail. Seems that no one ever had a problem with that so
> far.
They just went away, most probably.
But seriously, I miss a very important idea in this discussion: we control
the Git source code. So we _can_ add a upload_pack feature that a client
can ask for after the first failed attempt.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Jeff King @ 2009-08-18 22:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Matthieu Moy, git
In-Reply-To: <alpine.DEB.1.00.0908182337200.8306@pacific.mpi-cbg.de>
On Tue, Aug 18, 2009 at 11:42:58PM +0200, Johannes Schindelin wrote:
> > I think yours is nicer, especially as we have just added the
> > '-p|--patch' option, as well. With what is there now, you can do "git
> > stash -p", but not "git stash -p -k".
>
> But it is sloppy, in that it blindly accepts options that might be valid
> for several subcommands, not just "save".
>
> That was the reason I did not implement it this way.
>
> But we do not have such ambiguous options yet.
>
> Or do we? Look at what "list" accepts!
>
> So please register my objection.
I don't see the problem. Either the option works for "stash save" or it
does not. If I say "git stash --quiet", then it _must_ be "git stash
save --quiet", and not "git stash pop --quiet", because "save" is the
only default command. If I say "git stash --foobar", it is translated
to "git stash save --foobar", which should generate an error (it doesn't
right now, but that is a separate issue). I don't see any confusion:
"save" is always the default command, unless one is given as the first
argument.
The place where I would see a potential problem is if stash grew any
"global" options (e.g., in the same way that "git" can take options
before its subcommand name).
-Peff
^ permalink raw reply
* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Johannes Schindelin @ 2009-08-18 22:37 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, git
In-Reply-To: <vpqtz0423jk.fsf@bauges.imag.fr>
Hi,
On Tue, 18 Aug 2009, Matthieu Moy wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > But it is sloppy, in that it blindly accepts options that might be
> > valid for several subcommands, not just "save".
>
> I wouldn't call that sloppy. 'save' is the default command, if you don't
> provide any command, then 'save' will be used.
'save' might be the default command, but we don't trigger it with _any_
crap, for a very good reason. Read the commit log for git-stash.sh to
know why.
> > So please register my objection.
>
> I will if you register my objection to yours ;-).
>
> Jokes aside,
If you think that's funny, I don't. I actually put a lot of thought into
the issue whether to allow _any_ parameter with looks like an option to
trigger "save". And I very much had to come to the decision that no, that
is too dangerous.
So I cannot take your objection seriously.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Johannes Schindelin @ 2009-08-18 22:38 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Marius Storm-Olsen, Johan 't Hart, Junio C Hamano, msysgit,
git, lznuaa, bonzini
In-Reply-To: <40aa078e0908181502v32cbd223xcde1cd363dc76345@mail.gmail.com>
Hi,
On Wed, 19 Aug 2009, Erik Faye-Lund wrote:
> On Tue, Aug 18, 2009 at 11:53 PM, Johannes
> Schindelin<Johannes.Schindelin@gmx.de> wrote:
> > debuggin, and Visual Studio's feature to move to the next/previous
> > compile error.
>
> Nope, you're still able to skip through the erros (with F4) as ususal.
> I've been using a sed-script to translate gcc-style errors to msvc-style
> errors for makefile projects before with great success. In this case,
> the errors are already in msvc-style, so that part should really not be
> any issue.
At this point you are just piling work-around on work-around.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] graph API: fix bug in graph_is_interesting()
From: Adam Simpkins @ 2009-08-18 21:18 UTC (permalink / raw)
To: Git Mailing List; +Cc: Santi Béjar, Junio C Hamano
In-Reply-To: <1250628954.114121983@192.168.1.201>
Updated graph_is_interesting() to use simplify_commit() to determine if
a commit is interesting, just like get_revision() does. Previously, it
would sometimes incorrectly treat an interesting commit as
uninteresting. This resulted in incorrect lines in the graph output.
This problem was reported by Santi Béjar. The following command
would exhibit the problem before, but now works correctly:
git log --graph --simplify-by-decoration --oneline v1.6.3.3
Previously git graph did not display the output for this command
correctly between f29ac4f and 66996ec, among other places.
Signed-off-by: Adam Simpkins <simpkins@facebook.com>
---
Note that simplify_commit() may modify the revision list. Calling it
in graph_is_interesting() can modify the revision list earlier than it
otherwise would be (in get_revision()). I don't think this should
cause any problems, but figured I'd point it out in case anyone more
familiar with the code thinks otherwise.
graph.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/graph.c b/graph.c
index e466770..ea21e91 100644
--- a/graph.c
+++ b/graph.c
@@ -286,9 +286,10 @@ static int graph_is_interesting(struct git_graph *graph, struct commit *commit)
}
/*
- * Uninteresting and pruned commits won't be printed
+ * Otherwise, use simplify_commit() to see if this commit is
+ * interesting
*/
- return (commit->object.flags & (UNINTERESTING | TREESAME)) ? 0 : 1;
+ return simplify_commit(graph->revs, commit) == commit_show;
}
static struct commit_list *next_interesting_parent(struct git_graph *graph,
--
1.6.4.314.ge5db
^ permalink raw reply related
* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Johannes Schindelin @ 2009-08-18 22:46 UTC (permalink / raw)
To: Jeff King; +Cc: Matthieu Moy, git
In-Reply-To: <20090818223028.GB31176@coredump.intra.peff.net>
Hi,
On Tue, 18 Aug 2009, Jeff King wrote:
> On Tue, Aug 18, 2009 at 11:42:58PM +0200, Johannes Schindelin wrote:
>
> > > I think yours is nicer, especially as we have just added the
> > > '-p|--patch' option, as well. With what is there now, you can do
> > > "git stash -p", but not "git stash -p -k".
> >
> > But it is sloppy, in that it blindly accepts options that might be
> > valid for several subcommands, not just "save".
> >
> > That was the reason I did not implement it this way.
> >
> > But we do not have such ambiguous options yet.
> >
> > Or do we? Look at what "list" accepts!
> >
> > So please register my objection.
>
> I don't see the problem. Either the option works for "stash save" or it
> does not. If I say "git stash --quiet", then it _must_ be "git stash
> save --quiet", and not "git stash pop --quiet", because "save" is the
> only default command.
So if I say "git stash -q" by mistake, but wanted to say "git stash drop
-q", then I am borked?
Bah! I say: bah!
You're basically reintroducing at least part of the DWIMery that was
reverted in 9488e875 and I have the distinct feeling that some people in
this thread do not think hard enough about what would adher to the
principle of least surprise even in the future (or even for people
introducing other stash save options).
Well, you just go ahead and push through your patch, and I do what I
promised on my blog.
^ permalink raw reply
* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Erik Faye-Lund @ 2009-08-18 22:48 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Marius Storm-Olsen, Johan 't Hart, Junio C Hamano, msysgit,
git, lznuaa, bonzini
In-Reply-To: <alpine.DEB.1.00.0908190038110.8306@pacific.mpi-cbg.de>
On Wed, Aug 19, 2009 at 12:38 AM, Johannes
Schindelin<Johannes.Schindelin@gmx.de> wrote:
>> Nope, you're still able to skip through the erros (with F4) as ususal.
>> I've been using a sed-script to translate gcc-style errors to msvc-style
>> errors for makefile projects before with great success. In this case,
>> the errors are already in msvc-style, so that part should really not be
>> any issue.
>
> At this point you are just piling work-around on work-around.
At what point? This works out of the box for makefile projects that
use msvc for compilation. Sure, for my gcc-setup, yeah. But how is
that being a stack of work-arounds relevant in this context?
I'm not arguing either way here - I'm perfectly happy to stick with MinGW.
I'm just saying that one of the problems you pointed out is not really
a problem. The rest of them sounds like very much like real problems
to me - not particularly big issues, you're the one who just said that
perfect is the enemy of the good, no? ;)
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Jeff King @ 2009-08-18 23:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Matthieu Moy, git
In-Reply-To: <alpine.DEB.1.00.0908190041200.8306@pacific.mpi-cbg.de>
On Wed, Aug 19, 2009 at 12:46:27AM +0200, Johannes Schindelin wrote:
> So if I say "git stash -q" by mistake, but wanted to say "git stash drop
> -q", then I am borked?
>
> Bah! I say: bah!
Yep, though it is only one of many borkings that currently exist. Try:
# oops, what was the name of that option?
git stash save --index
# does apply take --patch, too?
git stash apply --patch
Still, other parts of the option parsing being awful aren't an excuse to
mess this one up. So I see your point.
> You're basically reintroducing at least part of the DWIMery that was
> reverted in 9488e875 and I have the distinct feeling that some people in
> this thread do not think hard enough about what would adher to the
> principle of least surprise even in the future (or even for people
> introducing other stash save options).
I don't think it is quite as bad as that, as arbitrary crap will not get
passed through, only crap that looks like options. Which is perhaps a
step up, but it is debatable how much.
I think Matthieu's proposal to be strict about matching the options, but
to take multiple options is probably the best bet for now. As it is now,
accepting "git stash -k" or "git stash -p" but not "git stash -k -p" is
pretty counterintuitive. But with explicit matching it should be no less
safe than it is now.
> Well, you just go ahead and push through your patch, and I do what I
> promised on my blog.
I don't know why you need to be so confrontational. I don't even have a
patch to "push through". I just said "I don't see the problem", and I'm
glad you brought up the previous stash behavior, because I had forgotten
the pain it caused. And that is why discussing on a public forum is
nice; it lets people contribute things that others might have missed.
-Peff
^ permalink raw reply
* Re: [PATCH 0/3] short syntaxes for 'git stash'
From: Jeff King @ 2009-08-18 23:31 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1250631523-10524-1-git-send-email-Matthieu.Moy@imag.fr>
On Tue, Aug 18, 2009 at 11:38:40PM +0200, Matthieu Moy wrote:
> This small patch serie is based on the following commit in pu:
>
> dda1f2a Implement 'git stash save --patch'
>
> It is meant to replace two commits already there:
>
> ea41cfc Make 'git stash -k' a short form for 'git stash save --keep-index'
> f300fab DWIM 'git stash save -p' for 'git stash -p'
Actually, these are already in 'next', so they can't be simply replaced.
So you would need to re-roll patches 1 and 2 at the very least.
However, thinking more on it, I think we can address Dscho's concern
with your proposal to accept only a limited set of options. And looking
at what's in f300fab, it actually does make an attempt to allow multiple
options, but it doesn't cover all cases (e.g., I can use "-p
--no-keep-index" but not "--no-keep-index -p". Nor can I do "-p -k";
even though "-k" is implied by "-p", you will get a very strange usage
mention instead of it being a silent no-op).
So there are two issues:
- refactoring to allow arbitrary combinations of -k/-p and variants.
- allowing other options; I believe "-q" is the only one. That seems
to be specific to Dscho's objection, as it is ambiguous with other
subcommands. Though "-p" may also become ambiguous, if we get "stash
apply -p" soon.
I think the first one should be fairly uncontroversial. I'm not sure
about the second.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] stash: reject stash name starting with a dash.
From: Jeff King @ 2009-08-18 23:35 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1250631523-10524-4-git-send-email-Matthieu.Moy@imag.fr>
On Tue, Aug 18, 2009 at 11:38:43PM +0200, Matthieu Moy wrote:
> This avoids typos like 'git stash save --invalid-option', particularly
> nasty since one can omit the 'save' subcommand. The syntax
> 'git stash save -- "-name starting with dash" still allows such stash name.
Aside from the documentation and usage lines, this one is actually
independent of the other two, and I think makes sense regardless of what
happens.
> +test_expect_success 'stash --invalid-option' '
> + echo bar5 > file &&
> + echo bar6 > file2 &&
> + git add file2 &&
> + ! git stash --invalid-option &&
> + test bar5,bar6 = $(cat file),$(cat file2) &&
> + git stash -- -message-starting-with-dash &&
> + test bar,bar2 = $(cat file),$(cat file2)
> +'
Should this actually be "git stash save --invalid-option", since it is
really testing the actual save option parsing, and not the behavior to
automatically push options to "git stash save"?
Other than that, patch looks good.
-Peff
^ permalink raw reply
* Re: Continue git clone after interruption
From: Nicolas Pitre @ 2009-08-18 23:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jakub Narebski, Tomasz Kontusz, git
In-Reply-To: <alpine.DEB.1.00.0908190022090.8306@pacific.mpi-cbg.de>
On Wed, 19 Aug 2009, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 18 Aug 2009, Nicolas Pitre wrote:
>
> > On Tue, 18 Aug 2009, Jakub Narebski wrote:
> >
> > > There is another way which we can go to implement resumable clone.
> > > Let's git first try to clone whole repository (single pack; BTW what
> > > happens if this pack is larger than file size limit for given
> > > filesystem?).
> >
> > We currently fail. Seems that no one ever had a problem with that so
> > far.
>
> They just went away, most probably.
Most probably they simply don't exist. I would be highly surprised
otherwise.
> But seriously, I miss a very important idea in this discussion: we control
> the Git source code. So we _can_ add a upload_pack feature that a client
> can ask for after the first failed attempt.
Indeed. So what do you think about my proposal? It was included in my
first reply to this thread.
Nicolas
^ permalink raw reply
* Re: [PATCH 3/3] stash: reject stash name starting with a dash.
From: Junio C Hamano @ 2009-08-18 23:45 UTC (permalink / raw)
To: Jeff King; +Cc: Matthieu Moy, git, gitster
In-Reply-To: <20090818233535.GB6304@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Aug 18, 2009 at 11:38:43PM +0200, Matthieu Moy wrote:
>
>> This avoids typos like 'git stash save --invalid-option', particularly
>> nasty since one can omit the 'save' subcommand. The syntax
>> 'git stash save -- "-name starting with dash" still allows such stash name.
>
> Aside from the documentation and usage lines, this one is actually
> independent of the other two, and I think makes sense regardless of what
> happens.
Yes, and I'd rather see it near 'maint' even if we were to (mind you, we
won't) reject all changes in 'next' on this program.
>> +test_expect_success 'stash --invalid-option' '
>> + echo bar5 > file &&
>> + echo bar6 > file2 &&
>> + git add file2 &&
>> + ! git stash --invalid-option &&
>> + test bar5,bar6 = $(cat file),$(cat file2) &&
>> + git stash -- -message-starting-with-dash &&
>> + test bar,bar2 = $(cat file),$(cat file2)
>> +'
>
> Should this actually be "git stash save --invalid-option", since it is
> really testing the actual save option parsing, and not the behavior to
> automatically push options to "git stash save"?
It would be ideal if
git stash save --invalid-option
failed, while
git stash --invalid-option should be trapped
and/or
git stash "--invalid-option should be trapped"
are accepted as valid quickie ways to name a WIP stash before attending to
an unrelated emergency.
^ permalink raw reply
* Re: [PATCH] graph API: fix bug in graph_is_interesting()
From: Junio C Hamano @ 2009-08-18 23:53 UTC (permalink / raw)
To: Git Mailing List; +Cc: Santi Béjar
In-Reply-To: <20090818211812.GL8147@facebook.com>
Adam Simpkins <simpkins@facebook.com> writes:
> - return (commit->object.flags & (UNINTERESTING | TREESAME)) ? 0 : 1;
> + return simplify_commit(graph->revs, commit) == commit_show;
If you do this after revision.c finished the traversal (e.g. "limited"
case), I think it should be Ok.
But calling simplify_commit() while the traversal is still in progress is
asking for trouble. I do not recall the details anymore but when I tried
to make the "simplify-merges" algorithm incremental, I had seen funny
breakage caused by calling simplify_commit() twice on the same commit.
I suspect that this change will break the primary traversal.
^ permalink raw reply
* [PATCH][resend 3] git-svn: Use GIT_SSH setting if SVN_SSH is not set
From: Karthik R @ 2009-08-18 23:54 UTC (permalink / raw)
To: git; +Cc: Eric Wong
If SVN_SSH is defined, it will be used. Else value in
GIT_SSH is copied to SVN_SSH & then, only on Windows,
the \s are escaped.
On Windows, the shell-variables must be set as follows
GIT_SSH="C:\Program Files\PuTTY\plink.exe"
SVN_SSH="C:\\Program Files\\PuTTY\\plink.exe"
See http://code.google.com/p/msysgit/issues/detail?id=305
Signed-off-by: Karthik Rajagopalan <karthikr@fastmail.fm>
---
Signed off this time.
Previous life of patch:
[PATCH] GIT_SSH does not override ssh in git-svn
[PATCH][resend] git-svn: Respect GIT_SSH setting
git-svn.perl | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index b0bfb74..ef86d10 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -21,6 +21,15 @@ $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} ||
'git-svn';
$Git::SVN::Ra::_log_window_size = 100;
$Git::SVN::_minimize_url = 'unset';
+if (! exists $ENV{SVN_SSH}) {
+ if (exists $ENV{GIT_SSH}) {
+ $ENV{SVN_SSH} = $ENV{GIT_SSH};
+ if ($^O eq 'msys') {
+ $ENV{SVN_SSH} =~ s/\\/\\\\/g;
+ }
+ }
+}
+
$Git::SVN::Log::TZ = $ENV{TZ};
$ENV{TZ} = 'UTC';
$| = 1; # unbuffer STDOUT
--
1.5.4.3
^ permalink raw reply related
* Re: [PATCH 3/3] stash: reject stash name starting with a dash.
From: Jeff King @ 2009-08-18 23:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, git
In-Reply-To: <7vocqc3cuz.fsf@alter.siamese.dyndns.org>
On Tue, Aug 18, 2009 at 04:45:40PM -0700, Junio C Hamano wrote:
> > Should this actually be "git stash save --invalid-option", since it is
> > really testing the actual save option parsing, and not the behavior to
> > automatically push options to "git stash save"?
>
> It would be ideal if
>
> git stash save --invalid-option
>
> failed, while
>
> git stash --invalid-option should be trapped
>
> and/or
>
> git stash "--invalid-option should be trapped"
>
> are accepted as valid quickie ways to name a WIP stash before attending to
> an unrelated emergency.
I thought that was the exact DWIM that was rejected previously so that
you don't accidentally make commits with "git stash llist" or "git stash
--almost-a-valid-optio".
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] stash: reject stash name starting with a dash.
From: Junio C Hamano @ 2009-08-18 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Matthieu Moy, git
In-Reply-To: <7vocqc3cuz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> It would be ideal if
>
> git stash save --invalid-option
>
> failed, while
>
> git stash --invalid-option should be trapped
>
> and/or
>
> git stash "--invalid-option should be trapped"
>
> are accepted as valid quickie ways to name a WIP stash before attending to
> an unrelated emergency.
Meh, let me take it back. I somehow was living in 13 months ago for a
short while when I wrote the above.
9488e87 (git-stash: require "save" to be explicit and update
documentation, 2007-07-01) made the latter two invalid.
^ permalink raw reply
* [RFC/PATCH 0/6] Git submodule: 'foreach' enhancements and nested submodule handling
From: Johan Herland @ 2009-08-19 1:45 UTC (permalink / raw)
To: git; +Cc: Johan Herland, gitster, mlevedahl, hjemli
Hi,
This patch series attempts to expand 'git submodule' command in two
regards:
1. Clean up, selftest, and enhance the 'git submodule foreach' command.
2. Provide better handling for nested submodules where the user want to
operate on _all_ submodules simultenously.
The first 3 patches in the series are fairly trivial and straightforward.
The last 3 patches are slightly more RFC in nature, although their
implementation is still fairly straighforward.
Patch 3/6 is a resend of a patch that I sent stand-alone on 2009-08-16.
Have fun! :)
...Johan
Johan Herland (6):
git submodule: Cleanup usage string and add option parsing to cmd_foreach()
Add selftest for 'git submodule foreach'
git submodule foreach: Provide access to submodule name, as '$name'
git submodule foreach: Add --recursive to recurse into nested submodules
git submodule update: Introduce --recursive to update nested submodules
git submodule status: Add --recursive to recurse into nested submodules
Documentation/git-submodule.txt | 23 ++++-
git-submodule.sh | 79 ++++++++++++--
t/t7407-submodule-foreach.sh | 223 +++++++++++++++++++++++++++++++++++++++
3 files changed, 313 insertions(+), 12 deletions(-)
create mode 100755 t/t7407-submodule-foreach.sh
^ permalink raw reply
* [RFC/PATCH 1/6] git submodule: Cleanup usage string and add option parsing to cmd_foreach()
From: Johan Herland @ 2009-08-19 1:45 UTC (permalink / raw)
To: git; +Cc: Johan Herland, gitster, mlevedahl, hjemli
In-Reply-To: <1250646324-961-1-git-send-email-johan@herland.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
git-submodule.sh | 28 +++++++++++++++++++++++++---
1 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9bdd6ea..c58eca4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,9 +4,14 @@
#
# Copyright (c) 2007 Lars Hjemli
-USAGE="[--quiet] [--cached|--files] \
-[add [-b branch] <repo> <path>]|[status|init|update [-i|--init] [-N|--no-fetch] [--rebase|--merge]|summary [-n|--summary-limit <n>] [<commit>]] \
-[--] [<path>...]|[foreach <command>]|[sync [--] [<path>...]]"
+dashless=$(basename "$0" | sed -e 's/-/ /')
+USAGE="[--quiet] add [-b branch] [--reference <repository>] [--] <repository> <path>
+ or: $dashless [--quiet] status [--cached] [--] [<path>...]
+ or: $dashless [--quiet] init [--] [<path>...]
+ or: $dashless [--quiet] update [--init] [-N|--no-fetch] [--rebase] [--reference <repository>] [--merge] [--] [<path>...]
+ or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
+ or: $dashless [--quiet] foreach <command>
+ or: $dashless [--quiet] sync [--] [<path>...]"
OPTIONS_SPEC=
. git-sh-setup
. git-parse-remote
@@ -238,6 +243,23 @@ cmd_add()
#
cmd_foreach()
{
+ # parse $args after "submodule ... foreach".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ GIT_QUIET=1
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
module_list |
while read mode sha1 stage path
do
--
1.6.4.304.g1365c.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