[PATCH 1/3] grep: generalize header grep code to accept arbitrary headers
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano gits...@pobox.com wrote:
enum grep_header_field {
GREP_HEADER_AUTHOR = 0,
- GREP_HEADER_COMMITTER
+ GREP_HEADER_COMMITTER,
+ GREP_HEADER_REFLOG,
+ GREP_HEADER_FIELD_MAX
};
-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
Please add comment to ensure that FIELD_MAX stays at the end; if you
ensure that, the result is much better than the original we know
committer is at the end so add one.
It's probably even better to remove the enum. Say one day I got drunk
and decided to add --grep-encoding. This patch makes sure that I could
submit such a patch even in drunk state.
grep.c | 76 +-
grep.h | 11 +++--
revision.c | 8 +++
3 files changed, 47 insertions(+), 48 deletions(-)
diff --git a/grep.c b/grep.c
index 898be6e..0c72262 100644
--- a/grep.c
+++ b/grep.c
@@ -10,7 +10,7 @@ static int grep_source_is_binary(struct grep_source *gs);
static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
const char *origin, int no,
enum grep_pat_token t,
- enum grep_header_field field)
+ const char *header, size_t header_len)
{
struct grep_pat *p = xcalloc(1, sizeof(*p));
p-pattern = xmemdupz(pat, patlen);
@@ -18,7 +18,8 @@ static struct grep_pat *create_grep_pat(const char *pat,
size_t patlen,
p-origin = origin;
p-no = no;
p-token = t;
- p-field = field;
+ p-header = header;
+ p-header_len = header_len;
return p;
}
@@ -45,7 +46,8 @@ static void do_append_grep_pat(struct grep_pat ***tail,
struct grep_pat *p)
if (!nl)
break;
new_pat = create_grep_pat(nl + 1, len - 1, p-origin,
- p-no, p-token, p-field);
+ p-no, p-token,
+ p-header, p-header_len);
new_pat-next = p-next;
if (!p-next)
*tail = new_pat-next;
@@ -59,11 +61,13 @@ static void do_append_grep_pat(struct grep_pat ***tail,
struct grep_pat *p)
}
}
+/* header must not be freed while grep is running */
void append_header_grep_pattern(struct grep_opt *opt,
- enum grep_header_field field, const char *pat)
+ const char *header, const char *pat)
{
struct grep_pat *p = create_grep_pat(pat, strlen(pat), header, 0,
-GREP_PATTERN_HEAD, field);
+GREP_PATTERN_HEAD,
+header, strlen(header));
do_append_grep_pat(opt-header_tail, p);
}
@@ -76,7 +80,7 @@ void append_grep_pattern(struct grep_opt *opt, const char
*pat,
void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen,
const char *origin, int no, enum grep_pat_token t)
{
- struct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, 0);
+ struct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, NULL,
0);
do_append_grep_pat(opt-pattern_tail, p);
}
@@ -92,7 +96,7 @@ struct grep_opt *grep_opt_dup(const struct grep_opt *opt)
for(pat = opt-pattern_list; pat != NULL; pat = pat-next)
{
if(pat-token == GREP_PATTERN_HEAD)
- append_header_grep_pattern(ret, pat-field,
+ append_header_grep_pattern(ret, pat-header,
pat-pattern);
else
append_grep_pat(ret, pat-pattern, pat-patternlen,
@@ -359,7 +363,7 @@ static void dump_grep_pat(struct grep_pat *p)
switch (p-token) {
default: break;
case GREP_PATTERN_HEAD:
- fprintf(stderr, head %d, p-field); break;
+ fprintf(stderr, head %s, p-header); break;
case GREP_PATTERN_BODY:
fprintf(stderr, body); break;
}
@@ -436,9 +440,10 @@ static struct grep_expr *grep_or_expr(struct grep_expr
*left, struct grep_expr *
static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
{
struct grep_pat *p;
- struct grep_expr *header_expr;
- struct grep_expr *(header_group[GREP_HEADER_FIELD_MAX]);
- enum grep_header_field fld;
+ struct grep_expr *header_expr = NULL;
+ struct grep_expr **header_group;
+ struct string_list header = STRING_LIST_INIT_NODUP;
+ int fld;
if (!opt-header_list)
return NULL;
@@ -446,37
[PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages
Similar to --author/--committer which filters commits by author and
committer header fields. --grep-reflog adds a fake reflog header to
commit and a grep filter to search on that line.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano gits...@pobox.com
wrote:
I am debating myself if it is sane for this option to have no hint
that it is about limiting in its name. --author/--committer
don't and it is clear from the context of the command that they are
not about setting author/committer, so --reflog-message may be
interpreted the same, perhaps.
The entry in the context above talks about multiple occurrence of
that option. Shouldn't this new one also say what happens when it
is
given twice?
Fixed.
I think I wrote prep_header_patterns() and compile_grep_patterns()
carefully enough not to assume the headers are only the author and
committer names, so the various combinations i.e. all-match,
author(s), committer(s), grep(s), and reflog-message(s), should
work
out of the box, but have you actually tested them?
I did not. I do now, tests are also added.
On Fri, Sep 28, 2012 at 12:28 AM, Jeff King p...@peff.net wrote:
I also found the name confusing on first-read. While --author is
an
example in one direction, the fact that --grep is not called
--body
is a counter-example.
I'd much rather see it as --grep-reflog or something. You could
also
do --grep-reflog-message, which would match a later
--grep-reflog-author, but I am not sure anybody would want the
latter,
and it makes the current name a lot longer.
Changed it to --grep-reflog. I was tempted to add --grep-* but I
can't error out if user gives an invalid header. So --grep-reflog
only for now.
Documentation/rev-list-options.txt | 8
revision.c | 20 ++--
t/t7810-grep.sh| 26 ++
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 1fc2a18..aa7cd9d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -51,6 +51,14 @@ endif::git-rev-list[]
commits whose author matches any of the given patterns are
chosen (similarly for multiple `--committer=pattern`).
+--grep-reflog=pattern::
+
+ Limit the commits output to ones with reflog entries that
+ match the specified pattern (regular expression). With
+ more than one `--grep-reflog`, commits whose reflog message
+ matches any of the given patterns are chosen. Ignored unless
+ `--walk-reflogs` is given.
+
--grep=pattern::
Limit the commits output to ones with log message that
diff --git a/revision.c b/revision.c
index f7cf385..cfa0e2e 100644
--- a/revision.c
+++ b/revision.c
@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
} else if ((argcount = parse_long_opt(committer, argv, optarg))) {
add_header_grep(revs, committer, optarg);
return argcount;
+ } else if ((argcount = parse_long_opt(grep-reflog, argv, optarg))) {
+ add_header_grep(revs, reflog, optarg);
+ return argcount;
} else if ((argcount = parse_long_opt(grep, argv, optarg))) {
add_message_grep(revs, optarg);
return argcount;
@@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs,
struct commit *commit)
static int commit_match(struct commit *commit, struct rev_info *opt)
{
+ int retval;
+ struct strbuf buf = STRBUF_INIT;
if (!opt-grep_filter.pattern_list !opt-grep_filter.header_list)
return 1;
- return grep_buffer(opt-grep_filter,
- commit-buffer, strlen(commit-buffer));
+ if (opt-reflog_info) {
+ strbuf_addstr(buf, reflog );
+ get_reflog_message(buf, opt-reflog_info);
+ strbuf_addch(buf, '\n');
+ strbuf_addstr(buf, commit-buffer);
+ }
+ if (buf.len)
+ retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
+ else
+ retval = grep_buffer(opt-grep_filter,
+commit-buffer, strlen(commit-buffer));
+ strbuf_release(buf);
+ return retval;
}
static inline int want_ancestry(struct rev_info *revs)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 91db352..f42a605 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -546,6 +546,32 @@ test_expect_success 'log grep (6)' '
test_cmp expect actual
'
+test_expect_success 'log grep (7)' '
+ git log -g --grep-reflog=commit: third --pretty=tformat:%s actual
+ echo third expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log grep (8)' '
+ git log -g --grep-reflog=commit:
Re: git gui does not open bare repositories
On Fri, Sep 28, 2012 at 9:50 AM, Angelo Borsotti angelo.borso...@gmail.com wrote: I have created a bare repository: $ mkdir remote.git angelo@ANGELO-PC /d/gtest (master) $ git init --bare Initialized empty Git repository in d:/gtest/ This creates a bare repository in d:/gtest, not in d:/gtest/remote.git. You'll need to cd into remote.git to create a bare repo. and then tried to open it with the git gui, but have got the message: remote.git is not a git repository. So this message is correct. Be as it may, as soon as you've initialized the repo as required, git gui will (probably) tell you: Cannot use bare repository: d:/gtest/remote.git So nothing gained there. So I'm wondering what you expect git gui to give you in a bare repo? It seems to me that this tool is specifically aimed at non-bare repos, allowing for creating commits, merges and whatnot; all things you cannot typically do in a bare repo. Frans -- 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 gui does not open bare repositories
Hi, Please remember to reply to all when discussing things on the git mailing list. On Fri, Sep 28, 2012 at 10:29 AM, Angelo Borsotti angelo.borso...@gmail.com wrote: Hello I apologise for having used the wrong script to reproduce the error. This is the right one: angelo@ANGELO-PC /d/gtest (master) $ mkdir remote.git angelo@ANGELO-PC /d/gtest (master) $ cd remote.git angelo@ANGELO-PC /d/gtest/remote.git (master) $ git init --bare Initialized empty Git repository in d:/gtest/remote.git/ Now with the git gui I try to open the d:/gtest/remote.git/ and receive the message that it is not a git repository. I understand that the gui is mostly aimed to non-bare repositories, but in such a case it would be better it could give me a message like, e.g. could not open a bare repository instead of telling me that it is not a git repository (I thought my bare repository was corrupt, and tried to figure out what was wrong with it). Actually git-gui 0.16.0 is telling me that it cannot use bare repositories, much in the vein of what I wrote earlier. Don't know if it matters that I'm on Linux though. Frans -- 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 gui does not open bare repositories
Am 28.09.2012 10:35, schrieb Frans Klaver:
Hi,
Please remember to reply to all when discussing things on the git mailing
list.
On Fri, Sep 28, 2012 at 10:29 AM, Angelo Borsotti
angelo.borso...@gmail.com wrote:
Hello
I apologise for having used the wrong script to reproduce the error.
This is the right one:
angelo@ANGELO-PC /d/gtest (master)
$ mkdir remote.git
angelo@ANGELO-PC /d/gtest (master)
$ cd remote.git
angelo@ANGELO-PC /d/gtest/remote.git (master)
$ git init --bare
Initialized empty Git repository in d:/gtest/remote.git/
Now with the git gui I try to open the d:/gtest/remote.git/ and
receive the message
that it is not a git repository.
I understand that the gui is mostly aimed to non-bare repositories,
but in such a case
it would be better it could give me a message like, e.g. could not
open a bare repository
instead of telling me that it is not a git repository (I thought my
bare repository was
corrupt, and tried to figure out what was wrong with it).
Actually git-gui 0.16.0 is telling me that it cannot use bare
repositories, much in the vein of what I wrote earlier. Don't know if
it matters that I'm on Linux though.
I get Not a Git repository: remote.git as well, when I run
git gui somewhere (i.e. not in remote.git) and the select
Open Existing Repository.
I get Cannot use bare repository: .../remote.git when I run git gui
from inside the remote.git directory.
Both on WinXP with msysGit.
Stefan
--
/dev/random says: Unless you're the lead dog, the view never changes.
python -c print
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] completion: fix non-critical bugs in __gitcomp() tests
When invoking __gitcomp() the $cur variable should hold the word to be completed, but two tests (checking the handling of prefix and suffix) set it to a bogus value. Luckily the bogus values haven't influenced the tests' correctness, because these two tests pass the word-to-be-matched as argument to __gitcomp(), which overrides $cur anyway. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9902-completion.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 92d7eb47..e7657537 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -121,7 +121,7 @@ test_expect_success '__gitcomp - prefix' ' EOF ( local -a COMPREPLY - cur=branch.me + cur=branch.maint.me __gitcomp remote merge mergeoptions rebase branch.maint. me IFS=$newline @@ -137,7 +137,7 @@ test_expect_success '__gitcomp - suffix' ' EOF ( local -a COMPREPLY - cur=branch.me + cur=branch.ma __gitcomp master maint next pu branch. ma . IFS=$newline -- 1.7.12.1.438.g7dfa67b -- 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 3/5] completion: add tests for the __gitcomp_nl() completion helper function
Test __gitcomp_nl()'s basic functionality, i.e. that trailing space,
prefix, and suffix are added correctly.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 47 +++
1 file changed, 47 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f5e68834..01f33220 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -146,6 +146,53 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
+test_expect_success '__gitcomp_nl - trailing space' '
+ sed -e s/Z$// expected -\EOF
+ maint Z
+ master Z
+ EOF
+ (
+ local -a COMPREPLY
+ cur=m
+ __gitcomp_nl maint${newline}master${newline}pu
+ IFS=$newline
+ echo ${COMPREPLY[*]} out
+ )
+ test_cmp expected out
+'
+
+test_expect_success '__gitcomp_nl - prefix' '
+ sed -e s/Z$// expected -\EOF
+ --fixup=maint Z
+ --fixup=master Z
+ EOF
+ (
+ local -a COMPREPLY
+ cur=--fixup=m
+ __gitcomp_nl maint${newline}master${newline}next${newline}pu\
+ --fixup= m
+ IFS=$newline
+ echo ${COMPREPLY[*]} out
+ )
+ test_cmp expected out
+'
+
+test_expect_success '__gitcomp_nl - suffix' '
+ sed -e s/Z$// expected -\EOF
+ branch.master.Z
+ branch.maint.Z
+ EOF
+ (
+ local -a COMPREPLY
+ cur=branch.ma
+ __gitcomp_nl master${newline}maint${newline}next${newline}pu\
+ branch. ma .
+ IFS=$newline
+ echo ${COMPREPLY[*]} out
+ )
+ test_cmp expected out
+'
+
test_expect_success 'basic' '
run_completion git
# built-in
--
1.7.12.1.438.g7dfa67b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] completion: fix args of run_completion() test helper
To simulate the the user hit 'git TAB, one of the completion tests
sets up the rather strange command line
git
i.e. the second word on the command line consists of two double
quotes. However, this is not what happens for real, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e7657537..f5e68834 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
@@ -147,7 +147,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion git \\
+ run_completion git
# built-in
grep -q ^add \$ out
# script
@@ -155,7 +155,7 @@ test_expect_success 'basic' '
# plumbing
! grep -q ^ls-files \$ out
- run_completion git f
+ run_completion git f
! grep -q -v ^f out
'
--
1.7.12.1.438.g7dfa67b
--
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 4/5] completion: test __gitcomp() and __gitcomp_nl() with expandable words
compgen performs expansion on all words in the list given to its -W
option. This breaks completion in various ways if one of those words
can be expanded. The funniest breakage is probably this, as the
command_not_found_handle kicks in:
$ git branch '$(foo)'
$ git branch TABNo command 'foo' found, did you mean:
Command 'fio' from package 'fio' (universe)
Command 'goo' from package 'goo' (universe)
Command 'fop' from package 'fop' (main)
Command 'fox' from package 'objcryst-fox' (universe)
Command 'xoo' from package 'xoo' (universe)
Command 'zoo' from package 'zoo' (universe)
foo: command not found
Document this breakage with tests.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 32
1 file changed, 32 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 01f33220..4af2a149 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -146,6 +146,22 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp - expandable words' '
+ sed -e s/Z$// expected -\EOF
+ $foo Z
+ $(bar) Z
+ ${baz} Z
+ EOF
+ (
+ local -a COMPREPLY
+ cur=
+ __gitcomp \$foo \$(bar) \${baz}
+ IFS=$newline
+ echo ${COMPREPLY[*]} out
+ )
+ test_cmp expected out
+'
+
test_expect_success '__gitcomp_nl - trailing space' '
sed -e s/Z$// expected -\EOF
maint Z
@@ -193,6 +209,22 @@ test_expect_success '__gitcomp_nl - suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp_nl - expandable words' '
+ sed -e s/Z$// expected -\EOF
+ $foo Z
+ $(bar) Z
+ ${baz} Z
+ EOF
+ (
+ local -a COMPREPLY
+ cur=
+ __gitcomp_nl \$foo$newline\$(bar)$newline\${baz}
+ IFS=$newline
+ echo ${COMPREPLY[*]} out
+ )
+ test_cmp expected out
+'
+
test_expect_success 'basic' '
run_completion git
# built-in
--
1.7.12.1.438.g7dfa67b
--
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
[POC PATCH 5/5] completion: avoid compgen to fix expansion issues in __gitcomp_nl()
compgen performs expansion on all words in the list given to its -W
option. This breaks completion in various ways if one of those words
can be expanded, as demonstrated by failing tests added in a previous
commit.
In __gitcomp_nl() we use only a small subset of compgen's
functionality: to filter matching possible completion words, and to
add a prefix and/or suffix to each of them. Since the list of words
is newline-separated, we can do all these just as well with a little
sed script. This way we can get rid of compgen and its additional
expansion of all words and don't need to perform excessive quoting to
circumvent it.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
We can also get rid of compgen in __gitcomp(); I already have working code
for that, but it still needs a bit of cleanup and commit messages.
contrib/completion/git-completion.bash | 6 +-
t/t9902-completion.sh | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e09..2df865fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -261,7 +261,11 @@ __gitcomp ()
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(echo $1 |sed -n /^${3-$cur}/ {
+ s/^/${2-}/
+ s/$/${4- }/
+ p
+ }))
}
__git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4af2a149..0ee91f64 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -209,7 +209,7 @@ test_expect_success '__gitcomp_nl - suffix' '
test_cmp expected out
'
-test_expect_failure '__gitcomp_nl - expandable words' '
+test_expect_success '__gitcomp_nl - expandable words' '
sed -e s/Z$// expected -\EOF
$foo Z
$(bar) Z
--
1.7.12.1.438.g7dfa67b
--
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: Stopping git-svn pulling a particular branch
On Fri, Sep 28, 2012 at 2:58 PM, Jon Seymour jon.seym...@gmail.com wrote: G'day An svn developer created a branch from a subdirectory of the trunk rather than by copying trunk itself. I want to avoid pulling this branch into my git repo with git svn fetch because the re-rooting pulls in too much history and content that I don't want. Is there any easy way to achieve this? I tried used the --ignore-paths option, but this doesn't seem suited to the purpose. I can delete the SVN branch if that will help, but past experience suggests that it won't. Answering my own question. There is an undocumented option to git-svn --ignore-refs which can be used to achieve this. I found that I had to manually delete the directories corresponding to the bogus branches from .git/svn/refs/remotes, then re-run git svn fetch with the --ignore-refs option set to a regex that matched the bogus branches. This allowed me to fetch everything else I needed to fetch and advanced the maxRevs metadata in .git/svn/.metadata past the problematic branches so that subsequent svn fetch calls avoided the attempts to fetch the bogus branches. I'll draft a documentation patch when I get a chance... jon. -- 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] Add __git_ps1_pc to use as PROMPT_COMMAND
Hi again
After the previous comments, I decided to attempt it using
PROMPT_COMMAND rather than calling a function from command substitution
in PS1. This new code works and doesn't have the wrapping issue anymore.
I've simplified some of the coloring stuff and for now there's no
parameters to customize the PS1. Users will have to customize the
function itself for now...
As a small observation, I think it's more logical to use PROMPT_COMMAND,
since that explicitly uses a function, rather than using command
substitution when setting PS1.
Obviously, as I didn't remove __git_ps1, the older use is unchanged, but
now there is about 80% duplication of code between __git_ps1 and
__git-ps1_pc
And AFAICT zsh is not supported here due to different escape characters
(\ in bash, % in zsh)
Please let me know if and how this can/should be integrated!
Cheers
Simon
The function can set the PS1 varible optionally with
Colored hints about the state of the tree when
GIT_PS1_SHOWCOLORHINTS is set to a nonempty value.
This version doesn't accept arguments to customize the
prompt. And it has not been tested with zsh.
Signed-off-by: Simon Oosthoek soosth...@nieuwland.nl
---
contrib/completion/git-prompt.sh | 135
++
1 file changed, 135 insertions(+)
diff --git a/contrib/completion/git-prompt.sh
b/contrib/completion/git-prompt.sh
index 29b1ec9..8ed6b84 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -48,6 +48,16 @@
# find one, or @{upstream} otherwise. Once you have set
# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
# setting the bash.showUpstream config variable.
+#
+# If you would like colored hints in the branchname shown in the prompt
+# you can set PROMPT_COMMAND (bash) to __git_ps1_pc and export the +#
variables GIT_PS1_SHOWDIRTYSTATE and GIT_PS1_SHOWCOLORHINT:
+# example:
+# export GIT_PS1_SHOWDIRTYSTATE=true
+# export GIT_PS1_SHOWCOLORHINT=true
+# export PROMPT_COMMAND=__git_ps1_pc
+# The prompt command function will directly set PS1, in order to +#
insert color commands in a way that doesn't mess up wrapping.
# __gitdir accepts 0 or 1 arguments (i.e., location)
# returns location of .git repo
@@ -287,3 +297,128 @@ __git_ps1 ()
printf -- ${1:- (%s)} $c${b##refs/heads/}${f:+ $f}$r$p
fi
}
+
+
+# __git_ps1_pc accepts 0 arguments (for now)
+# It is meant to be used as PROMPT_COMMAND, it sets PS1
+__git_ps1_pc ()
+{
+ local g=$(__gitdir)
+ if [ -n $g ]; then
+ local r=
+ local b=
+ if [ -f $g/rebase-merge/interactive ]; then
+ r=|REBASE-i
+ b=$(cat $g/rebase-merge/head-name)
+ elif [ -d $g/rebase-merge ]; then
+ r=|REBASE-m
+ b=$(cat $g/rebase-merge/head-name)
+ else
+ if [ -d $g/rebase-apply ]; then
+ if [ -f $g/rebase-apply/rebasing ]; then
+ r=|REBASE
+ elif [ -f $g/rebase-apply/applying ]; then
+ r=|AM
+ else
+ r=|AM/REBASE
+ fi
+ elif [ -f $g/MERGE_HEAD ]; then
+ r=|MERGING
+ elif [ -f $g/CHERRY_PICK_HEAD ]; then
+ r=|CHERRY-PICKING
+ elif [ -f $g/BISECT_LOG ]; then
+ r=|BISECTING
+ fi
+
+ b=$(git symbolic-ref HEAD 2/dev/null) || {
+
+ b=$(
+ case ${GIT_PS1_DESCRIBE_STYLE-} in
+ (contains)
+ git describe --contains HEAD ;;
+ (branch)
+ git describe --contains --all HEAD ;;
+ (describe)
+ git describe HEAD ;;
+ (* | default)
+ git describe --tags --exact-match HEAD
;;
+ esac 2/dev/null) ||
+
+ b=$(cut -c1-7 $g/HEAD 2/dev/null)... ||
+ b=unknown
+ b=($b)
+ }
+ fi
+
+ local w=
+ local i=
+ local s=
+ local u=
+ local c=
+ local p=
+
+ if [ true = $(git rev-parse --is-inside-git-dir
2/dev/null) ]; then
+ if [ true = $(git rev-parse --is-bare-repository 2/dev/null) ];
then
+ c=BARE:
+ else
+
Re: Using bitmaps to accelerate fetch and clone
On Thu, Sep 27, 2012 at 7:47 AM, Shawn Pearce spea...@spearce.org wrote: * https://git.eclipse.org/r/7939 Defines the new E003 index format and the bit set implementation logic. Quote from the patch's message: Currently, the new index format can only be used with pack files that contain a complete closure of the object graph e.g. the result of a garbage collection. You mentioned this before in your idea mail a while back. I wonder if it's worth storing bitmaps for all packs, not just the self contained ones. We could have one leaf bitmap per pack to mark all leaves where we'll need to traverse outside the pack. Commit leaves are the best as we can potentially reuse commit bitmaps from other packs. Tree leaves will be followed in the normal/slow way. For connectivity check, fewer trees/commits to deflate/parse means less time. And connectivity check is done on every git-fetch (I suspect the other end of a push also has the same check). It's not unusual for me to fetch some repos once every few months so these incomplete packs could be quite big and it'll take some time for gc --auto to kick in (of course we could adjust gc --auto to start based on the number of non-bitmapped objects, in additional to number of packs). -- 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
[PATCH 00/21] git p4: work on cygwin
This series fixes problems in git-p4, and its tests, so that git-p4 works on the cygwin platform. See the wiki for info on how to get started on cygwin: https://git.wiki.kernel.org/index.php/GitP4 Testing by people who use cygwin would be appreciated. It would be good to support cygwin more regularly. Anyone who had time to contribute to testing on cygwin, and reporting problems, would be welcome. There's more work requried to support msysgit. Those patches are not in good enough shape to ship out yet, but a lot of what is in this series is required for msysgit too. These patches: - fix bugs in git-p4 related to issues found on cygwin - cleanup some ugly code in git-p4 observed in error paths while getting tests to work on cygwin - simplify and refactor code and tests to make cygwin changes easier - handle newline and path issues for cygwin platform - speed up some aspects of git-p4 by removing extra shell invocations Pete Wyckoff (21): git p4: temp branch name should use / even on windows git p4: remove unused imports git p4: generate better error message for bad depot path git p4: fix error message when describe -s fails git p4 test: use client_view to build the initial client git p4 test: use client_view in t9806 git p4 test: start p4d inside its db dir git p4 test: translate windows paths for cygwin git p4: remove unreachable windows \r\n conversion code git p4: scrub crlf for utf16 files on windows git p4 test: newline handling git p4 test: use LineEnd unix in windows tests too git p4 test: avoid wildcard * in windows git p4: cygwin p4 client does not mark read-only git p4 test: disable chmod test for cygwin git p4: disable read-only attribute before deleting git p4: avoid shell when mapping users git p4: avoid shell when invoking git rev-list git p4: avoid shell when invoking git config --get-all git p4: avoid shell when calling git config git p4: introduce gitConfigBool git-p4.py | 122 -- t/lib-git-p4.sh | 60 +++-- t/t9800-git-p4-basic.sh | 5 ++ t/t9802-git-p4-filetype.sh| 117 t/t9806-git-p4-options.sh | 50 - t/t9807-git-p4-submit.sh | 14 - t/t9809-git-p4-client-view.sh | 14 +++-- t/t9812-git-p4-wildcards.sh | 37 ++--- t/t9815-git-p4-submit-fail.sh | 4 +- t/test-lib.sh | 3 ++ 10 files changed, 330 insertions(+), 96 deletions(-) -- 1.7.12.1.403.g28165e1 -- 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 01/21] git p4: temp branch name should use / even on windows
Commit fed2369 (git-p4: Search for parent commit on branch creation, 2012-01-25) uses temporary branches to help find the parent of a new p4 branch. The temp branches are of the form git-p4-tmp/%d for some p4 change number. Mistakenly, this string was made using os.path.join() instead of just string concatenation. On windows, this turns into a backslash (\), which is not allowed in git branch names. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 882b1bb..1e7a22a 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2599,7 +2599,7 @@ class P4Sync(Command, P4UserMap): blob = None if len(parent) 0: -tempBranch = os.path.join(self.tempBranchLocation, %d % (change)) +tempBranch = %s/%d % (self.tempBranchLocation, change) if self.verbose: print Creating temporary branch: + tempBranch self.commit(description, filesForCommit, tempBranch) -- 1.7.12.1.403.g28165e1 -- 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 02/21] git p4: remove unused imports
Found by pyflakes checker tool. Modules shelve, getopt were unused. Module os.path is exported by os. Reformat one-per-line as is PEP008 suggested style. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/git-p4.py b/git-p4.py index 1e7a22a..97699ef 100755 --- a/git-p4.py +++ b/git-p4.py @@ -7,10 +7,16 @@ #2007 Trolltech ASA # License: MIT http://www.opensource.org/licenses/mit-license.php # - -import optparse, sys, os, marshal, subprocess, shelve -import tempfile, getopt, os.path, time, platform -import re, shutil +import sys +import os +import optparse +import marshal +import subprocess +import tempfile +import time +import platform +import re +import shutil verbose = False -- 1.7.12.1.403.g28165e1 -- 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 03/21] git p4: generate better error message for bad depot path
Depot paths must start with //. Exit with a better explanation
when a bad depot path is supplied.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 1 +
t/t9800-git-p4-basic.sh | 5 +
2 files changed, 6 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index 97699ef..eef5c94 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3035,6 +3035,7 @@ class P4Clone(P4Sync):
self.cloneExclude = [/+p for p in self.cloneExclude]
for p in depotPaths:
if not p.startswith(//):
+sys.stderr.write('Depot paths must start with //: %s\n' % p)
return False
if not self.cloneDestination:
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index b7ad716..c5f4c88 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '
)
'
+test_expect_success 'depot typo error' '
+ test_must_fail git p4 clone --dest=$git /depot 2errs
+ grep -q Depot paths must start with errs
+'
+
test_expect_success 'git p4 clone @all' '
git p4 clone --dest=$git //depot@all
test_when_finished cleanup_git
--
1.7.12.1.403.g28165e1
--
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 04/21] git p4: fix error message when describe -s fails
The output was a bit nonsensical, including a bare %d. Fix it
to make it easier to understand.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index eef5c94..d7ee4b4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2679,7 +2679,8 @@ class P4Sync(Command, P4UserMap):
if r.has_key('time'):
newestTime = int(r['time'])
if newestTime is None:
-die(\describe -s\ on newest change %d did not give a time)
+die(Output from \describe -s\ on newest change %d did not give
a time %
+newestRevision)
details[time] = newestTime
self.updateOptionDict(details)
--
1.7.12.1.403.g28165e1
--
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 05/21] git p4 test: use client_view to build the initial client
Simplify the code a bit by using an existing function.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7061dce..890ee60 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -74,15 +74,8 @@ start_p4d() {
fi
# build a client
- (
- cd $cli
- p4 client -i -EOF
- Client: client
- Description: client
- Root: $cli
- View: //depot/... //client/...
- EOF
- )
+ client_view //depot/... //client/...
+
return 0
}
--
1.7.12.1.403.g28165e1
--
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 06/21] git p4 test: use client_view in t9806
Use the standard client_view function from lib-git-p4.sh
instead of building one by hand. This requires a bit of
rework, using the current value of $P4CLIENT for the client
name. It also reorganizes the test to isolate changes to
$P4CLIENT and $cli in a subshell.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 4 ++--
t/t9806-git-p4-options.sh | 50 ++-
2 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 890ee60..d558dd0 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -116,8 +116,8 @@ marshal_dump() {
client_view() {
(
cat -EOF
- Client: client
- Description: client
+ Client: $P4CLIENT
+ Description: $P4CLIENT
Root: $cli
View:
EOF
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index fa40cc8..37ca30a 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '
exec /dev/null
test_must_fail git p4 clone --dest=$git --use-client-spec
)
- cli2=$(test-path-utils real_path $TRASH_DIRECTORY/cli2)
+ # build a different client
+ cli2=$TRASH_DIRECTORY/cli2
mkdir -p $cli2
test_when_finished rmdir \$cli2\
- (
- cd $cli2
- p4 client -i -EOF
- Client: client2
- Description: client2
- Root: $cli2
- View: //depot/sub/... //client2/bus/...
- EOF
- )
- P4CLIENT=client2
test_when_finished cleanup_git
- git p4 clone --dest=$git --use-client-spec //depot/...
- (
- cd $git
- test_path_is_file bus/dir/f4
- test_path_is_missing file1
- )
- cleanup_git
-
- # same thing again, this time with variable instead of option
(
- cd $git
- git init
- git config git-p4.useClientSpec true
- git p4 sync //depot/...
- git checkout -b master p4/master
- test_path_is_file bus/dir/f4
- test_path_is_missing file1
+ # group P4CLIENT and cli changes in a sub-shell
+ P4CLIENT=client2
+ cli=$cli2
+ client_view //depot/sub/... //client2/bus/...
+ git p4 clone --dest=$git --use-client-spec //depot/...
+ (
+ cd $git
+ test_path_is_file bus/dir/f4
+ test_path_is_missing file1
+ )
+ cleanup_git
+ # same thing again, this time with variable instead of option
+ (
+ cd $git
+ git init
+ git config git-p4.useClientSpec true
+ git p4 sync //depot/...
+ git checkout -b master p4/master
+ test_path_is_file bus/dir/f4
+ test_path_is_missing file1
+ )
)
'
--
1.7.12.1.403.g28165e1
--
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 07/21] git p4 test: start p4d inside its db dir
This will avoid having to do native path conversion for
windows. Also may be a bit cleaner always to know that p4d
has that working directory, instead of wherever the function
was called from.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index d558dd0..402d736 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -40,8 +40,11 @@ start_p4d() {
mkdir -p $db $cli $git
rm -f $pidfile
(
- p4d -q -r $db -p $P4DPORT
- echo $! $pidfile
+ cd $db
+ {
+ p4d -q -p $P4DPORT
+ echo $! $pidfile
+ }
)
# This gives p4d a long time to start up, as it can be
--
1.7.12.1.403.g28165e1
--
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 08/21] git p4 test: translate windows paths for cygwin
Native windows binaries do not understand posix-like
path mapping offered by cygwin. Convert paths to native
using cygpath --windows before presenting them to p4d.
This is done using the AltRoots mechanism of p4. Both the
posix and windows forms are put in the client specification,
allowing p4 to find its location by native path even though
the environment reports a different PWD.
Shell operations in tests will use the normal form of $cli,
which will look like a posix path in cygwin, while p4 will
use AltRoots to match against the windows form of the working
directory.
Thanks-to: Sebastian Schuberth sschube...@gmail.com
Thanks-to: Johannes Sixt j...@kdbg.org
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 24 ++--
t/test-lib.sh | 3 +++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 402d736..e2941ac 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -8,7 +8,8 @@ TEST_NO_CREATE_REPO=NoThanks
. ./test-lib.sh
-if ! test_have_prereq PYTHON; then
+if ! test_have_prereq PYTHON
+then
skip_all='skipping git p4 tests; python not available'
test_done
fi
@@ -17,6 +18,24 @@ fi
test_done
}
+# On cygwin, the NT version of Perforce can be used. When giving
+# it paths, either on the command-line or in client specifications,
+# be sure to use the native windows form.
+#
+# Older versions of perforce were available compiled natively for
+# cygwin. Those do not accept native windows paths, so make sure
+# not to convert for them.
+native_path() {
+ path=$1
+ if test_have_prereq CYGWIN ! p4 -V | grep -q CYGWIN
+ then
+ path=$(cygpath --windows $path)
+ else
+ path=$(test-path-utils real_path $path)
+ fi
+ echo $path
+}
+
# Try to pick a unique port: guess a large number, then hope
# no more than one of each test is running.
#
@@ -32,7 +51,7 @@ P4EDITOR=:
export P4PORT P4CLIENT P4EDITOR
db=$TRASH_DIRECTORY/db
-cli=$(test-path-utils real_path $TRASH_DIRECTORY/cli)
+cli=$TRASH_DIRECTORY/cli
git=$TRASH_DIRECTORY/git
pidfile=$TRASH_DIRECTORY/p4d.pid
@@ -122,6 +141,7 @@ client_view() {
Client: $P4CLIENT
Description: $P4CLIENT
Root: $cli
+ AltRoots: $(native_path $cli)
View:
EOF
for arg ; do
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f8e3733..fd04870 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -624,12 +624,14 @@ case $(uname -s) in
# backslashes in pathspec are converted to '/'
# exec does not inherit the PID
test_set_prereq MINGW
+ test_set_prereq NOT_CYGWIN
test_set_prereq SED_STRIPS_CR
;;
*CYGWIN*)
test_set_prereq POSIXPERM
test_set_prereq EXECKEEPSPID
test_set_prereq NOT_MINGW
+ test_set_prereq CYGWIN
test_set_prereq SED_STRIPS_CR
;;
*)
@@ -637,6 +639,7 @@ case $(uname -s) in
test_set_prereq BSLASHPSPEC
test_set_prereq EXECKEEPSPID
test_set_prereq NOT_MINGW
+ test_set_prereq NOT_CYGWIN
;;
esac
--
1.7.12.1.403.g28165e1
--
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 09/21] git p4: remove unreachable windows \r\n conversion code
Replacing \r\n with \n on windows was added in c1f9197 (Replace \r\n with \n when importing from p4 on Windows, 2007-05-24), to work around an oddity with p4 print on windows. Text files are printed with \r\r\n endings, regardless of whether they were created on unix or windows, and regardless of the client LineEnd setting. As of d2c6dd3 (use p4CmdList() to get file contents in Python dicts. This is more robust., 2007-05-23), git-p4 uses p4 -G print, which generates files in a raw format. As the native line ending format if p4 is \n, there will be no \r\n in the raw text. Actually, it is possible to generate a text file so that the p4 representation includes embedded \r\n, even though this is not normal on either windows or unix. In that case the code would have mistakenly stripped them out, but now they will be left intact. More information on how p4 deals with line endings is here: http://kb.perforce.com/article/63 Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 9 - 1 file changed, 9 deletions(-) diff --git a/git-p4.py b/git-p4.py index d7ee4b4..b773b09 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2064,15 +2064,6 @@ class P4Sync(Command, P4UserMap): print \nIgnoring apple filetype file %s % file['depotFile'] return -# Perhaps windows wants unicode, utf16 newlines translated too; -# but this is not doing it. -if self.isWindows and type_base == text: -mangled = [] -for data in contents: -data = data.replace(\r\n, \n) -mangled.append(data) -contents = mangled - # Note that we do not try to de-mangle keywords on utf16 files, # even though in theory somebody may want that. pattern = p4_keywords_regexp_for_type(type_base, type_mods) -- 1.7.12.1.403.g28165e1 -- 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 10/21] git p4: scrub crlf for utf16 files on windows
Files of type utf16 are handled with p4 print instead of the normal p4 -G print interface due to how the latter does not produce correct output. See 55aa571 (git-p4: handle utf16 filetype properly, 2011-09-17) for details. On windows, though, p4 print can not be told which line endings to use, as there is no underlying client, and always chooses crlf, even for utf16 files. Convert the \r\n into \n when importing utf16 files. The fix for this is complex, in that the problem is a property of the NT version of p4. There are old versions of p4 that were compiled directly for cygwin that should not be subjected to text replacement. The right check here, then, is to look at the p4 version, not the OS version. Note also that on cygwin, platform.system() is CYGWIN_NT-5.1 or similar, not Windows. Add a function to memoize the p4 version string and use it to check for /NT, indicating the Windows build of p4. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index b773b09..5b2f73d 100755 --- a/git-p4.py +++ b/git-p4.py @@ -147,6 +147,22 @@ def p4_system(cmd): expand = isinstance(real_cmd, basestring) subprocess.check_call(real_cmd, shell=expand) +_p4_version_string = None +def p4_version_string(): +Read the version string, showing just the last line, which + hopefully is the interesting version bit. + + $ p4 -V + Perforce - The Fast Software Configuration Management System. + Copyright 1995-2011 Perforce Software. All rights reserved. + Rev. P4/NTX86/2011.1/393975 (2011/12/16). + +global _p4_version_string +if not _p4_version_string: +a = p4_read_pipe_lines([-V]) +_p4_version_string = a[-1].rstrip() +return _p4_version_string + def p4_integrate(src, dest): p4_system([integrate, -Dt, wildcard_encode(src), wildcard_encode(dest)]) @@ -1903,7 +1919,6 @@ class P4Sync(Command, P4UserMap): self.syncWithOrigin = True self.importIntoRemotes = True self.maxChanges = -self.isWindows = (platform.system() == Windows) self.keepRepoPath = False self.depotPaths = None self.p4BranchesInGit = [] @@ -2048,7 +2063,14 @@ class P4Sync(Command, P4UserMap): # operations. utf16 is converted to ascii or utf8, perhaps. # But ascii text saved as -t utf16 is completely mangled. # Invoke print -o to get the real contents. +# +# On windows, the newlines will always be mangled by print, so put +# them back too. This is not needed to the cygwin windows version, +# just the native NT type. +# text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) +if p4_version_string().find(/NT) = 0: +text = text.replace(\r\n, \n) contents = [ text ] if type_base == apple: -- 1.7.12.1.403.g28165e1 -- 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 11/21] git p4 test: newline handling
P4 stores newlines in the depos as \n. By default, git does this too, both on unix and windows. Test to make sure that this stays true. Both git and p4 have mechanisms to use \r\n in the working directory. Exercise these. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9802-git-p4-filetype.sh | 117 + 1 file changed, 117 insertions(+) diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh index 21924df..c5ab626 100755 --- a/t/t9802-git-p4-filetype.sh +++ b/t/t9802-git-p4-filetype.sh @@ -8,6 +8,123 @@ test_expect_success 'start p4d' ' start_p4d ' +# +# This series of tests checks newline handling Both p4 and +# git store newlines as \n, and have options to choose how +# newlines appear in checked-out files. +# +test_expect_success 'p4 client newlines, unix' ' + ( + cd $cli + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + printf unix\ncrlf\n f-unix + printf unix\r\ncrlf\r\n f-unix-as-crlf + p4 add -t text f-unix + p4 submit -d f-unix + + # LineEnd: unix; should be no change after sync + cp f-unix f-unix-orig + p4 sync -f + test_cmp f-unix-orig f-unix + + # make sure stored in repo as unix newlines + # use sed to eat python-appened newline + p4 -G print //depot/f-unix | marshal_dump data 2 |\ + sed \$d f-unix-p4-print + test_cmp f-unix-orig f-unix-p4-print + + # switch to win, make sure lf - crlf + p4 client -o | sed /LineEnd/s/:.*/:win/ | p4 client -i + p4 sync -f + test_cmp f-unix-as-crlf f-unix + ) +' + +test_expect_success 'p4 client newlines, win' ' + ( + cd $cli + p4 client -o | sed /LineEnd/s/:.*/:win/ | p4 client -i + printf win\r\ncrlf\r\n f-win + printf win\ncrlf\n f-win-as-lf + p4 add -t text f-win + p4 submit -d f-win + + # LineEnd: win; should be no change after sync + cp f-win f-win-orig + p4 sync -f + test_cmp f-win-orig f-win + + # make sure stored in repo as unix newlines + # use sed to eat python-appened newline + p4 -G print //depot/f-win | marshal_dump data 2 |\ + sed \$d f-win-p4-print + test_cmp f-win-as-lf f-win-p4-print + + # switch to unix, make sure lf - crlf + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + p4 sync -f + test_cmp f-win-as-lf f-win + ) +' + +test_expect_success 'ensure blobs store only lf newlines' ' + test_when_finished cleanup_git + ( + cd $git + git init + git p4 sync //depot@all + + # verify the files in .git are stored only with newlines + o=$(git ls-tree p4/master -- f-unix | cut -f1 | cut -d\ -f3) + git cat-file blob $o f-unix-blob + test_cmp $cli/f-unix-orig f-unix-blob + + o=$(git ls-tree p4/master -- f-win | cut -f1 | cut -d\ -f3) + git cat-file blob $o f-win-blob + test_cmp $cli/f-win-as-lf f-win-blob + + rm f-unix-blob f-win-blob + ) +' + +test_expect_success 'gitattributes setting eol=lf produces lf newlines' ' + test_when_finished cleanup_git + ( + # checkout the files and make sure core.eol works as planned + cd $git + git init + echo * eol=lf .gitattributes + git p4 sync //depot@all + git checkout master + test_cmp $cli/f-unix-orig f-unix + test_cmp $cli/f-win-as-lf f-win + ) +' + +test_expect_success 'gitattributes setting eol=crlf produces crlf newlines' ' + test_when_finished cleanup_git + ( + # checkout the files and make sure core.eol works as planned + cd $git + git init + echo * eol=crlf .gitattributes + git p4 sync //depot@all + git checkout master + test_cmp $cli/f-unix-as-crlf f-unix + test_cmp $cli/f-win-orig f-win + ) +' + +test_expect_success 'crlf cleanup' ' + ( + cd $cli + rm f-unix-orig f-unix-as-crlf + rm f-win-orig f-win-as-lf + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + p4 sync -f + ) +' + test_expect_success 'utf-16 file create' ' ( cd $cli -- 1.7.12.1.403.g28165e1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org
[PATCH 12/21] git p4 test: use LineEnd unix in windows tests too
In all clients, even those created on windows, use unix line
endings. This makes it possible to verify file contents without
doing OS-specific comparisons in all the tests.
Tests in t9802-git-p4-filetype.sh are used to make sure that
the other LineEnd options continue to work.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index e2941ac..fbd55ea 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -142,6 +142,7 @@ client_view() {
Description: $P4CLIENT
Root: $cli
AltRoots: $(native_path $cli)
+ LineEnd: unix
View:
EOF
for arg ; do
--
1.7.12.1.403.g28165e1
--
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 13/21] git p4 test: avoid wildcard * in windows
This character is not valid in windows filenames, even though it can appear in p4 depot paths. Avoid using it in tests on windows, both mingw and cygwin. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9809-git-p4-client-view.sh | 10 -- t/t9812-git-p4-wildcards.sh | 37 + 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh index 281be29..fd8fa89 100755 --- a/t/t9809-git-p4-client-view.sh +++ b/t/t9809-git-p4-client-view.sh @@ -365,7 +365,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' ' ( cd $git echo git-wild-hash dir1/git-wild#hash - echo git-wild-star dir1/git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo git-wild-star dir1/git-wild\*star + fi echo git-wild-at dir1/git-wild@at echo git-wild-percent dir1/git-wild%percent git add dir1/git-wild* @@ -376,7 +379,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' ' ( cd $cli test_path_is_file dir1/git-wild#hash - test_path_is_file dir1/git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_path_is_file dir1/git-wild\*star + fi test_path_is_file dir1/git-wild@at test_path_is_file dir1/git-wild%percent ) diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh index 143d413..6763325 100755 --- a/t/t9812-git-p4-wildcards.sh +++ b/t/t9812-git-p4-wildcards.sh @@ -14,7 +14,10 @@ test_expect_success 'add p4 files with wildcards in the names' ' printf file2\nhas\nsome\nrandom\ntext\n file2 p4 add file2 echo file-wild-hash file-wild#hash - echo file-wild-star file-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo file-wild-star file-wild\*star + fi echo file-wild-at file-wild@at echo file-wild-percent file-wild%percent p4 add -f file-wild* @@ -28,7 +31,10 @@ test_expect_success 'wildcard files git p4 clone' ' ( cd $git test -f file-wild#hash - test -f file-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test -f file-wild\*star + fi test -f file-wild@at test -f file-wild%percent ) @@ -40,7 +46,10 @@ test_expect_success 'wildcard files submit back to p4, add' ' ( cd $git echo git-wild-hash git-wild#hash - echo git-wild-star git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo git-wild-star git-wild\*star + fi echo git-wild-at git-wild@at echo git-wild-percent git-wild%percent git add git-wild* @@ -51,7 +60,10 @@ test_expect_success 'wildcard files submit back to p4, add' ' ( cd $cli test_path_is_file git-wild#hash - test_path_is_file git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_path_is_file git-wild\*star + fi test_path_is_file git-wild@at test_path_is_file git-wild%percent ) @@ -63,7 +75,10 @@ test_expect_success 'wildcard files submit back to p4, modify' ' ( cd $git echo new-line git-wild#hash - echo new-line git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo new-line git-wild\*star + fi echo new-line git-wild@at echo new-line git-wild%percent git add git-wild* @@ -74,7 +89,10 @@ test_expect_success 'wildcard files submit back to p4, modify' ' ( cd $cli test_line_count = 2 git-wild#hash - test_line_count = 2 git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_line_count = 2 git-wild\*star + fi test_line_count = 2 git-wild@at test_line_count = 2 git-wild%percent ) @@ -87,7 +105,7 @@ test_expect_success 'wildcard files submit back to p4, copy' ' cd $git cp file2 git-wild-cp#hash git add git-wild-cp#hash - cp
[PATCH 14/21] git p4: cygwin p4 client does not mark read-only
There are some old version of p4, compiled for cygwin, that
treat read-only files differently.
Normally, a file that is not open is read-only, meaning that
test -w on the file is false. This works on unix, and it works
on windows using the NT version of p4. The cygwin version
of p4, though, changes the permissions, but does not set the
windows read-only attribute, so test -w returns false.
Notice this oddity and make the tests work, even on cygiwn.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 13 +
t/t9807-git-p4-submit.sh | 14 --
t/t9809-git-p4-client-view.sh | 4 ++--
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index fbd55ea..23d01fd 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -150,3 +150,16 @@ client_view() {
done
) | p4 client -i
}
+
+is_cli_file_writeable() {
+ # cygwin version of p4 does not set read-only attr,
+ # will be marked 444 but -w is true
+ file=$1
+ if test_have_prereq CYGWIN p4 -V | grep -q CYGWIN
+ then
+ stat=$(stat --format=%a $file)
+ test $stat = 644
+ else
+ test -w $file
+ fi
+}
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 0ae048f..1fb7bc7 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -17,6 +17,16 @@ test_expect_success 'init depot' '
)
'
+test_expect_failure 'is_cli_file_writeable function' '
+ (
+ cd $cli
+ echo a a
+ is_cli_file_writeable a
+ ! is_cli_file_writeable file1
+ rm a
+ )
+'
+
test_expect_success 'submit with no client dir' '
test_when_finished cleanup_git
git p4 clone --dest=$git //depot
@@ -200,7 +210,7 @@ test_expect_success 'submit copy' '
(
cd $cli
test_path_is_file file5.ta
- test ! -w file5.ta
+ ! is_cli_file_writeable file5.ta
)
'
@@ -219,7 +229,7 @@ test_expect_success 'submit rename' '
cd $cli
test_path_is_missing file6.t
test_path_is_file file6.ta
- test ! -w file6.ta
+ ! is_cli_file_writeable file6.ta
)
'
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index fd8fa89..e0eb6b0 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -333,7 +333,7 @@ test_expect_success 'subdir clone, submit copy' '
(
cd $cli
test_path_is_file dir1/file11a
- test ! -w dir1/file11a
+ ! is_cli_file_writeable dir1/file11a
)
'
@@ -353,7 +353,7 @@ test_expect_success 'subdir clone, submit rename' '
cd $cli
test_path_is_missing dir1/file13
test_path_is_file dir1/file13a
- test ! -w dir1/file13a
+ ! is_cli_file_writeable dir1/file13a
)
'
--
1.7.12.1.403.g28165e1
--
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 gui does not open bare repositories
I have removed the Italian localization so as to make git gui use the English one. The result is the same as I have found before. The message is: Not a Git repository: remote.git. Thus, the misleading message is there. -Angelo -- 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 15/21] git p4 test: disable chmod test for cygwin
It does not notice chmod +x or -x; there is nothing for this test to do. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9815-git-p4-submit-fail.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh index d2b7b3d..2db1bf1 100755 --- a/t/t9815-git-p4-submit-fail.sh +++ b/t/t9815-git-p4-submit-fail.sh @@ -400,7 +400,9 @@ test_expect_success 'cleanup rename after submit cancel' ' ) ' -test_expect_success 'cleanup chmod after submit cancel' ' +# chmods are not recognized in cygwin; git has nothing +# to commit +test_expect_success NOT_CYGWIN 'cleanup chmod after submit cancel' ' test_when_finished cleanup_git git p4 clone --dest=$git //depot ( -- 1.7.12.1.403.g28165e1 -- 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 16/21] git p4: disable read-only attribute before deleting
On windows, p4 marks un-edited files as read-only. Not only are they read-only, but also they cannot be deleted. Remove the read-only attribute before deleting in both the copy and rename cases. This also happens in the RCS cleanup code, where a file is marked to be deleted, but must first be edited to remove adjust the keyword lines. Make sure it is editable before patching. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 10 ++ 1 file changed, 10 insertions(+) diff --git a/git-p4.py b/git-p4.py index 5b2f73d..a6806bc 100755 --- a/git-p4.py +++ b/git-p4.py @@ -17,6 +17,7 @@ import time import platform import re import shutil +import stat verbose = False @@ -1163,6 +1164,9 @@ class P4Submit(Command, P4UserMap): p4_edit(dest) pureRenameCopy.discard(dest) filesToChangeExecBit[dest] = diff['dst_mode'] +if self.isWindows: +# turn off read-only attribute +os.chmod(dest, stat.S_IWRITE) os.unlink(dest) editedFiles.add(dest) elif modifier == R: @@ -1181,6 +1185,8 @@ class P4Submit(Command, P4UserMap): p4_edit(dest) # with move: already open, writable filesToChangeExecBit[dest] = diff['dst_mode'] if not self.p4HasMoveCommand: +if self.isWindows: +os.chmod(dest, stat.S_IWRITE) os.unlink(dest) filesToDelete.add(src) editedFiles.add(dest) @@ -1221,6 +1227,10 @@ class P4Submit(Command, P4UserMap): for file in kwfiles: if verbose: print zapping %s with %s % (line,pattern) +# File is being deleted, so not open in p4. Must +# disable the read-only bit on windows. +if self.isWindows and file not in editedFiles: +os.chmod(file, stat.S_IWRITE) self.patchRCSKeywords(file, kwfiles[file]) fixed_rcs_keywords = True -- 1.7.12.1.403.g28165e1 -- 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 17/21] git p4: avoid shell when mapping users
The extra quoting and double-% are unneeded, just to work around the shell. Instead, avoid the shell indirection. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index a6806bc..a92d84f 100755 --- a/git-p4.py +++ b/git-p4.py @@ -982,7 +982,8 @@ class P4Submit(Command, P4UserMap): def p4UserForCommit(self,id): # Return the tuple (perforce user,git email) for a given git commit id self.getUserMapFromPerforceServer() -gitEmail = read_pipe(git log --max-count=1 --format='%%ae' %s % id) +gitEmail = read_pipe([git, log, --max-count=1, + --format=%ae, id]) gitEmail = gitEmail.strip() if not self.emails.has_key(gitEmail): return (None,gitEmail) -- 1.7.12.1.403.g28165e1 -- 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 18/21] git p4: avoid shell when invoking git rev-list
Invoke git rev-list directly, avoiding the shell, in P4Submit and P4Sync. The overhead of starting extra processes is significant in cygwin; this speeds things up on that platform. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index a92d84f..9c33af4 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1538,7 +1538,7 @@ class P4Submit(Command, P4UserMap): self.check() commits = [] -for line in read_pipe_lines(git rev-list --no-merges %s..%s % (self.origin, self.master)): +for line in read_pipe_lines([git, rev-list, --no-merges, %s..%s % (self.origin, self.master)]): commits.append(line.strip()) commits.reverse() @@ -2558,7 +2558,8 @@ class P4Sync(Command, P4UserMap): def searchParent(self, parent, branch, target): parentFound = False -for blob in read_pipe_lines([git, rev-list, --reverse, --no-merges, parent]): +for blob in read_pipe_lines([git, rev-list, --reverse, + --no-merges, parent]): blob = blob.strip() if len(read_pipe([git, diff-tree, blob, target])) == 0: parentFound = True -- 1.7.12.1.403.g28165e1 -- 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 19/21] git p4: avoid shell when invoking git config --get-all
Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 9c33af4..c0c738a 100755 --- a/git-p4.py +++ b/git-p4.py @@ -525,7 +525,8 @@ def gitConfig(key, args = None): # set args to --bool, for instance def gitConfigList(key): if not _gitConfig.has_key(key): -_gitConfig[key] = read_pipe(git config --get-all %s % key, ignore_error=True).strip().split(os.linesep) +s = read_pipe([git, config, --get-all, key], ignore_error=True) +_gitConfig[key] = s.strip().split(os.linesep) return _gitConfig[key] def p4BranchesInGit(branchesAreInRemotes = True): -- 1.7.12.1.403.g28165e1 -- 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 20/21] git p4: avoid shell when calling git config
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index c0c738a..007ef6b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -514,13 +514,16 @@ def gitBranchExists(branch):
return proc.wait() == 0;
_gitConfig = {}
-def gitConfig(key, args = None): # set args to --bool, for instance
+
+def gitConfig(key, args=None): # set args to --bool, for instance
if not _gitConfig.has_key(key):
-argsFilter =
-if args != None:
-argsFilter = %s % args
-cmd = git config %s%s % (argsFilter, key)
-_gitConfig[key] = read_pipe(cmd, ignore_error=True).strip()
+cmd = [ git, config ]
+if args:
+assert(args == --bool)
+cmd.append(args)
+cmd.append(key)
+s = read_pipe(cmd, ignore_error=True)
+_gitConfig[key] = s.strip()
return _gitConfig[key]
def gitConfigList(key):
--
1.7.12.1.403.g28165e1
--
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 21/21] git p4: introduce gitConfigBool
Make the intent of --bool more obvious by returning a direct True
or False value. Convert a couple non-bool users with obvious bool
intent.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 45 ++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 007ef6b..524df12 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -515,17 +515,25 @@ def gitBranchExists(branch):
_gitConfig = {}
-def gitConfig(key, args=None): # set args to --bool, for instance
+def gitConfig(key):
if not _gitConfig.has_key(key):
-cmd = [ git, config ]
-if args:
-assert(args == --bool)
-cmd.append(args)
-cmd.append(key)
+cmd = [ git, config, key ]
s = read_pipe(cmd, ignore_error=True)
_gitConfig[key] = s.strip()
return _gitConfig[key]
+def gitConfigBool(key):
+Return a bool, using git config --bool. It is True only if the
+ variable is set to true, and False if set to false or not present
+ in the config.
+
+if not _gitConfig.has_key(key):
+cmd = [ git, config, --bool, key ]
+s = read_pipe(cmd, ignore_error=True)
+v = s.strip()
+_gitConfig[key] = v == true
+return _gitConfig[key]
+
def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe([git, config, --get-all, key], ignore_error=True)
@@ -656,8 +664,7 @@ def p4PathStartsWith(path, prefix):
#
# we may or may not have a problem. If you have core.ignorecase=true,
# we treat DirA and dira as the same directory
-ignorecase = gitConfig(core.ignorecase, --bool) == true
-if ignorecase:
+if gitConfigBool(core.ignorecase):
return path.lower().startswith(prefix.lower())
return path.startswith(prefix)
@@ -892,7 +899,7 @@ class P4Submit(Command, P4UserMap):
self.usage += [name of git branch to submit into perforce depot]
self.origin =
self.detectRenames = False
-self.preserveUser = gitConfig(git-p4.preserveUser).lower() == true
+self.preserveUser = gitConfigBool(git-p4.preserveUser)
self.dry_run = False
self.prepare_p4_only = False
self.conflict_behavior = None
@@ -1000,7 +1007,7 @@ class P4Submit(Command, P4UserMap):
(user,email) = self.p4UserForCommit(id)
if not user:
msg = Cannot find p4 user for email %s in commit %s. %
(email, id)
-if gitConfig('git-p4.allowMissingP4Users').lower() == true:
+if gitConfigBool(git-p4.allowMissingP4Users):
print %s % msg
else:
die(Error: %s\nSet git-p4.allowMissingP4Users to true to
allow this. % msg)
@@ -1095,7 +1102,7 @@ class P4Submit(Command, P4UserMap):
message. Return true if okay to continue with the submit.
# if configured to skip the editing part, just submit
-if gitConfig(git-p4.skipSubmitEdit) == true:
+if gitConfigBool(git-p4.skipSubmitEdit):
return True
# look at the modification time, to check later if the user saved
@@ -,7 +1118,7 @@ class P4Submit(Command, P4UserMap):
# If the file was not saved, prompt to see if this patch should
# be skipped. But skip this verification step if configured so.
-if gitConfig(git-p4.skipSubmitEditCheck) == true:
+if gitConfigBool(git-p4.skipSubmitEditCheck):
return True
# modification time updated means user saved the file
@@ -1211,7 +1218,7 @@ class P4Submit(Command, P4UserMap):
# Patch failed, maybe it's just RCS keyword woes. Look through
# the patch to see if that's possible.
-if gitConfig(git-p4.attemptRCSCleanup,--bool) == true:
+if gitConfigBool(git-p4.attemptRCSCleanup):
file = None
pattern = None
kwfiles = {}
@@ -1506,7 +1513,7 @@ class P4Submit(Command, P4UserMap):
sys.exit(128)
self.useClientSpec = False
-if gitConfig(git-p4.useclientspec, --bool) == true:
+if gitConfigBool(git-p4.useclientspec):
self.useClientSpec = True
if self.useClientSpec:
self.clientSpecDirs = getClientSpec()
@@ -1546,7 +1553,7 @@ class P4Submit(Command, P4UserMap):
commits.append(line.strip())
commits.reverse()
-if self.preserveUser or (gitConfig(git-p4.skipUserNameCheck) ==
true):
+if self.preserveUser or gitConfigBool(git-p4.skipUserNameCheck):
self.checkAuthorship = False
else:
self.checkAuthorship = True
@@ -1582,7 +1589,7 @@ class P4Submit(Command, P4UserMap):
else:
self.diffOpts += -C%s % detectCopies
-if gitConfig(git-p4.detectCopiesHarder, --bool) == true:
+if gitConfigBool(git-p4.detectCopiesHarder):
Re: What's cooking in git.git (Sep 2012, #09; Thu, 27)
On 12-09-27 11:38 PM, Junio C Hamano wrote: * mb/remote-default-nn-origin (2012-07-11) 6 commits - Teach get_default_remote to respect remote.default. - Test that plain git fetch uses remote.default when on a detached HEAD. - Teach clone to set remote.default. - Teach git remote about remote.default. - Teach remote.c about the remote.default configuration setting. - Rename remote.c's default_remote_name static variables. When the user does not specify what remote to interact with, we often attempt to use 'origin'. This can now be customized via a configuration variable. Expecting a reroll. The first remote becomes the default bit is better done as a separate step. Unfortunately my days have been too full to progress this. I'm still planning to get to it when there's an opportunity. The next iteration would add the settings required to enable the migration plan that Junio outlined here: http://article.gmane.org/gmane.comp.version-control.git/201332 In the meantime, anyone else who feels like taking this up is more than welcome. The relevant threads are: http://thread.gmane.org/gmane.comp.version-control.git/200145 http://thread.gmane.org/gmane.comp.version-control.git/201065 http://thread.gmane.org/gmane.comp.version-control.git/201306 M. -- 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 gui does not open bare repositories
Hi Angelo, On Fri, Sep 28, 2012 at 2:09 PM, Angelo Borsotti angelo.borso...@gmail.com wrote: I have removed the Italian localization so as to make git gui use the English one. The result is the same as I have found before. The message is: Not a Git repository: remote.git. Thus, the misleading message is there. I'm not seeing what you're seeing. I'm running git 1.7.12.1 and I get exactly this message: Cannot use bare repository: /Users/bsmith/tmp/repo.git When I do exactly this: $ cd ~/tmp $ mkdir repo.git $ cd repo.git $ git init --bare $ git gui What, exactly, are you doing? // Ben -- 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: [POC PATCH 5/5] completion: avoid compgen to fix expansion issues in __gitcomp_nl()
On Fri, Sep 28, 2012 at 12:09:35PM +0200, SZEDER Gábor wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(echo $1 |sed -n /^${3-$cur}/ {
$cur can be a path, so it can contain /, which then breaks this sed
expression. Here's a fixup:
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 2df865fd..d30f376f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -260,8 +260,8 @@ __gitcomp ()
#appended.
__gitcomp_nl ()
{
- local IFS=$'\n'
- COMPREPLY=($(echo $1 |sed -n /^${3-$cur}/ {
+ local IFS=$'\n' cur_=${3-$cur}
+ COMPREPLY=($(echo $1 |sed -n /^${cur_//\//\\/}/ {
s/^/${2-}/
s/$/${4- }/
p
--
1.7.12.1.490.g14283db
--
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
Question About joshrobb.com
Hi there! My team is helping to spread the word about a really great resource page your customers and/or readers would probably appreciate called: 43 Free Cloud Services for Application Developers http://xeround.com/blog/2012/08/43-free-cloud-resources-for-application-developers When searching Google for Build Mangement, we found a post on joshrobb.com, so we thought you may want to share our resource with your readers. I hope that you have an amazing weekend! Let me know if you have any questions and/or comments and if you share, we'd love to return the favor in the future! Thanks! Mike Juba 425 North Prince Street Lancaster, PA 17603 (888)690-2152 We don't currently plan on emailing you again, but if you'd like to unsubscribe from any further emails, please visit our unsubscribe link at rankpop.com/email-unsubscribe/ -- 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 gui does not open bare repositories
Hi Ben, I am running git gui on Windows 7. Are you running it on Linux? -Angelo -- 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: What's cooking in git.git (Sep 2012, #09; Thu, 27)
Marc Branchaud marcn...@xiplink.com writes: On 12-09-27 11:38 PM, Junio C Hamano wrote: * mb/remote-default-nn-origin (2012-07-11) 6 commits - Teach get_default_remote to respect remote.default. - Test that plain git fetch uses remote.default when on a detached HEAD. - Teach clone to set remote.default. - Teach git remote about remote.default. - Teach remote.c about the remote.default configuration setting. - Rename remote.c's default_remote_name static variables. When the user does not specify what remote to interact with, we often attempt to use 'origin'. This can now be customized via a configuration variable. Expecting a reroll. The first remote becomes the default bit is better done as a separate step. Unfortunately my days have been too full to progress this. I'm still planning to get to it when there's an opportunity. The next iteration would add the settings required to enable the migration plan that Junio outlined here: http://article.gmane.org/gmane.comp.version-control.git/201332 In the meantime, anyone else who feels like taking this up is more than welcome. The relevant threads are: http://thread.gmane.org/gmane.comp.version-control.git/200145 http://thread.gmane.org/gmane.comp.version-control.git/201065 http://thread.gmane.org/gmane.comp.version-control.git/201306 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 3/3] revision: make --grep search in notes too if shown
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Notes are shown after commit body. From user perspective it looks
pretty much like commit body and they may assume --grep would search
in that part too. Make it so.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Sep 28, 2012 at 1:16 AM, Junio C Hamano gits...@pobox.com
wrote:
The output from log --show-notes, on the other hand, is even more
conflated and a casual user would view it as part of the message,
so
I would imagine that if we ever do the extention to cover notes
data, the normal --grep should apply to it.
Something like this?
Yes, that was what I had in mind.
revision.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/revision.c b/revision.c
index cfa0e2e..febb4d7 100644
--- a/revision.c
+++ b/revision.c
@@ -2223,6 +2223,12 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
strbuf_addch(buf, '\n');
strbuf_addstr(buf, commit-buffer);
}
+ if (opt-show_notes) {
+ if (!buf.len)
+ strbuf_addstr(buf, commit-buffer);
+ format_display_notes(commit-object.sha1, buf,
+ get_log_output_encoding(), 0);
+ }
if (buf.len)
retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
else
--
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] Add __git_ps1_pc to use as PROMPT_COMMAND
Simon Oosthoek soosth...@nieuwland.nl writes:
+# __git_ps1_pc accepts 0 arguments (for now)
+# It is meant to be used as PROMPT_COMMAND, it sets PS1
+__git_ps1_pc ()
+{
+ local g=$(__gitdir)
+ if [ -n $g ]; then
+...
+ fi
+}
This looks awfully similar to the existing code in __git_ps1
function. Without refactoring to share the logic between them, it
won't be maintainable.
--
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] add t3420-rebase-topology
On Thu, Sep 27, 2012 at 5:20 AM, Chris Webb ch...@arachsys.com wrote: You're right that rebase --root without --onto always creates a brand new root as a result of the implementation using a sentinel commit. Clearly this is what's wanted with --interactive That's not as clear as one might first think. git rebase -i actually honors --force; if one marks a commit for edit, but then --continue without making any changes, the next commit(s) will be fast-forwarded. See the first few lines of pick_one (or pick_one_preserving_merges). but rebase --root with neither --onto nor --interactive is a slightly odd combination for which I struggle to imagine a natural use. Perhaps you're right that for consistency it should be a no-op unless --force-rebase is given? If we did this, this combination would be a no-op unconditionally as by definition we're always descended from the root of our current commit. For consistency, it seems like git rebase -p --root should always be a no-op, while git rebase [-i/-m] --root should be no-op if the history has no merges. Also, since git rebase -i tries to fast-forward through existing commits, it seems like git rebase -i --root should ideally not create a sentinel commit, but instead stop at the first commit marked for editing. If, OTOH, --force-rebase is given, we should rewrite history from the first commit, which in the case of --root would mean creating a sentinel commit. So, in short, I have a feeling that the sentinel commit should be created if and only if both --root and --force-rebase (but not --onto) are given. However, given the not-very-useful behaviour, I suspect that rebase --root is much more likely to be a mistyped version of rebase -i --root than rebase --root --force-rebase. (Unless I'm missing a reasonable use for this? History linearisation perhaps?) Yes, the not-very-useful-ness of this is the clear argument against making them no-ops. But I have to say I was slightly surprised when I tried git rebase --root for the first time and it created completely new history for me. As you say, git rebase --root is probably often a mistyped git rebase -i --root, and if that is the case, it seems nicer (in addition to being more consistent) if we don't do anything rather than rewriting the history. The history rewriting might even go unnoticed and come as an unpleasant surprise later. When working on a new project, git rebase -i --root might even be a convenient replacement for git rebase -i initial commit even when one does not want to rewrite the initial commit itself, and in such a case, the user would clearly not want a sentinel commit either. So I'm getting more and more convinced that the sentinel commit should only be created if --force-rebase was given. Let me know if I'm missing something. Martin -- 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/5] completion: fix args of run_completion() test helper
SZEDER Gábor sze...@ira.uka.de writes:
To simulate the the user hit 'git TAB, one of the completion tests
sets up the rather strange command line
git
i.e. the second word on the command line consists of two double
quotes. However, this is not what happens for real, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e7657537..f5e68834 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
I can understand the other three hunks, but this one is fishy.
Shouldn't $1 be inside a pair of dq? I.e.
+ run_completion $1
@@ -147,7 +147,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion git \\
+ run_completion git
# built-in
grep -q ^add \$ out
# script
@@ -155,7 +155,7 @@ test_expect_success 'basic' '
# plumbing
! grep -q ^ls-files \$ out
- run_completion git f
+ run_completion git f
! grep -q -v ^f out
'
--
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 gui does not open bare repositories
Hi Ben, I run the same test on Linux, and have got the same results as you did. So the problem is only on Windows. -Angelo -- 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/5] completion: fix args of run_completion() test helper
On Fri, Sep 28, 2012 at 11:04:05AM -0700, Junio C Hamano wrote:
SZEDER Gábor sze...@ira.uka.de writes:
To simulate the the user hit 'git TAB, one of the completion tests
s/the the/that the/
sets up the rather strange command line
git
i.e. the second word on the command line consists of two double
quotes. However, this is not what happens for real, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e7657537..f5e68834 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
I can understand the other three hunks, but this one is fishy.
Shouldn't $1 be inside a pair of dq? I.e.
+ run_completion $1
No. $1 holds all words on the command line. If it was between a pair
of dq, then the whole command line would be passed to the completion
script as a single word.
@@ -147,7 +147,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion git \\
+ run_completion git
# built-in
grep -q ^add \$ out
# script
@@ -155,7 +155,7 @@ test_expect_success 'basic' '
# plumbing
! grep -q ^ls-files \$ out
- run_completion git f
+ run_completion git f
! grep -q -v ^f out
'
--
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 06/21] git p4 test: use client_view in t9806
Pete Wyckoff p...@padd.com writes:
Use the standard client_view function from lib-git-p4.sh
instead of building one by hand. This requires a bit of
rework, using the current value of $P4CLIENT for the client
name. It also reorganizes the test to isolate changes to
$P4CLIENT and $cli in a subshell.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 4 ++--
t/t9806-git-p4-options.sh | 50
++-
2 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 890ee60..d558dd0 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -116,8 +116,8 @@ marshal_dump() {
client_view() {
(
cat -EOF
- Client: client
- Description: client
+ Client: $P4CLIENT
+ Description: $P4CLIENT
Root: $cli
View:
EOF
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index fa40cc8..37ca30a 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '
exec /dev/null
test_must_fail git p4 clone --dest=$git --use-client-spec
)
- cli2=$(test-path-utils real_path $TRASH_DIRECTORY/cli2)
+ # build a different client
+ cli2=$TRASH_DIRECTORY/cli2
mkdir -p $cli2
test_when_finished rmdir \$cli2\
test_when_finished cleanup_git
...
- # same thing again, this time with variable instead of option
(
...
+ # group P4CLIENT and cli changes in a sub-shell
+ P4CLIENT=client2
+ cli=$cli2
+ client_view //depot/sub/... //client2/bus/...
+ git p4 clone --dest=$git --use-client-spec //depot/...
+ (
+ cd $git
+ test_path_is_file bus/dir/f4
+ test_path_is_missing file1
+ )
+ cleanup_git
Hmm, the use of test-path-utils real_path to form cli2 in the
original was not necessary at all?
+ # same thing again, this time with variable instead of option
+ (
+ cd $git
+ git init
+ git config git-p4.useClientSpec true
+ git p4 sync //depot/...
+ git checkout -b master p4/master
+ test_path_is_file bus/dir/f4
+ test_path_is_missing file1
+ )
Do you need a separate sub-shell inside a sub-shell we are already
in that you called client_view in?
)
'
--
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 05/21] git p4 test: use client_view to build the initial client
Pete Wyckoff p...@padd.com writes:
Simplify the code a bit by using an existing function.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7061dce..890ee60 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -74,15 +74,8 @@ start_p4d() {
fi
# build a client
- (
- cd $cli
- p4 client -i -EOF
- Client: client
- Description: client
- Root: $cli
- View: //depot/... //client/...
- EOF
- )
+ client_view //depot/... //client/...
+
return 0
}
Assuming that writing //depot/... //client/... on the next line
indented by a tab is equivalent to writing it on View: line (which I
think it is), this looks like an obviously good reuse of the code.
I have to wonder if the use of printf in client_view implementation
should be tighted up, though.
diff --git i/t/lib-git-p4.sh w/t/lib-git-p4.sh
index 7061dce..4e58289 100644
--- i/t/lib-git-p4.sh
+++ w/t/lib-git-p4.sh
@@ -128,8 +128,6 @@ client_view() {
Root: $cli
View:
EOF
- for arg ; do
- printf \t$arg\n
- done
+ printf \t%s\n $@
) | p4 client -i
}
--
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 04/21] git p4: fix error message when describe -s fails
Pete Wyckoff p...@padd.com writes:
The output was a bit nonsensical, including a bare %d. Fix it
to make it easier to understand.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index eef5c94..d7ee4b4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2679,7 +2679,8 @@ class P4Sync(Command, P4UserMap):
if r.has_key('time'):
newestTime = int(r['time'])
if newestTime is None:
-die(\describe -s\ on newest change %d did not give a time)
+die(Output from \describe -s\ on newest change %d did not
give a time %
+newestRevision)
Shouldn't it say p4 describe -s?
details[time] = newestTime
self.updateOptionDict(details)
--
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/5] completion: fix args of run_completion() test helper
SZEDER Gábor sze...@ira.uka.de writes:
On Fri, Sep 28, 2012 at 11:04:05AM -0700, Junio C Hamano wrote:
SZEDER Gábor sze...@ira.uka.de writes:
To simulate the the user hit 'git TAB, one of the completion tests
s/the the/that the/
sets up the rather strange command line
git
i.e. the second word on the command line consists of two double
quotes. However, this is not what happens for real, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e7657537..f5e68834 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
I can understand the other three hunks, but this one is fishy.
Shouldn't $1 be inside a pair of dq? I.e.
+ run_completion $1
No. $1 holds all words on the command line. If it was between a pair
of dq, then the whole command line would be passed to the completion
script as a single word.
And these words can be split at $IFS boundaries without any
issues? IOW, nobody would ever want to make words array in the
run_completion function to ['git' 'foo bar' 'baz']?
--
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 15/21] git p4 test: disable chmod test for cygwin
Am 28.09.2012 14:04, schrieb Pete Wyckoff: It does not notice chmod +x or -x; there is nothing for this test to do. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9815-git-p4-submit-fail.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh index d2b7b3d..2db1bf1 100755 --- a/t/t9815-git-p4-submit-fail.sh +++ b/t/t9815-git-p4-submit-fail.sh @@ -400,7 +400,9 @@ test_expect_success 'cleanup rename after submit cancel' ' ) ' -test_expect_success 'cleanup chmod after submit cancel' ' +# chmods are not recognized in cygwin; git has nothing +# to commit +test_expect_success NOT_CYGWIN 'cleanup chmod after submit cancel' ' test_when_finished cleanup_git git p4 clone --dest=$git //depot ( In the git part, you could use test_chmod to change the executable bit. But if you cannot test it in the p4 part later on, it is probably not worth it. -- Hannes -- 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/5] completion: fix args of run_completion() test helper
Jeff King p...@peff.net writes:
On Fri, Sep 28, 2012 at 12:23:47PM -0700, Junio C Hamano wrote:
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
I can understand the other three hunks, but this one is fishy.
Shouldn't $1 be inside a pair of dq? I.e.
+ run_completion $1
No. $1 holds all words on the command line. If it was between a pair
of dq, then the whole command line would be passed to the completion
script as a single word.
And these words can be split at $IFS boundaries without any
issues? IOW, nobody would ever want to make words array in the
run_completion function to ['git' 'foo bar' 'baz']?
It might be simpler to just convert test_completion into the
test_completion_long I added in my series; the latter takes the expected
output on stdin, leaving the actual arguments free to represent the real
command-line. E.g., your example would become:
test_completion git foo bar baz -\EOF
... expected output ...
EOF
I realize that the way my question was stated was misleading. It
was not meant as a rhetorical You would never be able to pass
['git' 'foo bar' 'baz'] with that interface, and the patch sucks.
but was meant as a pure question Do we want to pass such word
list?. test_completion is almost always used to test completion
with inputs without any $IFS letters in it, so not being able to
test such an input via this interface is fine. If needed, we can
give another less often used interface to let you pass such an
input is perfectly fine by me.
But I suspect that the real reason test_completion requires the
caller to express the list of inputs to run_completion as $IFS
separate list is because it needs to also get expected from the
command line:
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
I wonder if doing something like this would be a far simpler
solution:
test_completion ()
{
case $1 in
'')
;;
*)
echo $1 expect
shift
;;
esac
run_completion $@
test_cmp expect output
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] completion: fix args of run_completion() test helper
On Fri, Sep 28, 2012 at 12:23:47PM -0700, Junio C Hamano wrote:
SZEDER Gábor sze...@ira.uka.de writes:
On Fri, Sep 28, 2012 at 11:04:05AM -0700, Junio C Hamano wrote:
SZEDER Gábor sze...@ira.uka.de writes:
To simulate the the user hit 'git TAB, one of the completion tests
s/the the/that the/
sets up the rather strange command line
git
i.e. the second word on the command line consists of two double
quotes. However, this is not what happens for real, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e7657537..f5e68834 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
-_words=( $1 )
+_words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
-run_completion $@
+run_completion $1
test_cmp expected out
}
I can understand the other three hunks, but this one is fishy.
Shouldn't $1 be inside a pair of dq? I.e.
+ run_completion $1
No. $1 holds all words on the command line. If it was between a pair
of dq, then the whole command line would be passed to the completion
script as a single word.
And these words can be split at $IFS boundaries without any
issues? IOW, nobody would ever want to make words array in the
run_completion function to ['git' 'foo bar' 'baz']?
Maybe we would, but making it work would be a separate fix. You can't
do that with the current version either.
--
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] gitk: refresh the index before running diff-files
Jeff King p...@peff.net writes:
Potentially the reload command should reset the need_index_refresh
flag, too.
Yeah, I think that is a sane enhancement to think about.
gitk | 10 ++
1 file changed, 10 insertions(+)
diff --git a/gitk b/gitk
index 379582a..561be23 100755
--- a/gitk
+++ b/gitk
@@ -5112,6 +5112,14 @@ proc dodiffindex {} {
filerun $fd [list readdiffindex $fd $lserial $i]
}
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+ exec sh -c git update-index --refresh /dev/null 21 || true
+ set need_index_refresh false
+}
+}
+
proc readdiffindex {fd serial inst} {
global viewmainheadid nullid nullid2 curview commitinfo commitdata
lserial
global vfilelimit
@@ -5131,6 +5139,7 @@ proc readdiffindex {fd serial inst} {
}
# now see if there are any local changes not checked in to the index
+refresh_index
set cmd |git diff-files
if {$vfilelimit($curview) ne {}} {
set cmd [concat $cmd -- $vfilelimit($curview)]
@@ -11670,6 +11679,7 @@ set want_ttk 1
set autosellen 40
set perfile_attrs 0
set want_ttk 1
+set need_index_refresh true
if {[tk windowingsystem] eq aqua} {
set extdifftool opendiff
--
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 diff-file bug?
On Fri, Sep 28, 2012 at 07:55:10PM +0100, Scott Batchelor wrote: I'm fairly new to git and am witnessing some strange behavior with git that I suspect may be a bug. Can anyone set my mind at rest. It's not a bug. Every so often (I've not quite figured out the exact set of circumstances yet) git diff-files shows that every file in my repo is modified, when I expect none to be. Diff-files only looks at the cached sha1 in the index. It will not re-read the file to update its index entry if its stat information appears out of date. So what is happening is that another program[1] is updating the timestamp or other information about each file, but not the content. Diff-files does not re-read the content to find out there is in fact no change. If you are using plumbing like diff-files, you are expected to run git update-index --refresh yourself first. The reasoning is that for performance reasons, a script that issues many git commands would only want to pay the price to refresh the index once, at the beginning of the script. If you use the git diff porcelain, it will automatically refresh the index for you. If I type git status (which shows what I expect - no files modified) and then git diff-files again, git now shows that no files have been modified (which is what I expect). It's like git status is resetting something that git diff-files uses. Exactly. git status automatically refreshes the index, and the result is saved. I'm trying to figure out what the problem with git diff-files is because gitk uses it under the hood, and I think that gitk is reporting erroneous changes (which are also reset by performing a git status in the repo) in the patch files list. gitk should probably be calling update-index --refresh on startup. If it already is, then it may be that whatever is updating the files is doing it while gitk is running. -Peff [1] Usually stat updates are caused by another program. But we have had cases where unusual filesystems yielded inconsistent results from stat(). -- 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] gitk: refresh the index before running diff-files
Jeff King p...@peff.net writes:
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+ exec sh -c git update-index --refresh /dev/null 21 || true
I think the usual idiom for ignoring errors is to use catch around exec,
avoiding the extra shell wrapper:
catch { exec git update-index --refresh }
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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] gitk: refresh the index before running diff-files
On Sat, Sep 29, 2012 at 12:11:58AM +0200, Andreas Schwab wrote:
Jeff King p...@peff.net writes:
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+ exec sh -c git update-index --refresh /dev/null 21 || true
I think the usual idiom for ignoring errors is to use catch around exec,
avoiding the extra shell wrapper:
catch { exec git update-index --refresh }
Thanks. I don't speak tcl at all, but your version makes much more
sense.
-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] gitk: refresh the index before running diff-files
Jeff King p...@peff.net writes:
On Sat, Sep 29, 2012 at 12:11:58AM +0200, Andreas Schwab wrote:
Jeff King p...@peff.net writes:
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+ exec sh -c git update-index --refresh /dev/null 21 || true
I think the usual idiom for ignoring errors is to use catch around exec,
avoiding the extra shell wrapper:
catch { exec git update-index --refresh }
Thanks. I don't speak tcl at all, but your version makes much more
sense.
But isn't the redirection still needed? Otherwise the Needs
update messages will go to the terminal, no?
--
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] gitk: refresh the index before running diff-files
On Fri, Sep 28, 2012 at 04:02:32PM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+exec sh -c git update-index --refresh /dev/null 21 || true
I think the usual idiom for ignoring errors is to use catch around exec,
avoiding the extra shell wrapper:
catch { exec git update-index --refresh }
Thanks. I don't speak tcl at all, but your version makes much more
sense.
But isn't the redirection still needed? Otherwise the Needs
update messages will go to the terminal, no?
I think the weird tcl-catches-stderr thing kicks in (at least it did for
me in a simple experiment). But like I said, I am not an expert.
-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] gitk: refresh the index before running diff-files
Jeff King p...@peff.net writes: I think the weird tcl-catches-stderr thing kicks in (at least it did for me in a simple experiment). But like I said, I am not an expert. OK, I'll believe you ;-) -- 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] gitk: refresh the index before running diff-files
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
On Sat, Sep 29, 2012 at 12:11:58AM +0200, Andreas Schwab wrote:
Jeff King p...@peff.net writes:
+proc refresh_index {} {
+global need_index_refresh
+if { $need_index_refresh } {
+ exec sh -c git update-index --refresh /dev/null 21 || true
I think the usual idiom for ignoring errors is to use catch around exec,
avoiding the extra shell wrapper:
catch { exec git update-index --refresh }
Thanks. I don't speak tcl at all, but your version makes much more
sense.
But isn't the redirection still needed? Otherwise the Needs
update messages will go to the terminal, no?
The exec command captures both stdout and stderr and returns it as its
value, and catch ignores it.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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/3] revision: add --grep-reflog to filter commits by reflog messages
On Sat, Sep 29, 2012 at 12:55 AM, Junio C Hamano gits...@pobox.com wrote: For that to happen, the code _must_ know what kind of headers we would support; discarding the existing enum is going in a wrong direction. Or what kind of manipulation is required for a header. The caller can decide if it wants such manipulation or not. Somebody might want to grep committer's date, for example. When we introduce git log --header=frotz:xyzzy option that looks for a generic frotz header and tries to see if xyzzy textually appears in the field, we may want to add a generic this is not a header that we have special treatment for enum to the mix. But for known kinds of headers, we would need a list of what each header is and what semantics it wants. So please reconsider undoing [1/3], and rerolling [2/3] that depends on it. Done. The enum is kept. I added a few tests about grepping timestamp in 1/3 to keep people (or myself) from making the same mistake I did. 3/3 is reposted for completeness, I don't care much about notes (so far). It's up to you to take or drop it. Nguyễn Thái Ngọc Duy (3): grep: prepare for new header field filter revision: add --grep-reflog to filter commits by reflog messages revision: make --grep search in notes too if shown Documentation/rev-list-options.txt | 8 grep.c | 10 +- grep.h | 7 +-- revision.c | 26 -- t/t7810-grep.sh| 38 ++ 5 files changed, 84 insertions(+), 5 deletions(-) -- 1.7.12.1.406.g6ab07c4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] grep: prepare for new header field filter
grep supports only author and committer headers, which have the same
special treatment that later headers may or may not have. Check for
field type and only strip_timestamp() when the field is either author
or committer.
GREP_HEADER_FIELD_MAX is put in the grep_header_field enum to be
calculated automatically, correctly, as long as it's at the end of the
enum.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
grep.c | 9 -
grep.h | 6 --
t/t7810-grep.sh | 12
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/grep.c b/grep.c
index 898be6e..8d73995 100644
--- a/grep.c
+++ b/grep.c
@@ -720,7 +720,14 @@ static int match_one_pattern(struct grep_pat *p, char
*bol, char *eol,
if (strncmp(bol, field, len))
return 0;
bol += len;
- saved_ch = strip_timestamp(bol, eol);
+ switch (p-field) {
+ case GREP_HEADER_AUTHOR:
+ case GREP_HEADER_COMMITTER:
+ saved_ch = strip_timestamp(bol, eol);
+ break;
+ default:
+ break;
+ }
}
again:
diff --git a/grep.h b/grep.h
index 8a28a67..d54adbe 100644
--- a/grep.h
+++ b/grep.h
@@ -29,9 +29,11 @@ enum grep_context {
enum grep_header_field {
GREP_HEADER_AUTHOR = 0,
- GREP_HEADER_COMMITTER
+ GREP_HEADER_COMMITTER,
+
+ /* Must be at the end of the enum */
+ GREP_HEADER_FIELD_MAX
};
-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
struct grep_pat {
struct grep_pat *next;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 91db352..30eaa9a 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -628,6 +628,18 @@ test_expect_success 'log --all-match --grep --grep
--author takes intersection'
test_cmp expect actual
'
+test_expect_success 'log --author does not search in timestamp' '
+ : expect
+ git log --author=$GIT_AUTHOR_DATE actual
+ test_cmp expect actual
+'
+
+test_expect_success 'log --committer does not search in timestamp' '
+ : expect
+ git log --committer=$GIT_COMMITTER_DATE actual
+ test_cmp expect actual
+'
+
test_expect_success 'grep with CE_VALID file' '
git update-index --assume-unchanged t/t
rm t/t
--
1.7.12.1.406.g6ab07c4
--
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 3/3] revision: make --grep search in notes too if shown
Notes are shown after commit body. From user perspective it looks
pretty much like commit body and they may assume --grep would search
in that part too. Make it so.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
revision.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/revision.c b/revision.c
index 109bec1..dff6fb7 100644
--- a/revision.c
+++ b/revision.c
@@ -2223,6 +2223,12 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
strbuf_addch(buf, '\n');
strbuf_addstr(buf, commit-buffer);
}
+ if (opt-show_notes) {
+ if (!buf.len)
+ strbuf_addstr(buf, commit-buffer);
+ format_display_notes(commit-object.sha1, buf,
+get_log_output_encoding(), 0);
+ }
if (buf.len)
retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
else
--
1.7.12.1.406.g6ab07c4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages
Similar to --author/--committer which filters commits by author and
committer header fields. --grep-reflog adds a fake reflog header to
commit and a grep filter to search on that line.
All rules to --author/--committer apply except no timestamp stripping.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/rev-list-options.txt | 8
grep.c | 1 +
grep.h | 1 +
revision.c | 20 ++--
t/t7810-grep.sh| 26 ++
5 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 1fc2a18..aa7cd9d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -51,6 +51,14 @@ endif::git-rev-list[]
commits whose author matches any of the given patterns are
chosen (similarly for multiple `--committer=pattern`).
+--grep-reflog=pattern::
+
+ Limit the commits output to ones with reflog entries that
+ match the specified pattern (regular expression). With
+ more than one `--grep-reflog`, commits whose reflog message
+ matches any of the given patterns are chosen. Ignored unless
+ `--walk-reflogs` is given.
+
--grep=pattern::
Limit the commits output to ones with log message that
diff --git a/grep.c b/grep.c
index 8d73995..d70dcdf 100644
--- a/grep.c
+++ b/grep.c
@@ -697,6 +697,7 @@ static struct {
} header_field[] = {
{ author , 7 },
{ committer , 10 },
+ { reflog , 7 },
};
static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
diff --git a/grep.h b/grep.h
index d54adbe..6e78b96 100644
--- a/grep.h
+++ b/grep.h
@@ -30,6 +30,7 @@ enum grep_context {
enum grep_header_field {
GREP_HEADER_AUTHOR = 0,
GREP_HEADER_COMMITTER,
+ GREP_HEADER_REFLOG,
/* Must be at the end of the enum */
GREP_HEADER_FIELD_MAX
diff --git a/revision.c b/revision.c
index ae12e11..109bec1 100644
--- a/revision.c
+++ b/revision.c
@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
} else if ((argcount = parse_long_opt(committer, argv, optarg))) {
add_header_grep(revs, GREP_HEADER_COMMITTER, optarg);
return argcount;
+ } else if ((argcount = parse_long_opt(grep-reflog, argv, optarg))) {
+ add_header_grep(revs, GREP_HEADER_REFLOG, optarg);
+ return argcount;
} else if ((argcount = parse_long_opt(grep, argv, optarg))) {
add_message_grep(revs, optarg);
return argcount;
@@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs,
struct commit *commit)
static int commit_match(struct commit *commit, struct rev_info *opt)
{
+ int retval;
+ struct strbuf buf = STRBUF_INIT;
if (!opt-grep_filter.pattern_list !opt-grep_filter.header_list)
return 1;
- return grep_buffer(opt-grep_filter,
- commit-buffer, strlen(commit-buffer));
+ if (opt-reflog_info) {
+ strbuf_addstr(buf, reflog );
+ get_reflog_message(buf, opt-reflog_info);
+ strbuf_addch(buf, '\n');
+ strbuf_addstr(buf, commit-buffer);
+ }
+ if (buf.len)
+ retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
+ else
+ retval = grep_buffer(opt-grep_filter,
+commit-buffer, strlen(commit-buffer));
+ strbuf_release(buf);
+ return retval;
}
static inline int want_ancestry(struct rev_info *revs)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 30eaa9a..3a5d0fd 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -546,6 +546,32 @@ test_expect_success 'log grep (6)' '
test_cmp expect actual
'
+test_expect_success 'log grep (7)' '
+ git log -g --grep-reflog=commit: third --pretty=tformat:%s actual
+ echo third expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log grep (8)' '
+ git log -g --grep-reflog=commit: third --grep-reflog=commit: second
--pretty=tformat:%s actual
+ {
+ echo third echo second
+ } expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log grep (9)' '
+ git log -g --grep-reflog=commit: third --author=Thor
--pretty=tformat:%s actual
+ echo third expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log grep (9)' '
+ git log -g --grep-reflog=commit: third --author=non-existant
--pretty=tformat:%s actual
+ : expect
+ test_cmp expect actual
+'
+
test_expect_success 'log with multiple --grep uses union' '
git log --grep=i --grep=r --format=%s actual
{
--
1.7.12.1.406.g6ab07c4
--
To unsubscribe from this list: send the line
Re: [PATCH 1/8] Introduce new static function real_path_internal()
On 09/27/2012 11:27 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
@@ -54,20 +73,36 @@ const char *real_path(const char *path)
}
if (*buf) {
-if (!*cwd !getcwd(cwd, sizeof(cwd)))
-die_errno (Could not get current working
directory);
+if (!*cwd !getcwd(cwd, sizeof(cwd))) {
+if (die_on_error)
+die_errno(Could not get current
working directory);
+else
+goto error_out;
+}
-if (chdir(buf))
-die_errno (Could not switch to '%s', buf);
+if (chdir(buf)) {
+if (die_on_error)
+die_errno(Could not switch to '%s',
buf);
+else
+goto error_out;
+}
+}
The patch makes sense, but while you are touching this code, I have
to wonder if there is an easy way to tell, before entering the loop,
if we have to chdir() around in the loop. That would allow us to
hoist the getcwd() that is done only so that we can come back to
where we started outside the loop, making it clear why the call is
there, instead of cryptic if (!*cwd to ensure we do getcwd()
once and before doing any chdir().
I don't see an easy way to predict, before entering the loop, whether
chdir() will be needed. For example, compare
touch filename
ln -s filename foo
./test-path-utils real_path foo
with
touch filename
ln -s $(pwd)/filename foo
./test-path-utils real_path foo
In the first case no chdir() is needed, whereas in the second case a
chdir() is needed but only on the second loop iteration.
All I can offer you is a palliative like the one below.
Michael
diff --git a/abspath.c b/abspath.c
index 5748b91..40cdc46 100644
--- a/abspath.c
+++ b/abspath.c
@@ -35,7 +35,14 @@ static const char *real_path_internal(const char
*path, int die_on_error)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf =
bufs[1];
char *retval = NULL;
+
+ /*
+* If we have to temporarily chdir(), store the original CWD
+* here so that we can chdir() back to it at the end of the
+* function:
+*/
char cwd[1024] = ;
+
int buf_index = 1;
int depth = MAXDEPTH;
--
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 1/3] grep: prepare for new header field filter
On Sat, Sep 29, 2012 at 11:41:27AM +0700, Nguyen Thai Ngoc Duy wrote:
diff --git a/grep.c b/grep.c
index 898be6e..8d73995 100644
--- a/grep.c
+++ b/grep.c
@@ -720,7 +720,14 @@ static int match_one_pattern(struct grep_pat *p, char
*bol, char *eol,
if (strncmp(bol, field, len))
return 0;
bol += len;
- saved_ch = strip_timestamp(bol, eol);
+ switch (p-field) {
+ case GREP_HEADER_AUTHOR:
+ case GREP_HEADER_COMMITTER:
+ saved_ch = strip_timestamp(bol, eol);
+ break;
+ default:
+ break;
+ }
Reading this hunk, I wondered what happens to saved_ch if we do not set
it here. Fortunately it is initialized to 0, as we already have to
handle the non-header case. Then later we do this, which does introduce
a new condition (saved_ch was not set, but we trigger the first half of
the conditional):
if (p-token == GREP_PATTERN_HEAD saved_ch)
*eol = saved_ch;
However, the second half of that conditional (which previously was only
triggered when we tried to split the timestamp, but there was a bogus
line with no on it) prevents us from overwriting *eol.
So I think it is good, but it was non-obvious enough that I wanted to
save other reviewers from investigating it. The rest of the patch looks
good to me, as well.
-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 3/8] longest_ancestor_length(): use string_list_split()
On 09/28/2012 12:48 AM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
-for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
-for (colon = ceil; *colon *colon != PATH_SEP; colon++);
-len = colon - ceil;
+string_list_split(prefixes, prefix_list, PATH_SEP, -1);
+
+for (i = 0; i prefixes.nr; i++) {
+const char *ceil = prefixes.items[i].string;
+int len = strlen(ceil);
+
Much nicer than the yucky original ;-)
If your winky-smiley implies irony, then I would like to object. Even
though the original is not difficult to understand, it is more difficult
to review than the new version because one has to think about off-by-one
errors etc. The new version has a bit more boilerplate, but a quick
read suffices both to understand it and to see that it is correct.
Though of course I admit that the improvement is small.
But the main point of this change is to move towards using more testable
parts, so it is a step forward regardless of whether it is more readable.
if (len == 0 || len PATH_MAX || !is_absolute_path(ceil))
continue;
-strlcpy(buf, ceil, len+1);
+memcpy(buf, ceil, len+1);
if (normalize_path_copy(buf, buf) 0)
continue;
Why do you need this memcpy in the first place? Isn't ceil already
a NUL terminated string unlike the original code that points into a
part of the prefix_list string? IOW, why not
normalize_path_copy(buf, ceil);
or something?
Good point. I will fix this in v2.
Can normalize_path_copy() overflow buf[PATH_MAX+1] here (before or
after this patch)?
normalize_path_copy() can only shrink paths, not grow them. So the
length check on ceil guarantees that the result of normalize_path_copy()
will fit in buf.
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 1/8] Introduce new static function real_path_internal()
Michael Haggerty mhag...@alum.mit.edu writes:
The patch makes sense, but while you are touching this code, I have
to wonder if there is an easy way to tell, before entering the loop,
if we have to chdir() around in the loop. That would allow us to
hoist the getcwd() that is done only so that we can come back to
where we started outside the loop, making it clear why the call is
there, instead of cryptic if (!*cwd to ensure we do getcwd()
once and before doing any chdir().
I don't see an easy way to predict, before entering the loop, whether
chdir() will be needed. For example, compare
touch filename
ln -s filename foo
./test-path-utils real_path foo
with
touch filename
ln -s $(pwd)/filename foo
./test-path-utils real_path foo
In the first case no chdir() is needed, whereas in the second case a
chdir() is needed but only on the second loop iteration.
Thanks for an example, and it is perfectly OK if we really have to
have that only fires once and only if needed logic in the loop.
All I can offer you is a palliative like the one below.
I think that is an improvement; or we could rename it not cwd[] but
some other name that includes the word original in it.
Thanks.
Michael
diff --git a/abspath.c b/abspath.c
index 5748b91..40cdc46 100644
--- a/abspath.c
+++ b/abspath.c
@@ -35,7 +35,14 @@ static const char *real_path_internal(const char
*path, int die_on_error)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf =
bufs[1];
char *retval = NULL;
+
+ /*
+* If we have to temporarily chdir(), store the original CWD
+* here so that we can chdir() back to it at the end of the
+* function:
+*/
char cwd[1024] = ;
+
int buf_index = 1;
int depth = MAXDEPTH;
--
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/8] longest_ancestor_length(): use string_list_split()
Michael Haggerty mhag...@alum.mit.edu writes:
On 09/28/2012 12:48 AM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
- for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
- for (colon = ceil; *colon *colon != PATH_SEP; colon++);
- len = colon - ceil;
+ string_list_split(prefixes, prefix_list, PATH_SEP, -1);
+
+ for (i = 0; i prefixes.nr; i++) {
+ const char *ceil = prefixes.items[i].string;
+ int len = strlen(ceil);
+
Much nicer than the yucky original ;-)
If your winky-smiley implies irony, then I would like to object.
No irony. The original was hard to read, especially with the for
loop that does in-line strchr() on a single line. The updated one
is much easier to read.
normalize_path_copy() can only shrink paths, not grow them. So the
length check on ceil guarantees that the result of normalize_path_copy()
will fit in buf.
OK.
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 7/8] longest_ancestor_length(): resolve symlinks before comparing paths
On 09/28/2012 12:51 AM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
longest_ancestor_length() relies on a textual comparison of directory
parts to find the part of path that overlaps with one of the paths in
prefix_list. But this doesn't work if any of the prefixes involves a
symbolic link, because the directories will look different even though
they might logically refer to the same directory. So canonicalize the
paths listed in prefix_list using real_path_if_valid() before trying
to find matches.
path is already in canonical form, so doesn't need to be canonicalized
again.
This fixes some problems with using GIT_CEILING_DIRECTORIES that
contains paths involving symlinks, including t4035 if run with --root
set to a path involving symlinks.
Remove a number of tests of longest_ancestor_length(). It is awkward
to test longest_ancestor_length() now, because its new path
normalization behavior depends on the contents of the whole
filesystem. But we can live without the tests, because
longest_ancestor_length() is now built of reusable components that are
themselves tested separately: string_list_split(),
string_list_longest_prefix(), and real_path_if_valid().
Errr, components may be correct but the way to combine and construct
could go faulty, so...
I don't see a realistic alternative. Testing real_path() is itself is
already quite awkward (see t0060), so testing longest_ancestor_length()
would be even more so. Of course, the GIT_CEILING_DIRECTORIES tests
indirectly test longest_ancestor_length(), though not systematically.
If you have a better suggestion, please let me know.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
path.c| 17 --
t/t0060-path-utils.sh | 64
---
2 files changed, 10 insertions(+), 71 deletions(-)
diff --git a/path.c b/path.c
index 5cace83..981bb06 100644
--- a/path.c
+++ b/path.c
@@ -570,22 +570,25 @@ int normalize_path_copy(char *dst, const char *src)
static int normalize_path_callback(struct string_list_item *item, void
*cb_data)
{
-char buf[PATH_MAX+2];
+char *buf;
const char *ceil = item-string;
-int len = strlen(ceil);
+const char *realpath;
+int len;
-if (len == 0 || len PATH_MAX || !is_absolute_path(ceil))
+if (!*ceil || !is_absolute_path(ceil))
return 0;
-memcpy(buf, ceil, len+1);
-if (normalize_path_copy(buf, buf) 0)
+realpath = real_path_if_valid(ceil);
+if (!realpath)
return 0;
-len = strlen(buf);
+len = strlen(realpath);
+buf = xmalloc(len + 2); /* Leave space for possible trailing slash */
+strcpy(buf, realpath);
if (len == 0 || buf[len-1] != '/') {
buf[len++] = '/';
buf[len++] = '\0';
}
Nice.
I just noticed that the second len++ in the final if is misleading.
I will fix that in v2.
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 2/3] revision: add --grep-reflog to filter commits by reflog messages
Jeff King p...@peff.net writes:
+if (opt-reflog_info) {
+strbuf_addstr(buf, reflog );
+get_reflog_message(buf, opt-reflog_info);
+strbuf_addch(buf, '\n');
+strbuf_addstr(buf, commit-buffer);
+}
+if (buf.len)
+retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
+else
+retval = grep_buffer(opt-grep_filter,
+ commit-buffer, strlen(commit-buffer));
+strbuf_release(buf);
+return retval;
I like how callers not doing a reflog walk do not have to pay the price
to do the extra allocating. We could further limit it to only when
--grep-reflog is in effect, but I guess that would mean wading through
grep_filter's patterns, since it could be buried amidst ANDs and ORs?
One alternative would be to set a bit in the grep_opt when we call
append_header_grep_pattern. It feels a bit like a layering violation,
though. I guess the bit could also go into rev_info. It may not even be
a measurable slowdown, though. Premature optimization and all that.
I do not think it is a layering violation. compile_grep_exp()
should be aware of the short-cut possibilities and your our
expression is interested in reflog so we need to read it is very
similar in spirit to the existing opt-extended bit.
It will obviously allow us to avoid reading reflog information
unnecessarily here. I think it makes perfect sense.
We may also want to flag the use of the --grep-reflog option when
the --walk-reflogs option is not in effect in setup_revisions() as
an error, or something.
--
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
