* Re: Lack of detached signatures
From: Junio C Hamano @ 2011-09-29 0:28 UTC (permalink / raw)
To: Ted Ts'o
Cc: Jeff King, Joseph Parmelee, Carlos Martín Nieto,
Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <20110928230958.GJ19250@thunk.org>
Ted Ts'o <tytso@mit.edu> writes:
> On Wed, Sep 28, 2011 at 06:25:43PM -0400, Jeff King wrote:
>> [1] This is a minor nit, and probably not worth breaking away from the
>> way the rest of the world does it, but it is somewhat silly to sign the
>> compressed data. I couldn't care less about the exact bytes in the
>> compressed version; what I care about is the actual tar file. The
>> compression is just a transport.
>
> The worry I have is that many users don't check the GPG checksum files
> as it is. If they have to decompress the file, and then run gpg to
> check the checksum, they might never get around to doing it.
>
> That being said, I'm not sure I have a good solution. One is to ship
> the file without using detached signatures, and ship a foo.tar.gz.gpg
> file, and force them to use GPG to unwrap the file before it can be
> unpacked. But users would yell and scream if we did that...
I suspect that letting GPG do the compression and shipping foo.tar.gpg
would work just fine as well, and it is somewhat a tempting response to a
_demand_ to sign materials we distribute. Of course, a nicer response to a
_request_ would be to give a detached signature ;-)
I understand that the automated GPG signature k.org used to use on the
master machine was primarily to protect the copies that the mirrors serve
from getting tampered after they leave the master machine. Do you happen
to know what the new policy will be? Will the developers who distribute
their snapshot tarballs from the site be GPG signing them themselves
before uploading? That would improve the situation (I suspect that there
were some people who misunderstood that these GPG signature were to
protect against break-in at the master machine), but at the same time, it
may create the chicken-and-egg bootstrapping problem if public keys of too
many people need to be published securely.
^ permalink raw reply
* Re: Lack of detached signatures
From: Ted Ts'o @ 2011-09-28 23:09 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Joseph Parmelee, Carlos Martín Nieto,
Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <20110928222542.GA18120@sigill.intra.peff.net>
On Wed, Sep 28, 2011 at 06:25:43PM -0400, Jeff King wrote:
> [1] This is a minor nit, and probably not worth breaking away from the
> way the rest of the world does it, but it is somewhat silly to sign the
> compressed data. I couldn't care less about the exact bytes in the
> compressed version; what I care about is the actual tar file. The
> compression is just a transport.
The worry I have is that many users don't check the GPG checksum files
as it is. If they have to decompress the file, and then run gpg to
check the checksum, they might never get around to doing it.
That being said, I'm not sure I have a good solution. One is to ship
the file without using detached signatures, and ship a foo.tar.gz.gpg
file, and force them to use GPG to unwrap the file before it can be
unpacked. But users would yell and scream if we did that...
- Ted
^ permalink raw reply
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Junio C Hamano @ 2011-09-28 22:37 UTC (permalink / raw)
To: Pang Yan Han
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <20110928153935.GA7800@myhost>
Pang Yan Han <pangyanhan@gmail.com> writes:
> +/* For invalid refs */
> +static struct command **invalid_delete;
> +static size_t invalid_delete_nr;
> +static size_t invalid_delete_alloc;
Do you have to have these separately only to keep track of the corner case
errors? I would have expected that it would be more natural to mark them
by adding a single bitfield to "struct command".
> @@ -447,6 +467,8 @@ static const char *update(struct command *cmd)
> if (!parse_object(old_sha1)) {
> rp_warning("Allowing deletion of corrupt ref.");
> old_sha1 = NULL;
> + if (!ref_exists((char *) name))
> + invalid_delete_append(cmd);
This is not an "invalid" delete but deleting a non-existing ref. Perhaps
you would want to move the warning and optionally reword it as well, e.g.
if (!parse_object(old_sha1)) {
old_sha1 = NULL;
if (ref_exists(name)) {
rp_warning("Allowing deletion of corrupt ref.");
} else {
rp_warning("Deleting a ref that does not exist.");
cmd->did_not_exist = 1;
}
...
^ permalink raw reply
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Junio C Hamano @ 2011-09-28 23:28 UTC (permalink / raw)
To: Pang Yan Han
Cc: Junio C Hamano, git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <20110928230809.GA1798@myhost>
Pang Yan Han <pangyanhan@gmail.com> writes:
> I am unable to reply this until much later, but are the tests in this patch ok?
> It's the first time I'm writing test cases for git.
I'll squash in a bit more updates and later publish the result.
Thanks.
^ permalink raw reply
* Re: Lack of detached signatures
From: Joseph Parmelee @ 2011-09-28 22:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Carlos Martín Nieto, Olsen, Alan R, Michael Witten,
git@vger.kernel.org
In-Reply-To: <7v1uv01uqm.fsf@alter.siamese.dyndns.org>
On Wed, 28 Sep 2011, Junio C Hamano wrote:
> Joseph Parmelee <jparmele@wildbear.com> writes:
>
>> There is confusion here between the repository and the tarball. Once you
>> have produced the tarball there is NO cryptographic protection against
>> forgeries unless you sign it with GPG.
>
> True.
>
> If I give you a URL http://code.google.com/p/git-core/downloads/list with
> checksums
>
> $ sha1sum git-1.7.7.rc3.tar.gz
> c6ba05a833cab49dd66dd1e252306e187effbf2b git-1.7.7.rc3.tar.gz
>
> You either have to trust that code.google.com/ is not broken, or this
> message is coming from real Junio (provided if you can trust him in the
> first place).
>
How do I know that I am actually connected to code.google.com and not some
other site served up to me by a bogus proxy somewhere?
> BUT.
>
> The world is not so blank-and-white. Trust is ultimately among humans. If
> this message is not from the real Junio, don't you think you will hear
> something like "No, that c6ba05... is forgery, please don't use it!" from
> him, when he finds this message on the Git mailing list? If he does not
> exercise diligence to even do that much, does he deserve your trust in the
> first place?
>
> GPG does add security (if you have the key) but you can do pretty well
> even without it in practice.
>
Nonsense. There is a reason why responsible sites everywhere use detached
signatures on their release tarballs.
>> It is only because kernel.org exercised due diligence in the production of
>> tags and signatures on all their tarballs that the kernel code itself
>> withstood their recent intrusion....
>
> I do not think that is true at all. Developers just dropped *.tar.gz on a
> 'master' machine, and left the rest to a cron job that reflates the
> tarball into *.tar.bz2, sign both using a GPG key, and mirror them to the
> public-facing machines 'www'.
>
> Somebody who had access to the 'master' machine could add a new tarball
> and have it go thru the same exact process, getting signed by the cron.
>
The "cron job" provided the passphrase for the signing as well instead of
requiring a human to authorize the transaction by providing the passphrase
or by some other means? I suspect not. Have you actually used GPG to sign
something?
And even if that egregious error (no human authorization) had been made,
there is the matter of the secret signing key. Of course if the bad guys
have that (and the passphrase) then they have everything and can readily
prepare fraudulent packages. But without a detached signature you are
allowing them to do it even without going to the bother of stealing the
secret key and breaking/stealing the passphrase. Without a dual key
signature, you are providing ABSOLUTELY NO protection against
man-in-the-middle attacks that you will never know occurred, but which will
nevertheless (rightfully) reflect on your project. To just assert that "you
can do pretty well even without it in practice" is just plain irresponsible
and convinces me not to update our copies of git until it returns to
kernel.org and to administrators that understand the situation.
^ permalink raw reply
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Pang Yan Han @ 2011-09-28 23:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7voby4xo18.fsf@alter.siamese.dyndns.org>
Sorry Junio, disregard my earlier message. I didn't see this email.
On Wed, Sep 28, 2011 at 04:09:07PM -0700, Junio C Hamano wrote:
> Here is a quick fix-up on top.
>
> Points of interest:
>
> - Get rid of an unnecessary list; just a single bitfield is sufficient;
> while at it, make skip_update also a bitfield.
>
> - ref_exists(refname) calls resolve_ref() which wants "const char *",
> and does not touch refname itself. Make it to take "const char *",
> instead of casting const away in the new caller.
>
> - Properly indent test script. Also:
>
> - Use "cmd <<-EOF" to allow indenting the here text with leading tabs;
>
> - Strip useless use of single quote around "cmd <<'EOF'" inside
> test_expect_success; they were only stepping out of the sq context
> while the third argument to test_expect_success was being parsed. A
> construct like this:
>
> cat >pre-receive.expect <<'EOF'
> $orgmaster $newmaster refs/heads/master
> EOF
>
> only misleads the readers as if the file is getting literal variable
> names without substitution, which is not what is going on.
>
> I didn't fix this in this fix-up patch, but you also need to test the case
> where _ONLY_ deletion of a non-existent ref is requested, in which case,
> pre-receive and update should be told about it, but post-receive and
> post-update should not be even run (i.e. test the absense of these
> post-*.actual files).
>
> builtin/receive-pack.c | 41 ++++++---------------
> refs.c | 2 +-
> refs.h | 2 +-
> t/t5516-fetch-push.sh | 90 ++++++++++++++++++++++++-----------------------
> 4 files changed, 60 insertions(+), 75 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 8a0a9d2..b73f18a 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -147,32 +147,13 @@ static void write_head_info(void)
> struct command {
> struct command *next;
> const char *error_string;
> - unsigned int skip_update;
> + unsigned int skip_update:1,
> + did_not_exist:1;
> unsigned char old_sha1[20];
> unsigned char new_sha1[20];
> char ref_name[FLEX_ARRAY]; /* more */
> };
>
> -/* For invalid refs */
> -static struct command **invalid_delete;
> -static size_t invalid_delete_nr;
> -static size_t invalid_delete_alloc;
> -
> -static void invalid_delete_append(struct command *cmd)
> -{
> - ALLOC_GROW(invalid_delete, invalid_delete_nr + 1, invalid_delete_alloc);
> - invalid_delete[invalid_delete_nr++] = cmd;
> -}
> -
> -static int is_invalid_delete(struct command *cmd)
> -{
> - size_t i;
> - for (i = 0; i < invalid_delete_nr; i++)
> - if (invalid_delete[i] == cmd)
> - return 1;
> - return 0;
> -}
> -
> static const char pre_receive_hook[] = "hooks/pre-receive";
> static const char post_receive_hook[] = "hooks/post-receive";
>
> @@ -235,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
> int have_input = 0, code;
>
> for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
> - if (!cmd->error_string && !is_invalid_delete(cmd))
> + if (!cmd->error_string && !cmd->did_not_exist)
> have_input = 1;
> }
>
> @@ -268,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
> }
>
> for (cmd = commands; cmd; cmd = cmd->next) {
> - if (!cmd->error_string && !is_invalid_delete(cmd)) {
> + if (!cmd->error_string && !cmd->did_not_exist) {
> size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
> sha1_to_hex(cmd->old_sha1),
> sha1_to_hex(cmd->new_sha1),
> @@ -465,10 +446,13 @@ static const char *update(struct command *cmd)
>
> if (is_null_sha1(new_sha1)) {
> if (!parse_object(old_sha1)) {
> - rp_warning("Allowing deletion of corrupt ref.");
> old_sha1 = NULL;
> - if (!ref_exists((char *) name))
> - invalid_delete_append(cmd);
> + if (ref_exists(name)) {
> + rp_warning("Allowing deletion of corrupt ref.");
> + } else {
> + rp_warning("Deleting a non-existent ref.");
> + cmd->did_not_exist = 1;
> + }
> }
> if (delete_ref(namespaced_name, old_sha1, 0)) {
> rp_error("failed to delete %s", name);
> @@ -499,7 +483,7 @@ static void run_update_post_hook(struct command *commands)
> struct child_process proc;
>
> for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
> - if (cmd->error_string || is_invalid_delete(cmd))
> + if (cmd->error_string || cmd->did_not_exist)
> continue;
> argc++;
> }
> @@ -510,7 +494,7 @@ static void run_update_post_hook(struct command *commands)
>
> for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
> char *p;
> - if (cmd->error_string || is_invalid_delete(cmd))
> + if (cmd->error_string || cmd->did_not_exist)
> continue;
> p = xmalloc(strlen(cmd->ref_name) + 1);
> strcpy(p, cmd->ref_name);
> @@ -888,6 +872,5 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
> }
> if (use_sideband)
> packet_flush(1);
> - free(invalid_delete);
> return 0;
> }
> diff --git a/refs.c b/refs.c
> index a615043..43e9225 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,
> return 0;
> }
>
> -int ref_exists(char *refname)
> +int ref_exists(const char *refname)
> {
> unsigned char sha1[20];
> return !!resolve_ref(refname, sha1, 1, NULL);
> diff --git a/refs.h b/refs.h
> index dfb086e..4431205 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
> */
> extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
> extern void clear_extra_refs(void);
> -extern int ref_exists(char *);
> +extern int ref_exists(const char *);
>
> extern int peel_ref(const char *, unsigned char *);
>
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index c0d8a0e..47db4b1 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -43,33 +43,34 @@ mk_test () {
> mk_test_with_hooks() {
> mk_test "$@" &&
> (
> - cd testrepo &&
> - mkdir .git/hooks &&
> - cd .git/hooks &&
> + cd testrepo &&
> + mkdir .git/hooks &&
> + cd .git/hooks &&
>
> - cat >pre-receive <<'EOF' &&
> -#!/bin/sh
> -cat - >>pre-receive.actual
> -EOF
> + cat >pre-receive <<-'EOF' &&
> + #!/bin/sh
> + cat - >>pre-receive.actual
> + EOF
>
> - cat >update <<'EOF' &&
> -#!/bin/sh
> -printf "%s %s %s\n" "$@" >>update.actual
> -EOF
> - cat >post-receive <<'EOF' &&
> -#!/bin/sh
> -cat - >>post-receive.actual
> -EOF
> + cat >update <<-'EOF' &&
> + #!/bin/sh
> + printf "%s %s %s\n" "$@" >>update.actual
> + EOF
>
> - cat >post-update <<'EOF' &&
> -#!/bin/sh
> -for ref in "$@"
> -do
> - printf "%s\n" "$ref" >>post-update.actual
> -done
> -EOF
> + cat >post-receive <<-'EOF' &&
> + #!/bin/sh
> + cat - >>post-receive.actual
> + EOF
>
> - chmod u+x pre-receive update post-receive post-update
> + cat >post-update <<-'EOF' &&
> + #!/bin/sh
> + for ref in "$@"
> + do
> + printf "%s\n" "$ref" >>post-update.actual
> + done
> + EOF
> +
> + chmod +x pre-receive update post-receive post-update
> )
> }
>
> @@ -599,31 +600,32 @@ test_expect_success 'pushing valid refs triggers post-receive and post-update ho
> orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
> newnext=$_z40 &&
> git push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&
> - (cd testrepo/.git &&
> - cat >pre-receive.expect <<'EOF' &&
> -$orgmaster $newmaster refs/heads/master
> -$orgnext $newnext refs/heads/next
> -EOF
> + (
> + cd testrepo/.git &&
> + cat >pre-receive.expect <<-EOF &&
> + $orgmaster $newmaster refs/heads/master
> + $orgnext $newnext refs/heads/next
> + EOF
>
> - cat >update.expect <<'EOF' &&
> -refs/heads/master $orgmaster $newmaster
> -refs/heads/next $orgnext $newnext
> -EOF
> + cat >update.expect <<-EOF &&
> + refs/heads/master $orgmaster $newmaster
> + refs/heads/next $orgnext $newnext
> + EOF
>
> - cat >post-receive.expect <<'EOF' &&
> -$orgmaster $newmaster refs/heads/master
> -$orgnext $newnext refs/heads/next
> -EOF
> + cat >post-receive.expect <<-EOF &&
> + $orgmaster $newmaster refs/heads/master
> + $orgnext $newnext refs/heads/next
> + EOF
>
> - cat >post-update.expect <<'EOF' &&
> -refs/heads/master
> -refs/heads/next
> -EOF
> + cat >post-update.expect <<-EOF &&
> + refs/heads/master
> + refs/heads/next
> + EOF
>
> - test_cmp pre-receive.expect pre-receive.actual &&
> - test_cmp update.expect update.actual &&
> - test_cmp post-receive.expect post-receive.actual &&
> - test_cmp post-update.expect post-update.actual
> + test_cmp pre-receive.expect pre-receive.actual &&
> + test_cmp update.expect update.actual &&
> + test_cmp post-receive.expect post-receive.actual &&
> + test_cmp post-update.expect post-update.actual
> )
> '
>
> --
> 1.7.7.rc3.4.g8d714
>
^ permalink raw reply
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Pang Yan Han @ 2011-09-28 23:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7vsjngxphv.fsf@alter.siamese.dyndns.org>
On Wed, Sep 28, 2011 at 03:37:32PM -0700, Junio C Hamano wrote:
> Pang Yan Han <pangyanhan@gmail.com> writes:
>
> > +/* For invalid refs */
> > +static struct command **invalid_delete;
> > +static size_t invalid_delete_nr;
> > +static size_t invalid_delete_alloc;
>
> Do you have to have these separately only to keep track of the corner case
> errors? I would have expected that it would be more natural to mark them
> by adding a single bitfield to "struct command".
Yes they are purely for keeping track of deleting non-existent refs.
Ok I will add the bitfield to struct command.
>
> > @@ -447,6 +467,8 @@ static const char *update(struct command *cmd)
> > if (!parse_object(old_sha1)) {
> > rp_warning("Allowing deletion of corrupt ref.");
> > old_sha1 = NULL;
> > + if (!ref_exists((char *) name))
> > + invalid_delete_append(cmd);
>
> This is not an "invalid" delete but deleting a non-existing ref. Perhaps
> you would want to move the warning and optionally reword it as well, e.g.
>
> if (!parse_object(old_sha1)) {
> old_sha1 = NULL;
> if (ref_exists(name)) {
> rp_warning("Allowing deletion of corrupt ref.");
> } else {
> rp_warning("Deleting a ref that does not exist.");
> cmd->did_not_exist = 1;
> }
> ...
Sure thing.
I am unable to reply this until much later, but are the tests in this patch ok?
It's the first time I'm writing test cases for git.
Thanks for the feedback!
^ permalink raw reply
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Junio C Hamano @ 2011-09-28 23:09 UTC (permalink / raw)
To: Pang Yan Han
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <20110928153935.GA7800@myhost>
Here is a quick fix-up on top.
Points of interest:
- Get rid of an unnecessary list; just a single bitfield is sufficient;
while at it, make skip_update also a bitfield.
- ref_exists(refname) calls resolve_ref() which wants "const char *",
and does not touch refname itself. Make it to take "const char *",
instead of casting const away in the new caller.
- Properly indent test script. Also:
- Use "cmd <<-EOF" to allow indenting the here text with leading tabs;
- Strip useless use of single quote around "cmd <<'EOF'" inside
test_expect_success; they were only stepping out of the sq context
while the third argument to test_expect_success was being parsed. A
construct like this:
cat >pre-receive.expect <<'EOF'
$orgmaster $newmaster refs/heads/master
EOF
only misleads the readers as if the file is getting literal variable
names without substitution, which is not what is going on.
I didn't fix this in this fix-up patch, but you also need to test the case
where _ONLY_ deletion of a non-existent ref is requested, in which case,
pre-receive and update should be told about it, but post-receive and
post-update should not be even run (i.e. test the absense of these
post-*.actual files).
builtin/receive-pack.c | 41 ++++++---------------
refs.c | 2 +-
refs.h | 2 +-
t/t5516-fetch-push.sh | 90 ++++++++++++++++++++++++-----------------------
4 files changed, 60 insertions(+), 75 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8a0a9d2..b73f18a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -147,32 +147,13 @@ static void write_head_info(void)
struct command {
struct command *next;
const char *error_string;
- unsigned int skip_update;
+ unsigned int skip_update:1,
+ did_not_exist:1;
unsigned char old_sha1[20];
unsigned char new_sha1[20];
char ref_name[FLEX_ARRAY]; /* more */
};
-/* For invalid refs */
-static struct command **invalid_delete;
-static size_t invalid_delete_nr;
-static size_t invalid_delete_alloc;
-
-static void invalid_delete_append(struct command *cmd)
-{
- ALLOC_GROW(invalid_delete, invalid_delete_nr + 1, invalid_delete_alloc);
- invalid_delete[invalid_delete_nr++] = cmd;
-}
-
-static int is_invalid_delete(struct command *cmd)
-{
- size_t i;
- for (i = 0; i < invalid_delete_nr; i++)
- if (invalid_delete[i] == cmd)
- return 1;
- return 0;
-}
-
static const char pre_receive_hook[] = "hooks/pre-receive";
static const char post_receive_hook[] = "hooks/post-receive";
@@ -235,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
int have_input = 0, code;
for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
- if (!cmd->error_string && !is_invalid_delete(cmd))
+ if (!cmd->error_string && !cmd->did_not_exist)
have_input = 1;
}
@@ -268,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
}
for (cmd = commands; cmd; cmd = cmd->next) {
- if (!cmd->error_string && !is_invalid_delete(cmd)) {
+ if (!cmd->error_string && !cmd->did_not_exist) {
size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
sha1_to_hex(cmd->old_sha1),
sha1_to_hex(cmd->new_sha1),
@@ -465,10 +446,13 @@ static const char *update(struct command *cmd)
if (is_null_sha1(new_sha1)) {
if (!parse_object(old_sha1)) {
- rp_warning("Allowing deletion of corrupt ref.");
old_sha1 = NULL;
- if (!ref_exists((char *) name))
- invalid_delete_append(cmd);
+ if (ref_exists(name)) {
+ rp_warning("Allowing deletion of corrupt ref.");
+ } else {
+ rp_warning("Deleting a non-existent ref.");
+ cmd->did_not_exist = 1;
+ }
}
if (delete_ref(namespaced_name, old_sha1, 0)) {
rp_error("failed to delete %s", name);
@@ -499,7 +483,7 @@ static void run_update_post_hook(struct command *commands)
struct child_process proc;
for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
- if (cmd->error_string || is_invalid_delete(cmd))
+ if (cmd->error_string || cmd->did_not_exist)
continue;
argc++;
}
@@ -510,7 +494,7 @@ static void run_update_post_hook(struct command *commands)
for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
char *p;
- if (cmd->error_string || is_invalid_delete(cmd))
+ if (cmd->error_string || cmd->did_not_exist)
continue;
p = xmalloc(strlen(cmd->ref_name) + 1);
strcpy(p, cmd->ref_name);
@@ -888,6 +872,5 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
}
if (use_sideband)
packet_flush(1);
- free(invalid_delete);
return 0;
}
diff --git a/refs.c b/refs.c
index a615043..43e9225 100644
--- a/refs.c
+++ b/refs.c
@@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,
return 0;
}
-int ref_exists(char *refname)
+int ref_exists(const char *refname)
{
unsigned char sha1[20];
return !!resolve_ref(refname, sha1, 1, NULL);
diff --git a/refs.h b/refs.h
index dfb086e..4431205 100644
--- a/refs.h
+++ b/refs.h
@@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
*/
extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
extern void clear_extra_refs(void);
-extern int ref_exists(char *);
+extern int ref_exists(const char *);
extern int peel_ref(const char *, unsigned char *);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c0d8a0e..47db4b1 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -43,33 +43,34 @@ mk_test () {
mk_test_with_hooks() {
mk_test "$@" &&
(
- cd testrepo &&
- mkdir .git/hooks &&
- cd .git/hooks &&
+ cd testrepo &&
+ mkdir .git/hooks &&
+ cd .git/hooks &&
- cat >pre-receive <<'EOF' &&
-#!/bin/sh
-cat - >>pre-receive.actual
-EOF
+ cat >pre-receive <<-'EOF' &&
+ #!/bin/sh
+ cat - >>pre-receive.actual
+ EOF
- cat >update <<'EOF' &&
-#!/bin/sh
-printf "%s %s %s\n" "$@" >>update.actual
-EOF
- cat >post-receive <<'EOF' &&
-#!/bin/sh
-cat - >>post-receive.actual
-EOF
+ cat >update <<-'EOF' &&
+ #!/bin/sh
+ printf "%s %s %s\n" "$@" >>update.actual
+ EOF
- cat >post-update <<'EOF' &&
-#!/bin/sh
-for ref in "$@"
-do
- printf "%s\n" "$ref" >>post-update.actual
-done
-EOF
+ cat >post-receive <<-'EOF' &&
+ #!/bin/sh
+ cat - >>post-receive.actual
+ EOF
- chmod u+x pre-receive update post-receive post-update
+ cat >post-update <<-'EOF' &&
+ #!/bin/sh
+ for ref in "$@"
+ do
+ printf "%s\n" "$ref" >>post-update.actual
+ done
+ EOF
+
+ chmod +x pre-receive update post-receive post-update
)
}
@@ -599,31 +600,32 @@ test_expect_success 'pushing valid refs triggers post-receive and post-update ho
orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
newnext=$_z40 &&
git push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&
- (cd testrepo/.git &&
- cat >pre-receive.expect <<'EOF' &&
-$orgmaster $newmaster refs/heads/master
-$orgnext $newnext refs/heads/next
-EOF
+ (
+ cd testrepo/.git &&
+ cat >pre-receive.expect <<-EOF &&
+ $orgmaster $newmaster refs/heads/master
+ $orgnext $newnext refs/heads/next
+ EOF
- cat >update.expect <<'EOF' &&
-refs/heads/master $orgmaster $newmaster
-refs/heads/next $orgnext $newnext
-EOF
+ cat >update.expect <<-EOF &&
+ refs/heads/master $orgmaster $newmaster
+ refs/heads/next $orgnext $newnext
+ EOF
- cat >post-receive.expect <<'EOF' &&
-$orgmaster $newmaster refs/heads/master
-$orgnext $newnext refs/heads/next
-EOF
+ cat >post-receive.expect <<-EOF &&
+ $orgmaster $newmaster refs/heads/master
+ $orgnext $newnext refs/heads/next
+ EOF
- cat >post-update.expect <<'EOF' &&
-refs/heads/master
-refs/heads/next
-EOF
+ cat >post-update.expect <<-EOF &&
+ refs/heads/master
+ refs/heads/next
+ EOF
- test_cmp pre-receive.expect pre-receive.actual &&
- test_cmp update.expect update.actual &&
- test_cmp post-receive.expect post-receive.actual &&
- test_cmp post-update.expect post-update.actual
+ test_cmp pre-receive.expect pre-receive.actual &&
+ test_cmp update.expect update.actual &&
+ test_cmp post-receive.expect post-receive.actual &&
+ test_cmp post-update.expect post-update.actual
)
'
--
1.7.7.rc3.4.g8d714
^ permalink raw reply related
* Re: Lack of detached signatures
From: Jeff King @ 2011-09-28 22:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Joseph Parmelee, Carlos Martín Nieto, Olsen, Alan R,
Michael Witten, git@vger.kernel.org
In-Reply-To: <7v1uv01uqm.fsf@alter.siamese.dyndns.org>
On Wed, Sep 28, 2011 at 09:45:21AM -0700, Junio C Hamano wrote:
> The world is not so blank-and-white. Trust is ultimately among humans. If
> this message is not from the real Junio, don't you think you will hear
> something like "No, that c6ba05... is forgery, please don't use it!" from
> him, when he finds this message on the Git mailing list? If he does not
> exercise diligence to even do that much, does he deserve your trust in the
> first place?
>
> GPG does add security (if you have the key) but you can do pretty well
> even without it in practice.
Your suggestion above is something like an audit trail. It doesn't
prevent all mischief from happening, but after it happens and is
noticed, we can do some analysis, figuring out what happened and how to
clean up. Banks do this all the time with transactions.
At the same time, banks don't rely solely on an audit trail. They also
have up-front mechanisms, like passwords and ATM secrets, that help
prevent mischief from happening in the first place. And when they fail,
we fall back to the audit trail.
So having preventative mechanisms and audit mechanisms is not an
either-or situation. They complement each other; the strengths of one
can help when the other fails.
In this case, I think signed tarballs would be a nice complement to the
natural human audit trail. It can stop some attacks early, without
having to worry about the effort of analyzing and cleaning up after the
fact. Can the signature be wrong, or be checked improperly? Of course.
If you realize your machine has been hacked and your key stolen, then
you let everybody know and we fall back to auditing what has already
happened.
Each mechanism you put in place has a cost, of course. And it's worth
thinking about whether that cost is worthwhile. But it really is as easy
as running "gpg --detach-sign" when you upload a release, isn't it? You
already do something similar with the signed tags. So the cost of the
mechanism is quite low.
> I do not think that is true at all. Developers just dropped *.tar.gz on a
> 'master' machine, and left the rest to a cron job that reflates the
> tarball into *.tar.bz2, sign both using a GPG key, and mirror them to the
> public-facing machines 'www'.
>
> Somebody who had access to the 'master' machine could add a new tarball
> and have it go thru the same exact process, getting signed by the cron.
Right. In theory the master machine is harder to hack than the public
facing mirrors, but in this case it was not. Every link in the trust
chain introduces new possibilities for failure.
Given that git releases are all made by you, why not just sign them
locally with the same key you use to sign the release tags? It does mean
you have to generate and upload the .bz2 yourself[1], but is that really
that big a deal?
-Peff
[1] This is a minor nit, and probably not worth breaking away from the
way the rest of the world does it, but it is somewhat silly to sign the
compressed data. I couldn't care less about the exact bytes in the
compressed version; what I care about is the actual tar file. The
compression is just a transport.
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-28 22:10 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Christian Couder, Thomas Rast, Julian Phillips
In-Reply-To: <201109281338.04378.mfick@codeaurora.org>
On Wednesday, September 28, 2011 01:38:04 pm Martin Fick
wrote:
> On Monday, September 26, 2011 06:41:04 am Christian
> Couder
>
> wrote:
> > On Sun, Sep 25, 2011 at 10:43 PM, Martin Fick
>
> <mfick@codeaurora.org> wrote:
> ...
>
> > > git checkout
> > >
> > > can also take rather long periods of time > 3 mins
> > > when run on a repo with ~100K refs.
>
> ...
>
> > > So, I bisected this issue also, and it seems that
> > > the
> > >
> > > "offending" commit is
>
> ...
>
> > > commit 680955702990c1d4bfb3c6feed6ae9c6cb5c3c07
> > > Author: Christian Couder <chriscool@tuxfamily.org>
> > >
> > > replace_object: add mechanism to replace objects
> > >
> > > found in "refs/replace/"
>
> ...
>
> > I don't think there is an obvious problem with it, but
> > it would be nice if you could dig a bit deeper.
> >
> > The first thing that could take a lot of time is the
> > call to for_each_replace_ref() in this function:
> >
> > +static void prepare_replace_object(void)
> > +{
> > + static int replace_object_prepared;
> > +
> > + if (replace_object_prepared)
> > + return;
> > +
> > + for_each_replace_ref(register_replace_ref,
> > NULL); + replace_object_prepared = 1;
> > +}
>
> The time was actually spent in for_each_replace_ref()
> which calls get_loose_refs() which has the recursive bug
> that Julian Phillips fixed 2 days ago. Good to see that
> this fix helps other use cases too.
>
> So with that bug fixed, the thing taking the most time
> now for a git checkout with ~100K refs seems to be the
> orphan check as Thomas predicted. The strange part with
> this, is that the orphan check seems to take only about
> ~20s in the repo where the refs aren't packed. However,
> in the repo where they are packed, this check takes at
> least 5min! This seems a bit unusual, doesn't it? Is
> the filesystem that much better at indexing refs than
> git's pack mechanism? Seems unlikely, the unpacked refs
> take 312M in the FS, the packed ones only take about
> 4.3M. I suspect their is something else unexpected
> going on here in the packed ref case.
>
> Any thoughts? I will dig deeper...
I think the problem is that resolve_ref() walks a linked
list of searching for the packed ref. Does this mean that
packed refs are not indexed at all?
>
> -Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH v3] send-email: auth plain/login fix
From: Junio C Hamano @ 2011-09-28 22:00 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: joe, git
In-Reply-To: <1317205600-7210-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> This patch is tested by sending it :)
I am tempted to shorten the log message to cull the protocol trace.
send-email: auth plain/login fix
git send-email does not authenticate properly when communicating over TLS
with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the
standard server setup under debian with exim4 and probably everywhere
where system accounts are used.
The problem (only?) exists when libauthen-sasl-cyrus-perl (Authen::SASL::Cyrus)
is installed. Importing Authen::SASL::Perl makes Authen::SASL use the perl
implementation, which works better.
The solution is based on this forum thread:
http://www.perlmonks.org/?node_id=904354
This patch is tested by sending it. Without this fix, the interaction with
the server failed like this:
...
Password:
Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH
Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified
5.5.2 AUTH mechanism must be specified
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Around the line "the server failed like this:", it would be helpful if we
can say how others can reproduce the protocol exchange log shown above.
That would help those who may (or may not) be seeing a similar issue to
diagnose if this commit may help them (or is the culprit of a breakage
they find in the future).
Thanks.
^ permalink raw reply
* Re: [PATCH v3] Docs: git checkout --orphan: `root commit' and `branch head'
From: Junio C Hamano @ 2011-09-28 20:34 UTC (permalink / raw)
To: Michael Witten
Cc: Carlos Martín Nieto, vra5107, Michael J Gruber, Matthieu Moy,
Eric Raible, Philip Oakley, Jeff King, Jay Soffian, git
In-Reply-To: <e4f46b39e9ed4203bfab8a81e25eb600-mfwitten@gmail.com>
Michael Witten <mfwitten@gmail.com> writes:
> See:
>
> Re: Can a git changeset be created with no parent
> Carlos Martín Nieto <cmn@elego.de>
> Message-ID: <1317073309.5579.9.camel@centaur.lab.cmartin.tk>
> http://article.gmane.org/gmane.comp.version-control.git/182170
>
> and:
>
> git help glossary
I think I had to tell somebody on this list not to do this no more than a
month ago.
It is a good practice to point to earlier discussions while polishing
patch, and it also is good to include pointers in the commit log message
as a _supporting material_ (additional reading), but that is *NOT* a
substitute for a properly written commit log message. You need to state
what problem you are trying to fix and how the proposed patch fixes it.
Pretty please.
As to what the updated text wants to talk about, I think most of them are
improvement, but there are a few glitches and nits.
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index c0a96e6..0b6e528 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -125,29 +125,54 @@ explicitly give a name with '-b' in such a case.
> below for details.
>
> ---orphan::
> - Create a new 'orphan' branch, named <new_branch>, started from
> - <start_point> and switch to it. The first commit made on this
> - new branch will have no parents and it will be the root of a new
> - history totally disconnected from all the other branches and
> - commits.
> -+
> -The index and the working tree are adjusted as if you had previously run
> -"git checkout <start_point>". This allows you to start a new history
> -that records a set of paths similar to <start_point> by easily running
> -"git commit -a" to make the root commit.
> -+
> -This can be useful when you want to publish the tree from a commit
> -without exposing its full history. You might want to do this to publish
> -an open source branch of a project whose current tree is "clean", but
> -whose full history contains proprietary or otherwise encumbered bits of
> -code.
> -+
> -If you want to start a disconnected history that records a set of paths
> -that is totally different from the one of <start_point>, then you should
> -clear the index and the working tree right after creating the orphan
> -branch by running "git rm -rf ." from the top level of the working tree.
> -Afterwards you will be ready to prepare your new files, repopulating the
> -working tree, by copying them from elsewhere, extracting a tarball, etc.
> +--orphan::
> + Tell git to turn the next commit you create into a root commit
> + (that is, a commit without any parent); creating the next commit
> + is similar to creating the first commit after running "git{nbsp}init",
I do not think we ever used {nbsp} in our documentation set. Is it
absolutely necessary to use it in this context, and are you absolutely
sure this does not break various versions of documentation toolchain
people have?
> + except that the new commit will be referenced by the branch head
> + <new_branch> rather than "master".
The option works as a substitute for -B/-b in the sense that it takes a
name of the new branch, and the only difference is that the new branch
will start with no commit (yet). To reduce confusion, it might make sense
to update this part (and description of -b/-B) to begin like this:
--orphan <new_branch>::
to match the new explanation text, and the output from "git checkout -h".
By the way, shouldn't the entry for -b in "git checkout -h" output also
say <new branch>?
Micronit: "referenced by the branch head <new_branch>" might be easier to
read and understand with s/branch head/branch name/. Or perhaps
except that the new commit will be made on <new_branch> branch
rather than "master".
might be even better.
> ++
> +Furthermore, the working tree and index are adjusted as if you ran
> +"git{nbsp}checkout{nbsp}<start_point>"; thus, by just running
> +"git{nbsp}commit", you can create a root commit with a tree that is
> +exactly the same as the tree of <start_point>.
> ++
> +Naturally, before creating the commit, you may manipulate the index
> +in any way you want. ...
What value does "Naturally, " add to the understanding here? I would
understand if it were "Alternatively, ", though.
> +... For example, if you want to create a root commit
> +with a tree that is totally different from the tree of <start_point>,
> +then just clear the working tree and index first: From the top level
> +of the working tree, run "git{nbsp}rm{nbsp}-rf{nbsp}.", and then
> +prepare your new working tree and index as desired.
> ++
> +There are two common uses for this option:
> ++
> +--
> + Separate history::
> + Suppose that for convenience, you want to maintain
> + the website for your project in the same repository
> + as the project itself. In such a case, it may not
> + make much sense to interleave the history of the
> + website with the history of the project; you can use
> + the "--orphan" option in order to create these two
> + completely separate histories.
I suspect this is *not* common at all, and also because you would need a
working tree to update both lines of histories that are not related to
each other at all at the content level, I do not believe that "for
convenience" supports or justifies the use case at all. It would be less
convenient to update these two unrelated histories (switching between
branches would need to nuke the whole working tree, and the semantics to
carry local changes floating on top of the tip commit of the current
branch across branch switching actively works against you).
You would be better off using another repository to keep track of "the
website for your project".
In short, I do not think we would want to list the above as if we are
somehow encouraging the use of this option for such a use. It falls into
"because with --orphan you _could_", and definitely not "because having
these two unrelated projects in the same repository you work in is
convenient and/or necessary".
> + Hidden history::
> + Suppose you have a project that has proprietary
> + material that is never meant to be released to the
> + public, yet you now want to maintain an open source
> + history that may be published widely.
This cause does make sense; you would want the local changes floating on
top of the tip commit of the current branch carried across branch
switching.
> +In this case, it would not be enough just to remove the proprietary
> +material from the working tree and then create a new commit, because
> +the proprietary material would still be accessible through the new
> +commit's ancestry; the proprietary history must be hidden from the new
> +commit, and the "--orphan" option allows you to do so by ensuring that
> +the new commit has no parent.
Does this "In this case" paragraph format as part of the "Hidden history"
paragraph?
> ++
> +However, removing proprietary material from ancestry is usually a task
> +that is better performed by linkgit:git-filter-branch[1] and
> +linkgit:git-rebase[1], especially when there are multiple commits that
> +are already suitable for the open source history.
> +--
In general it is true, and not "especially". When you are just abandoning
the history and publishing tidied up tree without history afresh.
I suspect this is not exactly "filter-branch is better but you could use
checkout --orphan" like you made it sound in the above paragraph. If you
are bootstrapping a new open source project from the tip of a proprietary
tree, "checkout --orphan && edit to sanitize && commit" to start your
history afresh would be perfectly adequate for your PR people to say "Now
we are open", especially when you do not intend to accept fixes to older
revisions; you could filter-branch the older proprietary history but you
would not get much benefit out of the cost for doing so, I think.
Thanks.
^ permalink raw reply
* Re: Re* [PATCH 2/2] grep --no-index: don't use git standard exclusions
From: Junio C Hamano @ 2011-09-28 20:01 UTC (permalink / raw)
To: Bert Wesarg; +Cc: git
In-Reply-To: <CAKPyHN3=mbtEkyFUBdZAJCEVXfJhwPVhVFWkNNfX-yhtw9498w@mail.gmail.com>
Bert Wesarg <bert.wesarg@googlemail.com> writes:
> I would still vote to name the option --exclude-standard, like it is
> done in 'git ls-files'.
Thanks for catching.
^ permalink raw reply
* Re: [PATCH 1/3] parseopt: add OPT_NOOP_NOARG
From: Junio C Hamano @ 2011-09-28 19:47 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Pierre Habouzit
In-Reply-To: <4E835CFE.7020501@lsrfire.ath.cx>
Thanks.
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-28 19:38 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Christian Couder, Thomas Rast, Julian Phillips
In-Reply-To: <CAP8UFD3TWQHU0wLPuxMDnc3bRSz90Yd+yDMBe03kofeo-nr7yA@mail.gmail.com>
On Monday, September 26, 2011 06:41:04 am Christian Couder
wrote:
> On Sun, Sep 25, 2011 at 10:43 PM, Martin Fick
<mfick@codeaurora.org> wrote:
...
> > git checkout
> >
> > can also take rather long periods of time > 3 mins when
> > run on a repo with ~100K refs.
...
> > So, I bisected this issue also, and it seems that the
> > "offending" commit is
...
> > commit 680955702990c1d4bfb3c6feed6ae9c6cb5c3c07
> > Author: Christian Couder <chriscool@tuxfamily.org>
> >
> > replace_object: add mechanism to replace objects
> > found in "refs/replace/"
...
> I don't think there is an obvious problem with it, but it
> would be nice if you could dig a bit deeper.
>
> The first thing that could take a lot of time is the call
> to for_each_replace_ref() in this function:
>
> +static void prepare_replace_object(void)
> +{
> + static int replace_object_prepared;
> +
> + if (replace_object_prepared)
> + return;
> +
> + for_each_replace_ref(register_replace_ref, NULL);
> + replace_object_prepared = 1;
> +}
The time was actually spent in for_each_replace_ref()
which calls get_loose_refs() which has the recursive bug
that Julian Phillips fixed 2 days ago. Good to see that
this fix helps other use cases too.
So with that bug fixed, the thing taking the most time now
for a git checkout with ~100K refs seems to be the orphan
check as Thomas predicted. The strange part with this, is
that the orphan check seems to take only about ~20s in the
repo where the refs aren't packed. However, in the repo
where they are packed, this check takes at least 5min! This
seems a bit unusual, doesn't it? Is the filesystem that
much better at indexing refs than git's pack mechanism?
Seems unlikely, the unpacked refs take 312M in the FS, the
packed ones only take about 4.3M. I suspect their is
something else unexpected going on here in the packed ref
case.
Any thoughts? I will dig deeper...
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: Re* [PATCH 2/2] grep --no-index: don't use git standard exclusions
From: Bert Wesarg @ 2011-09-28 19:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vty7xwrsf.fsf_-_@alter.siamese.dyndns.org>
On Wed, Sep 28, 2011 at 00:21, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> Would '--untracked-too' only be a synonym for '--no-index
>> --exclude-standard', i.e. the current behavior?
>
> That basically would be the idea. Perhaps something like this on top of
> a9e6436 (grep --no-index: don't use git standard exclusions, 2011-09-15).
>
> -- >8 --
> Subject: [PATCH 1/2] grep: teach --untracked and --exclude options
I would still vote to name the option --exclude-standard, like it is
done in 'git ls-files'. Which also has a --exclude=<pattern> option.
And I think a --exclude=<pattern> option would be useful for 'git
grep', too.
Else:
Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>
Thanks.
Bert
>
> In a working tree of a git managed repository, "grep --untracked" would
> find the specified patterns from files in untracked files in addition to
> its usual behaviour of finding them in the tracked files.
>
> By default, when working with "--no-index" option, "grep" does not pay
> attention to .gitignore mechanism. "grep --no-index --exclude" can be
> used to tell the command to use .gitignore and stop reporting hits from
> files that would be ignored. Also, when working without "--no-index",
> "grep" honors .gitignore mechanism, and "grep --no-exclude" can be used
> to tell the command to include hits from files that are ignored.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/git-grep.txt | 15 ++++++++++++++-
> builtin-grep.c | 25 ++++++++++++++++++-------
> 2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index e019e76..2ccfb90 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -9,7 +9,7 @@ git-grep - Print lines matching a pattern
> SYNOPSIS
> --------
> [verse]
> -'git grep' [--cached]
> +'git grep' [--cached] [--untracked] [--excludes]
> [-a | --text] [-I] [-i | --ignore-case] [-w | --word-regexp]
> [-v | --invert-match] [-h|-H] [--full-name]
> [-E | --extended-regexp] [-G | --basic-regexp]
> @@ -36,6 +36,19 @@ OPTIONS
> Instead of searching in the working tree files, check
> the blobs registered in the index file.
>
> +--untracked::
> + In addition to searching in the tracked files in the working
> + tree, search also in untracked files.
> +
> +--no-excludes::
> + Also search in ignored files by not honoring the `.gitignore`
> + mechanism. Only useful with `--untracked`.
> +
> +--excludes::
> + Do not pay attention to ignored files specified via the `.gitignore`
> + mechanism. Only useful when searching files in the current directory
> + with `--no-index`.
> +
> -a::
> --text::
> Process binary files as if they were text.
> diff --git a/builtin-grep.c b/builtin-grep.c
> index a10946d..c6cfdf8 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -646,12 +646,14 @@ static int grep_object(struct grep_opt *opt, const char **paths,
> die("unable to grep from object of type %s", typename(obj->type));
> }
>
> -static int grep_directory(struct grep_opt *opt, const char **paths)
> +static int grep_directory(struct grep_opt *opt, const char **paths, int exc_std)
> {
> struct dir_struct dir;
> int i, hit = 0;
>
> memset(&dir, 0, sizeof(dir));
> + if (exc_std)
> + setup_standard_excludes(&dir);
>
> fill_directory(&dir, paths);
> for (i = 0; i < dir.nr; i++) {
> @@ -749,7 +751,7 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
> int cmd_grep(int argc, const char **argv, const char *prefix)
> {
> int hit = 0;
> - int cached = 0;
> + int cached = 0, untracked = 0, opt_exclude = -1;
> int seen_dashdash = 0;
> int external_grep_allowed__ignored;
> struct grep_opt opt;
> @@ -764,6 +766,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> { OPTION_BOOLEAN, 0, "index", &use_index, NULL,
> "finds in contents not managed by git",
> PARSE_OPT_NOARG | PARSE_OPT_NEGHELP },
> + OPT_BOOLEAN(0, "untracked", &untracked,
> + "search in both tracked and untracked files"),
> + OPT_SET_INT(0, "exclude", &opt_exclude,
> + "search also in ignored files", 1),
> OPT_GROUP(""),
> OPT_BOOLEAN('v', "invert-match", &opt.invert,
> "show non-matching lines"),
> @@ -950,18 +956,23 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> paths[1] = NULL;
> }
>
> - if (!use_index) {
> + if (!use_index && (untracked || cached))
> + die("--cached or --untracked cannot be used with --no-index.");
> +
> + if (!use_index || untracked) {
> int hit;
> - if (cached)
> - die("--cached cannot be used with --no-index.");
> + int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;
> if (list.nr)
> - die("--no-index cannot be used with revs.");
> - hit = grep_directory(&opt, paths);
> + die("--no-index or --untracked cannot be used with revs.");
> + hit = grep_directory(&opt, paths, use_exclude);
> if (use_threads)
> hit |= wait_all();
> return !hit;
> }
>
> + if (0 <= opt_exclude)
> + die("--exclude or --no-exclude cannot be used for tracked contents.");
> +
> if (!list.nr) {
> int hit;
> if (!cached)
> --
> 1.7.7.rc3.4.g8d714
>
>
>
^ permalink raw reply
* Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch
From: Jakub Narebski @ 2011-09-28 19:12 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> Hello,
>
> Whilst trying to do some work related to fetch, I came across a
> regression in the 'next' branch. Bisecting gave me this commit as
> breaking point (and I tried with the parent and there it worked). When
> doing 'git fetch', rev-list will complain about usage, and fetch will
> say that we didn't receive enough, even though earlier versions of git
> have no problems. This fails both on github and on git.or.cz and for git
> and http transports:
>
> $ ./git-fetch git://repo.or.cz/git
Have you tried
$ ./git fetch git://repo.or.cz/git
?
--
Jakub Narębski
^ permalink raw reply
* Re: SVN -> Git *but* with special changes
From: Jeff King @ 2011-09-28 19:07 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Stephen Bash, Abscissa, git
In-Reply-To: <vpq62kc36kn.fsf@bauges.imag.fr>
On Wed, Sep 28, 2011 at 07:44:24PM +0200, Matthieu Moy wrote:
> > I'd eliminate these on the SVN side before converting to Git. If you
> > have svnadmin access to the SVN repo svnadmin dump, svndumpfilter,
> > svnadmin load is a pretty easy process.
>
> Otherwise, you can do it on the Git side with:
>
> git filter-branch --tree-filter 'rm -f some-large-blob'
It's much more efficient to do:
git filter-branch --index-filter \
'git rm --cached --ignore-unmatch some-large-blob'
which avoids checking out the large files over and over[1], just to
delete them. Of course, it's even more efficient not to import them from
svn in the first place. :)
-Peff
[1] This is straight from the filter-branch manpage, btw.
^ permalink raw reply
* Re: SVN -> Git *but* with special changes
From: Jeff King @ 2011-09-28 19:04 UTC (permalink / raw)
To: Abscissa; +Cc: git
In-Reply-To: <1317227849979-6840904.post@n2.nabble.com>
On Wed, Sep 28, 2011 at 09:37:29AM -0700, Abscissa wrote:
> I have a couple big projects in SVN that I'd like to convert to Git. Being in
> SVN, they've operated under a couple assumptions that are not true under
> Git. These assumptions are:
>
> 1. Directories can exist even if there's nothing in them (just like any
> filesystem).
The usual technique is to put an empty .gitignore file into the empty
directory, which will make sure it always exists. If you import with
git-svn, it supports "--preserve-empty-dirs", which will do this for you
automatically.
> 2. Keeping binary files in version control isn't a big deal because the
> whole repo doesn't get copied to everyone's system or use up people's GitHub
> storage space.
git-svn --ignore-paths will handle this for you.
Side note on the GitHub thing: all of the forks will share objects, so
it's not much as space as you might think. And all of the paid plans
count repos, not bytes. Of course, the painful part is probably cloning
the big objects to everybody's workstation. :)
> (Also, there are tags and branches to be converted too, in the
> SVN-standard "tags" and "branches" directories.)
git svn --stdlayout ?
-Peff
^ permalink raw reply
* Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch
From: Jeff King @ 2011-09-28 18:53 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk>
On Wed, Sep 28, 2011 at 06:04:27PM +0200, Carlos Martín Nieto wrote:
> Whilst trying to do some work related to fetch, I came across a
> regression in the 'next' branch. Bisecting gave me this commit as
> breaking point (and I tried with the parent and there it worked). When
> doing 'git fetch', rev-list will complain about usage, and fetch will
> say that we didn't receive enough, even though earlier versions of git
> have no problems. This fails both on github and on git.or.cz and for git
> and http transports:
>
> $ ./git-fetch git://repo.or.cz/git
> usage: git rev-list [OPTION] <commit-id>... [ -- paths... ]
Hmm. I notice you're running a not-installed version of fetch. Might
this be a problem with a new git fetch running an older, installed
version of rev-list?
The commit you mention calls rev-list with --verify-objects, but that
feature is only added in the parent commit. So I can reproduce your
issue with:
$ git checkout 6d4bb38~2 ;# or anything before --verify-objects
$ make install
$ git checkout 6d4bb38
$ make
$ ./git-fetch git://repo.or.cz/git
but this works (because it sets the exec path properly):
$ ./bin-wrappers/git fetch git://repo.or.cz/git
as does this:
$ make install
$ ./git-fetch git://repo.or.cz/git
So I don't think there's a bug. It's just that running compiled programs
straight out of the build directory isn't supported. It works _most_ of
the time, but as you can see, you may end up calling older, installed
versions of git. The bin-wrappers scripts set up the exec path properly
to let you test.
-Peff
^ permalink raw reply
* Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch
From: Junio C Hamano @ 2011-09-28 18:50 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> Whilst trying to do some work related to fetch, I came across a
> regression in the 'next' branch....
>
> $ ./git-fetch git://repo.or.cz/git
That invocation of ./git-fetch looks suspicious.
Are you sure that it internally invokes ./git-rev-list from the same
version that knows --verify-objects option (you just built in your current
directory), or is it invoking an old git-rev-list that is installed and is
reachable from your usual $PATH, which does not know that option yet?
When I try a new version that was just built in my current directory, here
is an incantation I use:
GIT_EXEC_PATH=`pwd`
PATH=`pwd`:/usr/bin:/bin
GITPERLLIB=`pwd`/perl/blib/lib
export GIT_EXEC_PATH PATH GITPERLLIB
^ permalink raw reply
* Re: [PATCH] Docs: git checkout --orphan: `root commit' and `branch head'
From: Junio C Hamano @ 2011-09-28 18:17 UTC (permalink / raw)
To: Matthieu Moy
Cc: Michael Witten, Jeff King, Philip Oakley, Eric Raible,
Michael J Gruber, Carlos Martín Nieto, vra5107, git
In-Reply-To: <vpq4nzwoj1o.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> Well, as Junio mentioned, "--orphan" could be deprecated in favor of
>> "--no-parent"; the uncommon nature of this task works to our
>> advantage, in that it makes a deprecation of "--orphan" easy.
>
> I agree that _some_ cases would be more natural with commit --no-parent,
> but I disagree that it is a better solution in general.
Just for the record, in my later message after reading Peff's response, I
said I agree that adding it to "commit" would be a mistake.
^ permalink raw reply
* Re: SVN -> Git *but* with special changes
From: Abscissa @ 2011-09-28 18:03 UTC (permalink / raw)
To: git
In-Reply-To: <16808473.33899.1317229852319.JavaMail.root@mail.hq.genarts.com>
>> 1. Directories can exist even if there's nothing in them (just like
>> any filesystem).
>
>What requires the empty directories to exist? The build system?
Yes.
>Can you just let them go away in Git and fix it downstream in the user's
working copy?
That won't work retroactively for already-existing revisions. I'd rather not
break the build system for older revisions just for such a goofy reason as
empty dirs being deemed unnecessary.
--
View this message in context: http://git.661346.n2.nabble.com/SVN-Git-but-with-special-changes-tp6840904p6841234.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: emacs as mergetool
From: Nick @ 2011-09-28 17:03 UTC (permalink / raw)
To: git; +Cc: thomas
In-Reply-To: <201109261821.28382.thomas@koch.ro>
On 26/09/11 17:21, Thomas Koch wrote:
> would you be so kind and give an emacs newbie (and vim refugee) a hint about
> using emacs as mergetool?
>
> - Which one? ediff vs. emerge (I don't bother learning, just give me the best)
I tend to use git.el - which provides the M-x git-status command - and therefore
ediff. It's not necessarily the best git interface, but I've been too lazy
learn anything else so far. git-status allows you to do basic git
adding/committing etc. and will show modification diffs.
Given a git-status buffer, a conflicted merge will show up with certain files in
red. You can then view the conflicted versions side-by-side positioning the
cursor over the conflicted file, and using the key-sequence 'd E' to start
interactive merges in ediff mode. Afterwards you have to save the buffers and
mark the file as 'resolved' in git-status ('r').
There are other things you can do in a git-status buffer. M-x describe-mode (C-h
m) and M-x describe-bindings (C-h b) will provide some idea.
> - I'd like to have a setup, where I can run mergetool with different options
> to
> - connect to a running emacs server
> - run emacs in X
> - or in the terminal (default)
git-status works with all the above. I don't use git-mergetool at all.
To keep things in one emacs session I sometimes use git directly in emacs shell
buffers. i.e.
- Run emacs
- Open the repo in dired: M-x find-file <path to your git repo>
- Invoke git-status: M-x git-status
- (do stuff)
- Open a shell M-x shell, M-x eshell, or M-x ansi-term
- (do more stuff)
> - Is it possible to resolve all conflicting files in one session instead of
> having emacs called again for every file?
As I said, I don't use git-mergetool, but git-status should allow most basic
things in fairly straightforward way. For anything more complicated (e.g. git
commit --amend, or git add --interactive) I open an shell in emacs.
If you tell git to use emacsclient as the editor, and then git commit and co.
will open an emacs buffer so you can invoke git from the command line without
leaving your emacs session. To do this:
- invoke: git config --global core.editor emacsclient
- in emacs: M-x server-start
(or add "(server-start)" to your ~/.emacs file)
At some point I might check out magit. I'd be interested to hear about
comparisons between git.el and the alternatives.
Cheers,
N
^ permalink raw reply
* [PATCH 3/3] apply: use OPT_NOOP_NOARG
From: René Scharfe @ 2011-09-28 17:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pierre Habouzit
In-Reply-To: <4E835CFE.7020501@lsrfire.ath.cx>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
builtin/apply.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index f2edc52..872e40a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3831,7 +3831,6 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
int i;
int errs = 0;
int is_not_gitdir = !startup_info->have_repository;
- int binary;
int force_apply = 0;
const char *whitespace_option = NULL;
@@ -3850,12 +3849,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
"ignore additions made by the patch"),
OPT_BOOLEAN(0, "stat", &diffstat,
"instead of applying the patch, output diffstat for the input"),
- { OPTION_BOOLEAN, 0, "allow-binary-replacement", &binary,
- NULL, "old option, now no-op",
- PARSE_OPT_HIDDEN | PARSE_OPT_NOARG },
- { OPTION_BOOLEAN, 0, "binary", &binary,
- NULL, "old option, now no-op",
- PARSE_OPT_HIDDEN | PARSE_OPT_NOARG },
+ OPT_NOOP_NOARG(0, "allow-binary-replacement"),
+ OPT_NOOP_NOARG(0, "binary"),
OPT_BOOLEAN(0, "numstat", &numstat,
"shows number of added and deleted lines in decimal notation"),
OPT_BOOLEAN(0, "summary", &summary,
--
1.7.7.rc2
^ 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