Re: [PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
Ilya Bobyr ilya.bo...@gmail.com writes: While it could be done, it looks less obvious than this: GIT_TEST_ONLY='1 4' ./t0001-init.sh If you are thinking about affecting only one test, then you shouldn't be mucking with environment variables in the first place, primarily because running: $ GIT_TEST_ONLY='1 4' make test to run test .1 and .4 of all the test scripts would not make any sense. I think your simplicity argument is a total red-herring. Of course if you do not have to say the test script name, your specification would be shorter, but that is only because your specification is not specific enough to be useful. Giving that as a command line argument to the specific script, e.g. $ sh ./t0001-init.sh --only='1 4' might make some sense, but the above GIT_TEST_ONLY does not make any sense from the UI point of view. There are many reasons that makes me unenthused about this line of change in the first place: * Both at the philosophical level and at the practical level, I've found that it always makes sense to run most of the tests, i.e. skipping ought to be an exception not the norm. Over the course of this project, I often saw an alleged fix to one part of the system introduces breakages that are caught by tests that checks parts of the system that does not have any superficial link to it (e.g. update the refs code and find a rebase test break). * Even though GIT_SKIP_TESTS mechanism still allows you to skip individual test pieces, it has never been a serious feature in the first place. Many of the tests unfortunately do rely on state previous sequences of tests left behind, so it is not realistic to expect that you can skip test pieces randomly and exercise later test pieces reliably. * The numbering of individual test pieces can easily change by new tests inserted in the middle; again, many tests do take advantge of the states earlier tests leave behind, so do not add new tests in the middle is not a realistic rule to enforce, unless you are willing to clean up existing test scripts so that each test piece is independent from all the previous ones. The latter two makes the ability to skip individual test pieces a theoretically it could be made useful but practically not so much misfeature. I am very hesitant to see the test framework code churned only to enhance its usefulness when there isn't any in the first place, without first making changes that fundamentally improves its usefulness (e.g. to solve test numbering is not stable problem, you could identify the tests with test names instead of numbers to make it more stable, but that is not what your patch is even attempting to do). I view such a change as merely a code churn, damaging the codebase that is already less nice than ideal and turning it more difficult to fix properly to make it truly nicer later. My suggestion to cram everything into GIT_SKIP_TESTS is primarily because it is one way I can easily see how it allows us to limit the damage to the codebase--the suggested change could be contained within a single shell function match_pattern_list and no other code has to change to support it. I am not saying it is the only way, but glancing at your patch, adding an extra environment variable would need to also modify its callers, so the extent of the damage to the codebase seemed somewhat larger. So, I dunno. I am not yet enthused. -- 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] cache_tree_find(): remove redundant checks
The beginning of the loop ensures that slash can never be NULL. So
don't keep checking whether it is NULL later in the loop.
Furthermore, there is no need for an early
return it;
from the loop if slash points at the end of the string, because that
is exactly what will happen when the while condition fails at the
start of the next iteration.
Helped-by: David Kastrup d...@gnu.org
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
I incorporated David's suggestion, and then realized that yet another
check was superfluous, so I removed that one too.
cache-tree.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..19252c3 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -551,25 +551,22 @@ static struct cache_tree *cache_tree_find(struct
cache_tree *it, const char *pat
if (!it)
return NULL;
while (*path) {
- const char *slash;
struct cache_tree_sub *sub;
+ const char *slash = strchr(path, '/');
- slash = strchr(path, '/');
if (!slash)
slash = path + strlen(path);
- /* between path and slash is the name of the
-* subtree to look for.
+ /*
+* Between path and slash is the name of the subtree
+* to look for.
*/
sub = find_subtree(it, path, slash - path, 0);
if (!sub)
return NULL;
it = sub-cache_tree;
- if (slash)
- while (*slash *slash == '/')
- slash++;
- if (!slash || !*slash)
- return it; /* prefix ended with slashes */
path = slash;
+ while (*path == '/')
+ path++;
}
return it;
}
--
1.9.0
--
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] commit.c: use skip_prefix() instead of starts_with()
In record_author_date() parse_gpg_output() ,using skip_prefix() instead of
starts_with() is a more suitable abstraction.
Helped-by: Max Horn m...@quendi.de
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Patch V3 Variable naming improved, removed assignments inside conditionals.
Thanks to Junio C Hamano and Max Horn.
Patch V2 Corrected email formatting ,reapplied the implementation according to
suggestions.
Thanks to Michael Haggerty.
This is in respect to GSoC microproject #10.
In record_author_date(), extra and useless calls to strlen due to using
starts_with()
were removed by using skip_prefix(). Extra variable skip was used as buf is
used in
for loop update check.
Other usages of starts_with() in the same file can be found with,
$ grep -n starts_with commit.c
1116: else if (starts_with(line, gpg_sig_header)
1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
The starts_with() in line 1116 was left as it is, as strlen values were pre
generated as
global variables.
The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix
part by
directly using the function.
Also skip_prefix() is inline when compared to starts_with().
commit.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..6c92acb 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
static void record_author_date(struct author_date_slab *author_date,
struct commit *commit)
{
- const char *buf, *line_end;
+ const char *buf, *line_end, *ident_line;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,16 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
+ buf = ident_line;
if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+buf,
+line_end - buf) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ /* At the very beginning of the buffer */
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
1.9.0
--
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 3/3] rebase: new convenient option to edit a single commit
On 03/04/2014 03:08 AM, Duy Nguyen wrote: On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: git rebase -e XYZ is basically the same as EDITOR=sed -i '1s/pick XYZ/edit XYZ/' $@ \ git rebase -i XYZ^ In English, it prepares the todo list for you to edit only commit XYZ to save your time. The time saving is only significant when you edit a lot of commits separately. Is it correct to single out only edit for special treatment? If allowing edit on the command-line, then shouldn't command-line reword also be supported? I, for one, often need to reword a commit message (or two or three); far more frequently than I need to edit a commit. (This is a genuine question about perceived favoritism of edit, as opposed to a request to further bloat the interface.) Heh I had the same thought yesterday. The same thing could be asked for git commit --fixup to send us back to the fixed up commit so we can do something about it. If we go along that line, then git commit may be a better interface to reword older commits.. I disagree. git commit --fixup doesn't rewrite history. It just adds a new commit with a special commit message that will make it easier to rewrite history later. I think it would be prudent to keep the history-rewriting functionality segregated in git rebase, which users already know they have to use with care [1]. But the next question is whether git rebase should have shortcuts for *most* of its line commands. All of the following seem to make sense: git rebase --edit COMMIT A long-form for the -e option we have been talking about. It is unfortunately that this spelling sounds like the --edit option on git commit --edit and git merge --edit, so people might use it when they really mean git rebase --reword COMMIT. git rebase --reword COMMIT git rebase --fixup COMMIT git rebase --squash COMMIT git rebase --kill COMMIT Remove the commit from history, like running git rebase --interactive then deleting that line. I'm quite confident that I would use all of these commands. Moreover, it would logically be reasonable to allow multiple of these options, at least as long as they have distinct COMMIT arguments. Though, as Duy points out, it might in practice be easier to edit the todo list in an editor rather than trying to do multiple edits at a time via the command line. Some thought would have to go into the question of if/how these commands should interact with git rebase --autosquash (which, don't forget, can also be requested via rebase.autosquash). Michael [1] OK, granted, there is git commit --amend, which rewrites history too. But it rewrites only the last commit, which is less likely to be problematic. -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
On 3/4/2014 12:29 AM, Junio C Hamano wrote: Ilya Bobyr ilya.bo...@gmail.com writes: While it could be done, it looks less obvious than this: GIT_TEST_ONLY='1 4' ./t0001-init.sh If you are thinking about affecting only one test, Yes, that is the use case: when I am developing a specific feature I want to run just one test for that feature over and over, while I am working on that specific thing. Not the whole test suite (like t0001), but just the new case that I've added to the end, for example. Plus one or more tests that setup enough environment for it. then you shouldn't be mucking with environment variables in the first place, primarily because running: $ GIT_TEST_ONLY='1 4' make test to run test .1 and .4 of all the test scripts would not make any sense. No it does not. It only makes sense for one test suite. I think your simplicity argument is a total red-herring. Of course if you do not have to say the test script name, your specification would be shorter, but that is only because your specification is not specific enough to be useful. In my case it is very useful :) This is why I am saying that we might be talking about different cases: you are talking about the test suite level, while the issue I am trying to address an issue at an individual test level. Giving that as a command line argument to the specific script, e.g. $ sh ./t0001-init.sh --only='1 4' might make some sense, but the above GIT_TEST_ONLY does not make any sense from the UI point of view. No problem, I guess I can make it look like that - with '--only'. Maybe '--tests'? Then the same negation syntax could be used as previously discussed. As well as range syntax. There are many reasons that makes me unenthused about this line of change in the first place: * Both at the philosophical level and at the practical level, I've found that it always makes sense to run most of the tests, i.e. skipping ought to be an exception not the norm. Over the course of this project, I often saw an alleged fix to one part of the system introduces breakages that are caught by tests that checks parts of the system that does not have any superficial link to it (e.g. update the refs code and find a rebase test break). My main argument is the time. When testing Git as a whole or a feature as a whole there is no reason to skip some tests. When working on a specific piece I may run the same test 100 times easily. Here is what I see on my Cygwin: $ time ./t0001-init.sh [...] 1..36 real0m6.693s user0m1.505s sys 0m3.937s $ time GIT_SKIP_TESTS='t0001.[36789] t0001.??' ./t0001-init.sh [...] 1..36 real0m3.313s user0m0.769s sys 0m1.844s So skipping 34 tests that I am not interested in save a bit more that 50% of the time. While it would be really nice if it would be faster, this speedup is a pretty simple one. * Even though GIT_SKIP_TESTS mechanism still allows you to skip individual test pieces, it has never been a serious feature in the first place. Many of the tests unfortunately do rely on state previous sequences of tests left behind, so it is not realistic to expect that you can skip test pieces randomly and exercise later test pieces reliably. * The numbering of individual test pieces can easily change by new tests inserted in the middle; again, many tests do take advantge of the states earlier tests leave behind, so do not add new tests in the middle is not a realistic rule to enforce, unless you are willing to clean up existing test scripts so that each test piece is independent from all the previous ones. Both are true, but do not apply to the TDD case. Neither they apply to a case when a test is broken and I want to execute everything up to that test. The latter two makes the ability to skip individual test pieces a theoretically it could be made useful but practically not so much misfeature. I am very hesitant to see the test framework code churned only to enhance its usefulness when there isn't any in the first place, without first making changes that fundamentally improves its usefulness (e.g. to solve test numbering is not stable problem, you could identify the tests with test names instead of numbers to make it more stable, but that is not what your patch is even attempting to do). If you see a way to address my problems, I might be able to code it the way you want it to be. [...] -- 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] implemented strbuf_write_or_die()
On 03/03/2014 07:31 PM, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Sat, Mar 1, 2014 at 7:51 AM, He Sun sunheeh...@gmail.com wrote: 2014-03-01 19:21 GMT+08:00 Faiz Kothari faiz.of...@gmail.com: diff --git a/remote-curl.c b/remote-curl.c index 10cb011..dee8716 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads) if (start_command(client)) exit(1); if (preamble) - write_or_die(client.in, preamble-buf, preamble-len); + strbuf_write_or_die(client.in, preamble); if (heads) write_or_die(client.in, heads-buf, heads-len); This should be changed. May be you can use Ctrl-F to search write_or_die(). Or if you are using vim, use / and n to find all. It's not obvious from the patch fragment, but 'heads' is not a strbuf, so Faiz correctly left this invocation alone. That is a very good sign why this change is merely a code-churn and not an improvement, isn't it? We know (and any strbuf user should know) that -buf and -len are the ways to learn the pointer and the length the strbuf holds. Why anybody thinks it is benefitial to introduce another function that is _only_ for writing out strbuf and cannot be used to write out a plain buffer is simply beyond me. I'm the guilty one. I like the change (obviously, since I suggested it). Writing strbufs comes up frequently and will hopefully increase in usage and I think it is a positive thing to encourage the use of strbufs by making them increasingly first-class citizens. But I can see your points too, and I humbly defer to the wisdom of the list. I will remove this suggestion from the list of microprojects. Faiz, this is the way things go on the Git mailing list. It would be boring if everybody agreed all the time :-) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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] cache_tree_find(): remove redundant checks
Michael Haggerty mhag...@alum.mit.edu writes: The beginning of the loop ensures that slash can never be NULL. So don't keep checking whether it is NULL later in the loop. Furthermore, there is no need for an early return it; from the loop if slash points at the end of the string, because that is exactly what will happen when the while condition fails at the start of the next iteration. Hm. Another suggestion. You have const char *slash = strchr(path, '/'); if (!slash) slash = path + strlen(path); [...] sub = find_subtree(it, path, slash - path, 0); [...] path = slash; while (*path == '/') path++; } At the price of introducing another variable, this could be const char *slash = strchr(path, '/'); size_t len = slash ? slash - path : strlen(path); [...] sub = find_subtree(it, path, len, 0); [...] if (!slash) break; for (path = slash; *path == '/';) path++; } This introduces another variable and another condition. The advantage is that slash indeed points at a slash or is NULL, so the variable names correspond better to what happens. Alternatively, it might make sense to rename slash into end or endpart or whatever. Since I can't think of a pretty name, I lean towards preferring the latter version as it reads nicer. I prefer code to read like children's books rather than mystery novels. -- David Kastrup -- 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] cache_tree_find(): remove redundant checks
On 03/04/2014 10:40 AM, David Kastrup wrote: Michael Haggerty mhag...@alum.mit.edu writes: The beginning of the loop ensures that slash can never be NULL. So don't keep checking whether it is NULL later in the loop. Furthermore, there is no need for an early return it; from the loop if slash points at the end of the string, because that is exactly what will happen when the while condition fails at the start of the next iteration. Hm. Another suggestion. You have const char *slash = strchr(path, '/'); if (!slash) slash = path + strlen(path); [...] sub = find_subtree(it, path, slash - path, 0); [...] path = slash; while (*path == '/') path++; } At the price of introducing another variable, this could be const char *slash = strchr(path, '/'); size_t len = slash ? slash - path : strlen(path); [...] sub = find_subtree(it, path, len, 0); [...] if (!slash) break; for (path = slash; *path == '/';) path++; } This introduces another variable and another condition. The advantage is that slash indeed points at a slash or is NULL, so the variable names correspond better to what happens. Alternatively, it might make sense to rename slash into end or endpart or whatever. Since I can't think of a pretty name, I lean towards preferring the latter version as it reads nicer. I prefer code to read like children's books rather than mystery novels. I think we're reaching the point of diminishing returns here. I can't muster a strong feeling either way about your suggestion to add a len variable. BTW, I purposely didn't use a for loop at the end (even though I usually like them) because I wanted to keep it prominent that path is being updated to the value of slash. Putting that assignment in a for loop makes it easy to overlook because it puts path in the spot that usually holds an inconsequential iteration variable. YMMV. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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 3/3] rebase: new convenient option to edit a single commit
On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine sunsh...@sunshineco.com wrote: Is it correct to single out only edit for special treatment? If allowing edit on the command-line, then shouldn't command-line reword also be supported? I, for one, often need to reword a commit message (or two or three); far more frequently than I need to edit a commit. (This is a genuine question about perceived favoritism of edit, as opposed to a request to further bloat the interface.) Heh I had the same thought yesterday. The same thing could be asked for git commit --fixup to send us back to the fixed up commit so we can do something about it. If we go along that line, then git commit may be a better interface to reword older commits.. I disagree. git commit --fixup doesn't rewrite history. It just adds a new commit with a special commit message that will make it easier to rewrite history later. I think it would be prudent to keep the history-rewriting functionality segregated in git rebase, which users already know they have to use with care [1]. Just to be clear I didn't mean to modify --fixup behavior. It could be --amend-old-commit or something like that. It's actually --amend that made me want to put the UI in git commit. But it's a bad idea (besides what you pointed out) because after you're done, you still need to do git rebase --continue. But the next question is whether git rebase should have shortcuts for *most* of its line commands. All of the following seem to make sense: git rebase --edit COMMIT A long-form for the -e option we have been talking about. It is unfortunately that this spelling sounds like the --edit option on git commit --edit and git merge --edit, so people might use it when they really mean git rebase --reword COMMIT. git rebase --reword COMMIT Sounds good. git rebase --fixup COMMIT git rebase --squash COMMIT This is not interactive (except when merge conflicts occur), is it? A bit off topic. I sometimes want to fix up a commit and make it stop there for me to test it again but there is no such command, is there? Maybe we could add support for fixup/edit (or fe for short) and squash/edit (se). Not really familiar with the code base to do that myself quickly though. git rebase --kill COMMIT Remove the commit from history, like running git rebase --interactive then deleting that line. Yes! Done this sometimes (not so often) but a definitely nice thing to have. I'd go with --remove or --delete though. -- Duy -- 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] cache_tree_find(): remove redundant checks
Michael Haggerty mhag...@alum.mit.edu writes: BTW, I purposely didn't use a for loop at the end (even though I usually like them) because I wanted to keep it prominent that path is being updated to the value of slash. Putting that assignment in a for loop makes it easy to overlook because it puts path in the spot that usually holds an inconsequential iteration variable. Reasonable. -- David Kastrup -- 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: Rewriting git history during git-svn conversion
On Mon, Mar 3, 2014 at 7:38 PM, Robert Dailey rcdailey.li...@gmail.com wrote: Is it safe to do this while still using git svn fetch? Will it properly continue to convert SVN commits on top of my rewritten history? If not, what changes can I make after I run the commands linked by the URL above so that git svn continues to work normally? I think it's OK. git-svn doesn't continuously verify the integrity of history already converted, I believe. Just try it out, it worked fine in a little demo setup I made (although I used rebase -i instead of filter-branch): git svn clone .. #maybe clone a little test repository to speed up the testing git filter-branch ... #remove unwanted files git svn fetch #this should work On a related note, maybe you'll enjoy my git-svn demos ideas here: http://www.tfnico.com/presentations/git-and-subversion -- 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] upload-pack: allow shallow fetching from a read-only repository
Before cdab485 (upload-pack: delegate rev walking in shallow fetch to
pack-objects - 2013-08-16) upload-pack does not write to the source
repository. cdab485 starts to write $GIT_DIR/shallow_XX if it's a
shallow fetch, so the source repo must be writable.
git:// servers do not need write access to repos and usually don't,
which mean cdab485 breaks shallow clone over git://
Fall back to $TMPDIR if $GIT_DIR/shallow_XX cannot be created in
this case. Note that in other cases that write $GIT_DIR/shallow_XX
and eventually rename it to $GIT_DIR/shallow, there is no fallback to
$TMPDIR.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Rebased on top of jk/shallow-update-fix
builtin/receive-pack.c | 2 +-
commit.h | 2 +-
fetch-pack.c | 2 +-
shallow.c| 13 +++--
t/t5537-fetch-shallow.sh | 13 +
upload-pack.c| 2 +-
6 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..d723099 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -932,7 +932,7 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
if (si-nr_ours || si-nr_theirs) {
- alt_shallow_file = setup_temporary_shallow(si-shallow);
+ alt_shallow_file = setup_temporary_shallow(si-shallow, 0);
argv_array_pushl(av, --shallow-file, alt_shallow_file, NULL);
}
diff --git a/commit.h b/commit.h
index 55631f1..d38e996 100644
--- a/commit.h
+++ b/commit.h
@@ -209,7 +209,7 @@ extern int write_shallow_commits(struct strbuf *out, int
use_pack_protocol,
extern void setup_alternate_shallow(struct lock_file *shallow_lock,
const char **alternate_shallow_file,
const struct sha1_array *extra);
-extern const char *setup_temporary_shallow(const struct sha1_array *extra);
+extern const char *setup_temporary_shallow(const struct sha1_array *extra, int
read_only);
extern void advertise_shallow_grafts(int);
struct shallow_info {
diff --git a/fetch-pack.c b/fetch-pack.c
index ae8550e..b71d186 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -853,7 +853,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
setup_alternate_shallow(shallow_lock, alternate_shallow_file,
NULL);
else if (si-nr_ours || si-nr_theirs)
- alternate_shallow_file = setup_temporary_shallow(si-shallow);
+ alternate_shallow_file = setup_temporary_shallow(si-shallow,
0);
else
alternate_shallow_file = NULL;
if (get_pack(args, fd, pack_lockfile))
diff --git a/shallow.c b/shallow.c
index c7602ce..ad28af6 100644
--- a/shallow.c
+++ b/shallow.c
@@ -224,7 +224,8 @@ static void remove_temporary_shallow_on_signal(int signo)
raise(signo);
}
-const char *setup_temporary_shallow(const struct sha1_array *extra)
+const char *setup_temporary_shallow(const struct sha1_array *extra,
+ int read_only)
{
static int installed_handler;
struct strbuf sb = STRBUF_INIT;
@@ -235,7 +236,15 @@ const char *setup_temporary_shallow(const struct
sha1_array *extra)
if (write_shallow_commits(sb, 0, extra)) {
strbuf_addstr(temporary_shallow, git_path(shallow_XX));
- fd = xmkstemp(temporary_shallow.buf);
+ fd = mkstemp(temporary_shallow.buf);
+ if (read_only fd 0) {
+ strbuf_grow(temporary_shallow, PATH_MAX);
+ fd = git_mkstemp(temporary_shallow.buf, PATH_MAX,
+shallow_XX);
+ }
+ if (fd 0)
+ die_errno(Unable to create temporary file '%s',
+ temporary_shallow.buf);
if (!installed_handler) {
atexit(remove_temporary_shallow);
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..171db88 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -173,6 +173,19 @@ EOF
)
'
+test_expect_success POSIXPERM 'shallow fetch from a read-only repo' '
+ cp -R .git read-only.git
+ find read-only.git -print | xargs chmod -w
+ test_when_finished find read-only.git -type d -print | xargs chmod +w
+ git clone --no-local --depth=2 read-only.git from-read-only
+ git --git-dir=from-read-only/.git log --format=%s actual
+ cat expect EOF
+add-1-back
+4
+EOF
+ test_cmp expect actual
+'
+
if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then
say 'skipping remaining tests, git built without http support'
test_done
diff --git a/upload-pack.c b/upload-pack.c
index
Re: [PATCH 3/3] rebase: new convenient option to edit a single commit
On 03/04/2014 11:24 AM, Duy Nguyen wrote: On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty mhag...@alum.mit.edu wrote: git rebase --fixup COMMIT git rebase --squash COMMIT This is not interactive (except when merge conflicts occur), is it? --fixup would not be interactive (is that a problem?), but --squash does open an editor to allow you to merge the commit messages. A bit off topic. I sometimes want to fix up a commit and make it stop there for me to test it again but there is no such command, is there? Maybe we could add support for fixup/edit (or fe for short) and squash/edit (se). Not really familiar with the code base to do that myself quickly though. Maybe we should allow edit to appear on a line by itself, without a SHA-1, in which case it would stop after all previous lines had been processed. Then you could change one line to fixup or squash, and then add a blank edit line after it. Though there is no really obvious way to do this using the hypothetical new command line options that we have been discussing. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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: Branch Name Case Sensitivity
Am 03.03.2014 18:51, schrieb Junio C Hamano:
Lee Hopkins leer...@gmail.com writes:
I went ahead and took a stab at a solution. My solution is more
aggressive than a warning, I actually prevent the creation of
ambiguous refs. My changes are also in refs.c, which may not be
appropriate, but it seemed like the natural place.
I have never contributed to Git (in fact this is my first dive into
the source) and my C is a bit rusty, so bear with me, this is just a
suggestion:
---
refs.c | 31 ---
1 files changed, 24 insertions(+), 7 deletions(-)
Starting something like this from forbidding is likely to turn out
to be a very bad idea that can break existing repositories.
Its sure worth considering what should be done with pre-existing duplicates.
However, repositories with such refs are already broken on case-insensitive
filesystems, and allowing something that's known to be broken is even more
dangerous, IMO.
An alternative approach could be to encode upper-case letters in loose refs if
core.ignorecase == true (e.g. Foo - %46oo). Although this may pose a
problem for commands that bypass the refs API / plumbing for whatever reason.
A new configuration
refs.caseInsensitive = {warn|error|allow}
s/caseInsensitive/caseSensitive/
Its case-sensitive refs that cause trouble, case-insensitive refs would be fine
on all platforms.
I still don't see why we need an extra setting for this. The problems are
inherently caused by case-insensitive filesystems, and we already have
'core.ignorecase' for that (its even automatically configured). Having an extra
setting for refs is somewhat like making 'core.ignorecase' configurable per
sub-directory.
that defaults to warn and the user can choose to set to error to
forbid, would be more palatable, I would say.
If the variable is not in 'core.' namespace, you should implement
this check at the Porcelain level, allowing lower-level tools like
update-ref as an escape hatch that let users bypass the restriction
to be used to correct breakages; it would mean an unconditional if
!stricmp(), it is an error in refs.c will not work well.
I think it might be OK to have
core.allowCaseInsentitiveRefs = {yes|no|warn}
which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
corresponds to 'error', in the previous suggestion), instead. If we
wanted to prevent even lower-level tools like update-ref from
bypassing the check, that is.
Its the plumbing that's broken, so implementing checks at the porcelain level
won't help much. In particular, git-update-ref currently drops branches (or
resets them to an earlier state) and messes up reflogs.
--
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 submodule foreach: Skip eval for more than one argument
Hey folks, On Thu, Sep 26, 2013 at 04:10:15PM -0400, Anders Kaseorg wrote: ‘eval $@’ created an extra layer of shell interpretation, which was probably not expected by a user who passed multiple arguments to git submodule foreach: It seems this patch has broken the use of $name, $path, etc. inside the command ran by foreach (when it contains more than one argument): matthijs@grubby:~/test$ git --version git version 1.9.0 matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name But it works on the single-argument version: matthijs@grubby:~/test$ git submodule foreach 'echo $name' Entering 'test' test And it used to work in older versions: matthijs@login:~/test$ git --version git version 1.7.5.4 matthijs@login:~/test$ git submodule foreach 'echo $name' Entering 'test' test matthijs@login:~/test$ git submodule foreach echo '$name' Entering 'test' test I'm not sure how to fix this exactly. Adding export for the variables in git-submodule.sh seems obvious but doesn't seem to be a complete solution. This makes the variables available in the environment of any commands called (so git submodule sh -c 'echo $name') works, but the git submodule foreach echo '$name' above still doesn't work, since the $@ used does not do any substitution, it just executes $@ as a commandline unmodified. Ideally, you would do variable substitution, but not word splitting, but I'm not sure how to do that. Also, you'd still need one more layer of backslash escapes, which is probably what this commit wanted to prevent... Note that saying you should use the single argument version if you need those variables doesn't seem possible in all cases. In particular, I'm creating an alias that calls git submodule foreach, where the alias contains part of the command and the rest of command comes from arguments to the alias, meaning we always have at least two arguments... Finally, the new behaviour (e.g., eval with one argument, directly execute with multiple) is not documented in the manpage, but it seems relevant enough to need documentation? Gr. Matthijs $ git grep ' [searches for single quotes] $ git submodule foreach git grep ' Entering '[submodule]' /usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string Stopping at '[submodule]'; script returned non-zero status. To fix this, if the user passed more than one argument, just execute $@ directly instead of passing it to eval. Signed-off-by: Anders Kaseorg ande...@mit.edu --- git-submodule.sh | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index c17bef1..3381864 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -545,7 +545,12 @@ cmd_foreach() sm_path=$(relative_path $sm_path) # we make $path available to scripts ... path=$sm_path - eval $@ + if [ $# -eq 1 ] + then + eval $1 + else + $@ + fi if test -n $recursive then cmd_foreach --recursive $@ -- 1.8.4 signature.asc Description: Digital signature
[RFC] make --set-upstream work for local branches not in refs/heads
It might be possible (in Gerrited setups) to have local branches outside refs/heads/, like for example in following fetch config: [remote origin] url = ssh://u...@example.com/my-project fetch = +refs/heads/*:refs/remotes/origin/* fetch = +refs/wip/*:refs/remotes/origin-wip/* Let's say that 'test' branch already exist in origin/refs/wip/. If I call: git checkout test then it will create a new branch and add an entry to .git/config: [branch test] remote = origin merge = refs/wip/test But if I create a branch 'test2' and call: git push --set-upstream origin test2:refs/wip/test2 then branch is pushed, but no entry in .git config is created. I have to do it manually. I attached a hack-patch to have it working just to check with you if anything like that would be accepted. Obviously the get_local_refs() would need to compute the actual list of local hierarchies (if it is possible at all). And it probably should get a better name. And probably something else. What do you think? Krzesimir Nowak (1): RFC: make --set-upstream work for branches not in refs/heads/ transport.c | 41 ++--- 1 file changed, 34 insertions(+), 7 deletions(-) -- 1.8.3.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] RFC: make --set-upstream work for branches not in refs/heads/
---
transport.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/transport.c b/transport.c
index ca7bb44..ac933ee 100644
--- a/transport.c
+++ b/transport.c
@@ -143,6 +143,25 @@ static void insert_packed_refs(const char *packed_refs,
struct ref **list)
}
}
+/* That of course should not be hardcoded. */
+static const char* list_of_local_refs[] = {refs/heads/, refs/wip/, NULL};
+static const char** get_local_refs(void)
+{
+ return list_of_local_refs;
+}
+
+static int is_local_ref(const char *ref)
+{
+ const char **local_refs = get_local_refs();
+ const char **iter;
+
+ for (iter = local_refs; iter != NULL; ++iter)
+ if (starts_with(ref, *iter))
+ return strlen(*iter);
+
+ return 0;
+}
+
static void set_upstreams(struct transport *transport, struct ref *refs,
int pretend)
{
@@ -153,6 +172,8 @@ static void set_upstreams(struct transport *transport,
struct ref *refs,
const char *remotename;
unsigned char sha[20];
int flag = 0;
+ int localadd = 0;
+ int remoteadd = 0;
/*
* Check suitability for tracking. Must be successful /
* already up-to-date ref create/modify (not delete).
@@ -169,23 +190,29 @@ static void set_upstreams(struct transport *transport,
struct ref *refs,
localname = ref-peer_ref-name;
remotename = ref-name;
tmp = resolve_ref_unsafe(localname, sha, 1, flag);
- if (tmp flag REF_ISSYMREF
- starts_with(tmp, refs/heads/))
- localname = tmp;
+
+ if (tmp flag REF_ISSYMREF) {
+ localadd = is_local_ref (tmp);
+ if (localadd 0)
+ localname = tmp;
+ }
+ if (localadd == 0)
+ localadd = is_local_ref(localname);
+ remoteadd = is_local_ref(remotename);
/* Both source and destination must be local branches. */
- if (!localname || !starts_with(localname, refs/heads/))
+ if (!localname || localadd == 0)
continue;
- if (!remotename || !starts_with(remotename, refs/heads/))
+ if (!remotename || remoteadd == 0)
continue;
if (!pretend)
install_branch_config(BRANCH_CONFIG_VERBOSE,
- localname + 11, transport-remote-name,
+ localname + localadd, transport-remote-name,
remotename);
else
printf(Would set upstream of '%s' to '%s' of '%s'\n,
- localname + 11, remotename + 11,
+ localname + localadd, remotename + remoteadd,
transport-remote-name);
}
}
--
1.8.3.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: [PATCH] git submodule foreach: Skip eval for more than one argument
On Tue, Mar 4, 2014 at 2:51 PM, Matthijs Kooijman matth...@stdin.nl wrote: matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name jherland@beta ~/test$ echo '$name' $name What would you expect echo '$name' to do? What happens if you use double instead of single quotes? ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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 submodule foreach: Skip eval for more than one argument
On Tue, Mar 04, 2014 at 03:53:24PM +0100, Johan Herland wrote: On Tue, Mar 4, 2014 at 2:51 PM, Matthijs Kooijman matth...@stdin.nl wrote: matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name jherland@beta ~/test$ echo '$name' $name What would you expect echo '$name' to do? If I run git submodule foreach each '$name', then my shell eats the single quotes (which are only to prevent my shell from interpreting $name). git submodule will see $name, so it will run echo $name, not echo '$name'. What happens if you use double instead of single quotes? Then my shell eats up the double quotes _and_ replaces $name with nothing, so I can't expect git submodule to replace it with the submodule name then :-) Does that help to clarify what I mean? Gr. Matthijs signature.asc Description: Digital signature
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Tue, Mar 4, 2014 at 3:57 PM, Matthijs Kooijman matth...@stdin.nl wrote: On Tue, Mar 04, 2014 at 03:53:24PM +0100, Johan Herland wrote: What would you expect echo '$name' to do? If I run git submodule foreach each '$name', then my shell eats the single quotes (which are only to prevent my shell from interpreting $name). git submodule will see $name, so it will run echo $name, not echo '$name'. What happens if you use double instead of single quotes? Then my shell eats up the double quotes _and_ replaces $name with nothing, so I can't expect git submodule to replace it with the submodule name then :-) Does that help to clarify what I mean? Ok, so IINM, Anders' original commit was about making git submodule foreach command behave more like command (from a naive user's perspective), while you rather expect to insert quotes/escapes to finely control exactly when shell interpretation happens. Aren't these POVs mutually incompatible? Is the only 'real' solution to forbid multitple arguments, and force everybody to quote the entire command? I don't particularly care which way it goes, as long as (a) the common case behaves as most users would expect, (b) the uncommon/complicated case is still _possible_ (though not necessarily simple), and (c) we don't break a sizable number of existing users. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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
Confgure option like tagopt=note to notice changed remote tags
Hello, also I'm using git since a long time, I can't remember that I've noticed that git doesn't make a note or warning if remotes tags have changed. E.g. what's often will be forgotten is to annotate tags before pushing them. The usual resolution is just to annotate them locally and push them again. But such a change never ends up at people which already have fetched the tag (without them using git fetch -t). They even don't receive a notice which could remind them to use git fetch -t. Unfortunately I'm not aware a lot about git internals, but would it be hard to make it configurable (tagopts comes into mind) that git outputs a warning if a remote tag and local tag do disagree about the commit, annotation or sign? I even would prefer such a warning as the default. Regards, Alexander Holler -- 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
git compile with debug symbols
Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. thanks, mpujari -- 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: git compile with debug symbols
Mahesh Pujari pujarimahesh_ku...@yahoo.com writes: Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. git is compiled with debug symbols by default. -- David Kastrup -- 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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
In record_author_date() :
Replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable indent_line.
In parse_signed_commit() :
Replace line + gpg_sig_header_len by skip_prefix(), which
is saved in a new const char variable indent_line.
In parse_gpg_output() :
Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by
skip_prefix(), which is saved in already available
variable found.
Signed-off-by: Karthik Nayak karthik@gmail.com
---
commit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
@@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab
*author_date,
struct ident_split ident;
char *date_end;
unsigned long date;
+ const char *indent_line;
if (!commit-buffer) {
unsigned long size;
@@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab
*author_date,
return;
}
+ indent_line = skip_prefix(buf, author );
+
for (buf = commit-buffer ? commit-buffer : buffer;
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ if (!indent_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
- if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+ if (split_ident_line(ident, indent_line,
+line_end -
indent_line) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1,
char *buffer = read_sha1_file(sha1, type, size);
int in_signature, saw_signature = -1;
char *line, *tail;
+ const char *indent_line;
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
@@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
-line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
+ sig = indent_line + 2;
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
@@ -1193,10 +1196,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check +1);
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
1.9.0.138.g2de3478
Hey Eric,
As per your suggestion in the previous mail :
http://article.gmane.org/gmane.comp.version-control.git/243316
I have made necessary changes:
1. Better Naming of variables as per suggestion
2. Proper replacement of skip_prefix() over starts_with() .
--
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: git compile with debug symbols
A quick look at the Makefile shows that -g is enabled by default. so debugging is enabled by default On Tue, Mar 4, 2014 at 9:16 PM, Mahesh Pujari pujarimahesh_ku...@yahoo.com wrote: Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. thanks, mpujari -- 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: git compile with debug symbols
David Kastrup d...@gnu.org writes: Mahesh Pujari pujarimahesh_ku...@yahoo.com writes: Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. git is compiled with debug symbols by default. ... but: 1) some Git commands are shell-scripts, on which you can't use gdb. 2) some Git commands fork other commands, and then you have to deal with multiple processes (i.e. putting a breakpoint in a piece of code executed by the subprocess won't work if gdb is running on the other one). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 submodule foreach: Skip eval for more than one argument
Hey Johan,
Ok, so IINM, Anders' original commit was about making git submodule
foreach command behave more like command (from a naive user's
perspective),
Ok, that makes sense.
while you rather expect to insert quotes/escapes to finely control
exactly when shell interpretation happens.
Well, I mostly expect that the $name and $path that git submodule makes
available to each command invocation can actually be used by the
command.
Aren't these POVs mutually incompatible? Is the only 'real' solution
to forbid multitple arguments, and force everybody to quote the entire
command?
Yes, I think you're right that they're mutually exclusive. Specifically,
if you expect git submodule foreach command to behave like command,
that means you expect the (interactive) shell to do all the
interpolation, word-splitting, etc. If so, you can't then later still do
interpolation (of course, you could do sed magic to just replace $name
and $path, etc., but that's broken).
I don't particularly care which way it goes, as long as (a) the common
case behaves as most users would expect, (b) the uncommon/complicated
case is still _possible_ (though not necessarily simple), and (c) we
don't break a sizable number of existing users.
Well, if you call submodule directly, you can now just put everything in
a single command and get $name interpolation.
As I mentioned, I couldn't do this because I was using a git alias.
However, a bit of fiddling showed a solution to that using a shell
function:
[alias]
each = !f(){ git submodule foreach --quiet \echo \\$name $*\;}; f
This uses a shell function to collect all alias arguments and then uses
$* to expand them again into the single submodule foreach argument. Note
that $* is expanded when evaluating the alias, while \\$name is expanded
later inside submodule.
This suggests that with the current code, the more complicated cases are
still possible. There is one catch in this approach, in that the
original word splitting is not preserved ($* expands to just the
unquoted arguments as a single word). I'm not sure if this is fixable
($@ expands to multiple quoted words, but then foreach sees multiple
arguments and doesn't do the eval). One would need to escape the output
of $@ somehow (e.g., add \ before , but that would become terribly
complicated I expect...).
Perhaps an explicit --eval switch to git submodule makes sense for
complete control? If it has a correspondning --no-eval, you can even
pass a single-argument command without evalling, while still keeping the
current least surprise approach as the default?
Whatever behaviour is settled for, it should be documented in the
submodule manpage (which I think is not the case now).
Gr.
Matthijs
signature.asc
Description: Digital signature
[PATCH] Setup.c: PATH_MAX is the length including the Nil
Signed-off-by: Sun He sunheeh...@gmail.com
---
Check the limit.h of linux and find out that the MACRO
#define PATH_MAX4096/* # chars in a path name including nul */
So if the magic number 40 is just the size it should be. (e.g. hash code)
It may bring bugs with the length(4056) of long name(gitdirenv).
As gitdirenv could be set by GIT_DIR_ENVIRONMENT.
If it is a bug, it will almost never occur.
But I need your help to know if there is the PATH_MAX of git is the mirror of
the
PATH_MAX of linux and if this fix is right?
If it was, there may be many places like PATH_MAX + 1 could be replaced by
just PATH_MAX. And there may be many places like this.
Cheers,
He Sun
setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index cffb6d6..1511612 100644
--- a/setup.c
+++ b/setup.c
@@ -395,7 +395,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
char *gitfile;
int offset;
- if (PATH_MAX - 40 strlen(gitdirenv))
+ if (PATH_MAX - 41 strlen(gitdirenv))
die('$%s' too big, GIT_DIR_ENVIRONMENT);
gitfile = (char*)read_gitfile(gitdirenv);
--
1.9.0.138.g2de3478.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: git compile with debug symbols
Thanks David for the reply. I think I need to do more ground work of going through how to use gdb. Basically I am java programmer and I was trying out to debug git source using eclipse CDT and as we do in java, I was trying out to set break point but failed with errors as No line 396 in file help.c. And using gdb too I end up with same error. # (gdb) break help.c:396 # No line 396 in file help.c. Am I missing something. thanks, mpujari On Tuesday, March 4, 2014 9:34 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: David Kastrup d...@gnu.org writes: Mahesh Pujari pujarimahesh_ku...@yahoo.com writes: Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. git is compiled with debug symbols by default. ... but: 1) some Git commands are shell-scripts, on which you can't use gdb. 2) some Git commands fork other commands, and then you have to deal with multiple processes (i.e. putting a breakpoint in a piece of code executed by the subprocess won't work if gdb is running on the other one). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: git compile with debug symbols
Mahesh Pujari pujarimahesh_ku...@yahoo.com writes: Thanks David for the reply. I think I need to do more ground work of going through how to use gdb. Basically I am java programmer and I was trying out to debug git source using eclipse CDT and as we do in java, I was trying out to set break point but failed with errors as No line 396 in file help.c. And using gdb too I end up with same error. # (gdb) break help.c:396 # No line 396 in file help.c. Am I missing something. There is just no line 396 known to gdb. It seems like you are indicating a function header. That's not code. Either take the function _name_ rather than a line number (that's usually most reliable) or take the first line of actual code. -- David Kastrup -- 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] Setup.c: PATH_MAX is the length including the Nil
On Tue, Mar 4, 2014 at 9:59 PM, Sun He sunheeh...@gmail.com wrote: Signed-off-by: Sun He sunheeh...@gmail.com --- Check the limit.h of linux and find out that the MACRO #define PATH_MAX4096/* # chars in a path name including nul */ So if the magic number 40 is just the size it should be. (e.g. hash code) It may bring bugs with the length(4056) of long name(gitdirenv). As gitdirenv could be set by GIT_DIR_ENVIRONMENT. If it is a bug, it will almost never occur. But I need your help to know if there is the PATH_MAX of git is the mirror of the PATH_MAX of linux and if this fix is right? If it was, there may be many places like PATH_MAX + 1 could be replaced by just PATH_MAX. And there may be many places like this. Cheers, He Sun Hi, I am not getting what exactly you are trying to tell, but git defines its own PATH_MAX. Its defined in git-compat-util.h: #define PATH_MAX 4096 That is why instead of making buffers using PATH_MAX, use strbuf. All these problems of buffer overflow will be gone. I hope you are concerned about buffer overflow. Cheers -Faiz -- 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] implemented strbuf_write_or_die()
I'm the guilty one. I like the change (obviously, since I suggested it). Writing strbufs comes up frequently and will hopefully increase in usage and I think it is a positive thing to encourage the use of strbufs by making them increasingly first-class citizens. But I can see your points too, and I humbly defer to the wisdom of the list. I will remove this suggestion from the list of microprojects. Faiz, this is the way things go on the Git mailing list. It would be boring if everybody agreed all the time :-) Michael Hi, Thank you all. Even I like the strbuf_write_or_die() but again its a code churn as pointed out. But if we want to use strbuf instead of static buffers we might need this function very often (Its just my opinion). Anyways, implementing it was an exercise and I enjoyed it. I agree with Michael Haggerty that it would be boring if everybody agreed all the time :D I enjoyed it and learnt from the exercise, so I don't think it was a waste or a bad exercise. At least it exposed me to practices of good software design and importance of layers in software. Thanks a lot. -Faiz -- 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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Karthik Nayak karthik.188 at gmail.com writes:
In record_author_date() :
Replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable indent_line.
In parse_signed_commit() :
Replace line + gpg_sig_header_len by skip_prefix(), which
is saved in a new const char variable indent_line.
In parse_gpg_output() :
Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by
skip_prefix(), which is saved in already available
variable found.
Signed-off-by: Karthik Nayak karthik.188 at gmail.com
---
commit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
at at -1098,6 +1100,7 at at int parse_signed_commit(const
unsigned char *sha1,
char *buffer = read_sha1_file(sha1, type, size);
int in_signature, saw_signature = -1;
char *line, *tail;
+ const char *indent_line;
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
at at -,11 +1114,11 at at int parse_signed_commit(const
unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
- line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
+ sig = indent_line + 2;
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
Hi,
I was attempting the same microproject so I thought I would share some
points that were told to me earlier .The mail subject should have a
increasing order of submission numbers for each revision(for example here it
should be [PATCH v2]).
Also write the superfluous stuff which you have written in the bottom
after ---(the three dashes after the signed off statement) .
In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header
variables are precomputed outside of the function, and also Junio said to
prefer starts_with(), whenever skip_prefix() advantages are not so important
in the context.So the replace may not be so advantageous ,I may be wrong in
this case.
Cheers,
Tanay Abhra.
--
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] disable grafts during fetch/push/bundle
When we are creating a pack to send to a remote, we should
make sure that we are not respecting grafts or replace refs.
Otherwise, we may end up sending a broken pack to the other
side that does not contain all objects (either omitting them
entirely, or using objects that the other side does not have
as delta bases).
We already make an attempt to do the right thing in several
places by turning off read_replace_refs. However, we missed
at least one case (during bundle creation), and we do
nothing anywhere to handle grafts.
This patch introduces a new function to disable both grafts
and replace refs both for the current process and for its
children. We add a call anywhere that packs objects for
sending: bundle create, upload-pack, and push.
This may seem like a rather heavy hammer, as we could just
teach pack-objects not to respect grafts. But this catches
more possible failures:
1. We may actually feed pack-objects with the results of
rev-list (e.g., bundle creation does this).
2. We may be negotiating the HAVEs and WANTs with the
other side, and our view should be consistent with what
we feed pack-objects.
3. It may be useful to let pack-objects use grafts in some
instances, as evidenced by --keep-true-parents.
Signed-off-by: Jeff King p...@peff.net
---
This is motivated by a real-world case of somebody trying to push to
GitHub with a graft on their local end.
I suspect many other spots that use read_replace_refs = 0 probably
want to disable grafts, too (e.g., fsck and index-pack) but I do not
know of any particular breakage offhand.
I am tempted to say that not using --keep-true-parents is insane, and it
should be the default, but perhaps there is some case I'm missing.
Note that disabling grafts here just turns off .git/info/grafts, which
explicitly leaves the shallow file enabled (even though it ends up in
the same graft list). I'm assuming that the shallow file is handled
properly where it's appropriate, and it would not want to be included in
any of this disabling (certainly fetch/push should be handling it
explicitly already).
builtin/push.c | 1 +
bundle.c | 2 +
cache.h | 1 +
commit.c | 5 +++
commit.h | 1 +
environment.c| 22 ++
t/t6051-replace-grafts-remote.sh | 94
upload-pack.c| 2 +-
8 files changed, 127 insertions(+), 1 deletion(-)
create mode 100755 t/t6051-replace-grafts-remote.sh
diff --git a/builtin/push.c b/builtin/push.c
index 0e50ddb..448dcb5 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -527,6 +527,7 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
OPT_END()
};
+ disable_grafts_and_replace_refs();
packet_trace_identity(push);
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
diff --git a/bundle.c b/bundle.c
index e99065c..37b00a6 100644
--- a/bundle.c
+++ b/bundle.c
@@ -248,6 +248,8 @@ int create_bundle(struct bundle_header *header, const char
*path,
struct child_process rls;
FILE *rls_fout;
+ disable_grafts_and_replace_refs();
+
bundle_to_stdout = !strcmp(path, -);
if (bundle_to_stdout)
bundle_fd = 1;
diff --git a/cache.h b/cache.h
index db223e8..f538cc2 100644
--- a/cache.h
+++ b/cache.h
@@ -410,6 +410,7 @@ extern const char *get_git_work_tree(void);
extern const char *read_gitfile(const char *path);
extern const char *resolve_gitdir(const char *suspect);
extern void set_git_work_tree(const char *tree);
+extern void disable_grafts_and_replace_refs(void);
#define ALTERNATE_DB_ENVIRONMENT GIT_ALTERNATE_OBJECT_DIRECTORIES
diff --git a/commit.c b/commit.c
index 6bf4fe0..886dbfe 100644
--- a/commit.c
+++ b/commit.c
@@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const char *buf,
const char *tail)
static struct commit_graft **commit_graft;
static int commit_graft_alloc, commit_graft_nr;
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
+
static int commit_graft_pos(const unsigned char *sha1)
{
int lo, hi;
diff --git a/commit.h b/commit.h
index 16d9c43..dde0618 100644
--- a/commit.h
+++ b/commit.h
@@ -186,6 +186,7 @@ typedef int (*each_commit_graft_fn)(const struct
commit_graft *, void *);
struct commit_graft *read_graft_line(char *buf, int len);
int register_commit_graft(struct commit_graft *, int);
struct commit_graft *lookup_commit_graft(const unsigned char *sha1);
+int commit_grafts_loaded(void);
extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit
*rev2, int cleanup);
extern struct commit_list *get_merge_bases_many(struct commit *one, int n,
struct commit **twos, int cleanup);
diff --git a/environment.c b/environment.c
index 4a3437d..b960293 100644
---
Re: git compile with debug symbols
Mahesh Pujari pujarimahesh_kumar at yahoo.com writes: Hello, I am trying to compile git with debug symbols and failed to do so (basically I am a noob), can some one direct me to links or mailing list (have searched but couldn't find) or doc's so that I can debug git using gdb. thanks, mpujari Hi, I tried to put a break point at help.c:396 and it was successful . I think that the problem is either your symbols are not loaded. Nevertheless I will walk you through the steps. $ git clone https://github.com/git/git $ make $ gdb ./git $(gdb) gdb break help.c:396 Breakpoint 1 at 0x80f8b40: file help.c, line 396. Cheers, Tanay Abhra. -- 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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Hey Tanay,
1. Yes just getting used to git send-email now, should follow that from now
2. I thought it shouldn't be a part of the commit, so i put it after
the last ---
3. I did have a thought on your lines also , but concluded to it being
more advantageous, you might be right though
Nice to hear from you.
Thanks
-Karthik Nayak
On Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra tanay...@gmail.com wrote:
Karthik Nayak karthik.188 at gmail.com writes:
In record_author_date() :
Replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable indent_line.
In parse_signed_commit() :
Replace line + gpg_sig_header_len by skip_prefix(), which
is saved in a new const char variable indent_line.
In parse_gpg_output() :
Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by
skip_prefix(), which is saved in already available
variable found.
Signed-off-by: Karthik Nayak karthik.188 at gmail.com
---
commit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
at at -1098,6 +1100,7 at at int parse_signed_commit(const
unsigned char *sha1,
char *buffer = read_sha1_file(sha1, type, size);
int in_signature, saw_signature = -1;
char *line, *tail;
+ const char *indent_line;
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
at at -,11 +1114,11 at at int parse_signed_commit(const
unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
- line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
+ sig = indent_line + 2;
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
Hi,
I was attempting the same microproject so I thought I would share some
points that were told to me earlier .The mail subject should have a
increasing order of submission numbers for each revision(for example here it
should be [PATCH v2]).
Also write the superfluous stuff which you have written in the bottom
after ---(the three dashes after the signed off statement) .
In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header
variables are precomputed outside of the function, and also Junio said to
prefer starts_with(), whenever skip_prefix() advantages are not so important
in the context.So the replace may not be so advantageous ,I may be wrong in
this case.
Cheers,
Tanay Abhra.
--
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 v2] upload-pack: allow shallow fetching from a read-only repository
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Before cdab485 (upload-pack: delegate rev walking in shallow fetch to
pack-objects - 2013-08-16) upload-pack does not write to the source
repository. cdab485 starts to write $GIT_DIR/shallow_XX if it's a
shallow fetch, so the source repo must be writable.
git:// servers do not need write access to repos and usually don't,
which mean cdab485 breaks shallow clone over git://
Fall back to $TMPDIR if $GIT_DIR/shallow_XX cannot be created in
this case. Note that in other cases that write $GIT_DIR/shallow_XX
and eventually rename it to $GIT_DIR/shallow, there is no fallback to
$TMPDIR.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Rebased on top of jk/shallow-update-fix
Hmph.
I notice that the original code, with or without this change, allows
upload-pack spawned by daemon to attempt to write into GIT_DIR.
As upload-pack is supposed to be a read-only operation, this is
quite bad.
Perhaps we should give server operators an option to run their
daemon - upload-pack chain to always write to a throw-away
directory of their choice, without ever attempting to write to
GIT_DIR it serves?
How well is the access to the temporary shallow file controlled in
your code (sorry, but I do not recall carefully reading your patch
that added the mechanism with security issues in mind, so now I am
asking)? When it is redirected to TMPDIR (let's forget GIT_DIR for
now---if an attacker can write into there, the repository is already
lost), can an attacker race with us to cause us to overwrite we do
not expect to?
Even if it turns out that this patch is secure enough as-is, we
definitely need to make sure that server operators, who want to keep
their upload-pack truly a read-only operation, know that it is
necessary to (1) keep the system user they run git-daemon under
incapable of writing into GIT_DIR, and (2) make sure TMPDIR points
at somewhere only git-daemon user and nobody else can write into,
somewhere in the documentation.
diff --git a/fetch-pack.c b/fetch-pack.c
index ae8550e..b71d186 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -853,7 +853,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
setup_alternate_shallow(shallow_lock, alternate_shallow_file,
NULL);
else if (si-nr_ours || si-nr_theirs)
- alternate_shallow_file = setup_temporary_shallow(si-shallow);
+ alternate_shallow_file = setup_temporary_shallow(si-shallow,
0);
else
alternate_shallow_file = NULL;
if (get_pack(args, fd, pack_lockfile))
diff --git a/shallow.c b/shallow.c
index c7602ce..ad28af6 100644
--- a/shallow.c
+++ b/shallow.c
@@ -224,7 +224,8 @@ static void remove_temporary_shallow_on_signal(int signo)
raise(signo);
}
-const char *setup_temporary_shallow(const struct sha1_array *extra)
+const char *setup_temporary_shallow(const struct sha1_array *extra,
+ int read_only)
{
static int installed_handler;
struct strbuf sb = STRBUF_INIT;
@@ -235,7 +236,15 @@ const char *setup_temporary_shallow(const struct
sha1_array *extra)
if (write_shallow_commits(sb, 0, extra)) {
strbuf_addstr(temporary_shallow, git_path(shallow_XX));
- fd = xmkstemp(temporary_shallow.buf);
+ fd = mkstemp(temporary_shallow.buf);
+ if (read_only fd 0) {
+ strbuf_grow(temporary_shallow, PATH_MAX);
+ fd = git_mkstemp(temporary_shallow.buf, PATH_MAX,
+ shallow_XX);
+ }
+ if (fd 0)
+ die_errno(Unable to create temporary file '%s',
+ temporary_shallow.buf);
if (!installed_handler) {
atexit(remove_temporary_shallow);
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..171db88 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -173,6 +173,19 @@ EOF
)
'
+test_expect_success POSIXPERM 'shallow fetch from a read-only repo' '
s/POSIXPERM/,SANITY/, perhaps?
Thinking of it again, perhaps POSIXPERM should imply SANITY is required?
+ cp -R .git read-only.git
+ find read-only.git -print | xargs chmod -w
+ test_when_finished find read-only.git -type d -print | xargs chmod +w
+ git clone --no-local --depth=2 read-only.git from-read-only
+ git --git-dir=from-read-only/.git log --format=%s actual
+ cat expect EOF
+add-1-back
+4
+EOF
+ test_cmp expect actual
+'
+
if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then
say 'skipping remaining tests, git built without http support'
test_done
diff --git a/upload-pack.c b/upload-pack.c
index a3c52f6..b538f32 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -84,7
[PATCH] gitk: replace SHA1 entry field on keyboard paste
From: Ilya Bobyr ilya.bo...@gmail.com
Date: Thu, 27 Feb 2014 22:51:37 -0800
We already replace old SHA with the clipboard content for the mouse
paste event. It seems reasonable to do the same when pasting from
keyboard.
Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
* Paul? I do not use Paste on my keyboard, so I am not in the
position to say that this patch is correct (or not). I am just
forwarding it in case you think gitk users will find it useful.
The original patch was done against my tree, so I hand tweaked it
to apply to your tree.
Thanks.
gitk |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gitk b/gitk
index 90764e8..2f58bcf 100755
--- a/gitk
+++ b/gitk
@@ -2585,6 +2585,7 @@ proc makewindow {} {
bind $fstring Key-Return {dofind 1 1}
bind $sha1entry Key-Return {gotocommit; break}
bind $sha1entry PasteSelection clearsha1
+bind $sha1entry Paste clearsha1
bind $cflist 1 {sel_flist %W %x %y; break}
bind $cflist B1-Motion {sel_flist %W %x %y; break}
bind $cflist ButtonRelease-1 {treeclick %W %x %y}
--
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 1/3] rev-parse: support OPT_NUMBER_CALLBACK in --parseopt
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
If the option spec is
-NUM Help string
then rev-parse will accept and parse -([0-9]+) and return -NUM $1
Even though the hardcoded NUM token initially gave me a knee-jerk
Yuck reaction, that literal option name is very unlikely to be
desired by scripts/commands for their real option names, and being
in all uppercase it is very clear that it is magic convention
between the parsing mechanism and the script it uses.
It however felt funny to me without a matching (possibly hidden)
mechanism to allow parse-options machinery to consume such an output
as its input. In a script that uses this mechanism to parse out the
numeric option -NUM 3 out of git script -3 and uses that three
to drive an underlying command (e.g. git grep -3), wouldn't it be
more natural if that underlying command can be told to accept the
same notation (i.e. git grep -NUM 3)? For that to be consistent
with the rest of the system, -NUM would not be a good token; being
it multi-character, it must be --NUM or something with double-dash
prefix.
I kind of like the basic idea, the capability it tries to give
scripted Porcelain implementations. But my impression is that
rebase -i -4, which this mechanism was invented for, is not
progressing, so perhaps we should wait until the real user of this
feature appears.
Thanks.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/rev-parse.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..b37676f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -331,6 +331,8 @@ static int parseopt_dump(const struct option *o, const
char *arg, int unset)
struct strbuf *parsed = o-value;
if (unset)
strbuf_addf(parsed, --no-%s, o-long_name);
+ else if (o-type == OPTION_NUMBER)
+ strbuf_addf(parsed, -NUM);
else if (o-short_name (o-long_name == NULL || !stuck_long))
strbuf_addf(parsed, -%c, o-short_name);
else
@@ -338,7 +340,7 @@ static int parseopt_dump(const struct option *o, const
char *arg, int unset)
if (arg) {
if (!stuck_long)
strbuf_addch(parsed, ' ');
- else if (o-long_name)
+ else if (o-long_name || o-type == OPTION_NUMBER)
strbuf_addch(parsed, '=');
sq_quote_buf(parsed, arg);
}
@@ -439,7 +441,10 @@ static int cmd_parseopt(int argc, const char **argv,
const char *prefix)
if (s - sb.buf == 1) /* short option only */
o-short_name = *sb.buf;
- else if (sb.buf[1] != ',') /* long option only */
+ else if (s - sb.buf == 4 !strncmp(sb.buf, -NUM, 4)) {
+ o-type = OPTION_NUMBER;
+ o-flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG;
+ } else if (sb.buf[1] != ',') /* long option only */
o-long_name = xmemdupz(sb.buf, s - sb.buf);
else {
o-short_name = *sb.buf;
--
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 3/3] rebase: new convenient option to edit a single commit
Michael Haggerty mhag...@alum.mit.edu writes: ... All of the following seem to make sense: git rebase --edit COMMIT A long-form for the -e option we have been talking about. It is unfortunately that this spelling sounds like the --edit option on git commit --edit and git merge --edit, so people might use it when they really mean git rebase --reword COMMIT. I agree, so the --edit does *not* make sense as it invites confusion. git rebase --reword COMMIT Yes, that would make sense. git rebase --fixup COMMIT git rebase --squash COMMIT I am not sure about these. What does it even mean to --fixup (or --squash for that matter) a single commit without specifying what it is squashed into? Or are you assuming that all of these is only to affect pre-populated rebase-i insn sheet that is to be further edited before the actual rebasing starts? I somehow had an impression that the reason to have these new options is to skip the editing of the insn sheet in the editor altogether. git rebase --kill COMMIT This _does_ make sense under my assumption: remove this commit from the insn-sheet and go ahead with it, without bothering me to edit the insn-sheet further. I'm quite confident that I would use all of these commands. If --kill takes only one, I would probably do rebase --onto without bothering with -i at all, but if it lets me drop multiple non-consecutive commits, by accepting more than one --kill, I see how I would be using it myself. I can see how --reword/--amend would be useful even when dealing with only a single commit. I do not know about --fixup/--squash 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: [PATCH v2 1/2] i18n: proposed command missing leading dash
From: Sandy Carter sandy.car...@savoirfairelinux.com Date: Mon, 3 Mar 2014 09:55:53 -0500 Add missing leading dash to proposed commands in french output when using the command: git branch --set-upstream remotename/branchname and when upstream is gone Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * Forwarding to the i18n coordinator. I don't do French, but this seems trivially correct. po/fr.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/po/fr.po b/po/fr.po index e10263f..0b9d59e 100644 --- a/po/fr.po +++ b/po/fr.po @@ -1075,7 +1075,7 @@ msgstr Votre branche est basée sur '%s', mais la branche amont a disparu.\n #: remote.c:1875 msgid (use \git branch --unset-upstream\ to fixup)\n -msgstr (utilisez \git branch -unset-upstream\ pour corriger)\n +msgstr (utilisez \git branch --unset-upstream\ pour corriger)\n #: remote.c:1878 #, c-format @@ -3266,7 +3266,7 @@ msgstr git branch -d %s\n #: builtin/branch.c:1027 #, c-format msgid git branch --set-upstream-to %s\n -msgstr git branch -set-upstream-to %s\n +msgstr git branch --set-upstream-to %s\n #: builtin/bundle.c:47 #, c-format -- 1.9.0 -- 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] implemented strbuf_write_or_die()
Michael Haggerty mhag...@alum.mit.edu writes: On 03/03/2014 07:31 PM, Junio C Hamano wrote: That is a very good sign why this change is merely a code-churn and not an improvement, isn't it? We know (and any strbuf user should know) that -buf and -len are the ways to learn the pointer and the length the strbuf holds. ... ... Writing strbufs comes up frequently and will hopefully increase in usage and I think it is a positive thing to encourage the use of strbufs by making them increasingly first-class citizens. Yeah, I understand that. I suspect that the conclusion would have been very different if we were a C++ project; most likely it would be an excellent idea to add an often-used write_or_die() method to the strbuf class. But we are writing C. Faiz, this is the way things go on the Git mailing list. It would be boring if everybody agreed all the time :-) Surely, and 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: [PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
Ilya Bobyr ilya.bo...@gmail.com writes: On 3/4/2014 12:29 AM, Junio C Hamano wrote: ... then you shouldn't be mucking with environment variables in the first place, primarily because running: $ GIT_TEST_ONLY='1 4' make test to run test .1 and .4 of all the test scripts would not make any sense. No it does not. It only makes sense for one test suite. I think your simplicity argument is a total red-herring. Of course if you do not have to say the test script name, your specification would be shorter, but that is only because your specification is not specific enough to be useful. In my case it is very useful :) It invites a nonsense usage (i.e. running make test under that environment variable setting); that is not a good trade-off. * Even though GIT_SKIP_TESTS mechanism still allows you to skip individual test pieces, it has never been a serious feature in the first place. Many of the tests unfortunately do rely on state previous sequences of tests left behind, so it is not realistic to expect that you can skip test pieces randomly and exercise later test pieces reliably. * The numbering of individual test pieces can easily change by new tests inserted in the middle; again, many tests do take advantge of the states earlier tests leave behind, so do not add new tests in the middle is not a realistic rule to enforce, unless you are willing to clean up existing test scripts so that each test piece is independent from all the previous ones. Both are true, but do not apply to the TDD case. The existing tests are designed to be black-box tests, not function level unit tests, and touching lower level code carelessly affects other parts of the system you did not know the interactions about. What does TDD case change anything in that equation? -- 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] commit.c: use skip_prefix() instead of starts_with()
On 04.03.2014, at 09:42, Tanay Abhra tanay...@gmail.com wrote:
[...]
commit.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..6c92acb 100644
--- a/commit.c
+++ b/commit.c
[...]
@@ -566,14 +566,16 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
+ buf = ident_line;
if (split_ident_line(ident,
- buf + strlen(author ),
- line_end - (buf + strlen(author ))) ||
+ buf,
+ line_end - buf) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
Why not get rid of that assignment to buf, and use ident_line instead of buf
below? That seems like it would be more readable, wouldn't it?
@@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ /* At the very beginning of the buffer */
Do we really need that comment, and in that spot? The code seemed clear enough
to me without it. But if you think keeping is better, perhaps move it to
*before* the skip_prefix, and add a trailing ?
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
signature.asc
Description: Message signed with OpenPGP using GPGMail
Re: [PATCH] rev-parse --parseopt: option argument name hints
Ilya Bobyr ilya.bo...@gmail.com writes:
Built-in commands can specify names for option arguments, that are shown
when usage text is generated for the command. sh based commands should
be able to do the same.
Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace. Underscores are replaced with
whitespace. It is unlikely that an underscore would be useful in the
hint text.
Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
Documentation/git-rev-parse.txt | 11 +--
builtin/rev-parse.c | 17 -
t/t1502-rev-parse-parseopt.sh | 20
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
'git rev-parse --parseopt' input format is fully text based. It has two
parts,
separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.
Good spotting. I think the original author meant to say there
should be at least one line to serve as the usage string, so
updating it to should be one or more may be more accurate, but
could be more than one would also work.
The lines after the separator describe the options.
Each line of options has this format:
-opt_specflags* SP+ help LF
+opt_specflags*argh? SP+ help LF
`opt_spec`::
@@ -313,6 +313,12 @@ Each line of options has this format:
* Use `!` to not make the corresponding negated long option available.
+`argh`::
+ `argh`, if specified, is used as a name of the argument, if the
+ option takes an argument. `argh` is terminated by the first
+ whitespace. Angle braces are added automatically. Underscore symbols
+ are replaced with spaces.
I had a hard time understanding this Angle brackets are added
automatically one (obviously nobody wants extra angle brackets
added around option arguments given by the user), until I looked at
the addition of the test to realize that this description is only
about how it appears in the help output. The description needs to
be clarified to avoid confusion.
@@ -333,6 +339,7 @@ h,helpshow the help
foo some nifty option --foo
bar= some cool option --bar with an argument
+baz=arg another cool option --baz with an argument named arg
It probably is better not to have named arg at the end here, as
that gives an apparent-but-false contradiction with the Angle
brackets are added *automatically* and confuse readers. At least,
it confused _this_ reader.
After the eval in the existing example to parse the $@ argument
list in this part of the documentation, it may be a good idea to say
something like:
The above command, when $@ is --help, produces the
following help output:
... sample output here ...
to show the actual output. That way, we can illustrate how input
baz?arg description of baz is turned into --baz[=arg] output
clearly (yes, I am suggesting to use '?' in the new example, not '='
whose usage is already shown in the existing example).
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index aaeb611..83a769e 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv,
const char *prefix)
usage[unb++] = strbuf_detach(sb, NULL);
}
- /* parse: (short|short,long|long)[=?]? SP+ help */
+ /* parse: (short|short,long|long)[*=?!]*arghint? SP+ help */
while (strbuf_getline(sb, stdin, '\n') != EOF) {
const char *s;
+ const char *argh;
Let's spell that variable name out, e.g. arg_hint or something.
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
-C[...] option C with an optional argument
-d, --data[=...] short and long option with an optional argument
+Argument hints
+-b arg short option required argument
+--bar2 arg long option required argument
+-e, --fuz with spaces
+ short and long option required argument
+-s[some]short option optional argument
+--long[=data] long option optional argument
+-g, --fluf[=path] short and long option optional argument
+--longest a very long argument hint
+ a very long argument hint
+
Extras
--extra1 line above used to cause a segfault but no longer
does
@@ -39,6 +50,15 @@ b,baz a short and
Re: [PATCH v3] commit.c: use skip_prefix() instead of starts_with()
Tanay Abhra tanay...@gmail.com writes:
In record_author_date() parse_gpg_output() ,using skip_prefix() instead of
starts_with() is a more suitable abstraction.
Thanks. Will queue with a reworded message to clarify what exactly
A more suitable means.
Here is what I tentatively came up with.
-- 8 --
From: Tanay Abhra tanay...@gmail.com
Date: Tue, 4 Mar 2014 00:42:20 -0800
Subject: [PATCH] commit.c: use skip_prefix() instead of starts_with()
In record_author_date() parse_gpg_output(), the callers of
starts_with() not just want to know if the string starts with the
prefix, but also can benefit from knowing the string that follows
the prefix.
By using skip_prefix(), we can do both at the same time.
Helped-by: Max Horn m...@quendi.de
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
commit.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..6c92acb 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
static void record_author_date(struct author_date_slab *author_date,
struct commit *commit)
{
- const char *buf, *line_end;
+ const char *buf, *line_end, *ident_line;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,16 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
+ buf = ident_line;
if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+buf,
+line_end - buf) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ /* At the very beginning of the buffer */
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
1.9.0-186-gd464cb7
--
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] make --set-upstream work for local branches not in refs/heads
Krzesimir Nowak krzesi...@endocode.com writes:
It might be possible (in Gerrited setups) to have local branches
outside refs/heads/, like for example in following fetch config:
[remote origin]
url = ssh://u...@example.com/my-project
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/wip/*:refs/remotes/origin-wip/*
Let's say that 'test' branch already exist in origin/refs/wip/. If I
call:
git checkout test
then it will create a new branch and add an entry to .git/config:
[branch test]
remote = origin
merge = refs/wip/test
But if I create a branch 'test2' and call:
git push --set-upstream origin test2:refs/wip/test2
then branch is pushed, but no entry in .git config is created.
By definition anything otuside refs/heads/ is not a branch, so do
not call things in refs/wip branches. Retitle it to work for
local refs outside refs/heads or something.
Having said that, I do not see a major problem if we allowed
[branch test2]
remote = origin
merge = refs/wip/test2
to be created when push --setupstream is requested, making
test2@{upstream} to point at refs/remotes/origin-wip/test2.
I do not know what the correct implementation of such a feature
should be, 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
[PATCH v6 00/11] Add interpret-trailers builtin
This patch series implements a new command: git interpret-trailers and an infrastructure to process trailers that can be reused, for example in commit.c. 1) Rationale: This command should help with RFC 822 style headers, called trailers, that are found at the end of commit messages. (Note that these headers do not follow and are not intended to follow many rules that are in RFC 822. For example they do not follow the line breaking rules, the encoding rules and probably many other rules.) For a long time, these trailers have become a de facto standard way to add helpful information into commit messages. Until now git commit has only supported the well known Signed-off-by: trailer, that is used by many projects like the Linux kernel and Git. It is better to implement features for these trailers first in a new command rather than in builtin/commit.c, because this way the prepare-commit-msg and commit-msg hooks can reuse this command. 2) Current state: Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(token[(=|:)value])...] The following features are implemented: - the result is printed on stdout - the [token[=value]] arguments are interpreted - a commit message read from stdin is interpreted - the trailer.token.key options in the config are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifExist options are interpreted - the trailer.token.ifMissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - there are some tests - there is some documentation The following features are planned but not yet implemented: - add more tests related to commands - add examples in documentation - integration with git commit Possible improvements: - support GIT_COMMIT_PROTO env variable in commands 3) Changes since version 5, thanks to Junio and Eric: * the --infile file option has been removed * many small functions are back to just 'static' instead of 'static inline' * alnum_len() has been adjust to have a size_t len parameter and a size_t return value again * strcspn() is used in void parse_trailer() * some test setup commands have been moved in some proper tests * some commit messages have been improved * a patch to setup env variables for commands has been removed * all the memory leaks should have been fixed Christian Couder (11): Add data structures and basic functions for commit trailers trailer: process trailers from stdin and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 123 ++ Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 33 ++ git.c| 1 + t/t7513-interpret-trailers.sh| 261 trailer.c| 661 +++ trailer.h| 6 + 9 files changed, 1089 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt create mode 100644 builtin/interpret-trailers.c create mode 100755 t/t7513-interpret-trailers.sh create mode 100644 trailer.c create mode 100644 trailer.h -- 1.8.5.2.204.gcfe299d.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 v6 07/11] trailer: add interpret-trailers command
This patch adds the git interpret-trailers command.
This command uses the previously added process_trailers()
function in trailer.c.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
.gitignore | 1 +
Makefile | 1 +
builtin.h| 1 +
builtin/interpret-trailers.c | 33 +
git.c| 1 +
trailer.h| 6 ++
6 files changed, 43 insertions(+)
create mode 100644 builtin/interpret-trailers.c
create mode 100644 trailer.h
diff --git a/.gitignore b/.gitignore
index b5f9def..c870ada 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/git-index-pack
/git-init
/git-init-db
+/git-interpret-trailers
/git-instaweb
/git-log
/git-ls-files
diff --git a/Makefile b/Makefile
index ec90feb..a91465e 100644
--- a/Makefile
+++ b/Makefile
@@ -935,6 +935,7 @@ 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
diff --git a/builtin.h b/builtin.h
index d4afbfe..30f4c30 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,7 @@ extern int cmd_hash_object(int argc, const char **argv, const
char *prefix);
extern int cmd_help(int argc, const char **argv, const char *prefix);
extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
extern int cmd_init_db(int argc, const char **argv, const char *prefix);
+extern int cmd_interpret_trailers(int argc, const char **argv, const char
*prefix);
extern int cmd_log(int argc, const char **argv, const char *prefix);
extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
new file mode 100644
index 000..0c8ca72
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,33 @@
+/*
+ * Builtin git interpret-trailers
+ *
+ * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
+ *
+ */
+
+#include cache.h
+#include builtin.h
+#include parse-options.h
+#include trailer.h
+
+static const char * const git_interpret_trailers_usage[] = {
+ N_(git interpret-trailers [--trim-empty]
[(token[(=|:)value])...]),
+ NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+ int trim_empty = 0;
+
+ struct option options[] = {
+ OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_interpret_trailers_usage, 0);
+
+ process_trailers(trim_empty, argc, argv);
+
+ return 0;
+}
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
diff --git a/trailer.h b/trailer.h
new file mode 100644
index 000..9323b1e
--- /dev/null
+++ b/trailer.h
@@ -0,0 +1,6 @@
+#ifndef TRAILER_H
+#define TRAILER_H
+
+void process_trailers(int trim_empty, int argc, const char **argv);
+
+#endif /* TRAILER_H */
--
1.8.5.2.204.gcfe299d.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 v6 03/11] trailer: read and process config information
This patch implements reading the configuration
to get trailer information, and then processing
it and storing it in a doubly linked list.
The config information is stored in the list
whose first item is pointed to by:
static struct trailer_item *first_conf_item;
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 134 ++
1 file changed, 134 insertions(+)
diff --git a/trailer.c b/trailer.c
index a0124f2..5b8e28b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -25,6 +25,8 @@ struct trailer_item {
struct conf_info conf;
};
+static struct trailer_item *first_conf_item;
+
static int same_token(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
{
return !strncasecmp(a-token, b-token, alnum_len);
@@ -244,3 +246,135 @@ static void process_trailers_lists(struct trailer_item
**in_tok_first,
apply_arg_if_missing(in_tok_first, in_tok_last, arg_tok);
}
}
+
+static int set_where(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(after, value))
+ item-where = WHERE_AFTER;
+ else if (!strcasecmp(before, value))
+ item-where = WHERE_BEFORE;
+ else
+ return 1;
+ return 0;
+}
+
+static int set_if_exists(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(addIfDifferent, value))
+ item-if_exists = EXISTS_ADD_IF_DIFFERENT;
+ else if (!strcasecmp(addIfDifferentNeighbor, value))
+ item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
+ else if (!strcasecmp(add, value))
+ item-if_exists = EXISTS_ADD;
+ else if (!strcasecmp(overwrite, value))
+ item-if_exists = EXISTS_OVERWRITE;
+ else if (!strcasecmp(doNothing, value))
+ item-if_exists = EXISTS_DO_NOTHING;
+ else
+ return 1;
+ return 0;
+}
+
+static int set_if_missing(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(doNothing, value))
+ item-if_missing = MISSING_DO_NOTHING;
+ else if (!strcasecmp(add, value))
+ item-if_missing = MISSING_ADD;
+ else
+ return 1;
+ return 0;
+}
+
+enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
+TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
+
+static int set_name_and_type(const char *conf_key, const char *suffix,
+enum trailer_info_type type,
+char **pname, enum trailer_info_type *ptype)
+{
+ int ret = ends_with(conf_key, suffix);
+ if (ret) {
+ *pname = xstrndup(conf_key, strlen(conf_key) - strlen(suffix));
+ *ptype = type;
+ }
+ return ret;
+}
+
+static struct trailer_item *get_conf_item(const char *name)
+{
+ struct trailer_item *item;
+ struct trailer_item *previous;
+
+ /* Look up item with same name */
+ for (previous = NULL, item = first_conf_item;
+item;
+previous = item, item = item-next) {
+ if (!strcasecmp(item-conf.name, name))
+ return item;
+ }
+
+ /* Item does not already exists, create it */
+ item = xcalloc(sizeof(struct trailer_item), 1);
+ item-conf.name = xstrdup(name);
+
+ if (!previous)
+ first_conf_item = item;
+ else {
+ previous-next = item;
+ item-previous = previous;
+ }
+
+ return item;
+}
+
+static int git_trailer_config(const char *conf_key, const char *value, void
*cb)
+{
+ if (starts_with(conf_key, trailer.)) {
+ const char *orig_conf_key = conf_key;
+ struct trailer_item *item;
+ struct conf_info *conf;
+ char *name;
+ enum trailer_info_type type;
+
+ conf_key += 8;
+ if (!set_name_and_type(conf_key, .key, TRAILER_KEY, name,
type)
+ !set_name_and_type(conf_key, .command, TRAILER_COMMAND,
name, type)
+ !set_name_and_type(conf_key, .where, TRAILER_WHERE,
name, type)
+ !set_name_and_type(conf_key, .ifexists,
TRAILER_IF_EXISTS, name, type)
+ !set_name_and_type(conf_key, .ifmissing,
TRAILER_IF_MISSING, name, type))
+ return 0;
+
+ item = get_conf_item(name);
+ conf = item-conf;
+ free(name);
+
+ switch (type) {
+ case TRAILER_KEY:
+ if (conf-key)
+ warning(_(more than one %s), orig_conf_key);
+ conf-key = xstrdup(value);
+ break;
+ case TRAILER_COMMAND:
+ if (conf-command)
+ warning(_(more than one %s), orig_conf_key);
+
[PATCH v6 11/11] Documentation: add documentation for 'git interpret-trailers'
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-interpret-trailers.txt | 123 +++ 1 file changed, 123 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..75ae386 --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,123 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [(token[(=|:)value])...] + +DESCRIPTION +--- +Help add RFC 822-like headers, called 'trailers', at the end of the +otherwise free-form part of a commit message. + +This command is a filter. It reads the standard input for a commit +message and applies the `token` arguments, if any, to this +message. The resulting message is emited on the standard output. + +Some configuration variables control the way the `token` arguments are +applied to the message and the way any existing trailer in the message +is changed. They also make it possible to automatically add some +trailers. + +By default, a 'token=value' or 'token:value' argument will be added +only if no trailer with the same (token, value) pair is already in the +message. The 'token' and 'value' parts will be trimmed to remove +starting and trailing whitespace, and the resulting trimmed 'token' +and 'value' will appear in the message like this: + + +token: value + + +By default, if there are already trailers with the same 'token', the +new trailer will appear just after the last trailer with the same +'token'. Otherwise it will appear at the end of the message. + +Note that 'trailers' do not follow and are not intended to follow many +rules that are in RFC 822. For example they do not follow the line +breaking rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the 'value' part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + +CONFIGURATION VARIABLES +--- + +trailer.token.key:: + This 'key' will be used instead of 'token' in the + trailer. After some alphanumeric characters, it can contain + some non alphanumeric characters like ':', '=' or '#' that will + be used instead of ':' to separate the token from the value in + the trailer, though the default ':' is more standard. + +trailer.token.where:: + This can be either `after`, which is the default, or + `before`. If it is `before`, then a trailer with the specified + token, will appear before, instead of after, other trailers + with the same token, or otherwise at the beginning, instead of + at the end, of all the trailers. + +trailer.token.ifexist:: + This option makes it possible to choose what action will be + performed when there is already at least one trailer with the + same token in the message. ++ +The valid values for this option are: `addIfDifferent` (this is the +default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`. ++ +With `addIfDifferent`, a new trailer will be added only if no trailer +with the same (token, value) pair is already in the message. ++ +With `addIfDifferentNeighbor`, a new trailer will be added only if no +trailer with the same (token, value) pair is above or below the line +where the new trailer will be added. ++ +With `add`, a new trailer will be added, even if some trailers with +the same (token, value) pair are already in the message. ++ +With `overwrite`, the new trailer will overwrite an existing trailer +with the same token. ++ +With `doNothing`, nothing will be done, that is no new trailer will be +added if there is already one with the same token in the message. + +trailer.token.ifmissing:: + This option makes it possible to choose what action will be + performed when there is not yet any trailer with the same + token in the message. ++ +The valid values for this option are: `add` (this is the default) and +`doNothing`. ++ +With `add`, a new trailer will be added. ++ +With `doNothing`, nothing will be done. + +trailer.token.command:: + This option can be used to specify a shell command that will + be used to automatically add or modify a trailer with the + specified 'token'. ++ +When this option is specified, it is like if a special 'token=value' +argument is added at the end of the command line, where 'value' will +be given by the standard output of the specified command. ++ +If the command contains the `$ARG` string, this string will be +replaced with the 'value' part of an existing trailer with the same +token, if any, before the command
[PATCH v6 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put
the result into an arg_tok doubly linked list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 93 +++
1 file changed, 93 insertions(+)
diff --git a/trailer.c b/trailer.c
index 5b8e28b..5d69c00 100644
--- a/trailer.c
+++ b/trailer.c
@@ -378,3 +378,96 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
}
return 0;
}
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ size_t len = strcspn(trailer, =:);
+ if (len strlen(trailer)) {
+ strbuf_add(tok, trailer, len);
+ strbuf_trim(tok);
+ strbuf_addstr(val, trailer + len + 1);
+ strbuf_trim(val);
+ } else {
+ strbuf_addstr(tok, trailer);
+ strbuf_trim(tok);
+ }
+}
+
+
+static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
+{
+ *dst = *src;
+ if (src-name)
+ dst-name = xstrdup(src-name);
+ if (src-key)
+ dst-key = xstrdup(src-key);
+ if (src-command)
+ dst-command = xstrdup(src-command);
+}
+
+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
+char* tok, char* val)
+{
+ struct trailer_item *new = xcalloc(sizeof(*new), 1);
+ new-value = val;
+
+ if (conf_item) {
+ duplicate_conf(new-conf, conf_item-conf);
+ new-token = xstrdup(conf_item-conf.key);
+ free(tok);
+ } else
+ new-token = tok;
+
+ return new;
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ struct trailer_item *item;
+ int tok_alnum_len;
+
+ parse_trailer(tok, val, string);
+
+ tok_alnum_len = alnum_len(tok.buf, tok.len);
+
+ /* Lookup if the token matches something in the config */
+ for (item = first_conf_item; item; item = item-next) {
+ if (!strncasecmp(tok.buf, item-conf.key, tok_alnum_len) ||
+ !strncasecmp(tok.buf, item-conf.name, tok_alnum_len)) {
+ strbuf_release(tok);
+ return new_trailer_item(item, NULL, strbuf_detach(val,
NULL));
+ }
+ }
+
+ return new_trailer_item(NULL, strbuf_detach(tok, NULL),
strbuf_detach(val, NULL));;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+struct trailer_item **last,
+struct trailer_item *new)
+{
+ if (!*last) {
+ *first = new;
+ *last = new;
+ } else {
+ (*last)-next = new;
+ new-previous = *last;
+ *last = new;
+ }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char
**argv)
+{
+ int i;
+ struct trailer_item *arg_tok_first = NULL;
+ struct trailer_item *arg_tok_last = NULL;
+
+ for (i = 0; i argc; i++) {
+ struct trailer_item *new = create_trailer_item(argv[i]);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+
+ return arg_tok_first;
+}
--
1.8.5.2.204.gcfe299d.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 v6 06/11] trailer: put all the processing together and print
This patch adds the process_trailers() function that
calls all the previously added processing functions
and then prints the results on the standard output.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 48
1 file changed, 48 insertions(+)
diff --git a/trailer.c b/trailer.c
index e0e066f..ab93c16 100644
--- a/trailer.c
+++ b/trailer.c
@@ -67,6 +67,26 @@ static void free_trailer_item(struct trailer_item *item)
free(item);
}
+static void print_tok_val(const char *tok, const char *val)
+{
+ char c = tok[strlen(tok) - 1];
+ if (isalnum(c))
+ printf(%s: %s\n, tok, val);
+ else if (isspace(c) || c == '#')
+ printf(%s%s\n, tok, val);
+ else
+ printf(%s %s\n, tok, val);
+}
+
+static void print_all(struct trailer_item *first, int trim_empty)
+{
+ struct trailer_item *item;
+ for (item = first; item; item = item-next) {
+ if (!trim_empty || strlen(item-value) 0)
+ print_tok_val(item-token, item-value);
+ }
+}
+
static void add_arg_to_input_list(struct trailer_item *in_tok,
struct trailer_item *arg_tok)
{
@@ -547,3 +567,31 @@ static void process_stdin(struct trailer_item
**in_tok_first,
strbuf_list_free(lines);
}
+
+static void free_all(struct trailer_item **first)
+{
+ while (*first) {
+ struct trailer_item *item = remove_first(first);
+ free_trailer_item(item);
+ }
+}
+
+void process_trailers(int trim_empty, int argc, const char **argv)
+{
+ struct trailer_item *in_tok_first = NULL;
+ struct trailer_item *in_tok_last = NULL;
+ struct trailer_item *arg_tok_first;
+
+ git_config(git_trailer_config, NULL);
+
+ /* Print the non trailer part of stdin */
+ process_stdin(in_tok_first, in_tok_last);
+
+ arg_tok_first = process_command_line_args(argc, argv);
+
+ process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first);
+
+ print_all(in_tok_first, trim_empty);
+
+ free_all(in_tok_first);
+}
--
1.8.5.2.204.gcfe299d.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 v6 10/11] trailer: add tests for commands in config file
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 47 +++ 1 file changed, 47 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 3223b12..0badd0e 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -211,4 +211,51 @@ test_expect_success 'using ifMissing = doNothing' ' test_cmp expected actual ' +test_expect_success 'with simple command' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: C O Mitter commit...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using author information' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'setup a commit' ' + echo Content of the first commit. a.txt + git add a.txt + git commit -m Add file a.txt +' + +test_expect_success 'with command using $ARG' ' + git config trailer.fix.ifExists overwrite + git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG + FIXED=$(git log -1 --oneline --format=%h (%s) --abbrev-commit --abbrev=14 HEAD) + cat complex_message_body expected + printf Fixes: $FIXED\nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=HEAD complex_message actual + test_cmp expected actual +' + test_done -- 1.8.5.2.204.gcfe299d.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 v6 01/11] Add data structures and basic functions for commit trailers
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Makefile | 1 +
trailer.c | 49 +
2 files changed, 50 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..db93a63
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,49 @@
+#include cache.h
+/*
+ * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exists { EXISTS_ADD_IF_DIFFERENT,
EXISTS_ADD_IF_DIFFERENT_NEIGHBOR,
+ EXISTS_ADD, EXISTS_OVERWRITE, EXISTS_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
+
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exists if_exists;
+ enum action_if_missing if_missing;
+};
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info conf;
+};
+
+static int same_token(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
+{
+ return !strncasecmp(a-token, b-token, alnum_len);
+}
+
+static int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a-value, b-value);
+}
+
+static int same_trailer(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
+{
+ return same_token(a, b, alnum_len) same_value(a, b);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static size_t alnum_len(const char *buf, size_t len)
+{
+ while (len 0 !isalnum(buf[len - 1]))
+ len--;
+ return len;
+}
--
1.8.5.2.204.gcfe299d.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 v6 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output
the value to use for the specified trailer.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 64 +++
1 file changed, 64 insertions(+)
diff --git a/trailer.c b/trailer.c
index ab93c16..67e7baf 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,4 +1,5 @@
#include cache.h
+#include run-command.h
/*
* Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
*/
@@ -12,11 +13,14 @@ struct conf_info {
char *name;
char *key;
char *command;
+ unsigned command_uses_arg : 1;
enum action_where where;
enum action_if_exists if_exists;
enum action_if_missing if_missing;
};
+#define TRAILER_ARG_STRING $ARG
+
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -57,6 +61,13 @@ static inline int contains_only_spaces(const char *str)
return !*s;
}
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const char
*b)
+{
+ const char *ptr = strstr(sb-buf, a);
+ if (ptr)
+ strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b));
+}
+
static void free_trailer_item(struct trailer_item *item)
{
free(item-conf.name);
@@ -386,6 +397,7 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
if (conf-command)
warning(_(more than one %s), orig_conf_key);
conf-command = xstrdup(value);
+ conf-command_uses_arg = !!strstr(conf-command,
TRAILER_ARG_STRING);
break;
case TRAILER_WHERE:
if (set_where(conf, value))
@@ -420,6 +432,44 @@ static void parse_trailer(struct strbuf *tok, struct
strbuf *val, const char *tr
}
}
+static int read_from_command(struct child_process *cp, struct strbuf *buf)
+{
+ if (run_command(cp))
+ return error(running trailer command '%s' failed,
cp-argv[0]);
+ if (strbuf_read(buf, cp-out, 1024) 1)
+ return error(reading from trailer command '%s' failed,
cp-argv[0]);
+ strbuf_trim(buf);
+ return 0;
+}
+
+static const char *apply_command(const char *command, const char *arg)
+{
+ struct strbuf cmd = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *argv[] = {NULL, NULL};
+ const char *result = ;
+
+ strbuf_addstr(cmd, command);
+ if (arg)
+ strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
+
+ argv[0] = cmd.buf;
+ memset(cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.no_stdin = 1;
+ cp.out = -1;
+ cp.use_shell = 1;
+
+ if (read_from_command(cp, buf))
+ strbuf_release(buf);
+ else
+ result = strbuf_detach(buf, NULL);
+
+ strbuf_release(cmd);
+ return result;
+}
static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
{
@@ -442,6 +492,10 @@ static struct trailer_item *new_trailer_item(struct
trailer_item *conf_item,
duplicate_conf(new-conf, conf_item-conf);
new-token = xstrdup(conf_item-conf.key);
free(tok);
+ if (conf_item-conf.command_uses_arg || !val) {
+ new-value = apply_command(conf_item-conf.command,
val);
+ free(val);
+ }
} else
new-token = tok;
@@ -490,12 +544,22 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
int i;
struct trailer_item *arg_tok_first = NULL;
struct trailer_item *arg_tok_last = NULL;
+ struct trailer_item *item;
for (i = 0; i argc; i++) {
struct trailer_item *new = create_trailer_item(argv[i]);
add_trailer_item(arg_tok_first, arg_tok_last, new);
}
+ /* Add conf commands that don't use $ARG */
+ for (item = first_conf_item; item; item = item-next) {
+ if (item-conf.command !item-conf.command_uses_arg)
+ {
+ struct trailer_item *new = new_trailer_item(item, NULL,
NULL);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+ }
+
return arg_tok_first;
}
--
1.8.5.2.204.gcfe299d.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 v6 02/11] trailer: process trailers from stdin and arguments
Implement the logic to process trailers from stdin and arguments.
At the beginning trailers from stdin are in their own in_tok
doubly linked list, and trailers from arguments are in their own
arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be applied,
it is removed from its list and inserted into the in_tok list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 197 ++
1 file changed, 197 insertions(+)
diff --git a/trailer.c b/trailer.c
index db93a63..a0124f2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -47,3 +47,200 @@ static size_t alnum_len(const char *buf, size_t len)
len--;
return len;
}
+
+static void free_trailer_item(struct trailer_item *item)
+{
+ free(item-conf.name);
+ free(item-conf.key);
+ free(item-conf.command);
+ free((char *)item-token);
+ free((char *)item-value);
+ free(item);
+}
+
+static void add_arg_to_input_list(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf.where == WHERE_AFTER) {
+ arg_tok-next = in_tok-next;
+ in_tok-next = arg_tok;
+ arg_tok-previous = in_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = in_tok-previous;
+ in_tok-previous = arg_tok;
+ arg_tok-next = in_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf.where;
+ do {
+ if (!in_tok)
+ return 1;
+ if (same_trailer(in_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+* if we want to add a trailer after another one,
+* we have to check those before this one
+*/
+ in_tok = (where == WHERE_AFTER) ? in_tok-previous :
in_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exists(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len)
+{
+ switch (arg_tok-conf.if_exists) {
+ case EXISTS_DO_NOTHING:
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_OVERWRITE:
+ free((char *)in_tok-value);
+ in_tok-value = xstrdup(arg_tok-value);
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD:
+ add_arg_to_input_list(in_tok, arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 1))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 0))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(arg_tok);
+ break;
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
+
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_input_token(struct trailer_item *in_tok,
+ struct trailer_item **arg_tok_first,
+ enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int tok_alnum_len = alnum_len(in_tok-token, strlen(in_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(in_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf.where == where) {
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exists(in_tok, arg_tok, tok_alnum_len);
+ /*
+* If arg has been added to input,
+* then we need to process it too
[PATCH v6 05/11] trailer: parse trailers from stdin
Read trailers from stdin, parse them and put the result into a doubly linked
list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 76 +++
1 file changed, 76 insertions(+)
diff --git a/trailer.c b/trailer.c
index 5d69c00..e0e066f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -50,6 +50,13 @@ static size_t alnum_len(const char *buf, size_t len)
return len;
}
+static inline int contains_only_spaces(const char *str)
+{
+ const char *s;
+ for (s = str; *s isspace(*s); s++);
+ return !*s;
+}
+
static void free_trailer_item(struct trailer_item *item)
{
free(item-conf.name);
@@ -471,3 +478,72 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
return arg_tok_first;
}
+
+static struct strbuf **read_stdin(void)
+{
+ struct strbuf **lines;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+
+ lines = strbuf_split(sb, '\n');
+
+ strbuf_release(sb);
+
+ return lines;
+}
+
+/*
+ * Return the the (0 based) index of the first trailer line
+ * or the line count if there are no trailers.
+ */
+static int find_trailer_start(struct strbuf **lines)
+{
+ int count, start, empty = 1;
+
+ /* Get the line count */
+ for (count = 0; lines[count]; count++)
+ ; /* Nothing to do */
+
+ /*
+* Get the start of the trailers by looking starting from the end
+* for a line with only spaces before lines with one ':'.
+*/
+ for (start = count - 1; start = 0; start--) {
+ if (contains_only_spaces(lines[start]-buf)) {
+ if (empty)
+ continue;
+ return start + 1;
+ }
+ if (strchr(lines[start]-buf, ':')) {
+ if (empty)
+ empty = 0;
+ continue;
+ }
+ return count;
+ }
+
+ return empty ? count : start + 1;
+}
+
+static void process_stdin(struct trailer_item **in_tok_first,
+ struct trailer_item **in_tok_last)
+{
+ struct strbuf **lines = read_stdin();
+ int start = find_trailer_start(lines);
+ int i;
+
+ /* Print non trailer lines as is */
+ for (i = 0; lines[i] i start; i++) {
+ printf(%s, lines[i]-buf);
+ }
+
+ /* Parse trailer lines */
+ for (i = start; lines[i]; i++) {
+ struct trailer_item *new = create_trailer_item(lines[i]-buf);
+ add_trailer_item(in_tok_first, in_tok_last, new);
+ }
+
+ strbuf_list_free(lines);
+}
--
1.8.5.2.204.gcfe299d.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: Branch Name Case Sensitivity
On 2014-03-04 14.23, Karsten Blees wrote:
Am 03.03.2014 18:51, schrieb Junio C Hamano:
Lee Hopkins leer...@gmail.com writes:
I went ahead and took a stab at a solution. My solution is more
aggressive than a warning, I actually prevent the creation of
ambiguous refs. My changes are also in refs.c, which may not be
appropriate, but it seemed like the natural place.
I have never contributed to Git (in fact this is my first dive into
the source) and my C is a bit rusty, so bear with me, this is just a
suggestion:
---
refs.c | 31 ---
1 files changed, 24 insertions(+), 7 deletions(-)
Starting something like this from forbidding is likely to turn out
to be a very bad idea that can break existing repositories.
Its sure worth considering what should be done with pre-existing duplicates.
However, repositories with such refs are already broken on case-insensitive
filesystems, and allowing something that's known to be broken is even more
dangerous, IMO.
An alternative approach could be to encode upper-case letters in loose refs
if core.ignorecase == true (e.g. Foo - %46oo). Although this may pose a
problem for commands that bypass the refs API / plumbing for whatever reason.
A new configuration
refs.caseInsensitive = {warn|error|allow}
s/caseInsensitive/caseSensitive/
Its case-sensitive refs that cause trouble, case-insensitive refs would be
fine on all platforms.
I still don't see why we need an extra setting for this. The problems are
inherently caused by case-insensitive filesystems, and we already have
'core.ignorecase' for that (its even automatically configured). Having an
extra setting for refs is somewhat like making 'core.ignorecase' configurable
per sub-directory.
I start to agree here.
The case-insensitive file system does not allow branches foo and Foo at the
same time,
and the packed refs should simply follow this convention/restriction/behaviour.
(and everything else could and should go into another patch:
If we ever want Linux to ignore the case in refs,
to ease the cross-platform development with Windows.
Or if we allow Windows/Mac OS to handle case insensitive refs (by always
packing them)
to ease the co-working with e.g. Linux.
)
Lee, could you improve your change in refs.c into a real patch, with a commit
message?
(And please have a look at the indentation with TABs)
A test case could be good, if time allows I can make a suggestion.
Thanks for all comments
/Torsten
--
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] commit.c: use skip_prefix() instead of starts_with()
Max Horn m...@quendi.de writes:
+buf = ident_line;
if (split_ident_line(ident,
- buf + strlen(author ),
- line_end - (buf + strlen(author ))) ||
+ buf,
+ line_end - buf) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
Why not get rid of that assignment to buf, and use ident_line
instead of buf below? That seems like it would be more readable,
wouldn't it?
Yes, and also now the argument list is much shorter, you could
probably do it on two lines instead of three:
if (split_ident_line(ident,
ident_line, line_end - ident_line) ||
...
@@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
-if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-/* At the very beginning of the buffer */
-found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-} else {
+found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+/* At the very beginning of the buffer */
Do we really need that comment, and in that spot? The code seemed
clear enough to me without it. But if you think keeping is better,
perhaps move it to *before* the skip_prefix, and add a trailing
?
Both good suggestions (I tend to prefer the removal).
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: [PATCH] disable grafts during fetch/push/bundle
Jeff King p...@peff.net writes: When we are creating a pack to send to a remote, we should make sure that we are not respecting grafts or replace refs. Otherwise, we may end up sending a broken pack to the other side that does not contain all objects (either omitting them entirely, or using objects that the other side does not have as delta bases). We already make an attempt to do the right thing in several places by turning off read_replace_refs. However, we missed at least one case (during bundle creation), and we do nothing anywhere to handle grafts. Doing nothing for grafts has been pretty much a deliberate omission. Because we have no way to transfer how histories are grafted together, people cloning from a repository that grafts away a commit that records a mistakenly committed sekrit will end up with a disjoint history, instead of exposing the sekrit to them, and are expected to join the history by recreating grafts (perhaps a README of such a project instructs them to do so). That was deemed far better than exposing the hidden history, I think. And replace tries to do the right thing was an attempt to rectify that misfeature of grafts in that we now do have a way to transfer how the history is grafted together, so that project README does not have to instruct the fetcher of doing anything special. It _might_ be a misfeature, however, for the object connectivity layer to expose a part of the history replaced away to the party that fetches from such a repository. Ideally, the right thing ought to be to include history that would be omitted if we did not have the replacement (i.e. adding parents the underlying commit does not record), while not following the history that replacement wants to hide (i.e. excluding the commits replacement commits overlay). -- 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] cache_tree_find(): remove redundant checks
Michael Haggerty mhag...@alum.mit.edu writes:
while (*path) {
- const char *slash;
struct cache_tree_sub *sub;
+ const char *slash = strchr(path, '/');
- slash = strchr(path, '/');
if (!slash)
slash = path + strlen(path);
Isn't the above a strchrnul()?
Combining a freestanding decl with intializer assignment to lose one
line is sort of cheating on the line count, but replacing the three
lines with a single strchrnul() would be a real code reduction ;-)
- /* between path and slash is the name of the
- * subtree to look for.
+ /*
+ * Between path and slash is the name of the subtree
+ * to look for.
*/
sub = find_subtree(it, path, slash - path, 0);
if (!sub)
return NULL;
it = sub-cache_tree;
- if (slash)
- while (*slash *slash == '/')
- slash++;
- if (!slash || !*slash)
- return it; /* prefix ended with slashes */
path = slash;
+ while (*path == '/')
+ path++;
}
return it;
}
--
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 v4] commit.c: use skip_prefix() instead of starts_with()
In record_author_date() parse_gpg_output(), the callers of
starts_with() not just want to know if the string starts with the
prefix, but also can benefit from knowing the string that follows
the prefix.
By using skip_prefix(), we can do both at the same time.
Helped-by: Max Horn m...@quendi.de
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Patch V4 Identation improved, removed useless comment. [1]
Thanks to Junio C Hamano and Max Horn.
[1] http://article.gmane.org/gmane.comp.version-control.git/243388
Patch V3 Variable naming improved, removed assignments inside conditionals.
Thanks to Junio C Hamano and Max Horn.
Patch V2 Corrected email formatting ,reapplied the implementation according to
suggestions.
Thanks to Michael Haggerty.
This is in respect to GSoC microproject #10.
In record_author_date(), extra and useless calls to strlen due to using
starts_with()
were removed by using skip_prefix(). Extra variable ident_line was used as
buf is used in
for loop update check.
Other usages of starts_with() in the same file can be found with,
$ grep -n starts_with commit.c
1116: else if (starts_with(line, gpg_sig_header)
1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
The starts_with() in line 1116 was left as it is, as strlen values were pre
computed as
global variables, and replacing may hamper the clarity.
The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix
part by
directly using the function.
Also skip_prefix() is inline when compared to starts_with().
commit.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..01526f7 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
static void record_author_date(struct author_date_slab *author_date,
struct commit *commit)
{
- const char *buf, *line_end;
+ const char *buf, *line_end, *ident_line;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+ident_line, line_end -
ident_line) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
1.9.0
--
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
[microproject idea]
Find places where we scan a string twice unnecessarily, once with strchr() and then with strlen(), e.g. const char *colon = strchr(name, ':'); int namelen = colon ? colon - name : strlen(name); and rewrite such a pattern using strchrnul() as appropriate. The above example can become const char *colon = strchrnul(name, ':'); int namelen = colon - name; -- 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 v4] commit.c: use skip_prefix() instead of starts_with()
Tanay Abhra tanay...@gmail.com writes:
In record_author_date() parse_gpg_output(), the callers of
starts_with() not just want to know if the string starts with the
prefix, but also can benefit from knowing the string that follows
the prefix.
By using skip_prefix(), we can do both at the same time.
Helped-by: Max Horn m...@quendi.de
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
Do not add the last when sending; I never signed-off this particular
version of this patch.
diff --git a/commit.c b/commit.c
index 6bf4fe0..01526f7 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
static void record_author_date(struct author_date_slab *author_date,
struct commit *commit)
{
- const char *buf, *line_end;
+ const char *buf, *line_end, *ident_line;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
if (split_ident_line(ident,
- buf + strlen(author ),
- line_end - (buf + strlen(author ))) ||
+ ident_line, line_end -
ident_line) ||
Funny indentation with some SP followed by HT followed by SP.
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
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] branch.c: delete size check of newly tracked branch names
Since commit 6f084a56 the length of a newly tracked branch name is limited to 1009 = 1024 - 7 - 7 - 1 characters, a bound derived by having to store this name in a char[1024] with two strings of length at most 7 and a '\0' character. This is no longer necessary as of commit a9f2c136, which uses a strbuf (documented in Documentation/technical/api-strbuf.txt) to store this value. Remove this unneeded check and thus allow for branch names longer than 1009 characters. Signed-off-by: Jacopo Notarstefano jacopo.notarstef...@gmail.com --- branch.c | 4 1 file changed, 4 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..05feaff 100644 --- a/branch.c +++ b/branch.c @@ -114,10 +114,6 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, struct tracking tracking; int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE; - if (strlen(new_ref) 1024 - 7 - 7 - 1) - return error(_(Tracking not set up: name too long: %s), - new_ref); - memset(tracking, 0, sizeof(tracking)); tracking.spec.dst = (char *)orig_ref; if (for_each_remote(find_tracked_branch, tracking)) -- 1.9.0.138.g2de3478 -- 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 v5] commit.c: use skip_prefix() instead of starts_with()
In record_author_date() parse_gpg_output(), the callers of
starts_with() not just want to know if the string starts with the
prefix, but also can benefit from knowing the string that follows
the prefix.
By using skip_prefix(), we can do both at the same time.
Helped-by: Max Horn m...@quendi.de
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Patch V5 Minor revision of indentation
Patch V4 Identation improved, removed useless comment. [1]
Thanks to Junio C Hamano and Max Horn.
[1] http://article.gmane.org/gmane.comp.version-control.git/243388
Patch V3 Variable naming improved, removed assignments inside conditionals.
Thanks to Junio C Hamano and Max Horn.
Patch V2 Corrected email formatting ,reapplied the implementation according to
suggestions.
Thanks to Michael Haggerty.
This is in respect to GSoC microproject #10.
In record_author_date(), extra and useless calls to strlen due to using
starts_with()
were removed by using skip_prefix(). Extra variable ident_line was used as
buf is used in
for loop update check.
Other usages of starts_with() in the same file can be found with,
$ grep -n starts_with commit.c
1116: else if (starts_with(line, gpg_sig_header)
1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
The starts_with() in line 1116 was left as it is, as strlen values were pre
computed as
global variables, and replacing may hamper the clarity.
The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix
part by
directly using the function.
Also skip_prefix() is inline when compared to starts_with().
---
commit.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..d37675c 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
static void record_author_date(struct author_date_slab *author_date,
struct commit *commit)
{
- const char *buf, *line_end;
+ const char *buf, *line_end, *ident_line;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab
*author_date,
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ ident_line = skip_prefix(buf, author );
+ if (!ident_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+ident_line, line_end - ident_line) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check + 1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
--
1.9.0
--
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 v4] commit.c: use skip_prefix() instead of starts_with()
Junio C Hamano gits...@pobox.com writes:
+found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+if(!found) {
Missing SP between the control keyword and parenthesized expression
the keyword uses.
I've fixed this (and the broken indentation) locally and queued the
result to 'pu', so no need to resend to correct this one.
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: [BUG] Halt during fetch on MacOS
On Sun, Mar 2, 2014 at 3:02 AM, Kyle J. McKay mack...@gmail.com wrote: I can't reproduce, mostly, on Mac OS X 10.5.8 or 10.6.8. What I mean by mostly is that the very first time I ran the test script I got approximately 36 of these errors: fatal: unable to access 'https://android.googlesource.com/platform/external/tinyxml2/': Unknown SSL protocol error in connection to android.googlesource.com:443 The rest of the fetches completed. That was with Git 1.8.5.1. However, I was never able to reproduce those errors again. All the subsequent runs completed all fetches successfully using that same Git version so I also tried Git 1.8.5.2, 1.8.5.5 and Git 1.7.6.1 on the original and another machine. With Git 1.9.0.138.g2de3478 (latest build from source) on Mac OS X 10.9.2 I report similar results. A first run yielded several fatal: unable to access errors, while a second run yielded just one. -- 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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Thanks for the resend. Etiquette on this list is to cc: people who
commented on previous versions of the submission. As Tanay already
mentioned, use [PATCH vN] in the subject where N is the version number
of this attempt. The -v option of git format-email can help.
More below.
On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote:
In record_author_date() :
Replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable indent_line.
In parse_signed_commit() :
Replace line + gpg_sig_header_len by skip_prefix(), which
is saved in a new const char variable indent_line.
In parse_gpg_output() :
Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by
skip_prefix(), which is saved in already available
variable found.
It's not necessary to explain in prose what the patch itself already
states more concisely and precisely. All of this text should be
dropped from the commit message. Instead, explain the purpose of the
patch, the problem it solves, etc. Please read the (2) Describe your
changes well section of Documentation/SubmittingPatches to get an
idea of what sort of information to include in the commit message.
A better commit message should say something about how the affected
functions want to know not only if the string has a prefix, but also
the text following the prefix, and that skip_prefix() can provide both
pieces of information.
Signed-off-by: Karthik Nayak karthik@gmail.com
---
commit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
@@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab
*author_date,
struct ident_split ident;
char *date_end;
unsigned long date;
+ const char *indent_line;
Strange variable name. The code in question splits apart a person's
identification string (name, email, etc.). It has nothing to do with
indentation.
if (!commit-buffer) {
unsigned long size;
@@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab
*author_date,
return;
}
+ indent_line = skip_prefix(buf, author );
+
for (buf = commit-buffer ? commit-buffer : buffer;
buf;
buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ if (!indent_line) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
- if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+ if (split_ident_line(ident, indent_line,
+line_end -
indent_line) ||
Indent the second line (using tabs plus possible spaces) so that it
lines up with the ident in the line above. Be sure to set your editor
so tabs have width 8.
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1,
char *buffer = read_sha1_file(sha1, type, size);
int in_signature, saw_signature = -1;
char *line, *tail;
+ const char *indent_line;
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
@@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
Even stranger variable name for a GPG signature (which has nothing at
all to do with indentation).
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
-line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
+ sig = indent_line + 2;
Why is this adding 2 rather than 1?
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
@@ -1193,10 +1196,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf +
Re: [PATCH v2] cache_tree_find(): remove redundant checks
On 03/04/2014 10:05 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
while (*path) {
-const char *slash;
struct cache_tree_sub *sub;
+const char *slash = strchr(path, '/');
-slash = strchr(path, '/');
if (!slash)
slash = path + strlen(path);
Isn't the above a strchrnul()?
Oh, cool, I never realized that this GNU extension was blessed for use
in Git. Will change.
Combining a freestanding decl with intializer assignment to lose one
line is sort of cheating on the line count, but replacing the three
lines with a single strchrnul() would be a real code reduction ;-)
I suppose you are just teasing me, but for the record I consider line
count only a secondary metric. The reason for combining initialization
with declaration is to reduce by one the number of times that the reader
has to think about that variable when analyzing the code.
And as I am writing this I realize that after converting to the use of
strchrnul(), sub can also be initialized at its declaration.
I really wish we could mix declarations with statements because I think
it is a big help to readability.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 12:58 PM, karthik nayak karthik@gmail.com wrote:
Hey Tanay,
1. Yes just getting used to git send-email now, should follow that from now
2. I thought it shouldn't be a part of the commit, so i put it after
the last ---
3. I did have a thought on your lines also , but concluded to it being
more advantageous, you might be right though
Minor etiquette note: On this list, respond inline rather than top-posting.
To understand why, look at how much scrolling up and down a person has
to do to relate your points 1, 2, and 3 to review statements made by
Tanay at the very bottom of the email.
Nice to hear from you.
Thanks
-Karthik Nayak
On Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra tanay...@gmail.com wrote:
Karthik Nayak karthik.188 at gmail.com writes:
In record_author_date() :
Replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable indent_line.
In parse_signed_commit() :
Replace line + gpg_sig_header_len by skip_prefix(), which
is saved in a new const char variable indent_line.
In parse_gpg_output() :
Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by
skip_prefix(), which is saved in already available
variable found.
Signed-off-by: Karthik Nayak karthik.188 at gmail.com
---
commit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
at at -1098,6 +1100,7 at at int parse_signed_commit(const
unsigned char *sha1,
char *buffer = read_sha1_file(sha1, type, size);
int in_signature, saw_signature = -1;
char *line, *tail;
+ const char *indent_line;
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
at at -,11 +1114,11 at at int parse_signed_commit(const
unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
- line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
+ sig = indent_line + 2;
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
Hi,
I was attempting the same microproject so I thought I would share some
points that were told to me earlier .The mail subject should have a
increasing order of submission numbers for each revision(for example here it
should be [PATCH v2]).
Also write the superfluous stuff which you have written in the bottom
after ---(the three dashes after the signed off statement) .
In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header
variables are precomputed outside of the function, and also Junio said to
prefer starts_with(), whenever skip_prefix() advantages are not so important
in the context.So the replace may not be so advantageous ,I may be wrong in
this case.
Cheers,
Tanay Abhra.
--
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
--
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 v4] commit.c: use skip_prefix() instead of starts_with()
Junio C Hamano gitster at pobox.com writes:
Junio C Hamano gitster at pobox.com writes:
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+ if(!found) {
Missing SP between the control keyword and parenthesized expression
the keyword uses.
I've fixed this (and the broken indentation) locally and queued the
result to 'pu', so no need to resend to correct this one.
Thanks.
Sorry about the indentation, it was due to not setting the tab to eight
spaces. I found your mail late, so I had already
sent a revised patch [1]. Please ignore that mail.
Also , what should be my next step ,should I present a rough draft of a
proposal , or tackle other bugs on the mailing list?
Thanks for the suggestions and advice,
Regards,
Tanay Abhra.
[1] http://article.gmane.org/gmane.comp.version-control.git/243395
--
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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote:
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
@@ -1193,10 +1196,8 @@ static void parse_gpg_output(struct signature_check
*sigc)
for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
- /* At the very beginning of the buffer */
- found = buf + strlen(sigcheck_gpg_status[i].check +
1);
- } else {
+ found = skip_prefix(buf, sigcheck_gpg_status[i].check +1);
Add a space after the plus sign.
+ if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
'found' is being computed again here, even though you already computed
it just above via skip_prefix(). This seems pretty wasteful.
Ignore this objection. I must have misread the code as I was composing
the email.
--
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/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.
Permit to do a 'git clone --recursive' through git-gui.
Signed-off-by: Henri GEIST geist.he...@laposte.net
---
I have set the default checkbox state to 'true' by default has all my gui users
use it all the time this way.
But as it change the default behavior you may prefer to set it to 'false' by
default.
git-gui/lib/choose_repository.tcl | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/git-gui/lib/choose_repository.tcl
b/git-gui/lib/choose_repository.tcl
index 3c10bc6..47d436b 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -18,6 +18,7 @@ field local_path {} ; # Where this repository is locally
field origin_url {} ; # Where we are cloning from
field origin_name origin ; # What we shall call 'origin'
field clone_type hardlink ; # Type of clone to construct
+field recursive true ; # Recursive cloning flag
field readtree_err; # Error output from read-tree (if any)
field sorted_recent ; # recent repositories (sorted)
@@ -525,6 +526,11 @@ method _do_clone {} {
foreach r $w_types {
pack $r -anchor w
}
+ ${NS}::checkbutton $args.type_f.recursive \
+ -text [mc Recursive (For submodules)] \
+ -variable @recursive \
+ -onvalue true -offvalue false
+ pack $args.type_f.recursive
grid $args.type_l $args.type_f -sticky new
grid columnconfigure $args 1 -weight 1
@@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
fileevent $fd readable [cb _readtree_wait $fd]
}
+method _do_validate_submodule_cloning {ok} {
+ if {$ok} {
+ $o_cons done $ok
+ set done 1
+ } else {
+ _clone_failed $this [mc Cannot clone submodules.]
+ }
+}
+
+method _do_clone_submodules {} {
+ if {$recursive eq {true}} {
+ destroy $w_body
+ set o_cons [console::embed \
+ $w_body \
+ [mc Cloning submodules]]
+ pack $w_body -fill both -expand 1 -padx 10
+ $o_cons exec \
+ [list git submodule update --init --recursive] \
+ [cb _do_validate_submodule_cloning]
+ } else {
+ set done 1
+ }
+}
+
method _readtree_wait {fd} {
set buf [read $fd]
$o_cons update_meter $buf
@@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
} else {
- set done 1
+ _do_clone_submodules $this
}
}
@@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
hook_failed_popup post-checkout $pch_error 0
}
unset pch_error
- set done 1
+ _do_clone_submodules $this
return
}
fconfigure $fd_ph -blocking 0
--
1.7.9.3.369.gd715.dirty
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] cache_tree_find(): remove redundant checks
Michael Haggerty mhag...@alum.mit.edu writes:
Isn't the above a strchrnul()?
Oh, cool, I never realized that this GNU extension was blessed for use
in Git. Will change.
We do have our own fallbacks for non-glibc platforms, so it should
be safe.
Combining a freestanding decl with intializer assignment to lose one
line is sort of cheating on the line count, but replacing the three
lines with a single strchrnul() would be a real code reduction ;-)
I suppose you are just teasing me, but for the record I consider line
count only a secondary metric. The reason for combining initialization
with declaration is to reduce by one the number of times that the reader
has to think about that variable when analyzing the code.
...
I really wish we could mix declarations with statements because I think
it is a big help to readability.
Unfortunately, I think we are in violent disagreement.
A variable declaration block with initializations on only some but
not all variables is extremely annoying. If none of the variable
declaration has initialization (or initialization to trivial values
that do not depend on the logic flow), and the first statement is
separated from the decl block, then I do not have to read the decl
part when reading the code/logic *at all* (the compiler will find
missing variables, variables declared as a wrong type, etc.).
In other words, a trivial initialization at the beginning of the
block, if the logic flow only sometimes makes assignment to the
variable, is perfectly fine, e.g.
const char *string = NULL;
if (...) {
string = ...
}
But I would wish people stop doing this:
const char *string = strchrnul(name, ':');
... the real logic of the block that uses string follows ...
and instead say
const char *string;
string = strchrnul(name, ':');
... the real logic of the block that uses string follows ...
--
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] disable grafts during fetch/push/bundle
On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote:
diff --git a/commit.c b/commit.c
index 6bf4fe0..886dbfe 100644
--- a/commit.c
+++ b/commit.c
@@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const char *buf,
const char *tail)
static struct commit_graft **commit_graft;
static int commit_graft_alloc, commit_graft_nr;
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
Did you mean !!commit_graft ?
static int commit_graft_pos(const unsigned char *sha1)
{
int lo, hi;
--
1.8.5.2.500.g8060133
--
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 1/2] t3200-branch: test setting branch as own upstream
Brian Gesiak modoca...@gmail.com writes: No test asserts that git branch -u refs/heads/my-branch my-branch emits a warning. Add a test that does so. Signed-off-by: Brian Gesiak modoca...@gmail.com --- t/t3200-branch.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index fcdb867..6164126 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -507,6 +507,14 @@ EOF test_cmp expected actual ' +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' ' + git branch --set-upstream-to refs/heads/my13 my13 2actual + cat expected EOF +warning: Not setting branch my13 as its own upstream. +EOF + test_i18ncmp expected actual +' + Checking the error message is fine, but we are also interested in seeing that we do not leave such a nonsense configuration, if not more. Shouldn't we check the resulting config as well here? # Keep this test last, as it changes the current branch cat expect EOF $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 + branch: Created from master -- 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
What's cooking in git.git (Mar 2014, #01; Tue, 4)
What's cooking in git.git (Mar 2014, #01; Tue, 4) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. A handful of GSoC warm-up microprojects have been queued on 'pu'. Thanks for reviewing them. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * al/docs (2014-02-11) 4 commits (merged to 'next' on 2014-02-25 at 0c1a734) + docs/git-blame: explain more clearly the example pickaxe use + docs/git-clone: clarify use of --no-hardlinks option + docs/git-remote: capitalize first word of initial blurb + docs/merge-strategies: remove hyphen from mis-merges Originally merged to 'next' on 2014-02-13 A handful of documentation updates, all trivially harmless. * bc/gpg-sign-everywhere (2014-02-11) 9 commits (merged to 'next' on 2014-02-25 at 7db014c) + pull: add the --gpg-sign option. + rebase: add the --gpg-sign option + rebase: parse options in stuck-long mode + rebase: don't try to match -M option + rebase: remove useless arguments check + am: add the --gpg-sign option + am: parse options in stuck-long mode + git-sh-setup.sh: add variable to use the stuck-long mode + cherry-pick, revert: add the --gpg-sign option Originally merged to 'next' on 2014-02-13 Teach --gpg-sign option to many commands that create commits. * bk/refresh-missing-ok-in-merge-recursive (2014-02-24) 4 commits (merged to 'next' on 2014-02-25 at 2651cb0) + merge-recursive.c: tolerate missing files while refreshing index + read-cache.c: extend make_cache_entry refresh flag with options + read-cache.c: refactor --ignore-missing implementation + t3030-merge-recursive: test known breakage with empty work tree Originally merged to 'next' on 2014-01-29 Allow merge-recursive to work in an empty (temporary) working tree again when there are renames involved, correcting an old regression in 1.7.7 era. * bs/stdio-undef-before-redef (2014-01-31) 1 commit (merged to 'next' on 2014-02-25 at 77c4b5f) + git-compat-util.h: #undef (v)snprintf before #define them Originally merged to 'next' on 2014-01-31 When we replace broken macros from stdio.h in git-compat-util.h, #undef them to avoid re-definition warnings from the C preprocessor. * da/pull-ff-configuration (2014-01-15) 2 commits (merged to 'next' on 2014-02-25 at b9e4f61) + pull: add --ff-only to the help text + pull: add pull.ff configuration Originally merged to 'next' on 2014-01-22 git pull learned to pay attention to pull.ff configuration variable. * dk/blame-janitorial (2014-02-25) 5 commits (merged to 'next' on 2014-02-25 at d5faeb2) + builtin/blame.c::find_copy_in_blob: no need to scan for region end + blame.c: prepare_lines should not call xrealloc for every line + builtin/blame.c::prepare_lines: fix allocation size of sb-lineno + builtin/blame.c: eliminate same_suspect() + builtin/blame.c: struct blame_entry does not need a prev link Originally merged to 'next' on 2014-02-13 Code clean-up. * ds/rev-parse-required-args (2014-01-28) 1 commit (merged to 'next' on 2014-02-25 at bba6e79) + rev-parse: check i before using argv[i] against argc Originally merged to 'next' on 2014-01-31 git rev-parse --default without the required option argument did not diagnose it as an error. * ep/varscope (2014-01-31) 7 commits (merged to 'next' on 2014-02-25 at e967c7e) + builtin/gc.c: reduce scope of variables + builtin/fetch.c: reduce scope of variable + builtin/commit.c: reduce scope of variables + builtin/clean.c: reduce scope of variable + builtin/blame.c: reduce scope of variables + builtin/apply.c: reduce scope of variables + bisect.c: reduce scope of variable Originally merged to 'next' on 2014-01-31 Shrink lifetime of variables by moving their definitions to an inner scope where appropriate. * jk/config-path-include-fix (2014-01-28) 2 commits (merged to 'next' on 2014-02-25 at 3604f75) + handle_path_include: don't look at NULL value + expand_user_path: do not look at NULL path Originally merged to 'next' on 2014-01-31 include.path variable (or any variable that expects a path that can use ~username expansion) in the configuration file is not a boolean, but the code failed to check it. * jk/pack-bitmap (2014-02-12) 26 commits (merged to 'next' on 2014-02-25 at 5f65d26) + ewah: unconditionally ntohll ewah data + ewah: support platforms that require aligned reads + read-cache: use get_be32 instead of hand-rolled ntoh_l + block-sha1: factor out get_be and put_be wrappers + do not discard revindex when re-preparing packfiles + pack-bitmap: implement optional name_hash cache + t/perf: add tests for pack bitmaps + t: add basic bitmap functionality tests +
[RFC/PATCH] diff: simplify cpp funcname regex
The current funcname matcher for C files requires one or
more words before the function name, like:
static int foo(int arg)
{
However, some coding styles look like this:
static int
foo(int arg)
{
and we do not match, even though the default regex would.
This patch simplifies the regex significantly. Rather than
trying to handle all the variants of keywords and return
types, we simply look for an identifier at the start of the
line that contains a (, meaning it is either a function
definition or a function call, and then not containing ;
which would indicate it is a call or declaration.
This can be fooled by something like:
if (foo)
but it is unlikely to find that at the very start of a line
with no identation (and without having a complete set of
keywords, we cannot distinguish that from a function called
if taking foo anyway).
We had no tests at all for .c files, so this attempts to add
a few obvious cases (including the one we are fixing here).
Signed-off-by: Jeff King p...@peff.net
---
I tried accommodating this one case in the current regex, but it just
kept getting more complicated and unreadable. Maybe I am being naive to
think that this much simpler version can work. We don't have a lot of
tests or a known-good data set to check.
I did try git log -1000 -p before and after on git.git, diff'd the
results and manually inspected. The results were a mix of strict
improvement (mostly instances of the style I was trying to fix here) and
cases where there is no good answer. For example, for top-level changes
outside functions, we might find:
N_(some text that is long
that is part of:
const char *foo =
N_(some text that is long
and spans multiple lines);
Before this change, we would skip past it (using the cpp regex, that is;
the default one tends to find the same line) and either report nothing,
or whatever random function was before us. So it's a behavior change,
but the existing behavior is really no better.
t/t4018-diff-funcname.sh | 36
userdiff.c | 2 +-
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 38a092a..1e80b15 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -93,6 +93,29 @@ sed -e '
s/song;/song();/
' Beer.perl Beer-correct.perl
+cat Beer.c \EOF
+static int foo(void)
+{
+label:
+ int x = old;
+}
+
+struct foo; /* catch failure below */
+static int
+gnu(int arg)
+{
+ int x = old;
+}
+
+struct foo; /* catch failure below */
+int multiline(int arg,
+ char *arg2)
+{
+ int x = old;
+}
+EOF
+sed s/old/new/ Beer.c Beer-correct.c
+
test_expect_funcname () {
lang=${2-java}
test_expect_code 1 git diff --no-index -U1 \
@@ -127,6 +150,7 @@ test_expect_success 'set up .gitattributes declaring
drivers to test' '
cat .gitattributes -\EOF
*.java diff=java
*.perl diff=perl
+ *.c diff=cpp
EOF
'
@@ -158,6 +182,18 @@ test_expect_success 'perl pattern is not distracted by
forward declaration' '
test_expect_funcname package Beer;\$ perl
'
+test_expect_success 'c pattern skips labels' '
+ test_expect_funcname static int foo(void) c
+'
+
+test_expect_success 'c pattern matches GNU-style functions' '
+ test_expect_funcname gnu(int arg)\$ c
+'
+
+test_expect_success 'c pattern matches multiline functions' '
+ test_expect_funcname int multiline(int arg,\$ c
+'
+
test_expect_success 'custom pattern' '
test_config diff.java.funcname !static
!String
diff --git a/userdiff.c b/userdiff.c
index 10b61ec..b9d52b7 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -127,7 +127,7 @@
/* Jump targets or access declarations */
!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n
/* C/++ functions/methods at top level */
-^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[
\t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n
+^([A-Za-z_].*\\([^;]*)$\n
/* compound type at top level */
^((struct|class|enum)[^;]*)$,
/* -- */
--
1.8.5.2.500.g8060133
--
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] disable grafts during fetch/push/bundle
On Tue, Mar 04, 2014 at 06:36:07PM -0500, Eric Sunshine wrote:
On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote:
diff --git a/commit.c b/commit.c
index 6bf4fe0..886dbfe 100644
--- a/commit.c
+++ b/commit.c
@@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const char
*buf, const char *tail)
static struct commit_graft **commit_graft;
static int commit_graft_alloc, commit_graft_nr;
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
Did you mean !!commit_graft ?
Shouldn't they produce the same results?
-Peff
--
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] disable grafts during fetch/push/bundle
On Tue, Mar 04, 2014 at 12:52:18PM -0800, Junio C Hamano wrote: We already make an attempt to do the right thing in several places by turning off read_replace_refs. However, we missed at least one case (during bundle creation), and we do nothing anywhere to handle grafts. Doing nothing for grafts has been pretty much a deliberate omission. Because we have no way to transfer how histories are grafted together, people cloning from a repository that grafts away a commit that records a mistakenly committed sekrit will end up with a disjoint history, instead of exposing the sekrit to them, and are expected to join the history by recreating grafts (perhaps a README of such a project instructs them to do so). That was deemed far better than exposing the hidden history, I think. I see your point, but I would be tempted to say that the person trying to hide a secret with grafting is simply wrong to do so. You need to cement that history with a rewrite if you want to share with people. I do not recall any past discussion on this topic, and searching the archive only shows people echoing what I said above. Is this something we've promised to work in the past? I'm certainly sympathetic to systems failing to a secure default rather than doing something that the user does not expect. But at the same time, if using grafts for security isn't something people reasonably expect, then failing only hurts the non-security cases. And replace tries to do the right thing was an attempt to rectify that misfeature of grafts in that we now do have a way to transfer how the history is grafted together, so that project README does not have to instruct the fetcher of doing anything special. Perhaps the right response is grafts are broken, use git-replace instead. But then should we think about deprecating grafts? Again, this patch was spurred by a real user with a graft trying to push and getting a confusing error message. It _might_ be a misfeature, however, for the object connectivity layer to expose a part of the history replaced away to the party that fetches from such a repository. Ideally, the right thing ought to be to include history that would be omitted if we did not have the replacement (i.e. adding parents the underlying commit does not record), while not following the history that replacement wants to hide (i.e. excluding the commits replacement commits overlay). I don't really think it's worth the complexity. It's fairly common knowledge (or at least I think so) that replace refs are a _view_ onto the history. When you share the history graph, you share the true objects. You can _also_ share your views in replace/refs, but it is up to the client to fetch them. If you want to hide things, then you need to rewrite the true objects, end of story. I dunno. Maybe there are people who have different expectations. -Peff -- 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] disable grafts during fetch/push/bundle
On Tue, Mar 4, 2014 at 7:37 PM, Jeff King p...@peff.net wrote:
On Tue, Mar 04, 2014 at 06:36:07PM -0500, Eric Sunshine wrote:
On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote:
diff --git a/commit.c b/commit.c
index 6bf4fe0..886dbfe 100644
--- a/commit.c
+++ b/commit.c
@@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const char
*buf, const char *tail)
static struct commit_graft **commit_graft;
static int commit_graft_alloc, commit_graft_nr;
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
Did you mean !!commit_graft ?
Shouldn't they produce the same results?
Yes they should, but the use of !! seemed to imply that you wanted to
apply it to the pointer value. (If you indeed intended to use
commit_graft_nr, then 'return commit_graft_nr', without !!, would have
been sufficient and idiomatic C.)
--
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] disable grafts during fetch/push/bundle
On Tue, Mar 04, 2014 at 08:00:44PM -0500, Eric Sunshine wrote:
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
Did you mean !!commit_graft ?
Shouldn't they produce the same results?
Yes they should, but the use of !! seemed to imply that you wanted to
apply it to the pointer value. (If you indeed intended to use
commit_graft_nr, then 'return commit_graft_nr', without !!, would have
been sufficient and idiomatic C.)
I just wanted to normalize the return value to a boolean 0/1. Even when
the input is an int, it eliminates surprises when you try to assign the
result to a bitfield or other smaller-width type.
-Peff
--
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] disable grafts during fetch/push/bundle
On Tue, Mar 4, 2014 at 8:05 PM, Jeff King p...@peff.net wrote:
On Tue, Mar 04, 2014 at 08:00:44PM -0500, Eric Sunshine wrote:
+int commit_grafts_loaded(void)
+{
+ return !!commit_graft_nr;
+}
Did you mean !!commit_graft ?
Shouldn't they produce the same results?
Yes they should, but the use of !! seemed to imply that you wanted to
apply it to the pointer value. (If you indeed intended to use
commit_graft_nr, then 'return commit_graft_nr', without !!, would have
been sufficient and idiomatic C.)
I just wanted to normalize the return value to a boolean 0/1. Even when
the input is an int, it eliminates surprises when you try to assign the
result to a bitfield or other smaller-width type.
Thanks for the explanation.
--
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
New directory lost by git am
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 I applied a patch with git am that adds a new source file to a new directory, and later noticed that file was missing from the commit. It seems that git am fails to add the new file/directory to the index. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTFpCjAAoJEI5FoCIzSKrw1CsH/1E/0Wgs3RtXPLqWbwVoFy+U Bc7dW7TBmb8EScC+3DedI4u9ryjZigjbsnBg1Y8V/gEtmUSmvt1e8CWTdvMLQpvx bnasL4uia/CBOg/aZkJ1iEBiHA3sUi9Es4FqoHbuGBn0bkDrA2NQvt3bCqNf6n8H PCeWx/qb8+F4niI0I8T5ASeqOHMxxSegHvlGezl6XZoGHa5SeLRrg7JtW3ZoWKCO q6GRzR6dV4FWJckfajUo34IUQNS4YA7wLpmC3PVUn3+EgF+affAEigjVWGRWdf2k cuaNu6hUAuD/2EHhCt6YP+ubV+FYiU86QOvmVifVpH1Apd29Fw4Kqnvyq2zJVC0= =hXsK -END PGP SIGNATURE- -- 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: New directory lost by git am
Hi, On 05/03/14 15:49, Phillip Susi wrote: I applied a patch with git am that adds a new source file to a new directory, and later noticed that file was missing from the commit. It seems that git am fails to add the new file/directory to the index. Could you provide a few more details such as your git version (git --version) and an example of the failure. I've tried to reproduce the problem based on the description provided but everything seems to work as expected for me. git --version git version 1.9.0 mkdir test cd test git init echo hello world a.txt git add a.txt git commit -mInitial commit git checkout -b temp mkdir b echo lorem ipsum b/b.txt git add b/b.txt git commit -mAdd b/b.txt ls -R .: a.txt b ./b: b.txt git checkout master git format-patch temp -1 --stdout | git am ls -R .: a.txt b ./b: b.txt -- 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: New directory lost by git am
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 03/04/2014 10:08 PM, Chris Packham wrote: Could you provide a few more details such as your git version (git --version) and an example of the failure. I've tried to reproduce the problem based on the description provided but everything seems to work as expected for me. Version 1.8.3.2. git --version git version 1.9.0 mkdir test cd test git init echo hello world a.txt git add a.txt git commit -mInitial commit git checkout -b temp mkdir b echo lorem ipsum b/b.txt git add b/b.txt git commit -mAdd b/b.txt ls -R .: a.txt b ./b: b.txt git checkout master git format-patch temp -1 --stdout | git am ls -R .: a.txt b ./b: b.txt You are reapplying the patch while it is already applied. Do a reset HEAD~1 --hard, and git clean -x -f -d before git am. I didn't notice the missing file myself for some time because it is left in the working tree, just not added to the index and included in the commit. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTFphoAAoJEI5FoCIzSKrwx78H/iTLvtMVb2hmn2g2YDQuJWe3 nENrqlRNDF11YHpA9c7chxepcuP2CZaZjoXv45aCQG9Wx9XJyKPIbauhwqIIVUjR VYDORdtpn8u3Pf3WWyHYW+MEoupYyni4VYENVSjKnV6sLT951TuYI+4paHWat3lq /at9UkLy4d39hj2P/6M+voBbKWzblBZzP31lH6OY/Mno2zhh4eQChhsnZYPQ/Hfn REAeyB4WsLCjnPz+uEkOcWaEVVh+BwNU1UmK/tX+rzhBsaRzhDY5IIWTL9dfkD/z Af86IUSKdTjnMq7CTmVAmlxAfHXF0bgtlybrVY2Sdc8R/CqmWCz6USyKdUxgLIk= =Z3Z/ -END PGP SIGNATURE- -- 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 v4 27/27] count-objects: report unused files in $GIT_DIR/repos/...
On Sat, Mar 1, 2014 at 7:13 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
In linked checkouts, borrowed parts like config is taken from
$GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as
garbage.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/count-objects.c | 37 -
path.c | 4
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..725cd5f 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -78,6 +78,39 @@ static void count_objects(DIR *d, char *path, int len, int
verbose,
}
}
+static void report_linked_checkout_garbage(void)
+{
+ /*
+* must be more or less in sync with * path.c:update_common_dir().
+*
+* logs is let slip because logs/HEAD is in $GIT_DIR but the
+* remaining in $GIT_COMMON_DIR. Probably not worth traversing
+* the entire logs directory for that.
+*
+* The same gc.pid for because it's a temporary file.
+*/
+ const char *list[] = {
+ branches, hooks, info, lost-found, modules,
+ objects, refs, remotes, rr-cache, svn,
+ config, packed-refs, shallow, NULL
+ };
+ struct strbuf sb = STRBUF_INIT;
+ const char **p;
+ int len;
+
+ if (!file_exists(git_path(commondir)))
+ return;
+ strbuf_addf(sb, %s/, get_git_dir());
+ len = sb.len;
+ for (p = list; *p; p++) {
+ strbuf_setlen(sb, len);
+ strbuf_addstr(sb, *p);
+ if (file_exists(sb.buf))
+ report_garbage(unused in linked checkout, sb.buf);
+ }
+ strbuf_release(sb);
+}
+
static char const * const count_objects_usage[] = {
N_(git count-objects [-v] [-H | --human-readable]),
NULL
@@ -102,8 +135,10 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
- if (verbose)
+ if (verbose) {
report_garbage = real_report_garbage;
+ report_linked_checkout_garbage();
+ }
memcpy(path, objdir, len);
if (len objdir[len-1] != '/')
path[len++] = '/';
diff --git a/path.c b/path.c
index 47383ff..2e6035d 100644
--- a/path.c
+++ b/path.c
@@ -92,6 +92,10 @@ static void replace_dir(struct strbuf *buf, int len, const
char *newdir)
static void update_common_dir(struct strbuf *buf, int git_dir_len)
{
+ /*
+* Remember to report_linked_checkout_garbage()
+* builtin/count-objects.c
+*/
I couldn't figure out why this comment was telling me to remember to
report linked checkout garbage until I realized that you omitted the
word update (as in remember to update). It might be clearer to say
something along these lines:
Keep synchronized with related list in
builtin/count-objects.c:report_linked_checkout_garbage().
Is it not possible or just too much of a hassle to maintain this list
in just one place, as in a header which is included by these two
files? The exceptions, such as 'log' and 'gc.pid', could be marked by
a special character in the entry (!gc.pid, for example) or any such
scheme.
const char *common_dir_list[] = {
branches, hooks, info, logs, lost-found, modules,
objects, refs, remotes, repos, rr-cache, svn,
--
1.9.0.40.gaa8c3ea
--
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] cache_tree_find(): remove redundant checks
Junio C Hamano gits...@pobox.com writes:
Michael Haggerty mhag...@alum.mit.edu writes:
while (*path) {
-const char *slash;
struct cache_tree_sub *sub;
+const char *slash = strchr(path, '/');
-slash = strchr(path, '/');
if (!slash)
slash = path + strlen(path);
Isn't the above a strchrnul()?
Yes. I realized that previously, but since it's a GNU extension rather
than part of the C standards, I discarded that idea. Calling
git grep strchrnul
shows, however, that it _is_ used plentifully already.
That would, indeed, favor the current proposal but with strchnul.
Still worth thinking about whether there is no better name than slash
for something that indicated the end of the current path name segment.
--
David Kastrup
--
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
Bad git log behavior with multiple glob path arguments
git log seems to understand globs in the last path argument, and the last path argument only. I didn't see anything in the git log man page expressly saying this was to be expected, but it does seem like it ought to work for all the arguments or none of them. Here's a little shell script I ended up using to convince myself I wasn't going crazy. I'd expect the same output for all of the git log test, since they all specify (either with globs or not) all the files added to the repository. Alternatively, if globs aren't expected to work, I'd at least expect all the glob tests to return nothing. Note that glob matching doesn't seem to occur unless '--' is included. I'm not exactly clear on why that is. #!/bin/sh TESTREPO=$(pwd)/bad-glob-test-repo rm -rf $TESTREPO echo Running tests in $TESTREPO mkdir $TESTREPO cd $TESTREPO mkdir subdira mkdir subdirb mkdir subdirc git init echo a subdira/file.txt echo b subdirb/file.txt echo c subdirc/file.txt git add subdira/file.txt git commit -m 'a' git add subdirb/file.txt git commit -m 'b' git add subdirc/file.txt git commit -m 'c' echo Glob Test 1: git log --oneline -- 'subdira/*.txt' 'subdirb/*.txt' 'subdirc/*.txt' git log --oneline -- 'subdira/*.txt' 'subdirb/*.txt' 'subdirc/*.txt' echo Glob Test 2: git log --oneline -- 'subdira/*.txt' 'subdirc/*.txt' 'subdirb/*.txt' git log --oneline -- 'subdira/*.txt' 'subdirc/*.txt' 'subdirb/*.txt' echo Glob Test 3: git log --oneline -- 'subdirb/*.txt' 'subdira/*.txt' 'subdirc/*.txt' git log --oneline -- 'subdirb/*.txt' 'subdira/*.txt' 'subdirc/*.txt' echo Glob Test 4: git log --oneline -- 'subdirb/*.txt' 'subdirc/*.txt' 'subdira/*.txt' git log --oneline -- 'subdirb/*.txt' 'subdirc/*.txt' 'subdira/*.txt' echo Glob Test 5: git log --oneline -- 'subdirc/*.txt' 'subdira/*.txt' 'subdirb/*.txt' git log --oneline -- 'subdirc/*.txt' 'subdira/*.txt' 'subdirb/*.txt' echo Glob Test 6: git log --oneline -- 'subdirc/*.txt' 'subdirb/*.txt' 'subdira/*.txt' git log --oneline -- 'subdirc/*.txt' 'subdirb/*.txt' 'subdira/*.txt' echo Explicit Test 1: git log --oneline -- 'subdira/file.txt' 'subdirb/file.txt' 'subdirc/file.txt' git log --oneline -- 'subdira/file.txt' 'subdirb/file.txt' 'subdirc/file.txt' echo Explicit Test 2: git log --oneline -- 'subdira/file.txt' 'subdirc/file.txt' 'subdirb/file.txt' git log --oneline -- 'subdira/file.txt' 'subdirc/file.txt' 'subdirb/file.txt' echo Explicit Test 3: git log --oneline -- 'subdirb/file.txt' 'subdira/file.txt' 'subdirc/file.txt' git log --oneline -- 'subdirb/file.txt' 'subdira/file.txt' 'subdirc/file.txt' echo Explicit Test 4: git log --oneline -- 'subdirb/file.txt' 'subdirc/file.txt' 'subdira/file.txt' git log --oneline -- 'subdirb/file.txt' 'subdirc/file.txt' 'subdira/file.txt' echo Explicit Test 5: git log --oneline -- 'subdirc/file.txt' 'subdira/file.txt' 'subdirb/file.txt' git log --oneline -- 'subdirc/file.txt' 'subdira/file.txt' 'subdirb/file.txt' echo Explicit Test 6: git log --oneline -- 'subdirc/file.txt' 'subdirb/file.txt' 'subdira/file.txt' git log --oneline -- 'subdirc/file.txt' 'subdirb/file.txt' 'subdira/file.txt' -- Jeremy Nickurak -= Email/XMPP: -= jer...@nickurak.ca =- -- 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: git compile with debug symbols
Hello all, Thanks for replying back, figured out (offcourse had to search in net) that 'gdb' version I had was 6.7.1 (OS Ubuntu 12.04 LST), not sure how I got this. Then I upgraded gdb to version 7.4-2012.04 and things got going. thanks, mpujari On Tuesday, March 4, 2014 10:13 PM, David Kastrup d...@gnu.org wrote: Mahesh Pujari pujarimahesh_ku...@yahoo.com writes: Thanks David for the reply. I think I need to do more ground work of going through how to use gdb. Basically I am java programmer and I was trying out to debug git source using eclipse CDT and as we do in java, I was trying out to set break point but failed with errors as No line 396 in file help.c. And using gdb too I end up with same error. # (gdb) break help.c:396 # No line 396 in file help.c. Am I missing something. There is just no line 396 known to gdb. It seems like you are indicating a function header. That's not code. Either take the function _name_ rather than a line number (that's usually most reliable) or take the first line of actual code. -- David Kastrup -- 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] t3200-branch: test setting branch as own upstream
No test asserts that git branch -u refs/heads/my-branch my-branch emits a warning. Add a test that does so. Signed-off-by: Brian Gesiak modoca...@gmail.com --- t/t3200-branch.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index fcdb867..e6d4015 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -507,6 +507,16 @@ EOF test_cmp expected actual ' +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' ' + git branch --set-upstream-to refs/heads/my13 my13 2actual + cat expected EOF +warning: Not setting branch my13 as its own upstream. +EOF + test_i18ncmp expected actual + test_must_fail git config branch.my13.remote + test_must_fail git config branch.my13.merge +' + # Keep this test last, as it changes the current branch cat expect EOF $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 + branch: Created from master -- 1.8.3.4 (Apple Git-47) -- 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
Read
Hello, I am Ms Donna Kwok, HSBC Hong Kong, head of corporate sustainability Asia pacific region. A sum of USD$23,200,000.00 Million was deposited by our Late customer who died without declaring any next of kin before his death in 2006.My suggestion to you is to stand as the next of king to Fadel Ahmed.We shall share in the ratio of 50% for me, 50% for you.if interested please email don...@qq.com Thanks, Donna Kwok -- 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] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote:
diff --git a/commit.c b/commit.c
index 6bf4fe0..71a03e3 100644
--- a/commit.c
+++ b/commit.c
@@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
next = next ? next + 1 : tail;
+ indent_line = skip_prefix(line, gpg_sig_header);
Even stranger variable name for a GPG signature (which has nothing at
all to do with indentation).
if (in_signature line[0] == ' ')
sig = line + 1;
- else if (starts_with(line, gpg_sig_header)
-line[gpg_sig_header_len] == ' ')
- sig = line + gpg_sig_header_len + 1;
+ else if (indent_line indent_line[1] == ' ')
Also, shouldn't this be checking *indent_line (or indent_line[0])
rather than indent_line[1]?
+ sig = indent_line + 2;
Why is this adding 2 rather than 1?
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 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] diff: simplify cpp funcname regex
Am 3/5/2014 1:36, schrieb Jeff King:
The current funcname matcher for C files requires one or
more words before the function name, like:
static int foo(int arg)
{
However, some coding styles look like this:
static int
foo(int arg)
{
and we do not match, even though the default regex would.
This patch simplifies the regex significantly. Rather than
trying to handle all the variants of keywords and return
types, we simply look for an identifier at the start of the
line that contains a (, meaning it is either a function
definition or a function call, and then not containing ;
which would indicate it is a call or declaration.
Here is a patch that I'm carrying around since... a while.
What do you think?
The pattern I chose also catches variable definition, not just
functions. That is what I need, but it hurts grep --function-context
That's the reason I didn't forward the patch, yet.
--- 8 ---
From: Johannes Sixt j...@kdbg.org
Date: Tue, 25 Sep 2012 14:08:02 +0200
Subject: [PATCH] userdiff: have 'cpp' hunk header pattern catch more C++ anchor
points
The hunk header pattern 'cpp' is intended for C and C++ source code, but
it is actually not very useful for the latter, and even hurts some
use-cases for the former.
The parts of the pattern have the following flaws:
- The first part matches an identifier followed immediately by a colon and
arbitrary text and is intended to reject goto labels and C++ access
specifiers (public, private, protected). But this pattern also rejects
C++ constructs, which look like this:
MyClass::MyClass()
MyClass::~MyClass()
MyClass::Item MyClass::Find(...
- The second part matches an identifier followed by a list of qualified
names (i.e. identifiers separated by the C++ scope operator '::')
separated by space or '*' followed by an opening parenthesis (with space
between the tokens). It matches function declarations like
struct item* get_head(...
int Outer::Inner::Func(...
Since the pattern requires at least two identifiers, GNU-style function
definitions are ignored:
void
func(...
Moreover, since the pattern does not allow punctuation other than '*',
the following C++ constructs are not recognized:
. template definitions:
templateclass T int func(T arg)
. functions returning references:
const string get_message()
. functions returning templated types:
vectorint foo()
. operator definitions:
Value operator+(Value l, Value r)
- The third part of the pattern finally matches compound definitions. But
it forgets about unions and namespaces, and also skips single-line
definitions
struct random_iterator_tag {};
because no semicolon can occur on the line.
Change the first pattern to require a colon at the end of the line (except
for trailing space and comments), so that it does not reject constructor
or destructor definitions.
Notice that all interesting anchor points begin with an identifier or
keyword. But since there is a large variety of syntactical constructs after
the first word, the simplest is to require only this word and accept
everything else. Therefore, this boils down to a line that begins with a
letter or underscore (optionally preceded by the C++ scope operator '::'
to accept functions returning a type anchored at the global namespace).
Replace the second and third part by a single pattern that picks such a
line.
This has the following desirable consequence:
- All constructs mentioned above are recognized.
and the following likely desirable consequences:
- Definitions of global variables and typedefs are recognized:
int num_entries = 0;
extern const char* help_text;
typedef basic_stringwchar_t wstring;
- Commonly used marco-ized boilerplate code is recognized:
BEGIN_MESSAGE_MAP(CCanvas,CWnd)
Q_DECLARE_METATYPE(MyStruct)
PATTERNS(tex,...)
(The last one is from this very patch.)
but also the following possibly undesirable consequence:
- When a label is not on a line by itself (except for a comment) it is no
longer rejected, but can appear as a hunk header if it occurs at the
beginning of a line:
next:;
IMO, the benefits of the change outweigh the (possible) regressions by a
large margin.
Signed-off-by: Johannes Sixt j...@kdbg.org
---
userdiff.c | 8 +++-
13 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index ed958ef..49b2094 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -125,11 +125,9 @@ PATTERNS(tex,
^(((sub)*section|chapter|part)\\*{0,1}\\{.*)$,
[a-zA-Z@]+|.|[a-zA-Z0-9\x80-\xff]+),
PATTERNS(cpp,
/* Jump targets or access declarations */
-!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n
-/* C/++ functions/methods at top level */
-^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[
\t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n
-/* compound type at top level
