* Re: Query on git commit amend
From: Vijay Lakshminarayanan @ 2011-12-06 15:46 UTC (permalink / raw)
To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>
Viresh Kumar <viresh.kumar@st.com> writes:
> Hello,
>
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to change
> message.
>
> If there is no such way, then can we add a patch for this, if it looks a valid
> case.
I've found
$ GIT_EDITOR=cat git commit --amend
useful.
The benefit of this technique is that it even works for git-rebase -i.
In my typical git usage, I do a lot of git-commit --fixup's. After
reaching a level of stability, I change the history with:
GIT_EDITOR=cat git rebase -i --autosquash
and my history is adjusted without requiring manual intervention.
--
Cheers
~vijay
Gnus should be more complicated.
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 15:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcpthh97.fsf@alter.siamese.dyndns.org>
2011/12/6 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>
> Why should we treat this condition any differently from the case where the
> sender of a pack used beefier machine than you have and stuffed a huge
> object that the index-pack running on your box cannot hold in core,
> causing xmalloc() to die on your machine?
That's interesting. First of all xmalloc() is controlled by us while
index-pack code might lead to stack overflow exploit (never done it,
not sure if it's really pratical to do in this case).
But can I really use up all memory at server side by sending a huge pack?
> I do not think this is the right way to handle the issue. Your other patch
> to flatten the recursion to iteration looked a lot saner approach.
It may take me some time as I'm not really familar with this code.
Anybody is welcome to step up and flatten the function.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 15:30 UTC (permalink / raw)
To: Michael Haggerty; +Cc: kusmabite, git, Junio C Hamano
In-Reply-To: <4EDE2C95.5040804@alum.mit.edu>
2011/12/6 Michael Haggerty <mhagger@alum.mit.edu>:
> On 12/06/2011 01:17 PM, Erik Faye-Lund wrote:
>> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>>> a hard limit so that index-pack does not run out of stack. Also stop
>>> people from producing such a long delta chains using "pack-object
>>> --depth=<too large>"
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>> I used to make very long delta chains and triggered this in index-pack.
>>> I did not care reporting because it's my fault anyway. Think again,
>>> index-pack is called at server side and a malicious client can
>>> trigger this. This patch does not improve the situation much, but at
>>> least we won't get sigsegv at server side.
>>
>> Wouldn't it make more sense to make the limit a config option rather
>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>> all, different platforms have different stack-limitations...
>
> I'm confused: is the data only ever read by the same host that generated
> it?
index-pack is called at server side as part of a push. So in theory
the sending side can generate very long delta chains and bring down
the server side.
> Because if not, then the "creator" had better never be configured
> to use a chain depth that the "reader" cannot handle.
Normal creators (i.e. C Git) use default depth 50 so we should be safe.
> This in turn
> imply that there should be a common limit that is supported by all git
> clients and is a documented part of the protocol. (Or the code has to
> be rewritten to use an explicit stack instead of recursion.)
It's the implementation limitation, not the protocol. If the server
propagates error messages from index-pack back to client (I'm not
sure), then users can adjust --depth to be accepted by server. We
could negotiate the limit over the protocol but not sure we would want
to go that route.
The troubled code could be rewritten to avoid recursion. However long
delta chains may degrade performance, I'd rather have support to split
long chains (which can be done with --depth at client already) instead
of just recursion elimination. This patch is simpler, so it would be
easier to back port it if someone wants to do so.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Junio C Hamano @ 2011-12-06 15:06 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1323068688-31481-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Too deep delta chains can cause stack overflow in get_base_data(). Set
> a hard limit so that index-pack does not run out of stack. Also stop
> people from producing such a long delta chains using "pack-object
> --depth=<too large>"
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> I used to make very long delta chains and triggered this in index-pack.
> I did not care reporting because it's my fault anyway. Think again,
> index-pack is called at server side and a malicious client can
> trigger this. This patch does not improve the situation much, but at
> least we won't get sigsegv at server side.
Why should we treat this condition any differently from the case where the
sender of a pack used beefier machine than you have and stuffed a huge
object that the index-pack running on your box cannot hold in core,
causing xmalloc() to die on your machine?
I do not think this is the right way to handle the issue. Your other patch
to flatten the recursion to iteration looked a lot saner approach.
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Michael Haggerty @ 2011-12-06 14:54 UTC (permalink / raw)
To: kusmabite; +Cc: Nguyễn Thái Ngọc Duy, git, Junio C Hamano
In-Reply-To: <CABPQNSaE=TWGbBRMnthEuT181=XbOafPfgx88_JQnnQ6TiYyqw@mail.gmail.com>
On 12/06/2011 01:17 PM, Erik Faye-Lund wrote:
> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>
> Wouldn't it make more sense to make the limit a config option rather
> than a hard-coded value of 128 (which seems arbitrary to me)? After
> all, different platforms have different stack-limitations...
I'm confused: is the data only ever read by the same host that generated
it? Because if not, then the "creator" had better never be configured
to use a chain depth that the "reader" cannot handle. This in turn
imply that there should be a common limit that is supported by all git
clients and is a documented part of the protocol. (Or the code has to
be rewritten to use an explicit stack instead of recursion.)
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 0/8] nd/resolve-ref v2
From: Nguyen Thai Ngoc Duy @ 2011-12-06 14:07 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Junio C Hamano, Jeff King, Michael Haggerty,
Ramkumar Ramachandra
In-Reply-To: <20111117103924.GA5277@elie.hsd1.il.comcast.net>
2011/11/17 Jonathan Nieder <jrnieder@gmail.com>:
> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Nguyễn Thái Ngọc Duy (8):
>
> Here are some comments from a quick glance over the series, to avoid
> too much noise that would distract from reports about the upcoming
> release.
Sorry for the late reply. Somehow I managed to miss your mail.
>> Re-add resolve_ref() that always returns an allocated buffer
>
> Makes me nervous, since it would introduce memory leaks if some other
> patch in flight calls resolve_ref(). Why not call it ref_resolve() or
> something?
Yeah, I made the same mistake before and made it again.
>> Enable GIT_DEBUG_MEMCHECK on git_pathname()
>
> __VA_ARGS__ was introduced in C99. I suspect some compilers that
> currently can compile git don't support it. But if you need to use
> it, that wouldn't rule out doing so in some corner guarded with an
> #ifdef.
>
> Looks pleasant overall. I look forward to looking more closely at
> this later.
This patch is bonus anyway and I think I'll drop it. Keeping a bunch
of ifdefs looks really ugly. I think having git_pathname()'s static
buffer per callsite file would be a good thing to do instead.
--
Duy
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Nguyen Thai Ngoc Duy @ 2011-12-06 14:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vmqi98f.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/resolve-ref (2011-12-05) 2 commits
> (merged to 'next' on 2011-12-05 at cc79e86)
> + Copy resolve_ref() return value for longer use
> + Convert many resolve_ref() calls to read_ref*() and ref_exists()
>
> Will merge to 'master'.
I thought this would be replaced by a new version [1] I posted a while
ago (and forgot to address comments). Do you want me to rebase that on
top of this or replace it?
[1] http://article.gmane.org/gmane.comp.version-control.git/185580
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 12:48 UTC (permalink / raw)
To: kusmabite; +Cc: git, Junio C Hamano
In-Reply-To: <CABPQNSbOvVkSFAE32hXoZEZeHqg-+nHc93uGytfCPQMicu0uVg@mail.gmail.com>
On Tue, Dec 6, 2011 at 7:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> Wouldn't it make more sense to make the limit a config option rather
>>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>>> all, different platforms have different stack-limitations...
>>
>> Then it'd make more sense to make a compile time config based on
>> platform.
>
> Can how much stack each recursion use be calculated at compile-time?
> If so, I agree with you.
No, but at least we know default stack size of each platform and can
make pretty good limit based on that.
>> We could have a config option that can override the default,
>> but I really don't see the point of making long delta chains.
>
> Aha, I figured you _did_ see a point in this, because 128 seemed
> excessive to me already. I was thinking more that some platforms can
> have a much smaller stack than (I would expect to) fit in 128
> recursions (I've worked relatively recently with platforms with as
> small as a static 2k stack per process), so you might not be fixing
> the issue for such platforms. But that's not really your
> responsibility either ;)
Ah, I was thinking of an option that extends the limit, not shortens
it. Yes it makes sense in this case.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Erik Faye-Lund @ 2011-12-06 12:41 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8BuUn4htSR6jAJfbueOshE6-YVV5dZGSWTGXbkuA0HO=A@mail.gmail.com>
On Tue, Dec 6, 2011 at 1:32 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> 2011/12/6 Erik Faye-Lund <kusmabite@gmail.com>:
>> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>>> a hard limit so that index-pack does not run out of stack. Also stop
>>> people from producing such a long delta chains using "pack-object
>>> --depth=<too large>"
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>> I used to make very long delta chains and triggered this in index-pack.
>>> I did not care reporting because it's my fault anyway. Think again,
>>> index-pack is called at server side and a malicious client can
>>> trigger this. This patch does not improve the situation much, but at
>>> least we won't get sigsegv at server side.
>>>
>>
>> Wouldn't it make more sense to make the limit a config option rather
>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>> all, different platforms have different stack-limitations...
>
> Then it'd make more sense to make a compile time config based on
> platform.
Can how much stack each recursion use be calculated at compile-time?
If so, I agree with you.
> We could have a config option that can override the default,
> but I really don't see the point of making long delta chains.
Aha, I figured you _did_ see a point in this, because 128 seemed
excessive to me already. I was thinking more that some platforms can
have a much smaller stack than (I would expect to) fit in 128
recursions (I've worked relatively recently with platforms with as
small as a static 2k stack per process), so you might not be fixing
the issue for such platforms. But that's not really your
responsibility either ;)
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 12:32 UTC (permalink / raw)
To: kusmabite; +Cc: git, Junio C Hamano
In-Reply-To: <CABPQNSaE=TWGbBRMnthEuT181=XbOafPfgx88_JQnnQ6TiYyqw@mail.gmail.com>
2011/12/6 Erik Faye-Lund <kusmabite@gmail.com>:
> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>>
>
> Wouldn't it make more sense to make the limit a config option rather
> than a hard-coded value of 128 (which seems arbitrary to me)? After
> all, different platforms have different stack-limitations...
Then it'd make more sense to make a compile time config based on
platform. We could have a config option that can override the default,
but I really don't see the point of making long delta chains.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Erik Faye-Lund @ 2011-12-06 12:17 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1323068688-31481-1-git-send-email-pclouds@gmail.com>
2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> Too deep delta chains can cause stack overflow in get_base_data(). Set
> a hard limit so that index-pack does not run out of stack. Also stop
> people from producing such a long delta chains using "pack-object
> --depth=<too large>"
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> I used to make very long delta chains and triggered this in index-pack.
> I did not care reporting because it's my fault anyway. Think again,
> index-pack is called at server side and a malicious client can
> trigger this. This patch does not improve the situation much, but at
> least we won't get sigsegv at server side.
>
Wouldn't it make more sense to make the limit a config option rather
than a hard-coded value of 128 (which seems arbitrary to me)? After
all, different platforms have different stack-limitations...
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Erik Faye-Lund @ 2011-12-06 11:22 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <20111206055239.GA20671@sigill.intra.peff.net>
On Tue, Dec 6, 2011 at 6:52 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 05, 2011 at 09:01:52PM -0800, Junio C Hamano wrote:
>
>> * jk/credentials (2011-11-28) 20 commits
>> - fixup! 034c066e
>> - compat/getpass: add a /dev/tty implementation
>> - credential: use git_prompt instead of git_getpass
>> - prompt: add PROMPT_ECHO flag
>> - stub out getpass_echo function
>> - refactor git_getpass into generic prompt function
>> - move git_getpass to its own source file
>> - t: add test harness for external credential helpers
>> - credentials: add "store" helper
>> - strbuf: add strbuf_add*_urlencode
>> - credentials: add "cache" helper
>> - docs: end-user documentation for the credential subsystem
>> - credential: make relevance of http path configurable
>> - credential: add credential.*.username
>> - credential: apply helper config
>> - http: use credential API to get passwords
>> - credential: add function for parsing url components
>> - introduce credentials API
>> - t5550: fix typo
>> - test-lib: add test_config_global variant
>>
>> Expecting a reroll?
>
> Yes, I have a re-roll ready. I was holding back to see if some of the
> helper authors might comment, but got nothing. Perhaps the new version
> will spur some interest.
>
> Also, let's drop the top git_getpass bits from the topic for now (they
> will not be part of my rebase). They are a separate topic that can go on
> top, but I think there was some question from Erik of whether we should
> simply roll our own getpass().
>
>> * jk/upload-archive-use-start-command (2011-11-21) 1 commit
>> - upload-archive: use start_command instead of fork
>>
>> What's the status of this one?
>
> I think what you have in pu is good, but of course I didn't actually
> test it on Windows. Erik?
>
I tried to check out ee27ca4 and build, and got hit by a wall of warnings:
compat/mingw.h: In function 'waitpid':
compat/mingw.h:117: warning: pointer targets in passing argument 1 of
'_cwait' differ in signedness
d:\msysgit\mingw\bin\../lib/gcc/mingw32/4.4.0/../../../../include/process.h:60:
note: expected 'int *' but argument is of type 'unsigned int *'
This seems to be an issue that has been around since the birth of the
windows port, and can be fixed by making our waitpid signature
identical to what POSIX expects
(http://pubs.opengroup.org/onlinepubs/7908799/xsh/waitpid.html):
---8<---
diff --git a/compat/mingw.h b/compat/mingw.h
index a255898..2036013 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -111,7 +111,7 @@ static inline int mingw_unlink(const char *pathname)
}
#define unlink mingw_unlink
-static inline int waitpid(pid_t pid, unsigned *status, unsigned options)
+static inline int waitpid(pid_t pid, int *status, int options)
{
if (options == 0)
return _cwait(status, pid, 0);
---8<---
But then, there's a couple of much more scary warnings:
decorate.c: In function 'hash_obj':
decorate.c:11: warning: dereferencing type-punned pointer will break
strict-aliasing rules
<snip>
object.c: In function 'hash_obj':
object.c:48: warning: dereferencing type-punned pointer will break
strict-aliasing rules
<snip>
builtin-merge-recursive.c: In function 'cmd_merge_recursive':
builtin-merge-recursive.c:49: warning: unknown conversion type character 'z' in
format
builtin-merge-recursive.c:49: warning: format '%s' expects type 'char *', but ar
gument 2 has type 'unsigned int'
builtin-merge-recursive.c:49: warning: too many arguments for format
Looking at the code, the first two looks like the real thing, and not
just false positives.
IIRC, the strict aliasing rule is not a part of C89, but is in C99.
And GCC starts to generate code that depends on strict-aliasing rules
with -O3. We don't use -O3, so this might be theoretical, at least on
my machine at this time.
The last one is a usage if "%zu", which our printf doesn't support.
This comes from commit 73118f89 ("merge-recursive.c: Add more generic
merge_recursive_generic()"), and should probably be fixed by doing:
---8<---
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 703045b..02242cc 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -45,8 +45,9 @@ int cmd_merge_recursive(int argc, const char **argv,
const char *prefix)
bases[bases_count++] = sha;
}
else
- warning("Cannot handle more than %zu bases. "
- "Ignoring %s.", ARRAY_SIZE(bases)-1, argv[i]);
+ warning("Cannot handle more than %"PRIuMAX" bases. "
+ "Ignoring %s.",
+ (uintmax_t)ARRAY_SIZE(bases)-1, argv[i]);
}
if (argc - i != 3) /* "--" "<head>" "<remote>" */
die("Not handling anything other than two heads merge.");
---8<---
But no matter what, something has clearly happened to our warning
level, and that should probably be investigated.
Back to topic: after fixing these (apart from the strict aliasing
issues), t5000-tar-tree pass fine here. So I guess that's at least
something. I haven't tested from git-daemon yet, but I don't have time
for that right now...
I'll submit the first fix later tonight, as it's clearly an
improvement IMO... but perhaps it's better if Stephan just squash in
the latter in the next round of the series, since it seems to be in a
pu-only series so far?
^ permalink raw reply related
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Nguyen Thai Ngoc Duy @ 2011-12-06 11:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vmqi98f.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Here are the topics that have been cooking. Commits prefixed with '-' are
> only in 'pu' (proposed updates) while commits prefixed with '+' are in
> 'next'.
What can I do to get "build in repack" [1] series in or moved forward?
[1] http://thread.gmane.org/gmane.comp.version-control.git/185190
--
Duy
^ permalink raw reply
* Re: Roadmap for 1.7.9
From: Nguyen Thai Ngoc Duy @ 2011-12-06 11:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3c2lr36.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 3:07 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I expect the following will not make much progress without further
> discussion:
>
> * Ignored vs Precious (Nguyen Thai Ngoc Duy)
I'm working on adding --exclude-standard and --exclude-from to
read-tree. Apart from that I don't plan on supporting precious files
because I don't use them.
--
Duy
^ permalink raw reply
* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-06 9:22 UTC (permalink / raw)
To: Johannes Sixt; +Cc: flatworm, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDDE5C.6040200@st.com>
On 12/6/2011 2:50 PM, Viresh Kumar wrote:
> On 12/6/2011 2:41 PM, Johannes Sixt wrote:
>> Am 12/6/2011 9:23, schrieb Viresh Kumar:
>> $ git commit --amend -a -C HEAD
>>
>> But let's count keystrokes (after -a):
>>
>> <BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
>> 10 keystrokes (more if you release SHIFT before D)
>>
>> But if vi pops up you have:
>>
>> <ENTER><SHIFT>ZZ
>> 4 keystrokes
>>
>> Where is the advantage of the option?
>>
>
> Thanks guys.
>
> @Johannes: You are right but, i will make an alias for the entire command.
> So keystrokes are same for me. :)
>
There is one more benefit, we don't have to wait for git. We can simply switch to
some other window and work. And can write further command on the same window, in
the time git processes commit --amend.
--
viresh
^ permalink raw reply
* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-06 9:20 UTC (permalink / raw)
To: Johannes Sixt, flatworm; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDDC38.8080108@viscovery.net>
On 12/6/2011 2:41 PM, Johannes Sixt wrote:
> Am 12/6/2011 9:23, schrieb Viresh Kumar:
> $ git commit --amend -a -C HEAD
>
> But let's count keystrokes (after -a):
>
> <BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
> 10 keystrokes (more if you release SHIFT before D)
>
> But if vi pops up you have:
>
> <ENTER><SHIFT>ZZ
> 4 keystrokes
>
> Where is the advantage of the option?
>
Thanks guys.
@Johannes: You are right but, i will make an alias for the entire command.
So keystrokes are same for me. :)
--
viresh
^ permalink raw reply
* Re: Query on git commit amend
From: Johannes Sixt @ 2011-12-06 9:11 UTC (permalink / raw)
To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>
Am 12/6/2011 9:23, schrieb Viresh Kumar:
>
> Hello,
>
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to change
> message.
$ git commit --amend -a -C HEAD
But let's count keystrokes (after -a):
<BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
10 keystrokes (more if you release SHIFT before D)
But if vi pops up you have:
<ENTER><SHIFT>ZZ
4 keystrokes
Where is the advantage of the option?
-- Hannes
^ permalink raw reply
* Re: Query on git commit amend
From: Konstantin Khomoutov @ 2011-12-06 9:01 UTC (permalink / raw)
To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>
On Tue, 6 Dec 2011 13:53:00 +0530
Viresh Kumar <viresh.kumar@st.com> wrote:
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to
> change message.
>
> If there is no such way, then can we add a patch for this, if it
> looks a valid case.
git commit --amend -C HEAD
^ permalink raw reply
* Query on git commit amend
From: Viresh Kumar @ 2011-12-06 8:23 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Shiraz HASHIM
Hello,
Suppose i want to add few new changes to my last commit (HEAD).
The way i do it is
$ git add all_changed_files
$ git commit --amend
OR
$ git commit --amend -a
With both these ways, i get a screen to edit the message too.
I want to know if there is a way to skip this screen.
i.e.
$ git commit --amend -a -some_other_option
which simply adds new changes to existing commit, without asking to change
message.
If there is no such way, then can we add a patch for this, if it looks a valid
case.
--
viresh
^ permalink raw reply
* Re: Proposal: create meaningful aliases for git reset's hard/soft/mixed
From: Philippe Vaucher @ 2011-12-06 7:34 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, git, Christian Couder
In-Reply-To: <CABURp0pmnsgE1ywW-W2+QFNci=3Lm=JKj9Y3U8zjh8+Cg_NA6Q@mail.gmail.com>
> Think about why you need to use git-reset. Why do new users need to
> use git-reset? What is it they are after?
Ok, so let's forget about git reset and let's focus on the features
instead. If I got it right you suggested the features that people
wants most often are uncommit, unadd/undelete and undo. Here's a new
proposal (based on your input):
uncommit: git jump <commit> (currently "git reset --soft <commit>")
unadd/undelete: git unstage file (currently "git reset --mixed file"
(with "git checkout file" for deleted files)
undo: git checkout --force --clean <commit> (currently "git reset
--hard <commit> && git clean -fd")
So, let's try out some scenarios:
1) Newbie user clones/pulls a repository from somewhere. He hacks
around and then things go bad, and he decides to scratch away
everything he did to make sure things are like they're supposed to be.
He'd then type "git checkout --force --clean master". If he didn't
introduce new files, he would simply type "git checkout --force
master"
2) Newbie adds some file to the index, then realise he added one too
many. He wants to remove it from being added. He'd then type "git
unstage file".
3) Average user creates a commit and suddenly realise he actually
wanted to split that commit in two (he cannot use --amend, and he's
not a rebase -i guru yet). Or he did a "temp" commit because he don't
know about "git stash" yet and wants to discard it. He wants to simply
go back to the previous state while keeping his changes in the index
and the worktree. He'd then type "git jump HEAD^1".
Feel free to add more scenarios!
Philippe
^ permalink raw reply
* Re: Evaluation of ref-api branch status
From: Junio C Hamano @ 2011-12-06 6:57 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git discussion list
In-Reply-To: <4EDDAC4A.4030805@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 12/05/2011 07:26 PM, Junio C Hamano wrote:
> ...
>> My reading of your summary suggests that it would be easiest to drop the
>> three mh/ref-api* topics from my tree, especially the 'refs: loosen
>> over-strict "format" check' band-aid patches, and re-queue a re-roll from
>> you.
>
> OK, then, I will try to re-roll the series on top of master, and build
> the equivalent of your quick-fix into the logical point in the series.
These quick-fixes were necessary _only_ because the queued topics predate
the real fix already in 1.7.8 (and I generally refuse to criss cross merge
from master to topics), so I expect you won't need their equivalents if
the topic is rebased on top of 1.7.8
> ... How much time to I have to work on this
> while still leaving enough time to comfortably integrate it into 1.7.9?
I am hoping we can have rc0 very early next year [*1*].
[Reference]
*1* https://www.google.com/calendar/embed?src=jfgbl2mrlipp4pb6ieih0qr3so%40group.calendar.google.com&ctz=America/Los_Angeles
^ permalink raw reply
* Re: Suggestion on hashing
From: Nguyen Thai Ngoc Duy @ 2011-12-06 6:23 UTC (permalink / raw)
To: Bill Zaumen; +Cc: Chris West (Faux), Jeff King, Git Mailing List
In-Reply-To: <1323151347.1745.73.camel@yos>
On Tue, Dec 6, 2011 at 1:02 PM, Bill Zaumen <bill.zaumen@gmail.com> wrote:
> On Tue, 2011-12-06 at 11:46 +0700, Nguyen Thai Ngoc Duy wrote:
>> On Tue, Dec 6, 2011 at 8:56 AM, Chris West (Faux) <faux@goeswhere.com> wrote:
>> >
>> > Nguyen Thai Ngoc Duy wrote:
>> >>
>> >> SHA-1 charateristics (like 20 byte length) are hard coded everywhere
>> >> in git, it'd be a big audit.
>> >
>> >
>> > I was planning to look at this anyway. My branch[1] allows
>> > init/add/commit with SHA-256, SHA-512 and all the SHA-3 candidates.
>>
>> Great!
>
> If you are replacing SHA-1 as an object ID with another hash function,
> two things to watch are submodules and alternative object databases.
> Because of those, it is necessary to worry about the order in which
> repositories are converted. In the worst case for submodules, you'd
> have to do multiple repositories at the same time, switching between
> them depending on what you need at each point.
I know migration would be painful. But note that new repos can benefit
stronger digest without legacy (of course until it links to an old
repo). For submodules, I think we should extend it to become something
similar to soft-link: git link is an SHA-1 to a text file that
contains SHA-1 and maybe other digests of the submodule's tip.
--
Duy
^ permalink raw reply
* [PATCHv2 13/13] t: add test harness for external credential helpers
From: Jeff King @ 2011-12-06 6:23 UTC (permalink / raw)
To: git
In-Reply-To: <20111206062127.GA29046@sigill.intra.peff.net>
We already have tests for the internal helpers, but it's
nice to give authors of external tools an easy way to
sanity-check their helpers.
If you have written the "git-credential-foo" helper, you can
do so with:
GIT_TEST_CREDENTIAL_HELPER=foo \
make t0303-credential-external.sh
This assumes that your helper is capable of both storing and
retrieving credentials (some helpers may be read-only, and
they will fail these tests).
If your helper supports time-based expiration with a
configurable timeout, you can test that feature like this:
GIT_TEST_CREDENTIAL_HELPER_TIMEOUT="foo --timeout=1" \
make t0303-credential-external.sh
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0303-credential-external.sh | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
create mode 100755 t/t0303-credential-external.sh
diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh
new file mode 100755
index 0000000..79b046f
--- /dev/null
+++ b/t/t0303-credential-external.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description='external credential helper tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+if test -z "$GIT_TEST_CREDENTIAL_HELPER"; then
+ say "# skipping external helper tests (set GIT_TEST_CREDENTIAL_HELPER)"
+else
+ helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER"
+ helper_test "$GIT_TEST_CREDENTIAL_HELPER"
+# helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER"
+fi
+
+if test -z "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"; then
+ say "# skipping external helper timeout tests"
+else
+ helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"
+ helper_test_timeout "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"
+ helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"
+fi
+
+test_done
--
1.7.8.rc4.4.g884ec
^ permalink raw reply related
* [PATCHv2 12/13] credentials: add "store" helper
From: Jeff King @ 2011-12-06 6:23 UTC (permalink / raw)
To: git
In-Reply-To: <20111206062127.GA29046@sigill.intra.peff.net>
This is like "cache", except that we actually put the
credentials on disk. This can be terribly insecure, of
course, but we do what we can to protect them by filesystem
permissions, and we warn the user in the documentation.
This is not unlike using .netrc to store entries, but it's a
little more user-friendly. Instead of putting credentials in
place ahead of time, we transparently store them after
prompting the user for them once.
Signed-off-by: Jeff King <peff@peff.net>
---
.gitignore | 1 +
Documentation/git-credential-store.txt | 75 ++++++++++++++++
Documentation/gitcredentials.txt | 5 +
Makefile | 1 +
credential-store.c | 148 ++++++++++++++++++++++++++++++++
t/t0302-credential-store.sh | 9 ++
6 files changed, 239 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-credential-store.txt
create mode 100644 credential-store.c
create mode 100755 t/t0302-credential-store.sh
diff --git a/.gitignore b/.gitignore
index a6b0bd4..2b7a3f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@
/git-count-objects
/git-credential-cache
/git-credential-cache--daemon
+/git-credential-store
/git-cvsexportcommit
/git-cvsimport
/git-cvsserver
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
new file mode 100644
index 0000000..3109346
--- /dev/null
+++ b/Documentation/git-credential-store.txt
@@ -0,0 +1,75 @@
+git-credential-store(1)
+=======================
+
+NAME
+----
+git-credential-store - helper to store credentials on disk
+
+SYNOPSIS
+--------
+-------------------
+git config credential.helper 'store [options]'
+-------------------
+
+DESCRIPTION
+-----------
+
+NOTE: Using this helper will store your passwords unencrypted on disk,
+protected only by filesystem permissions. If this is not an acceptable
+security tradeoff, try linkgit:git-credential-cache[1], or find a helper
+that integrates with secure storage provided by your operating system.
+
+This command stores credentials indefinitely on disk for use by future
+git programs.
+
+You probably don't want to invoke this command directly; it is meant to
+be used as a credential helper by other parts of git. See
+linkgit:gitcredentials[7] or `EXAMPLES` below.
+
+OPTIONS
+-------
+
+--store=<path>::
+
+ Use `<path>` to store credentials. The file will have its
+ filesystem permissions set to prevent other users on the system
+ from reading it, but will not be encrypted or otherwise
+ protected. Defaults to `~/.git-credentials`.
+
+EXAMPLES
+--------
+
+The point of this helper is to reduce the number of times you must type
+your username or password. For example:
+
+------------------------------------------
+$ git config credential.helper store
+$ git push http://example.com/repo.git
+Username: <type your username>
+Password: <type your password>
+
+[several days later]
+$ git push http://example.com/repo.git
+[your credentials are used automatically]
+------------------------------------------
+
+STORAGE FORMAT
+--------------
+
+The `.git-credentials` file is stored in plaintext. Each credential is
+stored on its own line as a URL like:
+
+------------------------------
+https://user:pass@example.com
+------------------------------
+
+When git needs authentication for a particular URL context,
+credential-store will consider that context a pattern to match against
+each entry in the credentials file. If the protocol, hostname, and
+username (if we already have one) match, then the password is returned
+to git. See the discussion of configuration in linkgit:gitcredentials[7]
+for more information.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 4e3f860..066f825 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -71,6 +71,11 @@ cache::
Cache credentials in memory for a short period of time. See
linkgit:git-credential-cache[1] for details.
+store::
+
+ Store credentials indefinitely on disk. See
+ linkgit:git-credential-store[1] for details.
+
You may also have third-party helpers installed; search for
`credential-*` in the output of `git help -a`, and consult the
documentation of individual helpers. Once you have selected a helper,
diff --git a/Makefile b/Makefile
index 5d41c29..2537128 100644
--- a/Makefile
+++ b/Makefile
@@ -429,6 +429,7 @@ PROGRAM_OBJS += http-backend.o
PROGRAM_OBJS += sh-i18n--envsubst.o
PROGRAM_OBJS += credential-cache.o
PROGRAM_OBJS += credential-cache--daemon.o
+PROGRAM_OBJS += credential-store.o
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
diff --git a/credential-store.c b/credential-store.c
new file mode 100644
index 0000000..ed58768
--- /dev/null
+++ b/credential-store.c
@@ -0,0 +1,148 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+
+static struct lock_file credential_lock;
+
+static void parse_credential_file(const char *fn,
+ struct credential *c,
+ void (*match_cb)(struct credential *),
+ void (*other_cb)(struct strbuf *))
+{
+ FILE *fh;
+ struct strbuf line = STRBUF_INIT;
+ struct credential entry = CREDENTIAL_INIT;
+
+ fh = fopen(fn, "r");
+ if (!fh) {
+ if (errno != ENOENT)
+ die_errno("unable to open %s", fn);
+ return;
+ }
+
+ while (strbuf_getline(&line, fh, '\n') != EOF) {
+ credential_from_url(&entry, line.buf);
+ if (entry.username && entry.password &&
+ credential_match(c, &entry)) {
+ if (match_cb) {
+ match_cb(&entry);
+ break;
+ }
+ }
+ else if (other_cb)
+ other_cb(&line);
+ }
+
+ credential_clear(&entry);
+ strbuf_release(&line);
+ fclose(fh);
+}
+
+static void print_entry(struct credential *c)
+{
+ printf("username=%s\n", c->username);
+ printf("password=%s\n", c->password);
+}
+
+static void print_line(struct strbuf *buf)
+{
+ strbuf_addch(buf, '\n');
+ write_or_die(credential_lock.fd, buf->buf, buf->len);
+}
+
+static void rewrite_credential_file(const char *fn, struct credential *c,
+ struct strbuf *extra)
+{
+ if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
+ die_errno("unable to get credential storage lock");
+ if (extra)
+ print_line(extra);
+ parse_credential_file(fn, c, NULL, print_line);
+ if (commit_lock_file(&credential_lock) < 0)
+ die_errno("unable to commit credential store");
+}
+
+static void store_credential(const char *fn, struct credential *c)
+{
+ struct strbuf buf = STRBUF_INIT;
+
+ /*
+ * Sanity check that what we are storing is actually sensible.
+ * In particular, we can't make a URL without a protocol field.
+ * Without either a host or pathname (depending on the scheme),
+ * we have no primary key. And without a username and password,
+ * we are not actually storing a credential.
+ */
+ if (!c->protocol || !(c->host || c->path) ||
+ !c->username || !c->password)
+ return;
+
+ strbuf_addf(&buf, "%s://", c->protocol);
+ strbuf_addstr_urlencode(&buf, c->username, 1);
+ strbuf_addch(&buf, ':');
+ strbuf_addstr_urlencode(&buf, c->password, 1);
+ strbuf_addch(&buf, '@');
+ if (c->host)
+ strbuf_addstr_urlencode(&buf, c->host, 1);
+ if (c->path) {
+ strbuf_addch(&buf, '/');
+ strbuf_addstr_urlencode(&buf, c->path, 0);
+ }
+
+ rewrite_credential_file(fn, c, &buf);
+ strbuf_release(&buf);
+}
+
+static void remove_credential(const char *fn, struct credential *c)
+{
+ rewrite_credential_file(fn, c, NULL);
+}
+
+static int lookup_credential(const char *fn, struct credential *c)
+{
+ parse_credential_file(fn, c, print_entry, NULL);
+ return c->username && c->password;
+}
+
+int main(int argc, const char **argv)
+{
+ const char * const usage[] = {
+ "git credential-store [options] <action>",
+ NULL
+ };
+ const char *op;
+ struct credential c = CREDENTIAL_INIT;
+ char *file = NULL;
+ struct option options[] = {
+ OPT_STRING_LIST(0, "file", &file, "path",
+ "fetch and store credentials in <path>"),
+ OPT_END()
+ };
+
+ umask(077);
+
+ argc = parse_options(argc, argv, NULL, options, usage, 0);
+ if (argc != 1)
+ usage_with_options(usage, options);
+ op = argv[0];
+
+ if (!file)
+ file = expand_user_path("~/.git-credentials");
+ if (!file)
+ die("unable to set up default path; use --file");
+
+ if (credential_read(&c, stdin) < 0)
+ die("unable to read credential");
+
+ if (!strcmp(op, "get"))
+ lookup_credential(file, &c);
+ else if (!strcmp(op, "erase"))
+ remove_credential(file, &c);
+ else if (!strcmp(op, "store"))
+ store_credential(file, &c);
+ else
+ ; /* Ignore unknown operation. */
+
+ return 0;
+}
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
new file mode 100755
index 0000000..f61b40c
--- /dev/null
+++ b/t/t0302-credential-store.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+test_description='credential-store tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+helper_test store
+
+test_done
--
1.7.8.rc4.4.g884ec
^ permalink raw reply related
* [PATCHv2 11/13] strbuf: add strbuf_add*_urlencode
From: Jeff King @ 2011-12-06 6:23 UTC (permalink / raw)
To: git
In-Reply-To: <20111206062127.GA29046@sigill.intra.peff.net>
This just follows the rfc3986 rules for percent-encoding
url data into a strbuf.
Signed-off-by: Jeff King <peff@peff.net>
---
strbuf.c | 37 +++++++++++++++++++++++++++++++++++++
strbuf.h | 5 +++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 3ad2cc0..60e5e59 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -397,3 +397,40 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
return len;
}
+
+static int is_rfc3986_reserved(char ch)
+{
+ switch (ch) {
+ case '!': case '*': case '\'': case '(': case ')': case ';':
+ case ':': case '@': case '&': case '=': case '+': case '$':
+ case ',': case '/': case '?': case '#': case '[': case ']':
+ return 1;
+ }
+ return 0;
+}
+
+static int is_rfc3986_unreserved(char ch)
+{
+ return isalnum(ch) ||
+ ch == '-' || ch == '_' || ch == '.' || ch == '~';
+}
+
+void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
+ int reserved)
+{
+ strbuf_grow(sb, len);
+ while (len--) {
+ char ch = *s++;
+ if (is_rfc3986_unreserved(ch) ||
+ (!reserved && is_rfc3986_reserved(ch)))
+ strbuf_addch(sb, ch);
+ else
+ strbuf_addf(sb, "%%%02x", ch);
+ }
+}
+
+void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,
+ int reserved)
+{
+ strbuf_add_urlencode(sb, s, strlen(s), reserved);
+}
diff --git a/strbuf.h b/strbuf.h
index 46a33f8..cecd48c 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -115,4 +115,9 @@ struct strbuf_expand_dict_entry {
extern int strbuf_branchname(struct strbuf *sb, const char *name);
extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+extern void strbuf_add_urlencode(struct strbuf *, const char *, size_t,
+ int reserved);
+extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
+ int reserved);
+
#endif /* STRBUF_H */
--
1.7.8.rc4.4.g884ec
^ 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