Re: [PATCH] receive-pack.c: don't miss exporting unsolicited push certificates
On 09.01.2015 17:52, Junio C Hamano wrote:
Stefan Beller sbel...@google.com writes:
If the server is configured to not advertise push certificates,
a push certificate that gets pushed nevertheless will not be fully
recorded because push_cert_nonce is NULL.
Sorry, but I do not quite see what you are trying to get at.
When we did not advertise that this feature is supported, we know
the incoming nonce is meaningless junk because we didn't supply the
correct answer the pusher can give us; we do not even have the
correct answer ourselves.
Besides, while reviewing the other patch, didn't we agree that we
should reject such a push? Then there is nothing to log anyway, no?
Yes we did. This is unrelated to the previous patch. I stuffed it into
this thread as I found it was touching the same feature.
... reads the patch and code beyond the context while scratching
head ...
I notice that the above three lines do not correspond what you did
in this patch. The certificate is exported via the blob interface
fully with or without this change.
I had problems with wording the commit message because I have no
expertise with the feature. I am sorry for wasting your time there.
What is not given to the hook is the push-cert-nonce-status. While
it is sufficient to tell the hook that we are not getting a good
nonce (i.e. the hook does not see GIT_PUSH_CERT_NONCE_STATUS=G), we
are not showing that nonce-status is unsolicited, even though we
internally compute and decide that; is that what you are trying to
fix?
yes that's what I was trying to hint at. The hook would just see
it is unsolicited instead of not having the state available.
Still puzzled...
The recording of GIT_PUSH_CERT_NONCE_STATUS should be dependent on
the status being there instead of push_cert_nonce being non NULL.
Without this patch an unsolicited nonce never makes to the stage when
being exported with GIT_PUSH_CERT_NONCE_STATUS, because in the unsolicited
case push_cert_nonce is always NULL.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/receive-pack.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 628d13a..0e4878e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -504,18 +504,18 @@ static void prepare_push_cert_sha1(struct
child_process *proc)
sigcheck.key ? sigcheck.key : );
argv_array_pushf(proc-env_array, GIT_PUSH_CERT_STATUS=%c,
sigcheck.result);
-if (push_cert_nonce) {
+if (push_cert_nonce)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE=%s,
push_cert_nonce);
+if (nonce_status)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE_STATUS=%s,
nonce_status);
-if (nonce_status == NONCE_SLOP)
-argv_array_pushf(proc-env_array,
- GIT_PUSH_CERT_NONCE_SLOP=%ld,
- nonce_stamp_slop);
-}
+if (nonce_status == NONCE_SLOP)
+argv_array_pushf(proc-env_array,
+ GIT_PUSH_CERT_NONCE_SLOP=%ld,
+ nonce_stamp_slop);
}
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] receive-pack.c: don't miss exporting unsolicited push certificates
Stefan Beller sbel...@google.com writes:
If the server is configured to not advertise push certificates,
a push certificate that gets pushed nevertheless will not be fully
recorded because push_cert_nonce is NULL.
Sorry, but I do not quite see what you are trying to get at.
When we did not advertise that this feature is supported, we know
the incoming nonce is meaningless junk because we didn't supply the
correct answer the pusher can give us; we do not even have the
correct answer ourselves.
Besides, while reviewing the other patch, didn't we agree that we
should reject such a push? Then there is nothing to log anyway, no?
... reads the patch and code beyond the context while scratching
head ...
I notice that the above three lines do not correspond what you did
in this patch. The certificate is exported via the blob interface
fully with or without this change.
What is not given to the hook is the push-cert-nonce-status. While
it is sufficient to tell the hook that we are not getting a good
nonce (i.e. the hook does not see GIT_PUSH_CERT_NONCE_STATUS=G), we
are not showing that nonce-status is unsolicited, even though we
internally compute and decide that; is that what you are trying to
fix?
Still puzzled...
The recording of GIT_PUSH_CERT_NONCE_STATUS should be dependent on
the status being there instead of push_cert_nonce being non NULL.
Without this patch an unsolicited nonce never makes to the stage when
being exported with GIT_PUSH_CERT_NONCE_STATUS, because in the unsolicited
case push_cert_nonce is always NULL.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/receive-pack.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 628d13a..0e4878e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -504,18 +504,18 @@ static void prepare_push_cert_sha1(struct child_process
*proc)
sigcheck.key ? sigcheck.key : );
argv_array_pushf(proc-env_array, GIT_PUSH_CERT_STATUS=%c,
sigcheck.result);
- if (push_cert_nonce) {
+ if (push_cert_nonce)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE=%s,
push_cert_nonce);
+ if (nonce_status)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE_STATUS=%s,
nonce_status);
- if (nonce_status == NONCE_SLOP)
- argv_array_pushf(proc-env_array,
- GIT_PUSH_CERT_NONCE_SLOP=%ld,
- nonce_stamp_slop);
- }
+ if (nonce_status == NONCE_SLOP)
+ argv_array_pushf(proc-env_array,
+ GIT_PUSH_CERT_NONCE_SLOP=%ld,
+ nonce_stamp_slop);
}
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] receive-pack.c: don't miss exporting unsolicited push certificates
If the server is configured to not advertise push certificates,
a push certificate that gets pushed nevertheless will not be fully
recorded because push_cert_nonce is NULL.
The recording of GIT_PUSH_CERT_NONCE_STATUS should be dependent on
the status being there instead of push_cert_nonce being non NULL.
Without this patch an unsolicited nonce never makes to the stage when
being exported with GIT_PUSH_CERT_NONCE_STATUS, because in the unsolicited
case push_cert_nonce is always NULL.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/receive-pack.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 628d13a..0e4878e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -504,18 +504,18 @@ static void prepare_push_cert_sha1(struct child_process
*proc)
sigcheck.key ? sigcheck.key : );
argv_array_pushf(proc-env_array, GIT_PUSH_CERT_STATUS=%c,
sigcheck.result);
- if (push_cert_nonce) {
+ if (push_cert_nonce)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE=%s,
push_cert_nonce);
+ if (nonce_status)
argv_array_pushf(proc-env_array,
GIT_PUSH_CERT_NONCE_STATUS=%s,
nonce_status);
- if (nonce_status == NONCE_SLOP)
- argv_array_pushf(proc-env_array,
-GIT_PUSH_CERT_NONCE_SLOP=%ld,
-nonce_stamp_slop);
- }
+ if (nonce_status == NONCE_SLOP)
+ argv_array_pushf(proc-env_array,
+GIT_PUSH_CERT_NONCE_SLOP=%ld,
+nonce_stamp_slop);
}
}
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
[PATCH 0/2] Documentation/githooks: mention pwd, $GIT_PREFIX
A couple of patches to document and test that hooks are run from the top-level directory and that GIT_PREFIX is set to the subdirectory that Git was run from (like !aliases). The new documentation is mostly lifted from the documentation for alias.*. I don't think the new documentation is perfectly clear, but it's a start. In particular, what does Git do for hook pwd and GIT_PREFIX when it is run from within a bare repository? Or from within .git? Or if GIT_WORK_TREE (--work-tree) and/or GIT_DIR (--git-dir) are set? Many of these same questions apply to !aliases, so the documentation for alias.* should also be shored up. -Richard Richard Hansen (2): Documentation/githooks: mention pwd, $GIT_PREFIX t1020-subdirectory.sh: check hook pwd, $GIT_PREFIX Documentation/githooks.txt | 6 ++ t/t1020-subdirectory.sh| 34 ++ 2 files changed, 40 insertions(+) -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] t1020-subdirectory.sh: check hook pwd, $GIT_PREFIX
Make sure hooks are executed at the top-level directory and that GIT_PREFIX is set (as documented). Signed-off-by: Richard Hansen rhan...@bbn.com --- t/t1020-subdirectory.sh | 34 ++ 1 file changed, 34 insertions(+) diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh index 2edb4f2..03bb0a2 100755 --- a/t/t1020-subdirectory.sh +++ b/t/t1020-subdirectory.sh @@ -128,6 +128,23 @@ test_expect_success !MINGW '!alias expansion' ' test_cmp expect actual ' +test_expect_success 'hook pwd' ' + pwd expect + ( + rm -f actual + mkdir -p .git/hooks + ! test -e .git/hooks/post-checkout + cat -\EOF .git/hooks/post-checkout + #!/bin/sh + pwd actual + EOF + chmod +x .git/hooks/post-checkout + (cd dir git checkout -- two) + rm -f .git/hooks/post-checkout + ) + test_cmp expect actual +' + test_expect_success 'GIT_PREFIX for !alias' ' printf dir/ expect ( @@ -154,6 +171,23 @@ test_expect_success 'GIT_PREFIX for built-ins' ' test_cmp expect actual ' +test_expect_success 'GIT_PREFIX for hooks' ' + printf dir/ expect + ( + rm -f actual + mkdir -p .git/hooks + ! test -e .git/hooks/post-checkout + cat -\EOF .git/hooks/post-checkout + #!/bin/sh + printf %s $GIT_PREFIX actual + EOF + chmod +x .git/hooks/post-checkout + (cd dir git checkout -- two) + rm -f .git/hooks/post-checkout + ) + test_cmp expect actual +' + test_expect_success 'no file/rev ambiguity check inside .git' ' git commit -a -m 1 ( -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Documentation/githooks: mention pwd, $GIT_PREFIX
Document that hooks are run from the top-level directory and that GIT_PREFIX is set to the name of the original subdirectory (relative to the top-level directory). Signed-off-by: Richard Hansen rhan...@bbn.com --- Documentation/githooks.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 9ef2469..c08f4fd 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -26,6 +26,12 @@ executable by default. This document describes the currently defined hooks. +Hooks are executed from the top-level directory of a repository, which +may not necessarily be the current directory. +The 'GIT_PREFIX' environment variable is set as returned by running +'git rev-parse --show-prefix' from the original current directory. +See linkgit:git-rev-parse[1]. + HOOKS - -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised
Stefan Beller sbel...@google.com writes: Your proposal to acknowledge the correctness of the message leads to more questions. How would we proceed? How would it fail if we pretend that push-cert line had to be old/new/ref line? Failing the same way, but with a better diagnosis, would be sufficient. I expect such behavior only from malicious clients which actively want to abuse a feature which wasn't advertised,... Do not assume malice; it is not 2005 anymore. You have to remember that we are mature enough that there are many reimplementations of Git, all of which (us included ;-) start with a buggy version. When the protocol exchange gets to this state, in practice, we know we are talking with somebody who has push privilege into the repository, Yeah but what is one repository compared to the whole server? Huh? If an auth good enough for one repository allows things to another repository, then I consider that to that other repository the pusher also has push privilege. So what is the problem? But again, our first version could just be pretend we do not know anything about push-cert, with discussions on alternative considered in its log message. I do not think it is a blocker to lack the more helpful diagnosis feature. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git.c: remove unused includes
* cache.h and commit.h already included in builtin.h * quote.h was appeared in (6035d6aa GIT_TRACE: show which built-in/external commands are executed 25 Jun 2006) and sq_quote_print was removed at (82aae5c quote: remove sq_quote_print() Jul 30 2013) Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- git.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/git.c b/git.c index 09b3bcf..c9bec99 100644 --- a/git.c +++ b/git.c @@ -1,10 +1,7 @@ #include builtin.h -#include cache.h #include exec_cmd.h #include help.h -#include quote.h #include run-command.h -#include commit.h const char git_usage_string[] = git [--version] [--help] [-C path] [-c name=value]\n -- 2.2.1.522.g2561c04.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] cat-file: Remove unused includes
* exec_cmd.h became unnecessary at b931aa5a (Call builtin ls-tree in git-cat-file -p, 2006-05-26). * tag.h and tree.h became unnecessary at 21666f1a (convert object type handling from a string to a number, 2007-02-26). * diff.h was added at e5fba602 (textconv: support for cat_file, 2010-06-15). Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- builtin/cat-file.c | 4 1 file changed, 4 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index f8d8129..750b5a2 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -4,12 +4,8 @@ * Copyright (C) Linus Torvalds, 2005 */ #include cache.h -#include exec_cmd.h -#include tag.h -#include tree.h #include builtin.h #include parse-options.h -#include diff.h #include userdiff.h #include streaming.h -- 2.2.1.522.g2561c04.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cat-file: Remove unused includes
userdiff.h used in git_cat_file_config for getting textconv driver 2015-01-09 2:21 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: Will research this question and resend the patch. Thank you. 2015-01-09 1:45 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- builtin/cat-file.c | 4 1 file changed, 4 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index f8d8129..750b5a2 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -4,12 +4,8 @@ * Copyright (C) Linus Torvalds, 2005 */ #include cache.h -#include exec_cmd.h -#include tag.h -#include tree.h #include builtin.h #include parse-options.h -#include diff.h #include userdiff.h #include streaming.h Interesting. - exec_cmd.h became unnecessary at b931aa5a (Call builtin ls-tree in git-cat-file -p, 2006-05-26). - tag.h and tree.h became unnecessary at 21666f1a (convert object type handling from a string to a number, 2007-02-26). - diff.h was added at e5fba602 (textconv: support for cat_file, 2010-06-15), together with userdiff.h. Was it unnecessary from the beginning? I didn't dig further to find out the answer to the last question, but a patch to remove these include should explain these in its log message, I would think. Thanks. -- _ 0xAX -- _ 0xAX -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] receive-pack: support push-to-checkout hook
Hi Junio, On Thu, 8 Jan 2015, Junio C Hamano wrote: * This is an update to $gmane/260527; relative to what I have been keeping in 'pu', the only difference is that it comes with documentation updates. Thanks, it is very nice! Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cat-file: Remove unused includes
Hello Junio, Thanks for such great explanation, i understood you. I will update this patch and resend it tomorrow. Thank you. 2015-01-10 1:07 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: userdiff.h used in git_cat_file_config for getting textconv driver Yeah, but you know that I already know that when I pointed out about e5fba602 in the message you are responding to. And your patch does not remove it, we still need to include it; we do not need to dig that part of the change. Two corrections to my message you are responding to are in order. I said: I didn't dig further to find out the answer to the last question, but a patch to remove these include should explain these in its log message, I would think. but I think should was a bit too strong, especially without explaining why. It would have been nicer with such explanation is what I should have said. And while on the topic, I should explain why. Most of the time, removal of #include is done because we used to include the header for a good reason (i.e. the source used to need something that is declared in it) but with a code change to remove the last such reference we no longer need to include it. Commit 6d63baa4 (prio-queue: factor out compare and swap operations, 2014-07-14) [*1*] is an example. We used to mention 'struct commit' in the implementation of prio-queue, but the commit realizes that the use of prio-queue does not have to be limited to queuing commits, and removes the need to include commit.h. But this clean-up patch removes #includes without doing anything else. It is clear we _can_ remove them; the submitter of such a patch would have made sure that the code compiles and links fine without these includes. So Why can we remove them? is not a very interesting question. The interesting question is Why remove them *now*? Why do we have these unused #includes? Were they unnecessary from the start? Were they necessary but during the course of the development, we did something else that made them unnecessary and forgot to remove them? These are the natural questions that somebody reading a clean-up patch like this one may ask, and that is why I think it would have been nice if the proposed log message answered them before being asked. So here is an update after I dug a bit more. - exec_cmd.h became unnecessary at b931aa5a (Call builtin ls-tree in git-cat-file -p, 2006-05-26), when it changed an earlier code that used to delegate tree display to ls-tree via the run_command() interface (hence needing exec_cmd.h) to a direct call to cmd_ls_tree(). We should have removed the include in the same commit, but we forgot to do so. - diff.h was added at e5fba602 (textconv: support for cat_file, 2010-06-15), together with userdiff.h, but userdiff.h can be included without including diff.h; the header was unnecessary from the start. - tag.h and tree.h was necessary since 8e440259 (Use blob_, commit_, tag_, and tree_type throughout., 2006-04-02) as the code used to check the type of object by comparing typename with tree_type and tag_type (pointers to extern strings). 21666f1a (convert object type handling from a string to a number, 2007-02-26) made these type_type strings unnecessary, and it could have switched to include object.h to use typename(), but it forgot to do so. Because tag.h and tree.h include object.h, it did not need to include object.h in order to start using typename(). In today's code, we do not even have to include object.h after removing these two #includes, because builtin.h includes commit.h which in turn includes object.h these days. This happened at 7b9c0a69 (git-commit-tree: make it usable from other builtins, 2008-07-01). Having said all that, what the above satisifies is mostly curiosity, and gives whatever value Postmortems have by analysing what we could have done better. It is OK to omit the postmortem and instead just say These are no longer used; remove them., which was your original. So I shouldn't have said *should* explain. [Footnote] *1* I pulled this randomly from git log -Sinclude --grep=include output. -- _ 0xAX -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cat-file: Remove unused includes
Alexander Kuleshov kuleshovm...@gmail.com writes: userdiff.h used in git_cat_file_config for getting textconv driver Yeah, but you know that I already know that when I pointed out about e5fba602 in the message you are responding to. And your patch does not remove it, we still need to include it; we do not need to dig that part of the change. Two corrections to my message you are responding to are in order. I said: I didn't dig further to find out the answer to the last question, but a patch to remove these include should explain these in its log message, I would think. but I think should was a bit too strong, especially without explaining why. It would have been nicer with such explanation is what I should have said. And while on the topic, I should explain why. Most of the time, removal of #include is done because we used to include the header for a good reason (i.e. the source used to need something that is declared in it) but with a code change to remove the last such reference we no longer need to include it. Commit 6d63baa4 (prio-queue: factor out compare and swap operations, 2014-07-14) [*1*] is an example. We used to mention 'struct commit' in the implementation of prio-queue, but the commit realizes that the use of prio-queue does not have to be limited to queuing commits, and removes the need to include commit.h. But this clean-up patch removes #includes without doing anything else. It is clear we _can_ remove them; the submitter of such a patch would have made sure that the code compiles and links fine without these includes. So Why can we remove them? is not a very interesting question. The interesting question is Why remove them *now*? Why do we have these unused #includes? Were they unnecessary from the start? Were they necessary but during the course of the development, we did something else that made them unnecessary and forgot to remove them? These are the natural questions that somebody reading a clean-up patch like this one may ask, and that is why I think it would have been nice if the proposed log message answered them before being asked. So here is an update after I dug a bit more. - exec_cmd.h became unnecessary at b931aa5a (Call builtin ls-tree in git-cat-file -p, 2006-05-26), when it changed an earlier code that used to delegate tree display to ls-tree via the run_command() interface (hence needing exec_cmd.h) to a direct call to cmd_ls_tree(). We should have removed the include in the same commit, but we forgot to do so. - diff.h was added at e5fba602 (textconv: support for cat_file, 2010-06-15), together with userdiff.h, but userdiff.h can be included without including diff.h; the header was unnecessary from the start. - tag.h and tree.h was necessary since 8e440259 (Use blob_, commit_, tag_, and tree_type throughout., 2006-04-02) as the code used to check the type of object by comparing typename with tree_type and tag_type (pointers to extern strings). 21666f1a (convert object type handling from a string to a number, 2007-02-26) made these type_type strings unnecessary, and it could have switched to include object.h to use typename(), but it forgot to do so. Because tag.h and tree.h include object.h, it did not need to include object.h in order to start using typename(). In today's code, we do not even have to include object.h after removing these two #includes, because builtin.h includes commit.h which in turn includes object.h these days. This happened at 7b9c0a69 (git-commit-tree: make it usable from other builtins, 2008-07-01). Having said all that, what the above satisifies is mostly curiosity, and gives whatever value Postmortems have by analysing what we could have done better. It is OK to omit the postmortem and instead just say These are no longer used; remove them., which was your original. So I shouldn't have said *should* explain. [Footnote] *1* I pulled this randomly from git log -Sinclude --grep=include output. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Makefile: collect BUILTIN_OBJS instead of directly writing by hand
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- Makefile | 100 +-- 1 file changed, 1 insertion(+), 99 deletions(-) diff --git a/Makefile b/Makefile index 082d9ba..1f06624 100644 --- a/Makefile +++ b/Makefile @@ -797,105 +797,7 @@ LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o -BUILTIN_OBJS += builtin/add.o -BUILTIN_OBJS += builtin/annotate.o -BUILTIN_OBJS += builtin/apply.o -BUILTIN_OBJS += builtin/archive.o -BUILTIN_OBJS += builtin/bisect--helper.o -BUILTIN_OBJS += builtin/blame.o -BUILTIN_OBJS += builtin/branch.o -BUILTIN_OBJS += builtin/bundle.o -BUILTIN_OBJS += builtin/cat-file.o -BUILTIN_OBJS += builtin/check-attr.o -BUILTIN_OBJS += builtin/check-ignore.o -BUILTIN_OBJS += builtin/check-mailmap.o -BUILTIN_OBJS += builtin/check-ref-format.o -BUILTIN_OBJS += builtin/checkout-index.o -BUILTIN_OBJS += builtin/checkout.o -BUILTIN_OBJS += builtin/clean.o -BUILTIN_OBJS += builtin/clone.o -BUILTIN_OBJS += builtin/column.o -BUILTIN_OBJS += builtin/commit-tree.o -BUILTIN_OBJS += builtin/commit.o -BUILTIN_OBJS += builtin/config.o -BUILTIN_OBJS += builtin/count-objects.o -BUILTIN_OBJS += builtin/credential.o -BUILTIN_OBJS += builtin/describe.o -BUILTIN_OBJS += builtin/diff-files.o -BUILTIN_OBJS += builtin/diff-index.o -BUILTIN_OBJS += builtin/diff-tree.o -BUILTIN_OBJS += builtin/diff.o -BUILTIN_OBJS += builtin/fast-export.o -BUILTIN_OBJS += builtin/fetch-pack.o -BUILTIN_OBJS += builtin/fetch.o -BUILTIN_OBJS += builtin/fmt-merge-msg.o -BUILTIN_OBJS += builtin/for-each-ref.o -BUILTIN_OBJS += builtin/fsck.o -BUILTIN_OBJS += builtin/gc.o -BUILTIN_OBJS += builtin/get-tar-commit-id.o -BUILTIN_OBJS += builtin/grep.o -BUILTIN_OBJS += builtin/hash-object.o -BUILTIN_OBJS += builtin/help.o -BUILTIN_OBJS += builtin/index-pack.o -BUILTIN_OBJS += builtin/init-db.o -BUILTIN_OBJS += builtin/interpret-trailers.o -BUILTIN_OBJS += builtin/log.o -BUILTIN_OBJS += builtin/ls-files.o -BUILTIN_OBJS += builtin/ls-remote.o -BUILTIN_OBJS += builtin/ls-tree.o -BUILTIN_OBJS += builtin/mailinfo.o -BUILTIN_OBJS += builtin/mailsplit.o -BUILTIN_OBJS += builtin/merge.o -BUILTIN_OBJS += builtin/merge-base.o -BUILTIN_OBJS += builtin/merge-file.o -BUILTIN_OBJS += builtin/merge-index.o -BUILTIN_OBJS += builtin/merge-ours.o -BUILTIN_OBJS += builtin/merge-recursive.o -BUILTIN_OBJS += builtin/merge-tree.o -BUILTIN_OBJS += builtin/mktag.o -BUILTIN_OBJS += builtin/mktree.o -BUILTIN_OBJS += builtin/mv.o -BUILTIN_OBJS += builtin/name-rev.o -BUILTIN_OBJS += builtin/notes.o -BUILTIN_OBJS += builtin/pack-objects.o -BUILTIN_OBJS += builtin/pack-redundant.o -BUILTIN_OBJS += builtin/pack-refs.o -BUILTIN_OBJS += builtin/patch-id.o -BUILTIN_OBJS += builtin/prune-packed.o -BUILTIN_OBJS += builtin/prune.o -BUILTIN_OBJS += builtin/push.o -BUILTIN_OBJS += builtin/read-tree.o -BUILTIN_OBJS += builtin/receive-pack.o -BUILTIN_OBJS += builtin/reflog.o -BUILTIN_OBJS += builtin/remote.o -BUILTIN_OBJS += builtin/remote-ext.o -BUILTIN_OBJS += builtin/remote-fd.o -BUILTIN_OBJS += builtin/repack.o -BUILTIN_OBJS += builtin/replace.o -BUILTIN_OBJS += builtin/rerere.o -BUILTIN_OBJS += builtin/reset.o -BUILTIN_OBJS += builtin/rev-list.o -BUILTIN_OBJS += builtin/rev-parse.o -BUILTIN_OBJS += builtin/revert.o -BUILTIN_OBJS += builtin/rm.o -BUILTIN_OBJS += builtin/send-pack.o -BUILTIN_OBJS += builtin/shortlog.o -BUILTIN_OBJS += builtin/show-branch.o -BUILTIN_OBJS += builtin/show-ref.o -BUILTIN_OBJS += builtin/stripspace.o -BUILTIN_OBJS += builtin/symbolic-ref.o -BUILTIN_OBJS += builtin/tag.o -BUILTIN_OBJS += builtin/unpack-file.o -BUILTIN_OBJS += builtin/unpack-objects.o -BUILTIN_OBJS += builtin/update-index.o -BUILTIN_OBJS += builtin/update-ref.o -BUILTIN_OBJS += builtin/update-server-info.o -BUILTIN_OBJS += builtin/upload-archive.o -BUILTIN_OBJS += builtin/var.o -BUILTIN_OBJS += builtin/verify-commit.o -BUILTIN_OBJS += builtin/verify-pack.o -BUILTIN_OBJS += builtin/verify-tag.o -BUILTIN_OBJS += builtin/write-tree.o +BUILTIN_OBJS = $(patsubst %.c,%.o, $(wildcard builtin/*.c)) GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = -- 2.2.1.268.g1e6f5b2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] doc: core.ignoreStat update, and clarify the --assume-unchanged effect
Philip Oakley philipoak...@iee.org writes: diff --git a/Documentation/config.txt b/Documentation/config.txt index 52eeadd..fe179d0 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -375,15 +375,18 @@ This is useful for excluding servers inside a firewall from proxy use, while defaulting to a common proxy for external domains. core.ignoreStat:: + If true, Git will avoid using lstat() calls to detect if files have + changed. Git will set the assume-unchanged bit for those tracked files + which it has updated identically in both the index and working tree. I wonder if this is better stated in two seemingly independent sentences (like your version), or ... if files have changed by setting the assume-unchanged bit to clarify where the setting of the bits to these files come into the big picture, but it is minor. Either way, I think it is a lot easier to understand than what we have in 'master'. I had considered a number of different wordings, and wanted to keep the tricky parts separate to ease cognition. Hmph, but wouldn't the result get more confusing, by stating two tricky things in separate sentences without giving any clue to guess how these two trickies are related? That is why I suggested to say the same two things in a way that clarifies that avoid using lstat(2) is the effect and setting the assume-unchanged bit is the underlying implementation detail to cause that effect, i.e. If true, Git will avoid using lstat(2) calls to detect if files have changed by setting assume-unchanged bit for these tracked paths that Git has updated identically in both the index and in the working tree. On a separte note, this patch was a development from the problem noticed by Sérgio of the different actions of 'git commit .'and 'git commit -a' when --assume-unchanged was used. Did you have any thoughts regarding Duy's patch (05 December 2014 10:56) to his code given in $gmane/260865. I wasn't sure if it had just been missed, or if there was some other issue? I thought the reason why we are discussing this documentation clean-up (specifically, clarifying that assume-unchanged is a promise the user makes not to modify the paths marked as such and is not about telling Git to ignore changes to tracked paths), was because we agreed that such a change is a wrong thing to do. It is wrong for at least two reasons. - The I promise not to modify them, so please omit lstat(2) assuming that I keep that promise is a performance thing---we shouldn't add more code to cater to people who do not keep that promise. - Adding one more case of Git will hide changes to tracked paths that you promised not to change gives more chance to confuse users into an incorrect understanding of what assume-unchanged bit is about. By not applying $gmane/260865, we keep one more way for the users to notice that the bit is *not* a mechanism to hide changes to tracked paths. +When files are modified outside of Git, the user will need to stage +the modified files explicitly (e.g. see 'Examples' section in +linkgit:git-update-index[1]). +Git will not normally detect changes to those files. ++ +This is useful on systems where lstat() calls are very slow, such as +CIFS/Microsoft Windows. +False by default. I think you are trying to make the result more readable by using separate paragraphs for separate conceptual points, but then isn't it wrong to have False by default as part of stating which platforms are intended targets? I wonder if we want to have that last line as its own paragraph instead. I was happy with it being a simple separate sentence. I am also _for_ a separate sentence. But when a set of three paragraphs, i.e. A, something about A, and things about A. B, something about B, and things about B. C, something about C, and things about C. and you want to say something X that is not specific to A or B or C, would you add that X at the end of C's paragraph, resulting in: A, something about A, and things about A. B, something about B, and things about B. C, something about C, and things about C. X that applies to all of A, B and C. or would it be more clear to see: A, something about A, and things about A. B, something about B, and things about B. C, something about C, and things about C. X that applies to all of A, B and C. was the question. I think a simple separate sentence should not be part of the same In what situations this is useful paragraph. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git.c: remove unused includes
Alexander Kuleshov kuleshovm...@gmail.com writes: * cache.h and commit.h already included in builtin.h * quote.h was appeared in (6035d6aa GIT_TRACE: show which built-in/external commands are executed 25 Jun 2006) and sq_quote_print was removed at (82aae5c quote: remove sq_quote_print() Jul 30 2013) That's not 6035d6aa, though. We started to include quote.h at 575ba9d6 (GIT_TRACE: show which built-in/external commands are executed, 2006-06-25) that wanted to use sq_quote_print(). When 6ce4e61f (Trace into a file or an open fd and refactor tracing code., 2006-09-02) introduced trace.c API, the calls this file makes to sq_quote_print() were replaced by calls to trace_argv_printf() that are declared in cache.h, which this file already includes. We should have stopped including quote.h in that commit, but forgot to do so. Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- git.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/git.c b/git.c index 09b3bcf..c9bec99 100644 --- a/git.c +++ b/git.c @@ -1,10 +1,7 @@ #include builtin.h -#include cache.h #include exec_cmd.h #include help.h -#include quote.h #include run-command.h -#include commit.h const char git_usage_string[] = git [--version] [--help] [-C path] [-c name=value]\n -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-log: added --none-match option
2015-01-06 16:02 GMT-07:00 Junio C Hamano gits...@pobox.com:
Christoph Junghans ott...@gentoo.org writes:
Implements a inverted match for git log, like in the case of
git grep -v, which is useful from time to time to e.g. filter
FIXUP message out of git log.
Internally, a new bol 'none_match' has been introduces as
revs-grep_filter.invert inverts the match line-wise, which cannot
work as i.e. empty line always not match the pattern given.
Signed-off-by: Christoph Junghans ott...@gentoo.org
---
The patch itself looks like a good start, except that the above
description no longer matches the implementation.
I further suspect it would be better to rename all_match to
all_or_none and then you can lose the these two are mutually
incompatible check that is placed together with a wrong existing
comment. I also notice that you forgot to update the git grep
where the original --all-match came from.
That was on purpose. I am not quite sure what would be the point of
showing only matches from files that match no patterns (option
description from your patch below).
If a file matches none of the patterns, what matches are there to show?
The only useful thing I could image is using it in conjunction with
--files-with-matches, but that is what --files-without-match is for.
A partial fix-up may start like this on top of your version. By
renaming the variable used in the existing code, the compiler will
remind you that there are a few more places that your patch did not
touch that does something special for --all-match, which are a good
candidates you need to think if doing something similarly special
for the --none-match case is necessary.
Thanks.
diff --git a/builtin/grep.c b/builtin/grep.c
index 4063882..9ba4254 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -727,8 +727,12 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
close_callback },
OPT__QUIET(opt.status_only,
N_(indicate hit with exit status without
output)),
- OPT_BOOL(0, all-match, opt.all_match,
- N_(show only matches from files that match all
patterns)),
+ OPT_SET_INT(0, all-match, opt.all_or_none,
+ N_(show only matches from files that match all
patterns),
+ GREP_ALL_MATCH),
+ OPT_SET_INT(0, none-match, opt.all_or_none,
+ N_(show only matches from files that match no
patterns),
+ GREP_NONE_MATCH),
{ OPTION_SET_INT, 0, debug, opt.debug, NULL,
N_(show parse tree for grep expression),
PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
diff --git a/grep.c b/grep.c
index f486ee5..1ff5dea 100644
--- a/grep.c
+++ b/grep.c
@@ -1622,14 +1622,18 @@ static int chk_hit_marker(struct grep_expr *x)
int grep_source(struct grep_opt *opt, struct grep_source *gs)
{
- if(opt-none_match)
- return !grep_source_1(opt, gs, 0);
/*
* we do not have to do the two-pass grep when we do not check
-* buffer-wide all-match.
+* buffer-wide all-match or none-match.
*/
- if (!opt-all_match)
+ switch (opt-all_or_none) {
+ case GREP_ALL_MATCH:
return grep_source_1(opt, gs, 0);
+ case GREP_NONE_MATCH:
+ return !grep_source_1(opt, gs, 0);
+ default:
+ break;
+ }
/* Otherwise the toplevel or terms hit a bit differently.
* We first clear hit markers from them.
diff --git a/grep.h b/grep.h
index 8e50c95..2cdabf2 100644
--- a/grep.h
+++ b/grep.h
@@ -101,8 +101,9 @@ struct grep_opt {
int count;
int word_regexp;
int fixed;
- int all_match;
- int none_match;
+#define GREP_ALL_MATCH 1
+#define GREP_NONE_MATCH 2
+ int all_or_none;
int debug;
#define GREP_BINARY_DEFAULT0
#define GREP_BINARY_NOMATCH1
diff --git a/revision.c b/revision.c
index d43779e..b955848 100644
--- a/revision.c
+++ b/revision.c
@@ -2010,9 +2010,9 @@ static int handle_revision_opt(struct rev_info *revs,
int argc, const char **arg
} else if (!strcmp(arg, --perl-regexp)) {
grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE,
revs-grep_filter);
} else if (!strcmp(arg, --all-match)) {
- revs-grep_filter.all_match = 1;
+ revs-grep_filter.all_or_none = GREP_ALL_MATCH;
} else if (!strcmp(arg, --none-match)) {
- revs-grep_filter.none_match = 1;
+ revs-grep_filter.all_or_none = GREP_NONE_MATCH;
} else if ((argcount = parse_long_opt(encoding, argv, optarg))) {
if (strcmp(optarg, none))
git_log_output_encoding = xstrdup(optarg);
@@ -2335,8 +2335,6 @@ int
Re: [RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised
Stefan Beller sbel...@google.com writes:
If the server did not advertise the capability to have signed pushes
it should not accept signed pushes as stated in
Documentation/technical/protocol-capabilities.txt:
Client will then send a space separated list of capabilities it wants
to be in effect. The client MUST NOT ask for capabilities the server
did not say it supports.
Server MUST diagnose and abort if capabilities it does not understand
was sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.
After rereading the second paragraph I think they should also be reworded to
Server MUST diagnose and abort if capabilities it did not advertise
was sent.
Except for s/was sent/was requested/, I think that rule makes sense
very much.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4c069c5..628d13a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1276,7 +1276,8 @@ static struct command *read_head_info(struct sha1_array
*shallow)
use_atomic = 1;
}
- if (!strcmp(line, push-cert)) {
+ if (push_cert_nonce
+ !strcmp(line, push-cert)) {
int true_flush = 0;
char certbuf[1024];
This implementation is somewhat questionable.
The server knows how to parse push-cert line, knows that what
follows after that line up to push-cert-end line are shaped very
differently from protocol commands outside the push-cert block. In
other words, it knows how to parse the request meant for the capable
server; it just wants to refuse to serve that request.
The patched code will make it fail by hoping that queue_command()
that only handles 40-hex 40-hex ref will reject the line that
begins with push-cert. Instead of relying on such a hidden
dependency, wouldn't it be cleaner to actually parse the push-cert
block and then at the end notice and explictly say Your requests
were syntactically correct, but I am not going to honor your request
to use the push-cert extension, because I never told you that I'd
offer you that capability, instead of rejecting the request with I
was expecting old/new/ref but you sent a line with 'push-cert' on
it; what are you talking about?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cat-file: add short option '-c' for 'cat-file --textconv'
Alexander Kuleshov kuleshovm...@gmail.com writes: Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- Documentation/git-cat-file.txt | 5 +++-- builtin/cat-file.c | 4 ++-- t/t8007-cat-file-textconv.sh | 10 ++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index f6a16f4..b346a5d 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -9,14 +9,14 @@ git-cat-file - Provide content or type and size information for repository objec SYNOPSIS [verse] -'git cat-file' (-t | -s | -e | -p | type | --textconv ) object +'git cat-file' (-t | -s | -e | -p | type | (-c | --textconv) ) object Do we use -c for a shorthand for --textconv anywhere else? Is there any other command that has --textconv where -c does not mean --textconv? Or worse yet, where -c already means something completely different from --textconv? Unlike end-user facing Porcelain commands, plumbing commands are primarily meant to be used in scripts, and I am not sure how much benefit we are getting by introducing new short options to them. Ancient plumbing commands do have many single-letter options but they were added back when more modern set of end-user facing Porcelain commands did not exist. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-log: added --none-match option
Christoph Junghans ott...@gentoo.org writes: The only useful thing I could image is using it in conjunction with --files-with-matches, but that is what --files-without-match is for. Yes, -l was exactly what I had in mind and I was hoping that git grep -l --no-match -e WIP -e TODO -e FIXME -e NEEDSWORK may be a way to find perfect files without needing any work. You can say git grep -L -e WIP -e TODO -e FIXME -e NEEDSWORK instead. I missed that -L option. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised
Junio C Hamano gits...@pobox.com writes:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4c069c5..628d13a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1276,7 +1276,8 @@ static struct command *read_head_info(struct
sha1_array *shallow)
use_atomic = 1;
}
-if (!strcmp(line, push-cert)) {
+if (push_cert_nonce
+!strcmp(line, push-cert)) {
int true_flush = 0;
char certbuf[1024];
This implementation is somewhat questionable.
The server knows how to parse push-cert line, knows that what
follows after that line up to push-cert-end line are shaped very
differently from protocol commands outside the push-cert block. In
other words, it knows how to parse the request meant for the capable
server; it just wants to refuse to serve that request.
The patched code will make it fail by hoping that queue_command()
that only handles 40-hex 40-hex ref will reject the line that
begins with push-cert. Instead of relying on such a hidden
dependency, wouldn't it be cleaner to actually parse the push-cert
block and then at the end notice and explictly say Your requests
were syntactically correct, but I am not going to honor your request
to use the push-cert extension, because I never told you that I'd
offer you that capability, instead of rejecting the request with I
was expecting old/new/ref but you sent a line with 'push-cert' on
it; what are you talking about?
Note that I said somewhat questionable, not horribly broken,
because another part of me wants to say that this implementation may
be better than parse and reject from security point of view. The
pusher cannot tell if the push failed because the server is old to
know about the extension, or the server is new but is refusing, by
observing the failure.
But the other side of the same coin is that it makes it harder to
diagnose the failure.
When the protocol exchange gets to this state, in practice, we know
we are talking with somebody who has push privilege into the
repository, so revealing a bit more about the server by taking
parse and reject route may be a reasonable price to give diagnosis
that is more useful to help the users in this case, though.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised
On Fri, Jan 9, 2015 at 2:39 PM, Junio C Hamano gits...@pobox.com wrote: The patched code will make it fail by hoping that queue_command() that only handles 40-hex 40-hex ref will reject the line that begins with push-cert. Instead of relying on such a hidden dependency, wouldn't it be cleaner to actually parse the push-cert block and then at the end notice and explictly say Your requests were syntactically correct, but I am not going to honor your request to use the push-cert extension, because I never told you that I'd offer you that capability, instead of rejecting the request with I was expecting old/new/ref but you sent a line with 'push-cert' on it; what are you talking about? When reading the documentation at first I understood it the way: If I did not advertise the feature, I am expected to pretend it doesn't even exist. That lead me to the implementation as proposed. Your proposal to acknowledge the correctness of the message leads to more questions. How would we proceed? We would not accept the featured capability, but we could still do a normal push operation. This would not be desired by a authentic user, so aborting the whole push process would be ok. I expect such behavior only from malicious clients which actively want to abuse a feature which wasn't advertised, so I wouldn't mind being slightly dishonest to the client and pretend we don't know that capability. But relying on the hidden dependency is bad indeed. So maybe just answer more rudely I did not advertise one of the capability you requested, go away! (closes connection). But the other side of the same coin is that it makes it harder to diagnose the failure. The push-cert case is special compared to other capabilities as it is quite intrusive to the protocol compared to say quiet, so more diagnosis may be helpful. When the protocol exchange gets to this state, in practice, we know we are talking with somebody who has push privilege into the repository, Yeah but what is one repository compared to the whole server? (Just painting the devil on the wall now:) Say you could abuse one capability to gain access to the server or create a huge memory allocation such that the server becomes unresponsive for others. I know my argument is weak for the parse and reject route, so I will see if I can adjust the patch to give diagnosis but still reject the un-advertised capability. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] gettext.h: add parentheses around N_ expansion if supported
Kyle J. McKay mack...@gmail.com writes:
I'm not super attached to this change, it's just that it seems to me
that translation support for Git is a scarce resource. I'm guessing
that when considering the 7 complete translations (bg, ca, de, fr, sv,
vi and zh_CN) the average number of translators per language is in the
low single digits. So I hate to see unnecessary translation churn,
not when it can be so easily prevented.
Yes, I share the same feeling and I agree that it is a worthy goal.
I just did not want an unconditional #ifdef __GNUC__ that nobody
can override without editing the source, when __GNUC__ is a rough
approximation whether a specific language extension exists and is
enabled (we do not know which -Woption or options like --pedantic
that will be added in the future would interfere with us).
What I had in mind instead was something along this line (but with a
better make variable name). In an unconfigured build, it decides
between (msg) and msg using the same __GNUC__ heuristic, but
that can be overridden from the $(MAKE) command line in a pinch.
I do not do autoconf, but it would be trivial to set CAN_USE_* by
try-compiling a small program.
Makefile | 17 -
gettext.h | 24
git-compat-util.h | 7 +++
3 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 06e5d24..48f4ce2 100644
--- a/Makefile
+++ b/Makefile
@@ -343,6 +343,12 @@ all::
# return NULL when it receives a bogus time_t.
#
# Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
+#
+# Define CAN_USE_CONSTANT_STRING_IN_PARENTHESES to Yes if your compiler
+# happily compiles the following initialization:
+# static const char s[] = (FOO);
+# and define it to No if you need to remove the parentheses () around the
+# constant.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -950,6 +956,16 @@ ifneq (,$(SOCKLEN_T))
BASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T)
endif
+ifeq (Yes,$(CAN_USE_CONSTANT_STRING_IN_PARENTHESES))
+ BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=1
+else
+ifneq (,$(CAN_USE_CONSTANT_STRING_IN_PARENTHESES))
+ BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=0
+else
+ BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=-1
+endif
+endif
+
ifeq ($(uname_S),Darwin)
ifndef NO_FINK
ifeq ($(shell test -d /sw/lib echo y),y)
@@ -2486,4 +2502,3 @@ cover_db: coverage-report
cover_db_html: cover_db
cover -report html -outputdir cover_db_html cover_db
-
diff --git a/gettext.h b/gettext.h
index 7671d09..9b54ded 100644
--- a/gettext.h
+++ b/gettext.h
@@ -63,6 +63,30 @@ const char *Q_(const char *msgid, const char *plu, unsigned
long n)
}
/* Mark msgid for translation but do not translate it. */
+#if !USE_PARENS_AROUND_N
#define N_(msgid) msgid
+#else
+/*
+ * Strictly speaking, this will lead to invalid C when
+ * used this way:
+ * static const char s[] = N_(FOO);
+ * which will expand to
+ * static const char s[] = (FOO);
+ * and in valid C, the initializer on the right hand side must
+ * be without the parentheses. But many compilers do accept it
+ * as a language extension and it will allow us to catch mistakes
+ * like:
+ * static const char **msgs = {
+ *N_(one)
+ *N_(two),
+ *N_(three),
+ *NULL
+ * }
+ * (notice the missing comma on one of the lines) by forcing
+ * an compilation error, because parenthesised (one) (two)
+ * will not get silently turned into (onetwo).
+ */
+#define N_(msgid) (msgid)
+#endif
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index dcecd85..8640163 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -867,4 +867,11 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define gmtime_r git_gmtime_r
#endif
+#if USE_PARENS_AROUND_N == -1
+# ifdef __GNUC__
+#undef USE_PARENS_AROUND_N
+#define USE_PARENS_AROUND_N 1
+# endif
+#endif
+
#endif
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised
If the server did not advertise the capability to have signed pushes
it should not accept signed pushes as stated in
Documentation/technical/protocol-capabilities.txt:
Client will then send a space separated list of capabilities it wants
to be in effect. The client MUST NOT ask for capabilities the server
did not say it supports.
Server MUST diagnose and abort if capabilities it does not understand
was sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.
After rereading the second paragraph I think they should also be reworded to
Server MUST diagnose and abort if capabilities it did not advertise
was sent.
Suppose there would be hypothetical flaw in the capability of signed
pushes (or any capability for the current reasoning) which may harm
the server. This would require a bugfix release if it was severe and
would put us on time pressure to get it done.
A change like the one proposed would allow us to tell the community to
simply configure the server to not advertise the feature and if not
advertised the flaw could not be abused.
I am not saying there is a problem now, but I am rather saying patches
similar to this one would buy us time in case of problems arising.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
As I discovered the idea while composing the
atomic push series and the changes line of this
patch is closeby, this applies on top of
origin/sb/atomic-push (v12 as sent on Jan. 7th)
This patch is RFC, thinking about security best practice.
It's not enough to document the intended behavior in
Documentation/technical/protocol-capabilities.txt, but
rather enforce it in the code as well.
Any thoughts on that welcome!
Thanks,
Stefan
builtin/receive-pack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4c069c5..628d13a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1276,7 +1276,8 @@ static struct command *read_head_info(struct sha1_array
*shallow)
use_atomic = 1;
}
- if (!strcmp(line, push-cert)) {
+ if (push_cert_nonce
+ !strcmp(line, push-cert)) {
int true_flush = 0;
char certbuf[1024];
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] cat-file: Remove unused includes
* exec_cmd.h became unnecessary at b931aa5a (Call builtin ls-tree in git-cat-file -p, 2006-05-26). * tag.h and tree.h became unnecessary at 21666f1a (convert object type handling from a string to a number, 2007-02-26). * diff.h was added at e5fba602 (textconv: support for cat_file, 2010-06-15). Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- ndrivers 0 ndrivers 0 builtin/cat-file.c | 4 1 file changed, 4 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index f8d8129..750b5a2 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -4,12 +4,8 @@ * Copyright (C) Linus Torvalds, 2005 */ #include cache.h -#include exec_cmd.h -#include tag.h -#include tree.h #include builtin.h #include parse-options.h -#include diff.h #include userdiff.h #include streaming.h -- 2.2.1.522.g2561c04.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] doc: core.ignoreStat update, and clarify the --assume-unchanged effect
From: Junio C Hamano gits...@pobox.com Philip Oakley philipoak...@iee.org writes: The assume-unchanged bit, and consequently core.ignoreStat, can be misunderstood. Be assertive about the expectation that file changes should notified to Git. Overhaul the general wording thus: 1. direct description of what is ignored given first. 2. example instruction of the user manual action required. 3. use sideways indirection for assume-unchanged and update-index references. 4. add a 'normally' to give leeway for the change detection. Signed-off-by: Philip Oakley philipoak...@iee.org --- This is the corrected patch which now applies on top of next and has been checked on AsciiDoc. (was $gmane/261974/focus=262022) I have included the previous 'after three-dashes' comment and included them in the commit message. I'm happy for it to be tweaked as appropriate. Thanks. diff --git a/Documentation/config.txt b/Documentation/config.txt index 52eeadd..fe179d0 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -375,15 +375,18 @@ This is useful for excluding servers inside a firewall from proxy use, while defaulting to a common proxy for external domains. core.ignoreStat:: + If true, Git will avoid using lstat() calls to detect if files have + changed. Git will set the assume-unchanged bit for those tracked files + which it has updated identically in both the index and working tree. I wonder if this is better stated in two seemingly independent sentences (like your version), or ... if files have changed by setting the assume-unchanged bit to clarify where the setting of the bits to these files come into the big picture, but it is minor. Either way, I think it is a lot easier to understand than what we have in 'master'. I had considered a number of different wordings, and wanted to keep the tricky parts separate to ease cognition. On a separte note, this patch was a development from the problem noticed by Sérgio of the different actions of 'git commit .'and 'git commit -a' when --assume-unchanged was used. Did you have any thoughts regarding Duy's patch (05 December 2014 10:56) to his code given in $gmane/260865. I wasn't sure if it had just been missed, or if there was some other issue? ++ +When files are modified outside of Git, the user will need to stage +the modified files explicitly (e.g. see 'Examples' section in +linkgit:git-update-index[1]). +Git will not normally detect changes to those files. ++ +This is useful on systems where lstat() calls are very slow, such as +CIFS/Microsoft Windows. +False by default. I think you are trying to make the result more readable by using separate paragraphs for separate conceptual points, but then isn't it wrong to have False by default as part of stating which platforms are intended targets? I wonder if we want to have that last line as its own paragraph instead. I was happy with it being a simple separate sentence. Thanks. core.preferSymlinkRefs:: Instead of the default symref format for HEAD -- Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cat-file: add short option '-c' for 'cat-file --textconv'
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
Documentation/git-cat-file.txt | 5 +++--
builtin/cat-file.c | 4 ++--
t/t8007-cat-file-textconv.sh | 10 ++
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f6a16f4..b346a5d 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,14 +9,14 @@ git-cat-file - Provide content or type and size information
for repository objec
SYNOPSIS
[verse]
-'git cat-file' (-t | -s | -e | -p | type | --textconv ) object
+'git cat-file' (-t | -s | -e | -p | type | (-c | --textconv) ) object
'git cat-file' (--batch | --batch-check) list-of-objects
DESCRIPTION
---
In its first form, the command provides the content or the type of an object in
the repository. The type is required unless '-t' or '-p' is used to find the
-object type, or '-s' is used to find the object size, or '--textconv' is used
+object type, or '-s' is used to find the object size, or '-c/--textconv' is
used
(which implies type blob).
In the second form, a list of objects (separated by linefeeds) is provided on
@@ -52,6 +52,7 @@ OPTIONS
or to ask for a blob with object being a tag object that
points at it.
+-c
--textconv::
Show the content as transformed by a textconv filter. In this case,
object has be of the form tree-ish:path, or :path in order
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f8d8129..a8154a0 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -329,7 +329,7 @@ static int batch_objects(struct batch_options *opt)
}
static const char * const cat_file_usage[] = {
- N_(git cat-file (-t|-s|-e|-p|type|--textconv) object),
+ N_(git cat-file (-t|-s|-e|-p|type|(-c|--textconv)) object),
N_(git cat-file (--batch|--batch-check) list_of_objects),
NULL
};
@@ -373,7 +373,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
OPT_SET_INT('e', NULL, opt,
N_(exit with zero when there's no error), 'e'),
OPT_SET_INT('p', NULL, opt, N_(pretty-print object's
content), 'p'),
- OPT_SET_INT(0, textconv, opt,
+ OPT_SET_INT('c', textconv, opt,
N_(for blob objects, run textconv on object's
content), 'c'),
{ OPTION_CALLBACK, 0, batch, batch, format,
N_(show info and content of objects fed from the
standard input),
diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index eacd49a..994c5b0 100755
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -24,6 +24,11 @@ bin: test version 2
EOF
test_expect_success 'no filter specified' '
+ git cat-file -c :one.bin result
+ test_cmp expected result
+'
+
+test_expect_success 'no filter specified' '
git cat-file --textconv :one.bin result
test_cmp expected result
'
@@ -52,6 +57,11 @@ cat expected EOF
converted: test version 2
EOF
+test_expect_success 'cat-file -c on last commit' '
+ git cat-file -c :one.bin result
+ test_cmp expected result
+'
+
test_expect_success 'cat-file --textconv on last commit' '
git cat-file --textconv :one.bin result
test_cmp expected result
--
2.2.1.522.g2561c04.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
