Re: [PATCH 4/4] gc --aggressive: three phase repacking
On Tue, Mar 18, 2014 at 12:16 PM, Duy Nguyen pclo...@gmail.com wrote: But I think it's orthogonal to gc --aggressive improvement. There's another reason that improving gc may be a good idea (or not). It depends on how other git implementations handle long delta chains. If they hate long delta chains like current git too, then more reason to make gc less aggressive. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] gc --aggressive: three phase repacking
Jeff King p...@peff.net writes: On Tue, Mar 18, 2014 at 12:00:48PM +0700, Duy Nguyen wrote: On Tue, Mar 18, 2014 at 11:50 AM, Jeff King p...@peff.net wrote: On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote: As explained in the previous commit, current aggressive settings --depth=250 --window=250 could slow down repository access significantly. Notice that people usually work on recent history only, we could keep recent history more loosely packed, so that repo access is fast most of the time while the pack file remains small. One thing I have not seen is real-world timings showing the slowdown based on --depth. Did I miss them, or are we just making assumptions based on one old case from 2009 (that, AFAIK does not have real numbers, just speculation)? Has anyone measured the effect of bumping the delta cache size (and its hash implementation)? David tested it with git-blame [1]. I should probably run some tests too (I don't remember if I tested some operations last time). http://thread.gmane.org/gmane.comp.version-control.git/242277/focus=242435 Ah, thanks. I do remember that thread now. It looks like David's last word is that he gets a significant performance from bumping the delta base cache size (and number of buckets). Increasing number of buckets was having comparatively minor effects (that was the suggestion I started with), actually _degrading_ performance rather soon. The delta base cache size was much more noticeable. I had prepared a patch serious increasing it. The reason I have not submitted it yet is that I have not found a compelling real-world test case _apart_ from the fast git-blame that is still missing implementation of -M and -C options. There should be other commands digging through large amounts of old history, but I did not really find something benchmarking convincingly. Either most stuff is inefficient anyway, or the access order is better-behaved, causing fewer unwanted cache flushes. Access order in the optimized git-blame case is basically done with a reverse commit-time based priority queue leading to a breadth-first strategy. It still beats unsorted access solidly in its timing. Don't think I compared depth-first results (inversing the priority queue sorting condition) with regard to cache results, but it's bad for interactive use as it tends to leave some recent history unblamed for a long time while digging up stuff in the remote past. Moderate cache size increases seem like a better strategy, and the default size of 16M does not make a lot of sense with modern computers. In particular since the history digging is rarely competing with other memory intensive operations at the same time. And that matches the timings I just did. I suspect there are still pathological cases that could behave worse, but it really sounds like we should be looking into improving that cache as a first step. I can put up a patch. My git-blame experiments used 128M, and the patch proposes a more conservative 64M. I don't actually have made experiments for the 64M setting, though. The current default is 16M. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] gc --aggressive: three phase repacking
Jeff King p...@peff.net writes: On Tue, Mar 18, 2014 at 12:50:50AM -0400, Jeff King wrote: On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote: As explained in the previous commit, current aggressive settings --depth=250 --window=250 could slow down repository access significantly. Notice that people usually work on recent history only, we could keep recent history more loosely packed, so that repo access is fast most of the time while the pack file remains small. One thing I have not seen is real-world timings showing the slowdown based on --depth. Did I miss them, or are we just making assumptions based on one old case from 2009 (that, AFAIK does not have real numbers, just speculation)? Has anyone measured the effect of bumping the delta cache size (and its hash implementation)? Just as a very quick, rough data point, here are before-and-after timings for the patch below doing git rev-list --objects --all on my linux.git, which is a mix of --aggressive and normal packing (I didn't do a repack -f, but it's partially what I've downloaded from k.org and what I've repacked in various experiments over the past few months). [before] real0m28.824s user0m28.620s sys 0m0.232s [after] real0m21.694s user0m21.544s sys 0m0.172s The numbers below are completely pulled out of a hat, so we can perhaps do even better. But I think it shows that there is room for improvement in the delta base cache. --- diff --git a/environment.c b/environment.c index c3c8606..73ed670 100644 --- a/environment.c +++ b/environment.c @@ -37,7 +37,7 @@ int core_compression_seen; int fsync_object_files; size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; -size_t delta_base_cache_limit = 16 * 1024 * 1024; +size_t delta_base_cache_limit = 128 * 1024 * 1024; You need to change a file in Documentation as well. Can offer a patch. unsigned long big_file_threshold = 512 * 1024 * 1024; const char *pager_program; int pager_use_color = 1; diff --git a/sha1_file.c b/sha1_file.c index b37c6f6..a9ab8e3 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1944,7 +1944,7 @@ static void *unpack_compressed_entry(struct packed_git *p, return buffer; } -#define MAX_DELTA_CACHE (256) +#define MAX_DELTA_CACHE (1024) This one really needs experimentation. I found that increases here lead to performance degradation rather soon, probably because of decreased memory locality without significant reduction in cache collisions. Not sure whether it's worth touching at all. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] gc --aggressive: three phase repacking
Duy Nguyen pclo...@gmail.com writes: On Tue, Mar 18, 2014 at 12:16 PM, Duy Nguyen pclo...@gmail.com wrote: But I think it's orthogonal to gc --aggressive improvement. There's another reason that improving gc may be a good idea (or not). It depends on how other git implementations handle long delta chains. Other git implementations including current installations that have a half-life of at last a year. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] diff: simplify cpp funcname regex
Cc René; do you have any comments regarding grep --function-context?
Am 3/18/2014 6:24, schrieb Jeff King:
On Fri, Mar 14, 2014 at 07:56:46AM +0100, Johannes Sixt wrote:
Consider this code:
void above()
{}
static int Y;
static int A;
int bar()
{
return X;
}
void below()
{}
Thanks, this example is very helpful.
When you 'git grep --function-context X', then you get this output with
the current pattern, you proposal, and my proposal (file name etc omitted
for brevity):
int bar()
{
return X;
}
Right, that makes sense to me.
When you 'git grep --function-context Y', what do you want to see? With
the current pattern, and with your pattern that forbids semicolon we get:
void above()
{}
static int Y;
static int A;
and with my simple pattern, which allows semicolon, we get merely
static int Y;
because the line itself is a hunk header (and we do not look back any
further) and the next line is as well. That is not exactly function
context, and that is what I'm a bit worried about.
Hmm. To be honest, I do not see yours as all that bad. Is above() or
A actually interesting here? I'm not sure that they are. But then I do
not use --function-context myself.
I guess it violates the show things that are vaguely nearby, rather
than a container view of context that we discussed earlier. But somehow
that seems less important to me with --function-context.
So I dunno. I kind of like your version.
Then I'll polish my patch series (it also rewrites the test framework) and
post 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
Bug report -- Illegal instruction on Mac 10.6.8 without XCode installed
Hi, I am running a Mac 10.6.8 and tried to install git-1.9.0 off of the installer (git-1.9.0-intel-universal-snow-leopard.dmg). The installation worked fine and gave no error messages. But whenever I type in a git command (see below for some I tried), it gives me this error message: Illegal instruction I am completely new to git and mostly new of Unix, but here are some commands I tried: git git help git config git init git clean git config --global user.name John Doe git status However, typing man git displays typical man pages. I do not have Xcode installed. (it's very hard to find a legacy copy); the make command also is not present, so I can't use any of the workarounds I saw listed. I uninstalled git-1.9.0 successfully using the provided script, then downloaded the same file again (and installed it) to make sure I didn't get a corrupt copy. I had the same problem, however. If I can provide any more information just let me know. Thanks for any help you can provide. -Ray -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-rebase: Teach rebase - shorthand.
Teach rebase the same shorthand as checkout and merge; that is, that -
means the branch we were previously on.
Reported-by: Tim Chase g...@tim.thechases.com
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
git-rebase.sh | 4
t/t3400-rebase.sh | 6 ++
2 files changed, 10 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test $fork_point = auto fork_point=t
;;
*) upstream_name=$1
+ if test $upstream_name = -
+ then
+ upstream_name=@{-1}
+ fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
'
+test_expect_success 'rebase using shorthand' '
+ git checkout master
+ git checkout -b shorthand HEAD^
+ GIT_TRACE=1 git rebase -
+'
+
test_expect_success 'rebase a single mode change' '
git checkout master
git branch -D topic
--
1.8.5.2 (Apple Git-48)
--
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 7/7] run-command: mark run_hook_with_custom_index as deprecated
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- run-command.h | 1 + 1 file changed, 1 insertion(+) diff --git a/run-command.h b/run-command.h index 88460f9..3653bfa 100644 --- a/run-command.h +++ b/run-command.h @@ -51,6 +51,7 @@ extern int run_hook_le(const char *const *env, const char *name, ...); extern int run_hook_ve(const char *const *env, const char *name, va_list args); LAST_ARG_MUST_BE_NULL +__attribute__((deprecated)) extern int run_hook_with_custom_index(const char *index_file, const char *name, ...); #define RUN_COMMAND_NO_STDIN 1 -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] merge hook tests: fix and update tests
- update 'no editor' hook test and add 'editor' hook test
- make sure the tree is reset to a clean state after running a test
(using test_when_finished) so later tests are not impacted
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
t/t7505-prepare-commit-msg-hook.sh | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/t/t7505-prepare-commit-msg-hook.sh
b/t/t7505-prepare-commit-msg-hook.sh
index 5531abb..03dce09 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -134,14 +134,26 @@ test_expect_success 'with hook (-c)' '
test_expect_success 'with hook (merge)' '
- head=`git rev-parse HEAD`
- git checkout -b other HEAD@{1}
- echo more file
+ test_when_finished git checkout -f master
+ git checkout -B other HEAD@{1}
+ echo more file
+ git add file
+ git commit -m other
+ git checkout -
+ git merge --no-ff other
+ test `git log -1 --pretty=format:%s` = merge (no editor)
+'
+
+test_expect_success 'with hook and editor (merge)' '
+
+ test_when_finished git checkout -f master
+ git checkout -B other HEAD@{1}
+ echo more file
git add file
git commit -m other
git checkout -
- git merge other
- test `git log -1 --pretty=format:%s` = merge
+ env GIT_EDITOR=\\$FAKE_EDITOR\ git merge --no-ff -e other
+ test `git log -1 --pretty=format:%s` = merge
'
cat $HOOK 'EOF'
@@ -151,6 +163,7 @@ EOF
test_expect_success 'with failing hook' '
+ test_when_finished git checkout -f master
head=`git rev-parse HEAD`
echo more file
git add file
@@ -160,6 +173,7 @@ test_expect_success 'with failing hook' '
test_expect_success 'with failing hook (--no-verify)' '
+ test_when_finished git checkout -f master
head=`git rev-parse HEAD`
echo more file
git add file
@@ -169,6 +183,7 @@ test_expect_success 'with failing hook (--no-verify)' '
test_expect_success 'with failing hook (merge)' '
+ test_when_finished git checkout -f master
git checkout -B other HEAD@{1}
echo more file
git add file
@@ -178,7 +193,7 @@ test_expect_success 'with failing hook (merge)' '
exit 1
EOF
git checkout -
- test_must_fail git merge other
+ test_must_fail git merge --no-ff other
'
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] merge hook tests: use 'test_must_fail' instead of '!'
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7505-prepare-commit-msg-hook.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 1c95652..5531abb 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -154,7 +154,7 @@ test_expect_success 'with failing hook' ' head=`git rev-parse HEAD` echo more file git add file - ! GIT_EDITOR=\\$FAKE_EDITOR\ git commit -c $head + test_must_fail env GIT_EDITOR=\\$FAKE_EDITOR\ git commit -c $head ' @@ -163,7 +163,7 @@ test_expect_success 'with failing hook (--no-verify)' ' head=`git rev-parse HEAD` echo more file git add file - ! GIT_EDITOR=\\$FAKE_EDITOR\ git commit --no-verify -c $head + test_must_fail env GIT_EDITOR=\\$FAKE_EDITOR\ git commit --no-verify -c $head ' -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] test patch hunk editing with commit -p -m
Add (failing) tests: with commit changing the environment to let hooks know that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7514-commit-patch.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7514-commit-patch.sh diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh new file mode 100755 index 000..41dd37a --- /dev/null +++ b/t/t7514-commit-patch.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 +' + +test_expect_failure 'edit hunk commit -p -m message' ' + test_when_finished rm -f editor_was_started + rm -f editor_was_started + echo more file + echo e | env GIT_EDITOR=: editor_was_started git commit -p -m commit2 file + test -r editor_was_started +' + +test_expect_failure 'edit hunk commit --dry-run -p -m message' ' + test_when_finished rm -f editor_was_started + rm -f editor_was_started + echo more file + echo e | env GIT_EDITOR=: editor_was_started git commit -p -m commit3 file + test -r editor_was_started +' + +test_done -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] merge hook tests: fix missing '' in test
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7505-prepare-commit-msg-hook.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 3573751..1c95652 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -174,7 +174,7 @@ test_expect_success 'with failing hook (merge)' ' git add file rm -f $HOOK git commit -m other - write_script $HOOK -EOF + write_script $HOOK -EOF exit 1 EOF git checkout - -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] merge: fix GIT_EDITOR override for commit hook
Don't set GIT_EDITOR to : when calling prepare-commit-msg hook if the
editor is going to be called (e.g. with merge -e).
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
builtin/merge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index bdf6655..e15d0e1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -824,7 +824,7 @@ static void prepare_to_commit(struct commit_list
*remoteheads)
if (0 option_edit)
strbuf_commented_addf(msg, _(merge_editor_comment),
comment_line_char);
write_merge_msg(msg);
- if (run_commit_hook(1, get_index_file(), prepare-commit-msg,
+ if (run_commit_hook(0 option_edit, get_index_file(),
prepare-commit-msg,
git_path(MERGE_MSG), merge, NULL))
abort_commit(remoteheads, NULL);
if (0 option_edit) {
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add grep.fullName config variable
Junio C Hamano gits...@pobox.com writes: Andreas Schwab sch...@linux-m68k.org writes: This configuration variable sets the default for the --full-name option. Signed-off-by: Andreas Schwab sch...@linux-m68k.org --- Would this change break Porcelains (e.g. Emacs modes) and force them to be updated to explicitly pass --no-full-name to unbreak them? Yes, that would be required. On the other hand, currently it is impossible to cut-n-paste a file name without --full-name, since the pager is always started in top-level. Perhaps it is better to fix the latter? 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: Bug report -- Illegal instruction on Mac 10.6.8 without XCode installed
On Tue, 18 Mar 2014 01:33:25 -0700 Ray Hengst rkhen...@gmail.com wrote: Hi, I am running a Mac 10.6.8 and tried to install git-1.9.0 off of the installer (git-1.9.0-intel-universal-snow-leopard.dmg). The installation worked fine and gave no error messages. But whenever I type in a git command (see below for some I tried), it gives me this error message: Illegal instruction I am completely new to git and mostly new of Unix, but here are some commands I tried: git git help git config git init git clean git config --global user.name John Doe git status However, typing man git displays typical man pages. This has nothing to do with your problem: the `man` program is not part of Git and presumably works; it just finds and reads the specified manual page--which is just plain text--renders it and shows to you. And your problem is with misbehaving Git binary. I do not have Xcode installed. (it's very hard to find a legacy copy); the make command also is not present, so I can't use any of the workarounds I saw listed. I uninstalled git-1.9.0 successfully using the provided script, then downloaded the same file again (and installed it) to make sure I didn't get a corrupt copy. I had the same problem, however. If I can provide any more information just let me know. Thanks for any help you can provide. I once came across this thread [1] on SO which says this might be due to the binaries compiled for a newer version of the OS than you have installed. In any case, [2] suggests the installer is prepared by the guy behind [3], and that project's bug tracker has a bug for exactly your problem already filed [4]. You might want to chime in there with more details if you feel like it. 1. http://stackoverflow.com/q/14268887/720999 2. http://git-scm.com/download/mac 3. http://sourceforge.net/projects/git-osx-installer/ 4. http://sourceforge.net/p/git-osx-installer/tickets/97/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] diff: simplify cpp funcname regex
Am 18.03.2014 09:02, schrieb Johannes Sixt:
Cc René; do you have any comments regarding grep --function-context?
Am 3/18/2014 6:24, schrieb Jeff King:
On Fri, Mar 14, 2014 at 07:56:46AM +0100, Johannes Sixt wrote:
Consider this code:
void above()
{}
static int Y;
static int A;
int bar()
{
return X;
}
void below()
{}
Thanks, this example is very helpful.
When you 'git grep --function-context X', then you get this output with
the current pattern, you proposal, and my proposal (file name etc omitted
for brevity):
int bar()
{
return X;
}
Right, that makes sense to me.
When you 'git grep --function-context Y', what do you want to see? With
the current pattern, and with your pattern that forbids semicolon we get:
void above()
{}
static int Y;
static int A;
and with my simple pattern, which allows semicolon, we get merely
static int Y;
because the line itself is a hunk header (and we do not look back any
further) and the next line is as well. That is not exactly function
context, and that is what I'm a bit worried about.
In global context there is no function context, of course, so the
latter makes sense.
grep --function-context is about useful context and its implementation
piggy-backs on the hunk header definitions. If those are useful then
the grep output should be fine as well. IAW: No worries, go ahead. :)
However, I only use the defaults heuristic (which shows just the Y-line
as well) and don't know C++, so I my opinion on this matter isn't worth
that much.
René
--
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()
This patch replaces if chain that selects the message with
2 dimensional array of format strings and arguments.
Signed-off-by: Aleksey Mokhovikov moxobu...@gmail.com
---
This patch is unrelated with previous one, but related to GSoC.
So I don't know if I should create new thread for this patch.
Compare with original construction
Pros:
1) Remove if chain.
2) Single table contains all messages with arguments in one contruction.
3) Less code duplication.
Cons:
1) Harder to associate the case with message.
2) Harder for compiler to warn the programmer about not
enough arguments for format string.
3) Harder to add non-string argument to messages.
If !!(x) is out of the coding guide, then message_id construction
can be rewritten in several other ways.
The guideline tells that !!(x) is not welcome and should not be
unless needed. But looks like this is normal place for it.
P.S.
Thanks to comments I got, so
I've read more GNU gettext manual to get info
about translation workflow. When I posted a first patch
I've passed the same message format strings to gettext /*_()*/
as in original, to save the context of the message. And they
will be translated if every possible string combination
will be marked separately for translation. Because of
xgettext can extract string from source automatically,
it ruins the idea to generate message from parts. Even if the
exaclty same message format string is passed to gettext.
branch.c | 53 ++---
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..51a5faf 100644
--- a/branch.c
+++ b/branch.c
@@ -47,12 +47,32 @@ static int should_setup_rebase(const char *origin)
return 0;
}
+
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
const char *shortname = remote + 11;
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ int message_id = (!!remote_is_branch 2) | (!!origin 1) |
(!!rebasing);
+ const char *message_table[][4] =
+ {{N_(Branch %s set up to track local ref %s.),
+ local, remote},
+{N_(Branch %s set up to track local ref %s by rebasing.),
+ local, remote},
+{N_(Branch %s set up to track remote ref %s.),
+ local, remote},
+{N_(Branch %s set up to track remote ref %s by
rebasing.),
+ local, remote},
+{N_(Branch %s set up to track local branch %s.),
+ local, shortname},
+{N_(Branch %s set up to track local branch %s by
rebasing.),
+ local, shortname},
+{N_(Branch %s set up to track remote branch %s from %s.),
+ local, shortname, origin},
+{N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ local, shortname, origin}};
+ const char **message = NULL;
if (remote_is_branch
!strcmp(local, shortname)
@@ -68,7 +88,7 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_reset(key);
strbuf_addf(key, branch.%s.merge, local);
git_config_set(key.buf, remote);
-
+
if (rebasing) {
strbuf_reset(key);
strbuf_addf(key, branch.%s.rebase, local);
@@ -77,29 +97,16 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch
%s from %s by rebasing.) :
- _(Branch %s set up to track remote branch
%s from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch
%s by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s
by rebasing.) :
- _(Branch %s set up to track remote ref
%s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s
by rebasing.) :
- _(Branch %s set up to track local ref
%s.),
-
[PATCH][GSOC2014] add: Rewrite run_add_interactive to use struct argv_array
Origin code are code with own realisation argv array editing.
It was changed, and code modified for using unified argv-array
realisation from argv-array.h.
Commit for Google Summer of Code 2014
Signed-off-by: Movchan Pavel movchan...@gmail.com
---
builtin/add.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 4b045ba..258b491 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
#include diffcore.h
#include revision.h
#include bulk-checkin.h
+#include argv-array.h
static const char * const builtin_add_usage[] = {
N_(git add [options] [--] pathspec...),
@@ -141,23 +142,21 @@ static void refresh(int verbose, const struct pathspec
*pathspec)
int run_add_interactive(const char *revision, const char *patch_mode,
const struct pathspec *pathspec)
{
- int status, ac, i;
- const char **args;
+ int status, i;
+ struct argv_array *argv = ARGV_ARRAY_INIT;
- args = xcalloc(sizeof(const char *), (pathspec-nr + 6));
- ac = 0;
- args[ac++] = add--interactive;
+ argv_array_push(argv, add--interactive);
if (patch_mode)
- args[ac++] = patch_mode;
+ argv_array_push(argv, patch_mode);
if (revision)
- args[ac++] = revision;
- args[ac++] = --;
+ argv_array_push(argv, revision);
+ argv_array_push(argv, --);
for (i = 0; i pathspec-nr; i++)
/* pass original pathspec, to be re-parsed */
- args[ac++] = pathspec-items[i].original;
+ argv_array_push(argv, pathspec-items[i].original);
- status = run_command_v_opt(args, RUN_GIT_CMD);
- free(args);
+ status = run_command_v_opt(argv-argv, RUN_GIT_CMD);
+ argv_array_clear(argv);
return status;
}
--
1.7.10.4
--
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
I have a deal
Please is your email really active? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git won Linux Magazine's Linux New Media Award in the category Outstanding Contribution to Open Source/Linux/Free Software
Dear all, Git won an award in the main category of the English German Linux Magazine at CeBIT, this year. Jens Lehmann, Heiko Voigt, and myself were present to accept the award on behalf of the Git community as a whole. You can find a short blurb on my blog[1], including a picture of the physical prize. It seems the video of the award ceremony is not up yet, but I have been told it will come soon(tm). Best regards, Richard [1] http://richardhartmann.de/blog/posts/2014/03/14-Git_prize-Outstanding_Contribution_to_Open_Source__Linux__Free_Software/ -- 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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()
Eric Sunshine sunshine at sunshineco.com writes: The subject should be concise. Try to keep it at 65-70 characters or less. More detailed information can be written following the subject (separated from the subject by a blank line). Write in imperative tone: say replace X with Y rather than X is replaced with Y. Mention the module or function you're touching. You might say something like this: Subject: install_branch_config: replace if-chain with string composition Wrap lines to 65-70 characters. This prose is almost pure email commentary. It doesn't really convey useful information to a person reading the patch months or years from now. Place commentary below the --- line under your sign-off. Thanks a lot for you language and message formatting style advices. I've make a new patch taking into account the GNU gettext requirements. I don't know if I should create a new thread for another patch, but I'd be glad if you will give me some information about new patch: http://permalink.gmane.org/gmane.comp.version-control.git/244357 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Tue, Mar 18, 2014 at 9:27 PM, Duy Nguyen pclo...@gmail.com wrote: I thought about GIT_CAPABILITIES= git-upload-pack ... (and actually added it in pack-protocol.txt then deleted). The thing is, if you want to new upload-pack, you would need new git-upload-pack at the remote end that must understand git-upload-pack repo caps already. Making it aware about GIT_CAPABILITIES is extra cost for nothing. And we have to update git-shell to support it eventually. Well, the must understand part is not entirely true. If you make git-daemon pass the early capabilities via GIT_CAPABILITIES too, upload-pack does not have to support repo caps syntax. The upside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll break the protocol (see below) and can print friendly error messages. git-daemon has no way of printing friendly messages because it can't negotiate side-band. I should have read my mail one more time before sending. The git-upload-pack ignores... sentence is wrong. If it's old, its behavior is fixed and it cannot not send or do anything new. But on the other hand, this is good. The new protocol expects upload-pack to send its caps in a new pkt-line. The old upload-pack does not follow this, which should be the indicator for the client that this server does not support v2, so it could fall back to v1 gracefully. git:// still fails hard because git-daemon is likely old too and rejects it from the beginning. But ssh:// (without git-shell) should work, http:// too. This is a very good point for GIT_CAPABILITIES. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Tue, Mar 18, 2014 at 12:17:39AM -0400, Jeff King wrote: On Fri, Mar 14, 2014 at 05:09:45PM -0700, Shawn Pearce wrote: On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen pclo...@gmail.com wrote: On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce spea...@spearce.org wrote: You missed the SSH case. It doesn't have this slot to hide the data into. Right now we run this for ssh case: ssh host git-upload-pack repo-path. New client can do this instead ssh host git-upload-pack repo-path client capability flags Older servers will fail on this command, and the client must reconnect over SSH, which may mean supplying their password/passphrase again. But its remembered that the uploadPack2 didn't work so this can be blacklisted and not retried for a while. I wonder if we could use the environment for optional values. E.g., can we run: ssh host GIT_CAPABILITIES=... git-upload-pack repo-path That will not work everywhere, of course. Sites with git-shell will fail, as will sites with custom ssh handler (GitHub, for example, and I imagine Gerrit sites, if they support ssh). So we'd still need some fallback, but it would work out-of-the-box in a reasonable number of cases (and it is really not that different than the http case, which is just stuffing the values into $QUERY_STRING anyway :) ). Aggressively gc'ing linux-2.6 takes forever (and it's being timed so I can't really do any heavy lifting), so I outlined what the new protocol would be instead. Note that at least for upload-pack client capabilities can be advertised twice: the first time at transport connection level, the second time in the first want, like in v1. I think this will keep the code change down when we have to support both protocols. Moving all capabilities to the first negotiation may touch many places, but that's for now a baseless guess. The new capability negotiation is also added for push. We didn't pay much attention to it so far. I thought about GIT_CAPABILITIES= git-upload-pack ... (and actually added it in pack-protocol.txt then deleted). The thing is, if you want to new upload-pack, you would need new git-upload-pack at the remote end that must understand git-upload-pack repo caps already. Making it aware about GIT_CAPABILITIES is extra cost for nothing. And we have to update git-shell to support it eventually. Well, the must understand part is not entirely true. If you make git-daemon pass the early capabilities via GIT_CAPABILITIES too, upload-pack does not have to support repo caps syntax. The upside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll break the protocol (see below) and can print friendly error messages. git-daemon has no way of printing friendly messages because it can't negotiate side-band. I'm still not sure. But we should support either way, not both. Anyway the text for new protocols: -- 8 -- diff --git a/Documentation/config.txt b/Documentation/config.txt index 79e5768..c329eb1 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2084,6 +2084,16 @@ remote.pushdefault:: `branch.name.remote` for all branches, and is overridden by `branch.name.pushremote` for specific branches. +remote.useUploadPack2:: + Set to always to use only upload-pack version 2, never to + always use the original upload-pack, auto to use the + original protocol, but if the remote claims it support version + 2, then set remote.name.useUploadPack2 to + always. Default to auto. + +remote.name.useUploadPack2:: + Override remote.useUploadPack2 per remote. + remote.name.url:: The URL of a remote repository. See linkgit:git-fetch[1] or linkgit:git-push[1]. diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 39c6410..3db4219 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -40,15 +40,22 @@ hostname parameter, terminated by a NUL byte. 0032git-upload-pack /project.git\0host=myserver.com\0 +Some service may accept an extra argument (e.g. upload-pack version +2). The extra argument must follow host. + + 0042git-upload-pack /project.git\0host=myserver.com\0flags=someflags\0 + -- - git-proto-request = request-command SP pathname NUL [ host-parameter NUL ] + git-proto-request = request-command SP pathname NUL + [ host-parameter NUL [ flags-parameter NUL ] ] request-command = git-upload-pack / git-receive-pack / git-upload-archive ; case sensitive pathname = *( %x01-ff ) ; exclude NUL host-parameter= host= hostname [ : port ] + flags-parameter = flags= *( %x01-ff ) ; exclude NULL -- -Only host-parameter is allowed in the git-proto-request. Clients +No other parameters are allowed in the git-proto-request. Clients MUST NOT attempt to send additional parameters. It is used for the git-daemon name
Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()
This patch replaces if chain with
2 dimensional array of format strings and arguments.
Signed-off-by: Aleksey Mokhovikov moxobu...@gmail.com
---
This patch is unrelated with previous one, but related to GSoC.
So I don't know if I should create new thread for this patch.
Compare with original construction
Pros:
1) Remove if chain.
2) Single table contains all messages with arguments in one construction.
3) Less code duplication.
Cons:
1) Harder to associate the case with message.
2) Harder for compiler to warn the programmer about not
enough arguments for format string.
3) Harder to add non-string argument to messages.
If !!(x) is out of the coding guide, then message_id construction
can be rewritten in several other ways.
The guideline tells that !!(x) is not welcome and should not be
unless needed. But looks like this is normal place for it.
P.S.
Thanks to comments I got, so
I've read more GNU gettext manual to get info
about translation workflow. When I posted a first patch
I've passed the same message format strings to gettext /*_()*/
as in original, to save the context of the message. And they
will be translated if every possible string combination
will be marked separately for translation. Because of
xgettext can extract string from source automatically,
it ruins the idea to generate message from parts. Even if the
exaclty same message format string is passed to gettext.
branch.c | 53 ++---
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..51a5faf 100644
--- a/branch.c
+++ b/branch.c
@@ -47,12 +47,32 @@ static int should_setup_rebase(const char *origin)
return 0;
}
+
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
const char *shortname = remote + 11;
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ int message_id = (!!remote_is_branch 2) | (!!origin 1) |
(!!rebasing);
+ const char *message_table[][4] =
+ {{N_(Branch %s set up to track local ref %s.),
+ local, remote},
+{N_(Branch %s set up to track local ref %s by rebasing.),
+ local, remote},
+{N_(Branch %s set up to track remote ref %s.),
+ local, remote},
+{N_(Branch %s set up to track remote ref %s by rebasing.),
+ local, remote},
+{N_(Branch %s set up to track local branch %s.),
+ local, shortname},
+{N_(Branch %s set up to track local branch %s by rebasing.),
+ local, shortname},
+{N_(Branch %s set up to track remote branch %s from %s.),
+ local, shortname, origin},
+{N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ local, shortname, origin}};
+ const char **message = NULL;
if (remote_is_branch
!strcmp(local, shortname)
@@ -68,7 +88,7 @@ void install_branch_config(int flag, const char *local, const
char *origin, cons
strbuf_reset(key);
strbuf_addf(key, branch.%s.merge, local);
git_config_set(key.buf, remote);
-
+
if (rebasing) {
strbuf_reset(key);
strbuf_addf(key, branch.%s.rebase, local);
@@ -77,29 +97,16 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
+ if
Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()
Matthieu Moy Matthieu.Moy at grenoble-inp.fr writes: Hi, Aleksey Mokhovikov moxobukob at gmail.com writes: Please, read the threads for other submissions for this microproject. Most remarks done there also apply for your case. See for example: http://thread.gmane.org/gmane.comp.version-control.git/244210 Thank you for your reply. I've read a bit more GNU gettext manual to get information about translation with GNU gettext. Long story short, the idea to generate message from parts will produce even more problems, despite the message strings passed to the _() are equal before and after the patch. So I decided to make an array with all messages and mark them for translation with N_() to keep them together. Because we now have an array we can improve it to make a table with message format string and arguments. Now we need just to find the proper message index to print the message. Please look at another approach: http://permalink.gmane.org/gmane.comp.version-control.git/244357 -- 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: Apply commits from one branch to another branch (tree structure is different)
Jagan:
On Fri, Mar 14, 2014 at 1:11 PM, Jagan Teki jagannadh.t...@gmail.com wrote:
Don't know what happen, I'm unable to join #git channel
[23:40] Jagan hi
[23:40] == Cannot send to channel: #git
I'm not sure if this is the problem that you were having, but #git on
freenode is NickServ protected. You need to register with NickServ (a
bot on the network) and identify yourself with it in order to be
allowed to speak in #git. This cuts down on spam. You can `/msg
NickServ help' to learn how to use it (and also Google will be your
friend).
Alternatively, check the channel topic for an alternative solution.
Regards,
--
Brandon McCaig bamcc...@gmail.com bamcc...@castopulence.org
Castopulence Software https://www.castopulence.org/
Blog http://www.bamccaig.com/
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
--
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: Apply commits from one branch to another branch (tree structure is different)
Jagan:
On Fri, Mar 14, 2014 at 12:39 PM, Jagan Teki jagannadh.t...@gmail.com wrote:
Hi,
Hello,
I have two branch in one repo that I need to maintain for 2 different
deliveries.
Say branch1 and branch2 in test.git repo.
test.git
- branch1
foo_v1/text.txt
foo_v2/text.txt
- branch2
foo/text.txt
branch1 is developers branch all source looks version'ed manner and
branch2 is superset for branch1, example foo_v1 and foo_v2 are the directories
in branch1 where developer will update the latest one here foo_v2 and branch2
foo is same as the latest one of branch1 for an instance.
Suppose developer send 10 patches on branch1 where are changes in terms
of dir_version/ then I need to apply on my local repo branch1, till now
is fine then I need to apply same 10 patches on to my branch2 where source
tree dir which is quite question here how can I do.
Request for any help! let me know for any questions.
This just sounds like a painful workflow to me. I would suggest not
doing this at all, but rather using tags to mark specific releases,
and using individual branches for continued development (e.g., stable
or v1.x or whatever is most appropriate). You can have unstable or
master or dev or whatever for developers to work on freely without
breaking releases (albeit, there are many different workflows you can
use to manage the transition from unstable to stable code).
I would avoid using subtrees (subdirectories) within a Git repository
to represent different releases of the code. Git already tracks
versions. That is redundant and messy. It's really an outdated way of
thinking about version control.
/my 2 cents
Regards,
--
Brandon McCaig bamcc...@gmail.com bamcc...@castopulence.org
Castopulence Software https://www.castopulence.org/
Blog http://www.bamccaig.com/
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Documentation/gitk: Document new config file location
User config file location now complies with XDG base directory specification Signed-off-by: Astril Hayato astrilhay...@gmail.com --- Documentation/gitk.txt | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index 1e9e38a..c2aa514 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -166,8 +166,13 @@ gitk --max-count=100 --all \-- Makefile:: Files - -Gitk creates the .gitk file in your $HOME directory to store preferences -such as display options, font, and colors. +User configuration and preferences are stored at (in order of priority): + +* '$XDG_CONFIG_HOME/git/gitk' if it exists and '$XDG_CONFIG_HOME' is set +* '$HOME/.config/git/gitk' if it exists +* '$HOME/.gitk' if it exists + +If none of the above exist then '$HOME/.config/git/gitk' is created and used by default. History --- -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add grep.fullName config variable
Andreas Schwab sch...@linux-m68k.org writes: Yes, that would be required. On the other hand, currently it is impossible to cut-n-paste a file name without --full-name, since the pager is always started in top-level. Perhaps it is better to fix the latter? So far we never cared where the pager runs, but as a principle, I think it would be nice if we run it in the original subdirectory, not at the top of the working tree (unless we have to bend backwards to make the codepath involved too ugly, that is). Don't we have the exact same issue for the editor, by the way? Shouldn't we be running it in the original subdirectory as well? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] test-lib.sh: do not echo externally supplied strings
Uwe Storbeck u...@ibr.ch writes:
In some places we echo a string that is supplied by the calling
test script and may contain backslash sequences. The echo command
of some shells, most notably dash, interprets these backslash
sequences (POSIX.1 allows this) which may scramble the test
output.
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
Commit message rewritten to avoid title continuation in the body.
Thanks Junio C Hamano for the hint.
Here is what I queued yesterday. I was wrong to say control
characters; a backslash sequence is not necessarily a control
character (e.g. \c at the end that suppresses the final LF), so I'll
replace it with your version.
The test titles are not externally supplied but under our control,
so it is less problematic than the rebase -i case where we do get
bitten by user supplied commit title string. Still it is a good
idea to apply this change to protect ourselves.
Thanks.
commit 215cd588caebe86fe77115bdda97930b4659aecd
Author: Uwe Storbeck u...@ibr.ch
Date: Sat Mar 15 00:57:36 2014 +0100
test-lib.sh: do not echo test titles
The test title could be a string with backslash in it, and can be
interpreted as control characters by the echo command of some shells
(e.g. dash).
Signed-off-by: Uwe Storbeck u...@ibr.ch
Signed-off-by: Junio C Hamano gits...@pobox.com
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..3c7cb1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $* | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] general style: replaces memcmp() with starts_with()
Eric Sunshine sunsh...@sunshineco.com writes: A patch of this nature doesn't require much more description than stating what it does (replace memcmp() with starts_with()) and why (improve code clarity). The following rewrite might be sufficient: Subject: replace memcmp() with starts_with() starts_with() indicates the intention of the check more clearly than memcmp(). This is more concise; thank you. I will adapt this as the message for the next version of this patch. Would it be wise to mention magic numbers, as the discussion surrounding the rationale of this patch, especially with Junio and Michael, has centered around that? I was thinking of mentioning magic numbers in the example, but decided that their removal was a natural and obvious consequence of the improvement to code clarity, so it wasn't strictly necessary to talk about it. On the other hand, it is a good secondary justification, thus it should be perfectly acceptable to mention it. If I was writing the commit message, I might start by saying As an additional benefit... and then talk a bit about magic number retirement. I think your subject is good (as you said, it makes it clear that it is a project-wide clean-up by not mentioning any specific area), but indicates the intention of the check more clearly would not tell new readers who are unfamiliar with the helper API what intention it is talking about very much, so perhaps Subject: use starts_with() instead of !memcmp() When checking if a string begins with a constant string, using starts_with() is less error prone than calling !memcmp() with an explicit byte count. 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
Re: [PATCH] git-rebase: Teach rebase - shorthand.
On 03/18/2014 09:44 AM, Brian Gesiak wrote:
Teach rebase the same shorthand as checkout and merge; that is, that -
means the branch we were previously on.
Reported-by: Tim Chase g...@tim.thechases.com
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
git-rebase.sh | 4
t/t3400-rebase.sh | 6 ++
2 files changed, 10 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test $fork_point = auto fork_point=t
;;
*) upstream_name=$1
+ if test $upstream_name = -
+ then
+ upstream_name=@{-1}
+ fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
'
+test_expect_success 'rebase using shorthand' '
+ git checkout master
we schould have the^^
+ git checkout -b shorthand HEAD^
we schould have the ^^
+ GIT_TRACE=1 git rebase -
And why the GIT_TRACE ?
+'
+
test_expect_success 'rebase a single mode change' '
git checkout master
git branch -D topic
--
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: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?
Jacopo Notarstefano jacopo.notarstef...@gmail.com writes: It seems to me that the topic of adding the checkpatch.pl script to Git's source tree has cropped up several times in the past, as recently as a couple of days ago: $gmane/243607. It should be noted that its usage for its sake has been discouraged by Junio Hamano in $gmane/205998. I've never said any such thing. I only said I am not enthused against a proposal to add a build target that runs checkpatch or equivalent over *all* existing code, which will invite needless churn (read again the part of the message you quoted before I say I am not enthused). It is a totally separate issue for submitters to make sure they do not introduce *new* problems, and use of checkpatch --no-tree could be one way to do so. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: Teach rebase - shorthand.
Brian Gesiak modoca...@gmail.com writes:
Teach rebase the same shorthand as checkout and merge; that is, that -
means the branch we were previously on.
Reported-by: Tim Chase g...@tim.thechases.com
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
git-rebase.sh | 4
t/t3400-rebase.sh | 6 ++
2 files changed, 10 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test $fork_point = auto fork_point=t
;;
*) upstream_name=$1
+ if test $upstream_name = -
+ then
+ upstream_name=@{-1}
+ fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
'
+test_expect_success 'rebase using shorthand' '
+ git checkout master
+ git checkout -b shorthand HEAD^
+ GIT_TRACE=1 git rebase -
I'd rather not to see that TRACE there. We would also want to make
sure the result is what we expect to see, not only the command does
not error out, no?
+'
+
test_expect_success 'rebase a single mode change' '
git checkout master
git branch -D topic
--
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][GSOC2014] add: Rewrite run_add_interactive to use struct argv_array
Movchan Pavel movchan...@gmail.com writes:
Origin code are code with own realisation argv array editing.
It was changed, and code modified for using unified argv-array
realisation from argv-array.h.
Commit for Google Summer of Code 2014
Signed-off-by: Movchan Pavel movchan...@gmail.com
---
Thanks. Commit for ... is not something we would want to see in
git log output, though.
builtin/add.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 4b045ba..258b491 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
#include diffcore.h
#include revision.h
#include bulk-checkin.h
+#include argv-array.h
static const char * const builtin_add_usage[] = {
N_(git add [options] [--] pathspec...),
@@ -141,23 +142,21 @@ static void refresh(int verbose, const struct pathspec
*pathspec)
int run_add_interactive(const char *revision, const char *patch_mode,
const struct pathspec *pathspec)
{
- int status, ac, i;
- const char **args;
+ int status, i;
+ struct argv_array *argv = ARGV_ARRAY_INIT;
Where does that pointer point at and who allocated the piece of
memory used by it?
See http://thread.gmane.org/gmane.comp.version-control.git/244151
for an example solution.
- args = xcalloc(sizeof(const char *), (pathspec-nr + 6));
- ac = 0;
- args[ac++] = add--interactive;
+ argv_array_push(argv, add--interactive);
if (patch_mode)
- args[ac++] = patch_mode;
+ argv_array_push(argv, patch_mode);
if (revision)
- args[ac++] = revision;
- args[ac++] = --;
+ argv_array_push(argv, revision);
+ argv_array_push(argv, --);
for (i = 0; i pathspec-nr; i++)
/* pass original pathspec, to be re-parsed */
- args[ac++] = pathspec-items[i].original;
+ argv_array_push(argv, pathspec-items[i].original);
- status = run_command_v_opt(args, RUN_GIT_CMD);
- free(args);
+ status = run_command_v_opt(argv-argv, RUN_GIT_CMD);
+ argv_array_clear(argv);
return status;
}
--
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 grep.fullName config variable
Junio C Hamano gits...@pobox.com writes: Don't we have the exact same issue for the editor, by the way? Shouldn't we be running it in the original subdirectory as well? It's called with an absolute name, so it shouldn't care. 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: Git won Linux Magazine's Linux New Media Award in the category Outstanding Contribution to Open Source/Linux/Free Software
Richard Hartmann richih.mailingl...@gmail.com writes: Dear all, Git won an award in the main category of the English German Linux Magazine at CeBIT, this year. Jens Lehmann, Heiko Voigt, and myself were present to accept the award on behalf of the Git community as a whole. You can find a short blurb on my blog[1], including a picture of the physical prize. It seems the video of the award ceremony is not up yet, but I have been told it will come soon(tm). Best regards, Richard [1] http://richardhartmann.de/blog/posts/2014/03/14-Git_prize-Outstanding_Contribution_to_Open_Source__Linux__Free_Software/ Thanks, all. -- 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 grep.fullName config variable
Andreas Schwab sch...@linux-m68k.org writes: Junio C Hamano gits...@pobox.com writes: Andreas Schwab sch...@linux-m68k.org writes: This configuration variable sets the default for the --full-name option. Signed-off-by: Andreas Schwab sch...@linux-m68k.org --- Would this change break Porcelains (e.g. Emacs modes) and force them to be updated to explicitly pass --no-full-name to unbreak them? Yes, that would be required. On the other hand, currently it is impossible to cut-n-paste a file name without --full-name, since the pager is always started in top-level. Perhaps it is better to fix the latter? On the third hand, git grep isn't plumbing, so variation of output is to be expected? We already have grep.lineNumber and grep.patternType / grep.extendedRegexp (vc-git-grep uses -n itself, but does not protect against grep.patternType). 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
[PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
Originally, the code used subshells instead of FOO=BAR command because the variable would otherwise leak into the surrounding context of the POSIX shell when 'command' is a shell function. The subshell was used to hold the context for the test. Using 'env' in the test function sets the temp variables without leaking, removing the need of a subshell. Signed-off-by: David Tran unsignedz...@gmail.com --- Let's see if I replied correctly with send-email. Retrying this again. How do I 'reply' to a thread using send-email? I am David Tran a graduating CS/Math senior from Sonoma State University, United States. I would like to work with git for GSoC'14, specifically the line options for git rebase --interactive [1]. I have used git for a few years and know how destructive but important rebase is to git. I have created a few shell scripts here and there to make life using bash/zsh easier. I would like to apply these skills and work with the best. Github: unsignedzero [1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967 Oh, really ;-)? Missed that. Thanks. Getting closer, I think. Slowly but surely. Signed-off-by: David Tran unsignedz...@gmail.com --- t/t1300-repo-config.sh| 17 ++ t/t1510-repo-setup.sh |4 +-- t/t3200-branch.sh | 12 +-- t/t3301-notes.sh | 22 -- t/t3404-rebase-interactive.sh | 65 t/t3413-rebase-hook.sh|6 +--- t/t4014-format-patch.sh | 14 ++--- t/t5305-include-tag.sh|4 +-- t/t5602-clone-remote-exec.sh | 13 ++-- t/t5801-remote-helpers.sh |6 +-- t/t6006-rev-list-format.sh|9 ++ t/t7006-pager.sh | 18 ++- 12 files changed, 42 insertions(+), 148 deletions(-) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' test_expect_success SYMLINKS 'symlink to nonexistent configuration' ' ln -s doesnotexist linktonada ln -s linktonada linktolinktonada - ( - GIT_CONFIG=linktonada - export GIT_CONFIG - test_must_fail git config --list - GIT_CONFIG=linktolinktonada - test_must_fail git config --list - ) + test_must_fail env GIT_CONFIG=linktonada git config --list + test_must_fail env GIT_CONFIG=linktolinktonada git config --list ' test_expect_success 'check split_cmdline return' diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index cf2ee78..e1b2a99 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version) setup_repo 30 $here/30 gitfile true ( cd 30 - GIT_DIR=.git - export GIT_DIR - test_must_fail git symbolic-ref HEAD 2result + test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2result ) grep core.bare and core.worktree 30/result ' diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index fcdb867..d45e95c 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' ' write_script editor -\EOF echo New contents $1 EOF - ( - EDITOR=./editor - export EDITOR - test_must_fail git branch --edit-description no-such-branch - ) + test_must_fail env EDITOR=./editor git branch --edit-description no-such-branch ' test_expect_success 'refuse --edit-description on unborn branch for now' ' @@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' ' echo New contents $1 EOF git checkout --orphan unborn - ( - EDITOR=./editor - export EDITOR - test_must_fail git branch --edit-description - ) + test_must_fail env EDITOR=./editor git branch --edit-description ' test_expect_success '--merged catches invalid object names' ' diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index 3bb79a4..cfd67ff 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh export GIT_EDITOR test_expect_success 'cannot annotate
Re: [PATCH] Add grep.fullName config variable
Andreas Schwab sch...@linux-m68k.org writes: Junio C Hamano gits...@pobox.com writes: Don't we have the exact same issue for the editor, by the way? Shouldn't we be running it in the original subdirectory as well? It's called with an absolute name, so it shouldn't care. But we should not have to call with absolute paths when a short and sweet pathname relative to the user's current directory. That is the primary point of my comment. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
David Tran unsignedz...@gmail.com writes: Originally, the code used subshells instead of FOO=BAR command because the variable would otherwise leak into the surrounding context of the POSIX shell when 'command' is a shell function. The subshell was used to hold the context for the test. Using 'env' in the test function sets the temp variables without leaking, removing the need of a subshell. These are not temp variables ;-). You are improving the way how commands are run under a different settings to environment variables. Hmm, let's try to see if I can do better: Subject: tests: use env to run commands with temporary env-var settings Ordinarily, we would say VAR=VAL command to execute a tested command with environment variable(s) set only for that command. This however does not work if 'command' is a shell function (most notably 'test_must_fail'); the result of the assignment is retained and affects later commands. To avoid this, we used to assign and export environment variables and run such a test in a subshell, ( VAR=VAL export VAR test_must_fail git command to be tested ) but with env utility, we should be able to say test_must_fail env VAR=VAL git command to be tested which is much shorter and easier to read. Let's see if I replied correctly with send-email. Retrying this again. How do I 'reply' to a thread using send-email? Look for --in-reply-to option in man git-send-email. Signed-off-by: David Tran unsignedz...@gmail.com --- t/t1300-repo-config.sh| 17 ++ t/t1510-repo-setup.sh |4 +-- t/t3200-branch.sh | 12 +-- t/t3301-notes.sh | 22 -- t/t3404-rebase-interactive.sh | 65 t/t3413-rebase-hook.sh|6 +--- t/t4014-format-patch.sh | 14 ++--- t/t5305-include-tag.sh|4 +-- t/t5602-clone-remote-exec.sh | 13 ++-- t/t5801-remote-helpers.sh |6 +-- t/t6006-rev-list-format.sh|9 ++ t/t7006-pager.sh | 18 ++- 12 files changed, 42 insertions(+), 148 deletions(-) Thanks. The numbers look very good ;-) We love code reduction. diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' test_expect_success SYMLINKS 'symlink to nonexistent configuration' ' ln -s doesnotexist linktonada ln -s linktonada linktolinktonada - ( - GIT_CONFIG=linktonada - export GIT_CONFIG - test_must_fail git config --list - GIT_CONFIG=linktolinktonada - test_must_fail git config --list - ) + test_must_fail env GIT_CONFIG=linktonada git config --list + test_must_fail env GIT_CONFIG=linktolinktonada git config --list ' test_expect_success 'check split_cmdline return' diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index cf2ee78..e1b2a99 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version) setup_repo 30 $here/30 gitfile true ( cd 30 - GIT_DIR=.git - export GIT_DIR - test_must_fail git symbolic-ref HEAD 2result + test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2result ) grep core.bare and core.worktree 30/result ' diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index fcdb867..d45e95c 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' ' write_script editor -\EOF echo New contents $1 EOF - ( - EDITOR=./editor - export EDITOR - test_must_fail git branch --edit-description no-such-branch - ) + test_must_fail env EDITOR=./editor git branch --edit-description no-such-branch ' test_expect_success 'refuse --edit-description on unborn branch for now' ' @@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' ' echo New contents $1 EOF git checkout --orphan
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
On Tue, Mar 18, 2014 at 8:08 AM, David Tran unsignedz...@gmail.com wrote: Originally, the code used subshells instead of FOO=BAR command because the variable would otherwise leak into the surrounding context of the POSIX shell when 'command' is a shell function. The subshell was used to hold the context for the test. Using 'env' in the test function sets the temp variables without leaking, removing the need of a subshell. Signed-off-by: David Tran unsignedz...@gmail.com --- Oh, really ;-)? Missed that. Thanks. Getting closer, I think. Slowly but surely. Getting better. See below. Signed-off-by: David Tran unsignedz...@gmail.com --- diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 50e22b1..4c7364a 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' ' git checkout master ( set_fake_editor - FAKE_LINES=exec_echo_foo_file1 1 - export FAKE_LINES - test_must_fail git rebase -i HEAD^ + test_must_fail env FAKE_LINES=exec_echo_foo_file1 1 git rebase -i HEAD^ ) In a previous review, I asked if this subshell could be dropped or if it was required for set_fake_editor. I didn't quite understand your response, so I tested it myself, and found that the subshell can be eliminated safely without breaking this or later tests. test_cmp_rev master^ HEAD git reset --hard @@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' ' test_when_finished git rebase --abort ( set_fake_editor - FAKE_LINES=exec_this-command-does-not-exist 1 - export FAKE_LINES - test_must_fail git rebase -i HEAD^ actual 21 + test_must_fail env FAKE_LINES=exec_this-command-does-not-exist 1 \ + git rebase -i HEAD^ actual 21 ) Ditto for this subshell. ! grep Maybe git-rebase is broken actual ' @@ -528,11 +509,7 @@ test_expect_success 'aborted --continue does not squash commits after edit' ' FAKE_LINES=edit 1 git rebase -i HEAD^ echo edited again file7 git add file7 - ( - FAKE_COMMIT_MESSAGE= - export FAKE_COMMIT_MESSAGE - test_must_fail git rebase --continue - ) + test_must_fail env FAKE_COMMIT_MESSAGE= git rebase --continue Broken -chain. test $old = $(git rev-parse HEAD) git rebase --abort ' -- 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] branch.c: simplify chain of if statements
On Mon, Mar 17, 2014 at 7:46 AM, Dragos Foianu dragos.foi...@gmail.com wrote: Eric Sunshine sunshine at sunshineco.com writes: Matthieu already mentioned [2] that this sort of lego string construction is not internationalization-friendly. See section 4.3 [3] of the gettext manual for details. I was hoping to get away with using less memory by having only four entries in the table. I suppose that is not possible. The rebasing check can still be moved outside of the four if statements and calculate the index correctly. The strings would then have to be arranged in such a way to make this work. Using a multiple-dimension array as suggested in other submissions for this particular microproject would probably be better, but it has already been done. If a multi-dimension table is indeed better than other alternatives, then that's a good reason to choose it, even if others have already used that approach in their submissions. It's more important that the code is clean and easy to understand and maintain than to be clever. If you're really interested in trying an approach not already submitted by others, take a look at Jonathan's idea [1]. If you play around with it and find that it actually does make the code clearer and simpler, then perhaps it's worth submitting. If not, then not. [1]: http://thread.gmane.org/gmane.comp.version-control.git/198882/focus=198902 These hard-coded indexing constants (0, 1, 2, 3) are fragile and convey little meaning to the reader. Try to consider how to compute the index into verbose_prints[] based upon the values of 'remote_is_branch' and 'origin'. What are the different ways you could do so? I was going to do something like this: if !remote_is_branch the index goes incremented by 2, because the first two entries are of no interest and if !origin, the index is incremented by 1. This would correctly compute the index. It should also work with the rebasing check if the four rebasing-specific messages are at the end of the table and when rebasing the index is set to start at those messages. The reason I did not go with this is because I would still need the four ifs in order to keep the bug check part of the code. I might be able to find a work-around for it on the second attempt. Since the result is just a number, its possible to compute it directly without conditionals, however, it does start resembling a magical incantation. (I'll comment further in your v2 submission.) I have seen N_() used in other code but I wasn't sure what its purpose was. Thank you very much for the review. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote: diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' Isn't GIT_CONFIG here another way of saying: test_must_fail git config -f doesnotexist --list Perhaps that is shorter and more readable still (and there are a few similar cases in this patch. -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
[ANNOUNCE] Git v1.9.1
The latest maintenance release Git v1.9.1 is now available at the usual places. The release tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the v1.9.1 tag and the maint branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.9.1 Release Notes Fixes since v1.9.0 -- * git clean -d pathspec did not use the given pathspec correctly and ended up cleaning too much. * git difftool misbehaved when the repository is bound to the working tree with the .git file mechanism, where a textual file .git tells us where it is. * git push did not pay attention to branch.*.pushremote if it is defined earlier than remote.pushdefault; the order of these two variables in the configuration file should not matter, but it did by mistake. * Codepaths that parse timestamps in commit objects have been tightened. * git diff --external-diff incorrectly fed the submodule directory in the working tree to the external diff driver when it knew it is the same as one of the versions being compared. * git reset needs to refresh the index when working in a working tree (it can also be used to match the index to the HEAD in an otherwise bare repository), but it failed to set up the working tree properly, causing GIT_WORK_TREE to be ignored. * git check-attr when working on a repository with a working tree did not work well when the working tree was specified via the --work-tree (and obviously with --git-dir) option. * merge-recursive was broken in 1.7.7 era and stopped working in an empty (temporary) working tree, when there are renames involved. This has been corrected. * git rev-parse was loose in rejecting command line arguments that do not make sense, e.g. --default without the required value for that option. * include.path variable (or any variable that expects a path that can use ~username expansion) in the configuration file is not a boolean, but the code failed to check it. * git diff --quiet -- pathspec1 pathspec2 sometimes did not return correct status value. * Attempting to deepen a shallow repository by fetching over smart HTTP transport failed in the protocol exchange, when no-done extension was used. The fetching side waited for the list of shallow boundary commits after the sending end stopped talking to it. * Allow git cmd path/, when the 'path' is where a submodule is bound to the top-level working tree, to match 'path', despite the extra and unnecessary trailing slash (such a slash is often given by command line completion). Changes since v1.9.0 are as follows: Brad King (4): t3030-merge-recursive: test known breakage with empty work tree read-cache.c: refactor --ignore-missing implementation read-cache.c: extend make_cache_entry refresh flag with options merge-recursive.c: tolerate missing files while refreshing index David Aguilar (1): difftool: support repositories with .git-files David Sharp (1): rev-parse: check i before using argv[i] against argc Jeff King (12): expand_user_path: do not look at NULL path handle_path_include: don't look at NULL value tests: auto-set LIB_HTTPD_PORT from test name t4212: test bogus timestamps with git-log fsck: report integer overflow in author timestamps date: check date overflow against time_t log: handle integer overflow in timestamps log: do not segfault on gmtime errors remote: handle pushremote config in any order show_ident_date: fix tz range check clean: respect pathspecs with -d clean: simplify dir/not-dir logic Junio C Hamano (4): t0003: do not chdir the whole test process check-attr: move to the top of working tree when in non-bare repository t7800: add a difftool test for .git-files Git 1.9.1 Nguyễn Thái Ngọc Duy (17): test: rename http fetch and push test files pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done' protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt protocol-capabilities.txt: document no-done fetch-pack: fix deepen shallow over smart http with no-done cap t5537: move http tests out to t5539 reset: optionally setup worktree and refresh index on --mixed pathspec: convert some match_pathspec_depth() to ce_path_match() pathspec: convert some match_pathspec_depth() to dir_path_match() pathspec: rename match_pathspec_depth() to match_pathspec() dir.c:
Re: [PATCH] test-lib.sh: use printf instead of echo
Junio C Hamano wrote:
Uwe Storbeck wrote:
+ printf '%s\n' $@ | sed -e 's/^/# /'
This is wrong, isn't it? Why do we want one line per item here?
Yes, Hannes caught the same, too. Sorry for the sloppiness.
We currently use echo all over the place (e.g., 'echo $path' in
git-sh-setup), and every time we fix it there is a chance of making
mistakes. I wonder if it would make sense to add a helper to make the
echo calls easier to replace:
-- 8 --
Subject: git-sh-setup: introduce sane_echo helper
Using 'echo' with arguments that might contain backslashes or -e or
-n can produce confusing results that vary from platform to platform
(e.g., dash always interprets \ escape sequences and echoes -e
verbatim, whereas bash does not interpret \ escapes unless -e is
passed as an argument to echo and suppresses the -e from its
output).
Instead, we should use printf, which is more predictable:
printf '%s\n' $foo; # Just prints $foo, on all platforms.
Blindly replacing echo with printf '%s\n' would not be good enough
because that printf prints each argument on its own line. Provide a
sane_echo helper that prints its arguments, space-delimited, on a
single line, to make this easier to remember, and tweak 'say'
and 'die_with_status' to illustrate how it is used.
No functional change intended.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
git-sh-setup.sh | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git i/git-sh-setup.sh w/git-sh-setup.sh
index 256c89a..f35b5b9 100644
--- i/git-sh-setup.sh
+++ w/git-sh-setup.sh
@@ -43,6 +43,10 @@ git_broken_path_fix () {
# @@BROKEN_PATH_FIX@@
+sane_echo () {
+ printf '%s\n' $*
+}
+
die () {
die_with_status 1 $@
}
@@ -50,7 +54,7 @@ die () {
die_with_status () {
status=$1
shift
- printf 2 '%s\n' $*
+ sane_echo 2 $*
exit $status
}
@@ -59,7 +63,7 @@ GIT_QUIET=
say () {
if test -z $GIT_QUIET
then
- printf '%s\n' $*
+ sane_echo $*
fi
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
Jeff King p...@peff.net writes: On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote: diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' Isn't GIT_CONFIG here another way of saying: test_must_fail git config -f doesnotexist --list Perhaps that is shorter and more readable still (and there are a few similar cases in this patch. Surely, but are we assuming that git config correctly honors the equivalence between GIT_CONFIG=file and -f file, or is that also something we are testing in these tests? -- 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: [PATCHv2] branch.c: simplify chain of if statements
Thanks for the resubmission. Comments below...
On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu dragos.foi...@gmail.com wrote:
This patch uses a table to store the different messages that can
be emitted by the verbose install_branch_config function. It
computes an index based on the three flags and prints the message
located at the specific index in the table of messages. If the
index somehow is not within the table, we have a bug.
Most of this text can be dropped due to redundancy.
Saying This patch... is unnecessary.
The remaining text primarily says in prose what the patch itself
conveys more concisely and precisely. It's easier to read and
understand the actual code than it is to wade through a lengthy
description of the code change.
Speak in imperative voice: Use a table to store...
You might, for instance, say instead something like this:
install_branch_config() uses a long, somewhat complex if-chain to
select a message to display in verbose mode. Simplify the logic
by moving the messages to a table from which they can be
easily retrieved without complex logic.
Signed-off-by: Dragos Foianu dragos.foi...@gmail.com
---
branch.c | 44 +---
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..95645d5 100644
--- a/branch.c
+++ b/branch.c
@@ -54,6 +54,18 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *messages[] = {
+ N_(Branch %s set up to track local ref %s.),
+ N_(Branch %s set up to track remote ref %s.),
+ N_(Branch %s set up to track local branch %s.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.)
+ };
+ int index = 0;
+
if (remote_is_branch
!strcmp(local, shortname)
!origin) {
@@ -76,28 +88,22 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
}
strbuf_release(key);
+ if (origin)
+ index += 1;
+ if (remote_is_branch)
+ index += 2;
+ if (rebasing)
+ index += 4;
Other submissions have computed this value mathematically without need
for conditionals. For instance, we've seen:
index = (!!origin 0) + (!!remote_is_branch 1) + (!!rebasing 2)
as, well as the equivalent:
index = !!origin + !!remote_is_branch * 2 + !!rebasing * 4
Although this works, it does place greater cognitive demands on the
reader by requiring more effort to figure out what is going on and how
it relates to table position. The original (ungainly) chain of 'if'
statements in the original code does not suffer this problem. It
likewise is harder to understand than merely indexing into a
multi-dimension table where each variable is a key.
if (flag BRANCH_CONFIG_VERBOSE) {
if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch
%s from %s by rebasing.) :
- _(Branch %s set up to track remote branch
%s from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch
%s by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s
by rebasing.) :
- _(Branch %s set up to track remote ref
%s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s
by rebasing.) :
- _(Branch %s set up to track local ref
%s.),
- local, remote);
+ printf_ln(_(messages[index]),
+ local, shortname, origin);
else
+ printf_ln(_(messages[index]),
+ local, (!remote_is_branch) ? remote :
shortname);
It's possible to simplify this
Re: [PATCH] mv: prevent mismatched data when ignoring errors.
Michael Haggerty mhag...@alum.mit.edu writes:
I had recently been thinking along the same lines. In many of the
potential callers that I noticed, ALLOC_GROW() was used immediately
before making space in the array for a new element. So I suggest
something more like
+#define MOVE_DOWN(array, nr, at, count) \
+ memmove((array) + (at) + (count), \
+ (array) + (at), \
+ sizeof((array)[0]) * ((nr) - (at)))
+#define ALLOC_INSERT_GAP(array, nr, at, count, alloc) \
+ do { \
+ ALLOC_GROW((array), (nr) + (count), (alloc));\
+ MOVE_DOWN((array), (nr), (at), (count)); \
+ } while (0)
Also, count==1 is so frequent that this special case might deserve its
own macro pair.
Yeah, probably.
I'm not inspired by these macro names, though.
Me neither, about ups and downs.
Peff's suggestion to name these around the concept of gap sounded
sensible.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
On Tue, Mar 18, 2014 at 5:45 PM, Jeff King p...@peff.net wrote: On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote: diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' Isn't GIT_CONFIG here another way of saying: test_must_fail git config -f doesnotexist --list Perhaps that is shorter and more readable still (and there are a few similar cases in this patch. Such a change could be the subject of a separate cleanup patch, but is tangental to the GSoC microproject which begat this submission. -- 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] fsck.c:fsck_commit() use starts_with() and skip_prefix()
use of starts_with() instead of memcmp() use of skip_prefix instead of memcmp() and strlen() Signed-off-by: Othman Darraz darraz...@gmail.com --- I am planning to apply to GSOC 214 fsck.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c +++ b/fsck.c @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int err; - if (memcmp(buffer, tree , 5)) + if (starts_with(buffer, tree )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') return error_func(commit-object, FSCK_ERROR, invalid 'tree' line format - bad sha1); @@ -322,15 +322,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) if (p || parents) return error_func(commit-object, FSCK_ERROR, parent objects missing); } - if (memcmp(buffer, author , 7)) + if (starts_with(buffer, author )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'author' line); buffer += 7; err = fsck_ident(buffer, commit-object, error_func); if (err) return err; - if (memcmp(buffer, committer , strlen(committer ))) + buffer = (char* )skip_prefix(buffer,committer ); + if (!buffer) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'committer' line); - buffer += strlen(committer ); err = fsck_ident(buffer, commit-object, error_func); if (err) return err; -- 1.9.0.258.g00eda23.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell
On Tue, Mar 18, 2014 at 03:16:27PM -0700, Junio C Hamano wrote: Isn't GIT_CONFIG here another way of saying: test_must_fail git config -f doesnotexist --list Perhaps that is shorter and more readable still (and there are a few similar cases in this patch. Surely, but are we assuming that git config correctly honors the equivalence between GIT_CONFIG=file and -f file, or is that also something we are testing in these tests? I think we can assume that they are equivalent, and it is not worth testing (and they are equivalent in code since 270a344 (config: stop using config_exclusive_filename, 2012-02-16). My recollection is that GIT_CONFIG mostly exists as a historical footnote. Recall that at one time it affected all commands, but that had many problems and was done away with in dc87183 (Only use GIT_CONFIG in git config, not other programs, 2008-06-30). I think we left it in place for git-config mostly for backward compatibility, but I didn't see that point explicitly addressed in the list discussion (the main issue was that setting it for things besides git config is a bad idea, as it suppresses ~/.gitconfig). -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] fsck.c:fsck_commit() use starts_with() and skip_prefix()
Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz darraz...@gmail.com wrote: use of starts_with() instead of memcmp() use of skip_prefix instead of memcmp() and strlen() Write proper sentences to explain and justify the change; not sentence fragments. Signed-off-by: Othman Darraz darraz...@gmail.com --- I am planning to apply to GSOC 214 Your logic in these changes is rather severely flawed. Running the test suite (t1450, in particular), as the GSoC microproject page suggests, would have clued you in that something was amiss. fsck.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c +++ b/fsck.c @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int err; - if (memcmp(buffer, tree , 5)) + if (starts_with(buffer, tree )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') One of the reasons for using starts_with() rather than memcmp() is that it allows you to eliminate magic numbers, such as 5. However, if you look closely at this code fragment, you will see that the magic number is still present in the expression 'buffer+5'. starts_with(), might be a better fit. return error_func(commit-object, FSCK_ERROR, invalid 'tree' line format - bad sha1); @@ -322,15 +322,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) if (p || parents) return error_func(commit-object, FSCK_ERROR, parent objects missing); } - if (memcmp(buffer, author , 7)) + if (starts_with(buffer, author )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'author' line); buffer += 7; Same problem here with magic number 7. err = fsck_ident(buffer, commit-object, error_func); if (err) return err; - if (memcmp(buffer, committer , strlen(committer ))) + buffer = (char* )skip_prefix(buffer,committer ); Style: (char *) Style: whitespace: skip_prefix(foo, bar) Regarding the (char *) cast: Is 'buffer' ever modified in this function? If not, would it make sense to make it 'const' (and fix any other small fallout from that change)? + if (!buffer) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'committer' line); - buffer += strlen(committer ); err = fsck_ident(buffer, commit-object, error_func); if (err) return err; -- 1.9.0.258.g00eda23.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()
On Tue, Mar 18, 2014 at 7:09 PM, Eric Sunshine sunsh...@sunshineco.com wrote: diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c +++ b/fsck.c @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int err; - if (memcmp(buffer, tree , 5)) + if (starts_with(buffer, tree )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') One of the reasons for using starts_with() rather than memcmp() is that it allows you to eliminate magic numbers, such as 5. However, if you look closely at this code fragment, you will see that the magic number is still present in the expression 'buffer+5'. starts_with(), might be a better fit. Of course, I meant skip_prefix() might be a better fit. -- 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] fsck.c:fsck_commit() use starts_with() and skip_prefix()
On 03/19/2014 12:09 AM, Eric Sunshine wrote: Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz darraz...@gmail.com wrote: use of starts_with() instead of memcmp() use of skip_prefix instead of memcmp() and strlen() Write proper sentences to explain and justify the change; not sentence fragments. Signed-off-by: Othman Darraz darraz...@gmail.com --- I am planning to apply to GSOC 214 Your logic in these changes is rather severely flawed. Running the test suite (t1450, in particular), as the GSoC microproject page suggests, would have clued you in that something was amiss. fsck.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c +++ b/fsck.c @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int err; - if (memcmp(buffer, tree , 5)) + if (starts_with(buffer, tree )) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') One of the reasons for using starts_with() rather than memcmp() is that it allows you to eliminate magic numbers, such as 5. However, if you look closely at this code fragment, you will see that the magic number is still present in the expression 'buffer+5'. starts_with(), might be a better fit. Eric, I think you meant to say that *skip_prefix()* might be a better fit. [...] 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: [PATCHv2] branch.c: simplify chain of if statements
On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu dragos.foi...@gmail.com wrote: This patch uses a table to store the different messages that can be emitted by the verbose install_branch_config function. It computes an index based on the three flags and prints the message located at the specific index in the table of messages. If the index somehow is not within the table, we have a bug. Signed-off-by: Dragos Foianu dragos.foi...@gmail.com --- branch.c | 44 +--- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..95645d5 100644 --- a/branch.c +++ b/branch.c @@ -54,6 +54,18 @@ void install_branch_config(int flag, const char *local, const char *origin, cons + int index = 0; + if (origin) + index += 1; + if (remote_is_branch) + index += 2; + if (rebasing) + index += 4; + + if (index 0 || index sizeof(messages) / sizeof(*messages)) die(BUG: impossible combination of %d and %p, remote_is_branch, origin); One other observation: You have a one-off error in your out-of-bounds check. It should be 'index = sizeof...' You can use ARRAY_SIZE() in place of sizeof(...)/sizeof(...). Since an out-of-bound index would be a programmer bug, it would probably be more appropriate to use an assert(), just after 'index' is computed, rather than if+die(). The original code used die() because it couldn't detect the error until the end of the if-chain. } -- 1.8.3.2 -- 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: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?
On Tue, Mar 18, 2014 at 8:29 PM, Junio C Hamano gits...@pobox.com wrote: I've never said any such thing. I only said I am not enthused against a proposal to add a build target that runs checkpatch or equivalent over *all* existing code, which will invite needless churn (read again the part of the message you quoted before I say I am not enthused). It is a totally separate issue for submitters to make sure they do not introduce *new* problems, and use of checkpatch --no-tree could be one way to do so. Sorry for misrepresenting your opinion. My summarization algorithm sometimes is _very_ lossy :) -- 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] [GSoC 2014]diff: Imported dir.h and renamed read_directory()
this was done in order to implement the GSoC
Micro project for diff-no-index.c this is the
first patch importing dir.h, for the use of
is_dot_or_dotdot(), and renaming
read_directory() to read_directory_contents()
in order to deal with the conflicting function
in dir.h
Signed-off-by: Brian Bourn bab2...@columbia.edu
---
I plan on applying to GSoC 2014
diff-no-index.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..ba915af 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -10,13 +10,14 @@
#include blob.h
#include tag.h
#include diff.h
+#include dir.h
#include diffcore.h
#include revision.h
#include log-tree.h
#include builtin.h
#include string-list.h
-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_contents(const char *path, struct string_list *list)
{
DIR *dir;
struct dirent *e;
@@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o,
int i1, i2, ret = 0;
size_t len1 = 0, len2 = 0;
- if (name1 read_directory(name1, p1))
+ if (name1 read_directory_contents(name1, p1))
return -1;
- if (name2 read_directory(name2, p2)) {
+ if (name2 read_directory_contents(name2, p2)) {
string_list_clear(p1, 0);
return -1;
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
Subject: [PATCH] Enable index-pack threading in msysgit.
This adds a Windows implementation of pread. Note that it is NOT
safe to intersperse calls to read() and pread() on a file
descriptor. According to the ReadFile spec, using the 'overlapped'
argument should not affect the implicit position pointer of the
descriptor. Experiments have shown that this is, in fact, a lie.
To accomodate that fact, this change also incorporates:
http://article.gmane.org/gmane.comp.version-control.git/196042
... which gives each index-pack thread its own file descriptor.
---
builtin/index-pack.c | 21 -
compat/mingw.c | 31 ++-
compat/mingw.h | 3 +++
config.mak.uname | 1 -
4 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2f37a38..c02dd4c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -51,6 +51,7 @@ struct thread_local {
#endif
struct base_data *base_cache;
size_t base_cache_used;
+ int pack_fd;
};
/*
@@ -91,7 +92,8 @@ static off_t consumed_bytes;
static unsigned deepest_delta;
static git_SHA_CTX input_ctx;
static uint32_t input_crc32;
-static int input_fd, output_fd, pack_fd;
+static const char *curr_pack;
+static int input_fd, output_fd;
#ifndef NO_PTHREADS
@@ -134,6 +136,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
*/
static void init_thread(void)
{
+ int i;
init_recursive_mutex(read_mutex);
pthread_mutex_init(counter_mutex, NULL);
pthread_mutex_init(work_mutex, NULL);
@@ -141,11 +144,17 @@ static void init_thread(void)
pthread_mutex_init(deepest_delta_mutex, NULL);
pthread_key_create(key, NULL);
thread_data = xcalloc(nr_threads, sizeof(*thread_data));
+ for (i = 0; i nr_threads; i++) {
+ thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
+ if (thread_data[i].pack_fd == -1)
+ die_errno(unable to open %s, curr_pack);
+ }
threads_active = 1;
}
static void cleanup_thread(void)
{
+ int i;
if (!threads_active)
return;
threads_active = 0;
@@ -155,6 +164,8 @@ static void cleanup_thread(void)
if (show_stat)
pthread_mutex_destroy(deepest_delta_mutex);
pthread_key_delete(key);
+ for (i = 0; i nr_threads; i++)
+ close(thread_data[i].pack_fd);
free(thread_data);
}
@@ -288,13 +299,13 @@ static const char *open_pack_file(const char *pack_name)
output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR,
0600);
if (output_fd 0)
die_errno(_(unable to create '%s'), pack_name);
- pack_fd = output_fd;
+ nothread_data.pack_fd = output_fd;
} else {
input_fd = open(pack_name, O_RDONLY);
if (input_fd 0)
die_errno(_(cannot open packfile '%s'), pack_name);
output_fd = -1;
- pack_fd = input_fd;
+ nothread_data.pack_fd = input_fd;
}
git_SHA1_Init(input_ctx);
return pack_name;
@@ -542,7 +553,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = pread(get_thread_data()-pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
@@ -1490,7 +1501,7 @@ static void show_pack_info(int stat_only)
int cmd_index_pack(int argc, const char **argv, const char *prefix)
{
int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
- const char *curr_pack, *curr_index;
+ const char *curr_index;
const char *index_name = NULL, *pack_name = NULL;
const char *keep_name = NULL, *keep_msg = NULL;
char *index_name_buf = NULL, *keep_name_buf = NULL;
diff --git a/compat/mingw.c b/compat/mingw.c
index 383cafe..6cc85d6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -329,7 +329,36 @@ int mingw_mkdir(const char *path, int mode)
return ret;
}
-int mingw_open (const char *filename, int oflags, ...)
+
+ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
+{
+ HANDLE hand = (HANDLE)_get_osfhandle(fd);
+ if (hand == INVALID_HANDLE_VALUE) {
+ errno = EBADF;
+ return -1;
+ }
+
+ LARGE_INTEGER offset_value;
+ offset_value.QuadPart = offset;
+
+ DWORD bytes_read = 0;
+ OVERLAPPED overlapped = {0};
+ overlapped.Offset = offset_value.LowPart;
+ overlapped.OffsetHigh = offset_value.HighPart;
+ BOOL result = ReadFile(hand, buf, count, bytes_read, overlapped);
+
+ ssize_t ret = bytes_read;
+
+ if (!result GetLastError() != ERROR_HANDLE_EOF)
[PATCH] Enable index-pack threading in msysgit.
This adds a Windows implementation of pread. Note that it is NOT
safe to intersperse calls to read() and pread() on a file
descriptor. According to the ReadFile spec, using the 'overlapped'
argument should not affect the implicit position pointer of the
descriptor. Experiments have shown that this is, in fact, a lie.
To accomodate that fact, this change also incorporates:
http://article.gmane.org/gmane.comp.version-control.git/196042
... which gives each index-pack thread its own file descriptor.
---
builtin/index-pack.c | 21 -
compat/mingw.c | 31 ++-
compat/mingw.h | 3 +++
config.mak.uname | 1 -
4 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2f37a38..c02dd4c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -51,6 +51,7 @@ struct thread_local {
#endif
struct base_data *base_cache;
size_t base_cache_used;
+ int pack_fd;
};
/*
@@ -91,7 +92,8 @@ static off_t consumed_bytes;
static unsigned deepest_delta;
static git_SHA_CTX input_ctx;
static uint32_t input_crc32;
-static int input_fd, output_fd, pack_fd;
+static const char *curr_pack;
+static int input_fd, output_fd;
#ifndef NO_PTHREADS
@@ -134,6 +136,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
*/
static void init_thread(void)
{
+ int i;
init_recursive_mutex(read_mutex);
pthread_mutex_init(counter_mutex, NULL);
pthread_mutex_init(work_mutex, NULL);
@@ -141,11 +144,17 @@ static void init_thread(void)
pthread_mutex_init(deepest_delta_mutex, NULL);
pthread_key_create(key, NULL);
thread_data = xcalloc(nr_threads, sizeof(*thread_data));
+ for (i = 0; i nr_threads; i++) {
+ thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
+ if (thread_data[i].pack_fd == -1)
+ die_errno(unable to open %s, curr_pack);
+ }
threads_active = 1;
}
static void cleanup_thread(void)
{
+ int i;
if (!threads_active)
return;
threads_active = 0;
@@ -155,6 +164,8 @@ static void cleanup_thread(void)
if (show_stat)
pthread_mutex_destroy(deepest_delta_mutex);
pthread_key_delete(key);
+ for (i = 0; i nr_threads; i++)
+ close(thread_data[i].pack_fd);
free(thread_data);
}
@@ -288,13 +299,13 @@ static const char *open_pack_file(const char *pack_name)
output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR,
0600);
if (output_fd 0)
die_errno(_(unable to create '%s'), pack_name);
- pack_fd = output_fd;
+ nothread_data.pack_fd = output_fd;
} else {
input_fd = open(pack_name, O_RDONLY);
if (input_fd 0)
die_errno(_(cannot open packfile '%s'), pack_name);
output_fd = -1;
- pack_fd = input_fd;
+ nothread_data.pack_fd = input_fd;
}
git_SHA1_Init(input_ctx);
return pack_name;
@@ -542,7 +553,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = pread(get_thread_data()-pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
@@ -1490,7 +1501,7 @@ static void show_pack_info(int stat_only)
int cmd_index_pack(int argc, const char **argv, const char *prefix)
{
int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
- const char *curr_pack, *curr_index;
+ const char *curr_index;
const char *index_name = NULL, *pack_name = NULL;
const char *keep_name = NULL, *keep_msg = NULL;
char *index_name_buf = NULL, *keep_name_buf = NULL;
diff --git a/compat/mingw.c b/compat/mingw.c
index 383cafe..6cc85d6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -329,7 +329,36 @@ int mingw_mkdir(const char *path, int mode)
return ret;
}
-int mingw_open (const char *filename, int oflags, ...)
+
+ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
+{
+ HANDLE hand = (HANDLE)_get_osfhandle(fd);
+ if (hand == INVALID_HANDLE_VALUE) {
+ errno = EBADF;
+ return -1;
+ }
+
+ LARGE_INTEGER offset_value;
+ offset_value.QuadPart = offset;
+
+ DWORD bytes_read = 0;
+ OVERLAPPED overlapped = {0};
+ overlapped.Offset = offset_value.LowPart;
+ overlapped.OffsetHigh = offset_value.HighPart;
+ BOOL result = ReadFile(hand, buf, count, bytes_read, overlapped);
+
+ ssize_t ret = bytes_read;
+
+ if (!result GetLastError() != ERROR_HANDLE_EOF)
+ {
+ errno =
[PATCH v5] use starts_with() instead of !memcmp()
Another version, this time very in line with the review and commentary of Junio, Eric, and Michael. This version boasts a revamped commit message and fewer but surer hunks changed. Thanks again for the guidance. Quint Guvernator (1): use starts_with() instead of !memcmp() builtin/apply.c| 4 ++-- builtin/for-each-ref.c | 2 +- builtin/mktag.c| 2 +- builtin/patch-id.c | 10 +- connect.c | 4 ++-- imap-send.c| 6 +++--- remote.c | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] use starts_with() instead of !memcmp()
When checking if a string begins with a constant string, using
starts_with() indicates the intention of the check more clearly and is
less error-prone than calling !memcmp() with an explicit byte count.
Signed-off-by: Quint Guvernator quintus.pub...@gmail.com
---
builtin/apply.c| 4 ++--
builtin/for-each-ref.c | 2 +-
builtin/mktag.c| 2 +-
builtin/patch-id.c | 10 +-
connect.c | 4 ++--
imap-send.c| 6 +++---
remote.c | 2 +-
7 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0189523..826b3e2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1631,7 +1631,7 @@ static int parse_fragment(const char *line, unsigned long
size,
* l10n of \ No newline... is at least that long.
*/
case '\\':
- if (len 12 || memcmp(line, \\ , 2))
+ if (len 12 || !starts_with(line, \\ ))
return -1;
break;
}
@@ -1646,7 +1646,7 @@ static int parse_fragment(const char *line, unsigned long
size,
* it in the above loop because we hit oldlines == newlines == 0
* before seeing it.
*/
- if (12 size !memcmp(line, \\ , 2))
+ if (12 size starts_with(line, \\ ))
offset += linelen(line, size);
patch-lines_added += added;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3e1d5c3..4135980 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -193,7 +193,7 @@ static int verify_format(const char *format)
at = parse_atom(sp + 2, ep);
cp = ep + 1;
- if (!memcmp(used_atom[at], color:, 6))
+ if (starts_with(used_atom[at], color:))
need_color_reset_at_eol = !!strcmp(used_atom[at],
color_reset);
}
return 0;
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 640ab64..d2d9310 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -57,7 +57,7 @@ static int verify_tag(char *buffer, unsigned long size)
/* Verify type line */
type_line = object + 48;
- if (memcmp(type_line - 1, \ntype , 6))
+ if (!starts_with(type_line - 1, \ntype ))
return error(char%d: could not find \\\ntype \, 47);
/* Verify tag-line */
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..23ef424 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -81,14 +81,14 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
}
/* Ignore commit comments */
- if (!patchlen memcmp(line, diff , 5))
+ if (!patchlen !starts_with(line, diff ))
continue;
/* Parsing diff header? */
if (before == -1) {
- if (!memcmp(line, index , 6))
+ if (starts_with(line, index ))
continue;
- else if (!memcmp(line, --- , 4))
+ else if (starts_with(line, --- ))
before = after = 1;
else if (!isalpha(line[0]))
break;
@@ -96,14 +96,14 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
/* Looking for a valid hunk header? */
if (before == 0 after == 0) {
- if (!memcmp(line, @@ -, 4)) {
+ if (starts_with(line, @@ -)) {
/* Parse next hunk, but ignore line numbers. */
scan_hunk_header(line, before, after);
continue;
}
/* Split at the end of the patch. */
- if (memcmp(line, diff , 5))
+ if (!starts_with(line, diff ))
break;
/* Else we're parsing another header. */
diff --git a/connect.c b/connect.c
index 4150412..5ae2aaa 100644
--- a/connect.c
+++ b/connect.c
@@ -30,11 +30,11 @@ static int check_ref(const char *name, int len, unsigned
int flags)
return 0;
/* REF_HEADS means that we want regular branch heads */
- if ((flags REF_HEADS) !memcmp(name, heads/, 6))
+ if ((flags REF_HEADS) starts_with(name, heads/))
return 1;
/* REF_TAGS means that we want tags */
- if ((flags REF_TAGS) !memcmp(name, tags/, 5))
+ if ((flags REF_TAGS) starts_with(name, tags/))
return 1;
/* All type bits clear means that we are ok with anything */
diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..019de18 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -524,7
Re: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()
Thanks for the submission. Comments below to give you a taste of the
Git review process...
On Tue, Mar 18, 2014 at 8:35 PM, Brian Bourn ba.bo...@gmail.com wrote:
Subject: diff: Imported dir.h and renamed read_directory()
Use imperative voice: import dir.h and rename read_directory()
this was done in order to implement the GSoC
Micro project for diff-no-index.c
This commentary won't be relevant to anyone reading the commit message
months or years from now. Place it below the --- line following your
sign-off.
this is the
first patch importing dir.h, for the use of
is_dot_or_dotdot(), and renaming
read_directory() to read_directory_contents()
in order to deal with the conflicting function
in dir.h
Since this preparatory change and the one which will follow are so
closely related, they should be submitted together as a two-patch
series.
The commit message itself it somewhat rambling. It would be clearer if
you could explain the problem precisely, then the solution. Perhaps
something like this:
It would be desirable to replace manual checking of . or ..
in diff-no-index.c with is_dot_or_dotdot(), which is defined in
dir.h, however, dir.h declares a read_directory() which conflicts
with a (different) static read_directory() defined in
diff-no-index.c. As a preparatory step, rename the local
read_directory() to avoid the collision.
Signed-off-by: Brian Bourn bab2...@columbia.edu
---
I plan on applying to GSoC 2014
diff-no-index.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..ba915af 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -10,13 +10,14 @@
#include blob.h
#include tag.h
#include diff.h
+#include dir.h
#include diffcore.h
dir.h is not needed for this patch, so including it here only confuses
matters. Include it instead when it is actually required by the
follow-up patch which uses is_dot_or_dotdot().
(Was it intentional to place the new #include between diff.h and
diffcore.h? Just being curious; it's not particularly important.)
Other than this, the patch looks sane.
#include revision.h
#include log-tree.h
#include builtin.h
#include string-list.h
-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_contents(const char *path, struct string_list
*list)
{
DIR *dir;
struct dirent *e;
@@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o,
int i1, i2, ret = 0;
size_t len1 = 0, len2 = 0;
- if (name1 read_directory(name1, p1))
+ if (name1 read_directory_contents(name1, p1))
return -1;
- if (name2 read_directory(name2, p2)) {
+ if (name2 read_directory_contents(name2, p2)) {
string_list_clear(p1, 0);
return -1;
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code
in accordance with the GSoC Microproject implemented the call is_dot_or_dotdot() in the code in order to further universalize the call to the function and increase code continuity. Signed-off-by: Brian Bourn bab2...@columbia.edu --- diff-no-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff-no-index.c b/diff-no-index.c index ba915af..44cce25 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path, struct string_list *list) return error(Could not open directory %s, path); while ((e = readdir(dir))) - if (strcmp(., e-d_name) strcmp(.., e-d_name)) + if (!is_dot_or_dotdot(e-d_name)) string_list_insert(list, e-d_name); closedir(dir); -- 1.9.0 -- View this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html Sent from the git mailing list archive at Nabble.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
[PATCH v3] tests: use env to run commands with temporary env-var settings
Originally, we would use VAR=VAL command to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like test_must_fail); the result of the assignment is retained and affects later commands. To avoid this, we assigned and exported the environment variables and run the test(s) in a subshell like this, ( VAR=VAL export VAR test_must_fail git command to be tested ) Using the env utility, we should be able to say test_must_fail git command to be tested which is much short and easier to read. Signed-off-by: David Tran unsignedz...@gmail.com --- Let's see if I replied correctly with send-email. Retrying this again. How do I 'reply' to a thread using send-email? Look for --in-reply-to option in man git-send-email. Git prompts me for this but I don't know what to type. I tried typing just the id, like 244379 and that didn't work. I guess I'll try 244...@gmane.comp.version-control.git? Google didn't bring many results on how to use it but many results of what it is and its goal. Isn't GIT_CONFIG here another way of saying: test_must_fail git config -f doesnotexist --list Perhaps that is shorter and more readable still (and there are a few similar cases in this patch. I'll ignore this for now. If needed I can make another patch to resolve this. Hopefully this should be all of it. I am David Tran a graduating CS/Math senior from Sonoma State University, United States. I would like to work with git for GSoC'14, specifically the line options for git rebase --interactive [1][2]. I have used git for a few years and know how destructive but important rebase is to git. I have created a few shell scripts here and there to make life using bash/zsh easier. I would like to apply these skills and work with the best. Github: unsignedzero [1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967 [2]: http://thread.gmane.org/gmane.comp.version-control.git/242701 Signed-off-by: David Tran unsignedz...@gmail.com --- t/t1300-repo-config.sh| 17 ++ t/t1510-repo-setup.sh |4 +-- t/t3200-branch.sh | 12 +-- t/t3301-notes.sh | 22 +++- t/t3404-rebase-interactive.sh | 69 - t/t3413-rebase-hook.sh|6 +--- t/t4014-format-patch.sh | 14 ++-- t/t5305-include-tag.sh|4 +-- t/t5602-clone-remote-exec.sh | 13 ++-- t/t5801-remote-helpers.sh |6 +-- t/t6006-rev-list-format.sh|9 ++--- t/t7006-pager.sh | 18 ++- 12 files changed, 42 insertions(+), 152 deletions(-) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist - export GIT_CONFIG - test_must_fail git config --list - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' test_expect_success SYMLINKS 'symlink to nonexistent configuration' ' ln -s doesnotexist linktonada ln -s linktonada linktolinktonada - ( - GIT_CONFIG=linktonada - export GIT_CONFIG - test_must_fail git config --list - GIT_CONFIG=linktolinktonada - test_must_fail git config --list - ) + test_must_fail env GIT_CONFIG=linktonada git config --list + test_must_fail env GIT_CONFIG=linktolinktonada git config --list ' test_expect_success 'check split_cmdline return' diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index cf2ee78..e1b2a99 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version) setup_repo 30 $here/30 gitfile true ( cd 30 - GIT_DIR=.git - export GIT_DIR - test_must_fail git symbolic-ref HEAD 2result + test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2result ) grep core.bare and core.worktree 30/result ' diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index fcdb867..d45e95c 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' ' write_script editor -\EOF echo New contents $1 EOF - ( - EDITOR=./editor - export EDITOR -
Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code
On Tue, Mar 18, 2014 at 9:30 PM, babourn ba.bo...@gmail.com wrote: Subject: diff: used is_dot_or_dotdot() in code Use imperative voice: use rather than used in accordance with the GSoC Microproject implemented This commentary will not have much meaning to someone reading the commit log months or years from now. Place it below the --- line following your sign-off. the call is_dot_or_dotdot() in the code in order to further universalize the call to the function and increase code continuity. It should be sufficient to explain the patch by just saying: Subject: replace manual ./.. check with is_dot_or_dotdot() The rest of the explanatory text can be dropped since it doesn't add anything (meaningful) beyond what the subject says. Signed-off-by: Brian Bourn bab2...@columbia.edu --- diff-no-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff-no-index.c b/diff-no-index.c index ba915af..44cce25 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path, struct string_list *list) return error(Could not open directory %s, path); while ((e = readdir(dir))) - if (strcmp(., e-d_name) strcmp(.., e-d_name)) + if (!is_dot_or_dotdot(e-d_name)) The patch is severely whitespace-damaged. (Did you post it through Nabble?) string_list_insert(list, e-d_name); closedir(dir); -- 1.9.0 -- View this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html Sent from the git mailing list archive at Nabble.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 v3] tests: use env to run commands with temporary env-var settings
On Tue, Mar 18, 2014 at 2:54 PM, David Tran unsignedz...@gmail.com wrote: Originally, we would use VAR=VAL command to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like test_must_fail); the result of the assignment is retained and affects later commands. To avoid this, we assigned and exported the environment variables and run the test(s) in a subshell like this, ( VAR=VAL export VAR Append to this line. test_must_fail git command to be tested ) Using the env utility, we should be able to say test_must_fail git command to be tested which is much short and easier to read. s/short/shorter/ Signed-off-by: David Tran unsignedz...@gmail.com --- Hopefully this should be all of it. Much better. I didn't spot any errors in the patch this time around. One final note for future submissions: As a courtesy to reviewers, explain (below the --- line) what changed in the current version, and provide a reference to the previous attempt, like this [1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/244379 Signed-off-by: David Tran unsignedz...@gmail.com --- t/t1300-repo-config.sh| 17 ++ t/t1510-repo-setup.sh |4 +-- t/t3200-branch.sh | 12 +-- t/t3301-notes.sh | 22 +++- t/t3404-rebase-interactive.sh | 69 - t/t3413-rebase-hook.sh|6 +--- t/t4014-format-patch.sh | 14 ++-- t/t5305-include-tag.sh|4 +-- t/t5602-clone-remote-exec.sh | 13 ++-- t/t5801-remote-helpers.sh |6 +-- t/t6006-rev-list-format.sh|9 ++--- t/t7006-pager.sh | 18 ++- 12 files changed, 42 insertions(+), 152 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code
On Tue, Mar 18, 2014 at 11:58 PM, Brian Bourn ba.bo...@gmail.com wrote: On Tue, Mar 18, 2014 at 11:45 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Mar 18, 2014 at 9:30 PM, babourn ba.bo...@gmail.com wrote: Subject: diff: used is_dot_or_dotdot() in code Signed-off-by: Brian Bourn bab2...@columbia.edu --- diff-no-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff-no-index.c b/diff-no-index.c index ba915af..44cce25 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path, struct string_list *list) return error(Could not open directory %s, path); while ((e = readdir(dir))) - if (strcmp(., e-d_name) strcmp(.., e-d_name)) + if (!is_dot_or_dotdot(e-d_name)) The patch is severely whitespace-damaged. (Did you post it through Nabble?) I did post through Nabble, My email with the patch didn't seem to be going through. should I keep trying to resend it through email to undo the whitspace damage? It's probably not necessary to try resending this version of the patch since you'll (hopefully) be sending a newer version which takes reviewer comments into consideration. What method are you using to send the patches? git send-email? Something other? This particular mailing list rejects HTML-formatted messages, so that could be the culprit if you pasted the patch into your email client. It's a good idea to try sending patches to yourself via git send-email. If you can get that to work successfully, then they should be accepted by the mailing list when sent via the same mechanism. -- 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 v5] use starts_with() instead of !memcmp()
On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator quintus.pub...@gmail.com wrote: Another version, this time very in line with the review and commentary of Junio, Eric, and Michael. This version boasts a revamped commit message and fewer but surer hunks changed. Explaining what changed in this version is indeed a courtesy to reviewers. Thanks. For bonus points, provide a link to the previous attempt, like this [1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/244292 Thanks again for the guidance. Quint Guvernator (1): use starts_with() instead of !memcmp() builtin/apply.c| 4 ++-- builtin/for-each-ref.c | 2 +- builtin/mktag.c| 2 +- builtin/patch-id.c | 10 +- connect.c | 4 ++-- imap-send.c| 6 +++--- remote.c | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Motichek Loans @ 2%
Hi, In need of a loan for any purpose? Look no further, Motichek Loans Agency can help you with that loan you so seek and @ a 2% interest rate, you have access to funds between $10,000.00 to $20,000,000.00 If Interested, send your details via our email below for fast processing of that loan. Email: motic...@foxmail.com --- This email is free from viruses and malware because avast! Antivirus protection is active. http://www.avast.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
