Bug? git push --recurse-submodules=on-demand is not truly recursive
I have the following project structure: root-project | |-- A | | | |-- C | |-- B A and B are submodules of the root-project. C is in turn a submodule of project A. Suppose I have made changes to projects A,B and C and commited these changes to the respective indices. After that I update the references to A and B in the root-project and commit that change as well. When I push the commit of the root-project with the option --recurse-submodules=on-demand, git pushes the commits of projects A, B and the root-project but silently ignores all unpublished commits of project C. I end up publishing a project that no one can successfully clone because of the dangling link to C. Is this the expected behaviour or is this a bug? I have written a small shell script that sets up the project structure and executes the described scenario: https://gist.github.com/usommerl/6e8defcba94bd4ba1438 git version 2.3.3 Uwe Sommerlatt -- 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] read-cache: fix reading of split index
On Sat, Mar 21, 2015 at 4:43 AM, Thomas Gummerer t.gumme...@gmail.com wrote: The split index extension uses ewah bitmaps to mark index entries as deleted, instead of removing them from the index directly. This can result in an on-disk index, in which entries of stage #0 and higher stages appear, which are removed later when the index bases are merged. 15999d0 read_index_from(): catch out of order entries when reading an index file introduces a check which checks if the entries are in order after each index entry is read in do_read_index. This check may however fail when a split index is read. Fix this by moving checking the index after we know there is no split index or after the split index bases are successfully merged instead. Thank you for catching this. I was about to write would be nice to point out what tests fail so the reviewer has easier time trying themselves, but whoa.. quite a few of them! May I suggest a slight modification. Even though stage info is messed up before the index is merged, I think we should still check that both front and base indexes have all the names sorted correctly (and even stronger, the base index should never contain staged entries). If sorting order is messed up, it could lead to other problems. So instead of removing the test from do_read_index(), perhaps add a flag in check_ce_order() to optionally detect the stage problem, but print/do nothing, only set a flag so the caller know there may be a problem. In the two new call sites you added, we still call the new check_ce_order() again to make sure everything is in order. In the call site where split index is not active, if the previous check_ce_order() call from inside do_read_index() says everything is ok, we could even skip the check. -- 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: Git ignore help
On Tue, Mar 24, 2015 at 7:46 PM, mdc...@seznam.cz wrote: Duy, you wrote: This is true. To elaborate, if we have to recurse in excluded directories so that we can include some back, then the reason for excluding is already defeated as we may need to traverse the entire directory structure. However in this particular case where we do know in advance that only certain directories may have re-include rules, e.g. db, reports or scripts, we could keep going for a while. ... so according to that it sounds like including /db, /reports, /scripts should actually also NOT work. But it does work - i.e. when I add the following: # exclude /* # except !/db !/reports !/scripts then any content within those 3 directories (and their sub directories) is included and not ignored... It ONLY does not work when I add more levels - e.g.: !/reports/something In this case neither /reports nor /reports/something or any sub directory is included. Yes. It's the subtlety of optimizing ;-) If you read the man page really carefully (*), if the _parent_ directory of that file(**) is excluded and the parent of these three directories is _not_ excluded. (*) I'm not saying this is a good thing. Only docs such as language spec or RFCs need that level of attention. But I'm not a good document writer myself, can't blame others. Improvements are welcome though. (**) that file should be that file or directory but I guess simplification here is ok -- 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: per-repository and per-worktree config variables
On Thu, Mar 19, 2015 at 4:33 AM, Max Kirillov m...@max630.net wrote: On Sun, Feb 08, 2015 at 09:36:43AM -0800, Jens Lehmann wrote: I wonder if it's worth all the hassle to invent new names. Wouldn't it be much better to just keep a list of per-worktree configuration value names and use that inside the config code to decide where to find them for multiple work trees. That would also work easily for stuff like EOL-config and would push the complexity in the config machinery and not onto the user. I actually thought about the same, and now tend to think that most of config variables make sense to be per-worktree in some cases. Only few variable must always be per repository. I tried to summarize the variables which now (in current pu) should be common, also listed all the rest so somebody could scan through the list and spot anything I could miss. Thanks for compiling the list. At this point I think it may not be sensible to hard code some config vars (e.g. core.worktree) to be local or shared. So I'm thinking (out loud) that we may have a file $GIT_DIR/worktrees/id/local-config-patterns (or some other name) that would define what vars are local. gitignore syntax will be reused for this. The file would provide more flexibility.. -- 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: Git ignore help
Hi Duy, Eric, thx a lot. So net net - I can't really achieve this. It feels like something very basic and simple so pretty surprising but guess that's what it is :) Normally, I'd expect that the functionality will behave like with similar other blacklist/whitelist functionalities in Linux - that the more specific definition overrides the less specific one. And when there are 2 on the same level then one (include or exclude) has the priority by default... Anyways, what I am still not clear on is how come that including just the 'single level folder' work? Duy, you wrote: This is true. To elaborate, if we have to recurse in excluded directories so that we can include some back, then the reason for excluding is already defeated as we may need to traverse the entire directory structure. However in this particular case where we do know in advance that only certain directories may have re-include rules, e.g. db, reports or scripts, we could keep going for a while. ... so according to that it sounds like including /db, /reports, /scripts should actually also NOT work. But it does work - i.e. when I add the following: # exclude /* # except !/db !/reports !/scripts then any content within those 3 directories (and their sub directories) is included and not ignored... It ONLY does not work when I add more levels - e.g.: !/reports/something In this case neither /reports nor /reports/something or any sub directory is included. Martin -- Původní zpráva -- Od: Duy Nguyen Komu: Eric Sunshine Datum: 24. 3. 2015 10:40:33 Předmět: Re: Git ignore help On Tue, Mar 24, 2015 at 8:55 AM, Eric Sunshine wrote: e.g. db, reports or scripts, we could keep going for a while. I think I attempted to do this in the past and failed (don't remember exactly why). Maybe I'll try again some time in future. I also was pretty sure that you had attempted this, but couldn't find a reference to it, so I didn't mention it in my response. However, with some more digging, I finally located it[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/259870 Thank you. I only looked at my repo and no branch name suggested it (if only there is google search for a git repository..). So I gave up because of performance reasons again but that was for enabling it unconditionaly. If we enable it via a config variable and the user is made aware of the performance implications, I guess it would be ok. So it's back in my back log. -- 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 10/15] commit.c: fix a memory leak
On Sat, Mar 21, 2015 at 10:59 AM, Junio C Hamano gits...@pobox.com wrote: A further tangent (Duy Cc'ed for this point). We might want to rethink the interface to ce_path_match() and report_path_error() so that we do not have to do a separate allocation of has this pathspec been used? array. This was a remnant from the olden days back when pathspec were mere const char ** where we did not have any room to add per-item bit---these days pathspec is repreasented as an array of struct pathspec and we can afford to add a bit to the structure---which will make this kind of leak much less likely to happen. I just want to say noted (and therefore in my backlog). But no promise that it will happen any time soon. Low hanging fruit, perhaps some people may be interested.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gc: save log from daemonized gc --auto and print it next time
While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.
The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid gc
--auto running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.
Besides reverting 9f673f9 and looking at the problem from another
angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
--auto' will print the saved warnings, delete gc.log and exit.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/gc.c | 37 -
t/t6500-gc.sh | 20
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 5c634af..07769a9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -32,6 +32,8 @@ static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
static int detach_auto = 1;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
static const char *prune_expire = 2.weeks.ago;
static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
@@ -44,6 +46,15 @@ static char *pidfile;
static void remove_pidfile(void)
{
+ if (daemonized log_filename.len) {
+ struct stat st;
+
+ close(2);
+ if (stat(log_filename.buf, st) ||
+ !st.st_size ||
+ rename(log_filename.buf, git_path(gc.log)))
+ unlink(log_filename.buf);
+ }
if (pidfile)
unlink(pidfile);
}
@@ -324,13 +335,25 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
}
if (detach_auto) {
+ struct strbuf sb = STRBUF_INIT;
+
+ if (strbuf_read_file(sb, git_path(gc.log), 0) 0) {
+ warning(_(Last gc run reported the following,
gc skipped));
+ fputs(sb.buf, stderr);
+ strbuf_release(sb);
+ /* let the next gc --auto run as usual */
+ unlink(git_path(gc.log));
+ return 0;
+ }
+
if (gc_before_repack())
return -1;
/*
* failure to daemonize is ok, we'll continue
* in foreground
*/
- daemonize();
+ if (!daemonize())
+ daemonized = 1;
}
} else
add_repack_all_option();
@@ -343,6 +366,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
name, (uintmax_t)pid);
}
+ if (daemonized) {
+ int fd;
+
+ strbuf_addstr(log_filename, git_path(gc.log_XX));
+ fd = xmkstemp(log_filename.buf);
+ if (fd = 0) {
+ dup2(fd, 2);
+ close(fd);
+ } else
+ strbuf_release(log_filename);
+ }
+
if (gc_before_repack())
return -1;
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 63194d8..54bc9c4 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' '
test_i18ngrep [Uu]sage broken/usage
'
+test_expect_success !MINGW 'gc --auto and logging' '
+ git init abc
+ (
+ cd abc
+ # These create blobs starting with the magic number 17
+ for i in 901 944; do
+ echo $i test git hash-object -w test /dev/null
+ done
+ git config gc.auto 1
+ LANG=C git gc --auto
+ sleep 1 # give it time to daemonize
+ while test -f .git/gc.pid; do sleep 1; done
+ grep too many unreachable loose objects .git/gc.log
+ LANG=C git gc --auto 2error
+ grep skipped error
+ grep too many unreachable loose objects error
+ ! test -f .git/gc.log
+ )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
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 3/3] t0302: test credential-store support for XDG_CONFIG_HOME
Paul Tan pyoka...@gmail.com writes: Matthieu and Eric: I know I said I will try to re-order the patches to put the tests before the implementation, but after thinking and trying to rewrite the commit messages I realised it seems really weird to me. In this patch series, the implementation is split across the first two patches. The first patch should use the old tests, and ideally, the new tests should be squashed with the second patch because it seems more logical to me to implement the tests at the same time as the new feature. However, since the tests patch is very long, to make it easier to review it is split into a separate patch which is applied after the implementation patches. No problem, your version is very good. I was pointing out alternatives, but not requesting a change, and your reasoning makes perfect sense. I had reviewed v4 in details, and checked the diff between v4 and v5. The whole series is now Reviewed-by: Matthieu Moy matthieu@imag.fr Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git ignore help
On Tue, Mar 24, 2015 at 8:55 AM, Eric Sunshine sunsh...@sunshineco.com wrote: e.g. db, reports or scripts, we could keep going for a while. I think I attempted to do this in the past and failed (don't remember exactly why). Maybe I'll try again some time in future. I also was pretty sure that you had attempted this, but couldn't find a reference to it, so I didn't mention it in my response. However, with some more digging, I finally located it[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/259870 Thank you. I only looked at my repo and no branch name suggested it (if only there is google search for a git repository..). So I gave up because of performance reasons again but that was for enabling it unconditionaly. If we enable it via a config variable and the user is made aware of the performance implications, I guess it would be ok. So it's back in my back log. -- 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 18/25] t1301: use modern test_* helpers
Quoting Jeff King p...@peff.net:
This shortens the code and fixes some -chaining.
Signed-off-by: Jeff King p...@peff.net
---
t/t1301-shared-repo.sh | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 7eecfb8..ac10875 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -12,12 +12,11 @@ setfacl -k . 2/dev/null
# User must have read permissions to the repo - failure on --shared=0400
test_expect_success 'shared = 0400 (faulty permission u-w)' '
+ test_when_finished rm -rf sub
mkdir sub (
- cd sub git init --shared=0400
+ cd sub
+ test_must_fail git init --shared=0400
)
- ret=$?
- rm -rf sub
- test $ret != 0
'
modebits () {
@@ -33,7 +32,7 @@ do
git init --shared=1
test 1 = $(git config core.sharedrepository)
)
- actual=$(ls -l sub/.git/HEAD)
+ actual=$(ls -l sub/.git/HEAD)
case $actual in
-rw-rw-r--*)
: happy
This hunk could go into the moderate -chain breakage patch.
Doesn't really matter, what matters most is that it's fixed, but I
really liked your classification of missing s in the early patches.
@@ -90,10 +89,8 @@ do
rm -f .git/info/refs
git update-server-info
actual=$(modebits .git/info/refs)
- test x$actual = x-$y || {
- ls -lt .git/info
- false
- }
+ verbose test x$actual = x-$y
+
'
umask 077
@@ -102,10 +99,7 @@ do
rm -f .git/info/refs
git update-server-info
actual=$(modebits .git/info/refs)
- test x$actual = x-$x || {
- ls -lt .git/info
- false
- }
+ verbose test x$actual = x-$x
'
--
2.3.3.520.g3cfbb5d
--
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] gc: save log from daemonized gc --auto and print it next time
On Tue, Mar 24, 2015 at 8:17 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.
The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid gc
--auto running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.
Besides reverting 9f673f9 and looking at the problem from another
angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
--auto' will print the saved warnings, delete gc.log and exit.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/gc.c | 37 -
t/t6500-gc.sh | 20
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 5c634af..07769a9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -32,6 +32,8 @@ static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
static int detach_auto = 1;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
static const char *prune_expire = 2.weeks.ago;
static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
@@ -44,6 +46,15 @@ static char *pidfile;
static void remove_pidfile(void)
{
+ if (daemonized log_filename.len) {
+ struct stat st;
+
+ close(2);
+ if (stat(log_filename.buf, st) ||
+ !st.st_size ||
+ rename(log_filename.buf, git_path(gc.log)))
+ unlink(log_filename.buf);
+ }
if (pidfile)
unlink(pidfile);
}
@@ -324,13 +335,25 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
}
if (detach_auto) {
+ struct strbuf sb = STRBUF_INIT;
+
+ if (strbuf_read_file(sb, git_path(gc.log), 0) 0)
{
+ warning(_(Last gc run reported the
following, gc skipped));
When I read this message, it makes me think that the previous gc was
skipped, even though it's actually skipping the current one. Perhaps
rephrase as skipping gc; last gc reported:?
+ fputs(sb.buf, stderr);
+ strbuf_release(sb);
+ /* let the next gc --auto run as usual */
+ unlink(git_path(gc.log));
+ return 0;
+ }
+
if (gc_before_repack())
return -1;
/*
* failure to daemonize is ok, we'll continue
* in foreground
*/
- daemonize();
+ if (!daemonize())
+ daemonized = 1;
}
} else
add_repack_all_option();
@@ -343,6 +366,18 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
name, (uintmax_t)pid);
}
+ if (daemonized) {
+ int fd;
+
+ strbuf_addstr(log_filename, git_path(gc.log_XX));
+ fd = xmkstemp(log_filename.buf);
+ if (fd = 0) {
+ dup2(fd, 2);
+ close(fd);
+ } else
+ strbuf_release(log_filename);
+ }
+
if (gc_before_repack())
return -1;
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 63194d8..54bc9c4 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' '
test_i18ngrep [Uu]sage broken/usage
'
+test_expect_success !MINGW 'gc --auto and logging' '
+ git init abc
+ (
+ cd abc
+ # These create blobs starting with the magic number 17
+ for i in 901 944; do
+ echo $i test git hash-object -w test /dev/null
+ done
+ git config gc.auto 1
+ LANG=C git gc --auto
+ sleep 1 # give it time to daemonize
+ while test -f .git/gc.pid; do sleep 1; done
+ grep too many unreachable loose objects .git/gc.log
+ LANG=C git gc --auto 2error
+ grep skipped error
+ grep too many unreachable loose objects error
+ ! test -f .git/gc.log
+ )
+'
Re: A good time to pull from your gitk tree?
On Mon, Mar 23, 2015 at 12:03:37PM -0700, Junio C Hamano wrote: Is it a good time for me to pull from you, or do you recommend me to wait for a bit, expecting more? We'll go in the pre-release freeze soon-ish, so I thought I should ping. Now is a good time to pull from the usual place, thanks. Regards, Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
* pw/remote-set-url-fetch (2014-11-26) 1 commit - remote: add --fetch and --both options to set-url This has not seen any activity for a few months since $gmane/261483; is anybody still interested in resurrecting it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
Junio C Hamano gits...@pobox.com writes: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. This cycle is turning out to be a shoot for product excellence release. About half of the commits that have been merged to the 'master' so far have also been merged to 'maint' and v2.3.4 has been tagged. I've merged a few more topics to 'next' and hopefully will push the result out in a few hours. Some topics that are in 'next' but not in 'master' are high-impact ones that I'd feel more comfortable if we cooked them there for a bit more, and am planning to merge them (if there are no issues discovered in the meantime, of course) after this cycle finishes, but for others, I'm hoping to merge them to 'master' before we tag the -rc0 preview release. Please give them a good beating and report any regressions you find. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Sparse checkout not working as expected (colons in filenames on Windows)
From: Duy Nguyen pclo...@gmail.com On Fri, Mar 20, 2015 at 6:07 AM, Philip Oakley philipoak...@iee.org wrote: Hi, I was expecting that sparse checkout could be used to avoid the checking out, by git, of files which have colons in their name into the worktree when on Windows. Yue Lin Ho reported on the Msygit list [1] that he had a repo where there was already committed a file with a colon in it's name, which was a needed file and had been committed by a Linux user. The problem was how to work with that repo on a Windows box where such a file is prohibited to exist on the FS (hence the expectation that a sparse checkout would suffice). Yue has created a short test repo [2] Even after getting the pathspec escaping right, I still haven't been able to make this expected behaviour work [3]. Am I wrong to expect that sparse checkout (and the skip-worktree bit) can be used to avoid files with undesirable filenames hitting the OS's file system? If it should be OK, what's the correct recipe? -- Philip [1] https://groups.google.com/forum/?hl=en_US?hl%3Den#!topic/msysgit/D4HcHRpxPgU How to play around with the filename with colon on Windows? [2] Test repo https://github.com/t-pascal/tortoisegit-colons [3] test sequence:: $ mkdir colons cd colons $ git clone -n https://github.com/t-pascal/tortoisegit-colons $ cd tortoisegit-colons/ $ git config core.sparseCheckout true $ cat .git/info/sparse-checkout # external editor /* !ifcfg-eth0\:0 Colons have no special meaning in gitignore rules and therefore need not be escaped. The backslash is considered a literal character in this case, probably not what you want. $ git update-index --skip-worktree -- ifcfg-eth0\:0 Ignoring path ifcfg-eth0:0 $ git checkout -b test 7f35d34bc6160cc # tip commit, we are still unborn! error: Invalid path 'ifcfg-eth0:0 D ifcfg-eth0:0 Switched to a new branch 'test' -- I've corrected the sparse-checkout, but won't the command line 'git update-index --skip-worktree' will still need it? (demo commands below) That said, the final error (which I'd missed in the earlier post) is: fatal: make_cache_entry failed for path 'ifcfg-eth0:0' This is on the Windows (pre-compiled msysgit at v1.9.5) Git bash, so could be a catch path in that code for make_cache_entry (I've not checked the code yet). So at the moment it doesn't look like sparse checkout can be used to avoid colons in windows on-disk files based on the current code. -- Philip Philip@PHILIPOAKLEY /d/Git_repos/colons $ cd tortoisegit-colons/ Philip@PHILIPOAKLEY /d/Git_repos/colons/tortoisegit-colons (test) $ git update-index --skip-worktree -- ifcfg-eth0\:0 Ignoring path ifcfg-eth0:0 Philip@PHILIPOAKLEY /d/Git_repos/colons/tortoisegit-colons (test) $ git reset error: Invalid path 'ifcfg-eth0:0' fatal: make_cache_entry failed for path 'ifcfg-eth0: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] read-cache: tighten checks for do_read_index
On 03/24, Junio C Hamano wrote: Thomas Gummerer t.gumme...@gmail.com writes: 03f15a7 read-cache: fix reading of split index moved the checks for the correct order of index entries out of do_read_index. This loosens the checks more than necessary. Re-introduce the checks for the order, but don't error out when we have multiple stage-0 entries in the index. Return a flag for the caller instead, if we have multiple stage-0 entries and let the caller decide if we need to error out. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- This is a patch on top of my previous patch, as that one has already been merged to next. I am not convinced that this is a good change in the first place. The original before your fix was wrong exactly because it was too tightly tied to the implementation of the index file format where there was only one file whose contents must be sorted, and that is why it was a broken check in a new world with split-index. And your fix in 'next' is the right fix---it makes the verification happen only on the result is given to the caller for its consumption. It may be true that entries may have to be sorted in a certain order when reading the original index file format and also reading some steps in reading the split-index, but that merely happens to be an imprementation detail of the two format currently supported, and as we improve these formats (or even introduce yet another one) in the longer term, this patch would re-introduce the same issue your earlier fix corrected, wouldn't it? Yes, after looking at it again I completely agree. Sorry for the noise. -- 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] gc: save log from daemonized gc --auto and print it next time
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: While commit 9f673f9 (gc: config option for running --auto in background - 2014-02-08) helps reduce some complaints about 'gc --auto' hogging the terminal, it creates another set of problems. The latest in this set is, as the result of daemonizing, stderr is closed and all warnings are lost. This warning at the end of cmd_gc() is particularly important because it tells the user how to avoid gc --auto running repeatedly. Because stderr is closed, the user does not know, naturally they complain about 'gc --auto' wasting CPU. Besides reverting 9f673f9 and looking at the problem from another angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc --auto' will print the saved warnings, delete gc.log and exit. I wonder what this buys us if multiple auto-gc's are triggered because the user is running a long svn import or something similar. diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 63194d8..54bc9c4 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' ' test_i18ngrep [Uu]sage broken/usage ' +test_expect_success !MINGW 'gc --auto and logging' ' + git init abc + ( + cd abc + # These create blobs starting with the magic number 17 + for i in 901 944; do There are numbers smaller than these, like 263 and 410 ;-) + echo $i test git hash-object -w test /dev/null hash-object --stdin? + done + git config gc.auto 1 test_config? + LANG=C git gc --auto + sleep 1 # give it time to daemonize + while test -f .git/gc.pid; do sleep 1; done Yuck... + grep too many unreachable loose objects .git/gc.log + LANG=C git gc --auto 2error + grep skipped error + grep too many unreachable loose objects error + ! test -f .git/gc.log + ) +' For that 17/ has very many loose objects that are still young and unreachable issue, I wonder if the right solution is somehow to flag the repository and prevent gc --auto from running until the situation improves. I checked at this time and found too many in 17/; upon finding that flag file (with a timestamp), if there are new files in 17/ or if there are other reasons to do a gc (perhaps there are too many packfiles to be consolidated?), then do the gc but otherwise quite silently before spending too many cycles on it, or something along that line? -- 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 19/25] t6034: use modern test_* helpers
Quoting Jeff King p...@peff.net: These say roughly the same thing as the hand-rolled messages. We do lose the merge did not complete debug message, but merge and write-tree are prefectly capable of s/prefectly/perfectly/ writing useful error messages when they fail. Signed-off-by: Jeff King p...@peff.net --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: Add target to build PDF manpages
From: Michael J Gruber g...@drmicha.warpmail.net Junio C Hamano venit, vidit, dixit 20.03.2015 23:38: Stefan Beller sbel...@google.com writes: Thomas referencing reading the man page offline, made me wonder why you wouldn't read the man pages itself as they can also be carried around offline. But the striking point is on an iPad, which doesn't offer you the convenience of a shell etc, but pdf is fine to read there. Also you can add comments to pdfs more easily that html pages I'd guess. So the patch makes sense to me now. It's just a use case I'm personally not interested in for now, but I don't oppose it as is. Well, my comment was not about opposing to it, but was about questioning the usefulness of it, iow, who would benefit from having this patch in my tree? I didn't see (and I still do not quite see) why people would want to have separate pdf files for all the subcommands (instead of say an .epub or .pdf that binds all the man pages and perhaps user-manual, just like we do for .texi/.info). Exactly. For PDF, a combined document is more natural and will hopefully make crosslinks work as crossrefs within one document, rather than links to external documents. I'd say that would make a valuable target. As per the original request, it is useful to some, and the usefulness of a very large pdf containing all the documentation shouldn't be a reason to not have such a 'one at a time' target available (though personally I would suggest that it is the users responsibility to 'make' such a target, not the maintainers!). The single large pdf has also been discussed (http://thread.gmane.org/gmane.comp.version-control.git/207151/focus=207165) but didn't get into the code base either. The user-manual is available as a pdf target. Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] read-cache: tighten checks for do_read_index
Thomas Gummerer t.gumme...@gmail.com writes: 03f15a7 read-cache: fix reading of split index moved the checks for the correct order of index entries out of do_read_index. This loosens the checks more than necessary. Re-introduce the checks for the order, but don't error out when we have multiple stage-0 entries in the index. Return a flag for the caller instead, if we have multiple stage-0 entries and let the caller decide if we need to error out. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- This is a patch on top of my previous patch, as that one has already been merged to next. I am not convinced that this is a good change in the first place. The original before your fix was wrong exactly because it was too tightly tied to the implementation of the index file format where there was only one file whose contents must be sorted, and that is why it was a broken check in a new world with split-index. And your fix in 'next' is the right fix---it makes the verification happen only on the result is given to the caller for its consumption. It may be true that entries may have to be sorted in a certain order when reading the original index file format and also reading some steps in reading the split-index, but that merely happens to be an imprementation detail of the two format currently supported, and as we improve these formats (or even introduce yet another one) in the longer term, this patch would re-introduce the same issue your earlier fix corrected, wouldn't it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
Junio C Hamano gits...@pobox.com writes: * jn/gitweb-utf8-in-links (2014-05-27) 1 commit - gitweb: Harden UTF-8 handling in generated links This has been lingering in my 'pu' branch without seeing any updates since $gmane/250758; is anybody still interested in resurrecting it and moving it forward? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFH/PATCH] git-svn: adjust info to svn 1.7 and 1.8
t9119 refuses to run with svn versions greater than 1.6 since git svn
info does not even try to match the output of svn info for later
versions.
Adjust git svn info to match these versions and make t9119 run with
them. This requires the following changes:
* compute checksums with SHA1 instead of MD5 with svn = 1.7.0
* omit the line Revision: 0 for added content with svn = 1.8.0 (TBC)
* print the Repository UUID line even for added content with svn =
1.8.0 (TBC)
* add a Relative URL line for svn = 1.8.0
* add a Working Copy Root Path line for svn = 1.8.0 (TBC, RFH)
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Notes:
While trying to increase my test run coverage I noticed that most of us
won't
run t9119 at all. Bad bad.
My svn is 1.8.11 (r1643975) on Fedora 21.
I would appreciate help with the following items:
TBC = to be confirmed: confirm the svn version where this change kicked it,
or run this patch and t9119 with an svn version other than mine. Please
run with -v to make sure only the RFH item fails, see below.
RFH = request for help: I couldn't figure out how to get the working
copy root path in cmd_info.
18 subtests will fail because of the RFH item.
git-svn.perl| 38 +-
t/t9119-git-svn-info.sh | 2 +-
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 36f7240..00c9cc1 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1580,6 +1580,7 @@ sub cmd_info {
}
# canonicalize_path() will return to make libsvn 1.5.x happy,
+ my $rpath = $path;
$path = . if $path eq ;
my $full_url = canonicalize_url( add_path_to_url( $url, $path ) );
@@ -1591,7 +1592,9 @@ sub cmd_info {
my $result = Path: $path_arg\n;
$result .= Name: . basename($path) . \n if $file_type ne dir;
+ $result .= Working Copy Root Path: . $gs-path . \n if
::compare_svn_version('1.7.0') = 0; #TODO
$result .= URL: $full_url\n;
+ $result .= Relative URL: ^/ . $rpath . \n if
::compare_svn_version('1.8.0')=0;
eval {
my $repos_root = $gs-repos_root;
@@ -1603,8 +1606,10 @@ sub cmd_info {
}
::_req_svn();
$result .= Repository UUID: $uuid\n unless $diff_status eq A
+ ::compare_svn_version('1.8.0') 0
(::compare_svn_version('1.5.4') = 0 || $file_type ne dir);
- $result .= Revision: . ($diff_status eq A ? 0 : $rev) . \n;
+ $result .= Revision: . ($diff_status eq A ? 0 : $rev) . \n unless
+ $diff_status eq A ::compare_svn_version('1.8.0') = 0;
$result .= Node Kind: .
($file_type eq dir ? directory : file) . \n;
@@ -1653,19 +1658,19 @@ sub cmd_info {
command_output_pipe(qw(cat-file blob),
HEAD:$path);
if ($file_type eq link) {
my $file_name = $fh;
- $checksum = md5sum(link $file_name);
+ $checksum = md5sha1sum(link $file_name);
} else {
- $checksum = md5sum($fh);
+ $checksum = md5sha1sum($fh);
}
command_close_pipe($fh, $ctx);
} elsif ($file_type eq link) {
my $file_name =
command(qw(cat-file blob), HEAD:$path);
$checksum =
- md5sum(link . $file_name);
+ md5sha1sum(link . $file_name);
} else {
open FILE, , $path or die $!;
- $checksum = md5sum(\*FILE);
+ $checksum = md5sha1sum(\*FILE);
close FILE or die $!;
}
$result .= Checksum: . $checksum . \n;
@@ -2135,6 +2140,29 @@ sub md5sum {
return $md5-hexdigest();
}
+sub md5sha1sum {
+ my $arg = shift;
+ my $ref = ref $arg;
+ my $md5;
+ if (::compare_svn_version('1.7.0') 0) {
+ require Digest::MD5;
+ $md5 = Digest::MD5-new();
+ } else {
+ require Digest::SHA1;
+ $md5 = Digest::SHA1-new();
+ }
+if ($ref eq 'GLOB' || $ref eq 'IO::File' || $ref eq 'File::Temp') {
+ $md5-addfile($arg) or croak $!;
+ } elsif ($ref eq 'SCALAR') {
+ $md5-add($$arg) or croak $!;
+ } elsif (!$ref) {
+ $md5-add($arg) or croak $!;
+ } else {
+ fatal Can't provide MD5 hash for unknown ref type: ', $ref,
';
+ }
+ return $md5-hexdigest();
+}
+
sub gc_directory {
if (can_compress() -f $_ basename($_) eq unhandled.log) {
my $out_filename = $_ . .gz;
diff --git
Re: [PATCH] git.txt: list index versions in plain English
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: At the first look, a user may think the default version is 23. Even with UNIX background, there's no reference anywhere close that may indicate this is glob or regex. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Thanks. Documentation/git.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index b37f1ab..29d9257 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -766,7 +766,8 @@ Git so take care if using Cogito etc. 'GIT_INDEX_VERSION':: This environment variable allows the specification of an index version for new repositories. It won't affect existing index - files. By default index file version [23] is used. + files. By default index file version 2 or 3 is used. See + linkgit:git-update-index[1] for more information. 'GIT_OBJECT_DIRECTORY':: If the object storage directory is specified via this -- 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/RFC/GSOC] make git-pull a builtin
On Mon, Mar 23, 2015 at 6:18 PM, Duy Nguyen pclo...@gmail.com wrote: Could you share these changes? I'm just wondering if we can add kcov support to the test suite. In this case it's more of an embarrassing hack, as I just needed a way to make git run kcov outdir git-pull.sh whenever git pull is called since kcov will not instrument any spawned subprocesses. I modified execv_dashed_external() in git.c to prepend kcov to argv (diff probably munged by gmail): diff --git a/git.c b/git.c index 8c7ee9c..0f8e7d4 100644 --- a/git.c +++ b/git.c @@ -536,6 +536,8 @@ static void execv_dashed_external(const char **argv) struct strbuf cmd = STRBUF_INIT; const char *tmp; int status; + struct argv_array args = ARGV_ARRAY_INIT; + int i; if (use_pager == -1) use_pager = check_pager_config(argv[0]); @@ -551,6 +553,11 @@ static void execv_dashed_external(const char **argv) */ tmp = argv[0]; argv[0] = cmd.buf; + argv_array_push(args, kcov); + argv_array_push(args, /home/pyokagan/pyk/git/kcov); + argv_array_push(args, cmd.buf); + for (i = 1; argv[i]; i++) + argv_array_push(args, argv[i]); trace_argv_printf(argv, trace: exec:); @@ -558,7 +565,7 @@ static void execv_dashed_external(const char **argv) * if we fail because the command is not found, it is * OK to return. Otherwise, we just pass along the status code. */ - status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT); + status = run_command_v_opt(args.argv, RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT); if (status = 0 || errno != ENOENT) exit(status); I'm guessing a real solution is to follow what the test suite does for the --valgrind option, though I haven't looked into it in detail. -- 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 00/14] numparse module: systematically tighten up integer parsing
Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to enable it. What valid reason do you have to forbid it anywhere? Only because you do not accept it by default, you need to add to enable. Why is it a problem to allow git cmd --hexval=0x1234, even if git cmd --hexval=1234 would suffice? In some cases we would like to allow that flexibility; in some cases not. But the strtol()/strtoul() functions *always* allow it. The same issue. Whare are these some cases? -- 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 00/14] numparse module: systematically tighten up integer parsing
On 03/19/2015 06:26 AM, Jeff King wrote: On Tue, Mar 17, 2015 at 05:00:02PM +0100, Michael Haggerty wrote: My main questions: * Do people like the API? My main goal was to make these functions as painless as possible to use correctly, because there are so many call sites. * Is it too gimmicky to encode the base together with other options in `flags`? (I think it is worth it to avoid the need for another parameter, which callers could easily put in the wrong order.) I definitely like the overall direction of this. My first thought was that it does seem like there are a lot of possible options to the functions (and OR-ing the flags with the base does seem weird, though I can't think of a plausible case where it would actually cause errors). Many of those options don't seem used in the example conversions (I'm not clear who would want NUM_SATURATE, for example). There are a lot of options, but so far only few of them have been used. As the call sites are rewritten we will see which of these features are useful, and which can be dispensed with. If groups of options tend to be used together, we can define constants for them like I've done with NUM_SLOPPY and NUM_SIGN. Regarding NUM_SATURATE: I'm not sure who would want it either, but I thought there might be places where the user wants to specify (effectively) infinity, and it might be convenient to let him specify something easy to type like --max= rather than --max=2147483647. I wondered if we could do away with the radix entirely. Wouldn't we be asking for base 10 most of the time? Of course, your first few patches show octal parsing, but I wonder if we should actually have a separate parse_mode() for that, since that seems to be the general reason for doing octal parsing. 10644 does not overflow an int, but it is hardly a reasonable mode. Again, as a first pass I wanted to just have a really flexible API so that call sites can be rewritten without a lot of extra thought. If somebody wants to add a parse_mode() function, it will be easy to build on top of convert_ui(). But that change can be done after this one. I also wondered if we could get rid of NUM_SIGN in favor of just having the type imply it (e.g., convert_l would always allow negative numbers, whereas convert_ul would not). But I suppose there are times when we end up using an int to store an unsigned value for a good reason (e.g., -1 is a sentinel value, but we expect only positive values from the user). So that might be a bad idea. Yes, as I was rewriting call sites, I found many that used the unsigned variants of the parsing functions but stored the result in an int. Probably some of these use -1 to denote unset; it might be that there are other cases where the variable could actually be declared to be unsigned int. Prohibiting signs when parsing signed quantities isn't really elegant from an API purity point of view, but it sure is handy! I notice that you go up to unsigned long here for sizes. If we want to use this everywhere, we'll need something larger for parsing off_t values on 32-bit machines (though I would not at all be surprised with the current code if 32-bit machines have a hard time configuring a pack.packSizeLimit above 4G). Yes, probably. I haven't run into such call sites yet. I wonder how much of the boilerplate in the parse_* functions could be factored out to use a uintmax_t, with the caller just providing the range. That would make it easier to add new types like off_t, and possibly even constrained types (e.g., an integer from 0 to 100). On the other hand, you mentioned to me elsewhere that there may be some bugs in the range-checks of config.c's integer parsing. I suspect they are related to exactly this kind of refactoring, so perhaps writing things out is best. It's not a lot of code yet. If we find out we need variants for size_t and off_t and uintmax_t and intmax_t then such a refactoring would definitely be worth considering. * Am I making callers too strict? In many cases where a positive integer is expected (e.g., --abbrev=num), I have been replacing code like [...] IMHO most of the tightening happening here is a good thing, and means we are more likely to notice mistakes rather than silently doing something stupid. For sites that currently allow it, I could imagine people using hex notation for some values, though, depending on the context. It looks there aren't many of them ((it is just when the radix is 0, right?). Some of them look to be accidental (does anybody really ask for --threads=0x10?), but others might not be (the pack index-version contains an offset field that might be quite big). Yes, we can even debate whether we want to implement a general policy that user-entered integers can be specified in any radix. Probably nobody will ever specify --threads=0x10, but is there harm in allowing it? Against the gain in flexibility, I see the following
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On 03/19/2015 08:32 AM, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
I wonder how much of the boilerplate in the parse_* functions could be
factored out to use a uintmax_t, with the caller just providing the
range. That would make it easier to add new types like off_t, and
possibly even constrained types (e.g., an integer from 0 to 100). On the
other hand, you mentioned to me elsewhere that there may be some bugs in
the range-checks of config.c's integer parsing. I suspect they are
related to exactly this kind of refactoring, so perhaps writing things
out is best.
I like this idea very well. I wonder if we can implement the family
of
parse_{type}(const char *, unsigned int flags,
const char **endptr, {type} *result)
functions by calls a helper that internally deals with the numbers
in uintmax_t, and then checks if the value fits within the possible
range of the *result before returning.
int parse_i(const char *str, unsigned flags,
const char **endptr, int *result) {
uintmax_t val;
int sign = 1, status;
if (*str == '-') {
sign = -1;
str++;
}
status = parse_num(str, flags, endptr, val, INT_MAX);
if (status 0)
return status;
*result = sign 0 ? -val : val;
return 0;
}
(assuming the last parameter to parse_num is used to check if the
result fits within that range). Or that may be easier and cleaner
to be done in the caller with or something like that:
status = parse_num(str, flags, endptr, val);
if (status 0)
return status;
if (INT_MAX = val * sign || val * sign = INT_MIN) {
errno = ERANGE;
return -1;
}
If we wanted to, we may even be able to avoid duplication of
boilerplate by wrapping the above pattern within a macro,
parameterizing the TYPE_{MIN,MAX} and using token pasting, to
expand to four necessary result types.
There is no reason for the implementation of the parse_num() helper
to be using strtoul(3) or strtoull(3); its behaviour will be under
our total control. It can become fancier by enriching the flags
bits (e.g. allow scaling factor, etc.) only once and all variants
for various result types will get the same feature.
Parsing numbers is not rocket science, but there are a lot of pitfalls,
especially around overflow. It's even harder to write such code via
macros and the result is less readable.
This patch series is mostly about finding a reasonable API and whipping
the callers into shape. That seems ambitious enough for me. I'd rather
stick with boring wrappers for now and lean on strtol()/strtoul() to do
the dirty work. It will be easy for a motivated person to change the
implementation later.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
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 17/25] t0020: use modern test_* helpers
Quoting Jeff King p...@peff.net:
This test contains a lot of hand-rolled messages to show
when the test fails. We can omit most of these by using
verbose and test_must_fail. A few of them are for
update-index, but we can assume it produces reasonable error
messages when it fails.
Signed-off-by: Jeff King p...@peff.net
---
t/t0020-crlf.sh | 144
+++-
1 file changed, 28 insertions(+), 116 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index d2e51a8..9fa26df 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '
for f in one dir/two
do
append_cr $f tmp mv -f tmp $f
- git update-index -- $f || {
- echo Oops
- false
- break
- }
+ git update-index -- $f ||
+ break
done
Ah, these tests are evil, I remember them from the time when I was
fiddling with Jonathan's patch. They can fail silently without
testing what they were supposed to test.
If something in the loop fails, the break will leave the loop but it
will do so with zero return value and, consequently, the test will
continue as if everything were OK.
And unless it was 'git update-index' that failed in a way that left a
borked index behind, the 'git diff-index --cached' below will not
error out or produce some output that would cause the test to fail.
i.e. I tried e.g.
append_cr $f tmp mv -f tmp $f false
in the loop and the test succeeded.
I think the best fix would be to unroll the loop: after this patch the
loop body consists of only two significant lines and we iterate
through the loop only twice, so the test would be even shorter.
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
@@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '
for f in one dir/two
do
append_cr $f tmp mv -f tmp $f
- git update-index -- $f || {
- echo Oops $f
- false
- break
- }
+ git update-index -- $f ||
+ break
done
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
@@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '
for f in one dir/two
do
remove_cr $f tmp mv -f tmp $f
- git update-index -- $f || {
- echo Eh? $f
- false
- break
- }
+ verbose git update-index -- $f ||
+ break
done
test $one = $(git hash-object --stdin one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
test_expect_success 'checkout with autocrlf=input' '
@@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '
test $one = $(git hash-object --stdin one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
test_expect_success 'apply patch (autocrlf=input)' '
@@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '
git read-tree --reset -u HEAD
git apply patch.file
- test $patched = $(git hash-object --stdin one) || {
- echo Eh? apply without index
- false
- }
+ verbose test $patched = $(git hash-object --stdin one)
'
test_expect_success 'apply patch --cached (autocrlf=input)' '
@@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached
(autocrlf=input)' '
git read-tree --reset -u HEAD
git apply --cached patch.file
- test $patched = $(git rev-parse :one) || {
- echo Eh? apply with --cached
- false
- }
+ verbose test $patched = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=input)' '
@@ -226,11 +199,8 @@ test_expect_success 'apply patch --index
(autocrlf=input)' '
git read-tree --reset -u HEAD
git apply --index patch.file
- test $patched = $(git rev-parse :one)
- test $patched = $(git hash-object --stdin one) || {
- echo Eh? apply with --index
- false
- }
+ verbose test $patched =
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
On Tue, Mar 24, 2015 at 11:26 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: * jn/gitweb-utf8-in-links (2014-05-27) 1 commit - gitweb: Harden UTF-8 handling in generated links This has been lingering in my 'pu' branch without seeing any updates since $gmane/250758; is anybody still interested in resurrecting it and moving it forward? I can try to pick it up, but I am no longer sure that it is a good idea to solve the problem. In particular git does not require that paths (i.e. filesystem) use utf-8 encoding; it could use latin1 / iso-8859-1 and non-utf8 octet. -- Jakub Narębski -- 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 17/25] t0020: use modern test_* helpers
On Wed, Mar 25, 2015 at 01:23:23AM +0100, SZEDER Gábor wrote:
for f in one dir/two
do
append_cr $f tmp mv -f tmp $f
-git update-index -- $f || {
-echo Oops
-false
-break
-}
+git update-index -- $f ||
+break
done
Ah, these tests are evil, I remember them from the time when I was fiddling
with Jonathan's patch. They can fail silently without testing what they
were supposed to test.
If something in the loop fails, the break will leave the loop but it will do
so with zero return value and, consequently, the test will continue as if
everything were OK.
And unless it was 'git update-index' that failed in a way that left a borked
index behind, the 'git diff-index --cached' below will not error out or
produce some output that would cause the test to fail. i.e. I tried e.g.
append_cr $f tmp mv -f tmp $f false
in the loop and the test succeeded.
Ugh, you're right. I remembered that for loops were tricky in -chains,
but for some reason was thinking that break would give you the last
exit code, But I just re-tested, and of course it does not work.
I think the best fix would be to unroll the loop: after this patch the loop
body consists of only two significant lines and we iterate through the loop
only twice, so the test would be even shorter.
Yeah, unrolling may be the best thing here, given the size of the loops.
As a general rule, I think it has to be a subshell with an exit, like:
(
for i in one two three; do
echo $i
test $i = one ||
exit 1
done
)
echo exit=$?
which should yield one, two, and exit=1. 7b1732c (t7510: use consistent
-chains in loop, 2014-06-16) deals with this in another test.
-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 01/25] t/test-lib: introduce --chain-lint option
On Wed, Mar 25, 2015 at 03:53:52AM +0100, SZEDER Gábor wrote: cmd1 for i in a b c; do cmd2 $i done cmd3 which will not notice failures of cmd2 a or cmd b s/cmd b/cmd2 b/ ? Yes, but the patches are already in next, so it is sadly too late for commit message fixups. - it cannot find a missing -chain inside a block or subfunction, like: [...] And inside subshells. [...] Yeah, I had mentally filed them with block, but true subshells are probably the most common place. However, I'd suspect a good portion of them are going to be the trivial type, especially if they involve setting up the sub-environment at the top of the subshell. E.g., something like this: cmd1 ( FOO=bar; export FOO cmd2 ) cmd3 does not break the outer chain (which is what --chain-lint checks). It does break the chain inside the subshell, but we never expect variable assignment or export to fail (it is nice to fix it, of course, but the main purpose in fixing the ones in my trivial patch was more about shutting up --chain-lint to make the real breakages more obvious). -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 21/25] t9001: use test_when_finished
Quoting Jeff King p...@peff.net:
The confirmation tests in t9001 all save the value of
sendemail.confirm, do something to it, then restore it at
the end, in a way that breaks the -chain (they are not
wrong, because they save the $? value, but it fools
--chain-lint).
Instead, they can all use test_when_finished, and we can
even make the code simpler by factoring out the shared
lines.
I think that saving the value of 'sendemail.confirm' is not necessary.
There are two blocks of confirmation tests, this patch concerns only tests
of the second block. The first block of confirmation tests is nearly at
the beginning of the file in order to check the no confirm cases early.
If any of those fails the remainig tests in the file are skipped because
they might hang. The last of those tests sets 'sendemail.confirm' to
'never' and leaves it so to avoid unintentional prompting in the remaining
tests and then its value is not modified until that second block of
confirm tests are reached. This means that when those tests save the
value of 'sendemail.confirm' they always save 'never'. Then why save it,
just use test_when_finished to restore it to 'never' and all is well.
Note that we can _almost_ use test_config here, except that:
1. We do not restore the config with test_unconfig, but by
setting it back to some prior value.
2. We are not always setting a config variable. Sometimes
the change to be undone is unsetting it entirely.
We could teach test_config to handle these cases, but it's
not worth the complexity for a single call-site.
Signed-off-by: Jeff King p...@peff.net
---
t/t9001-send-email.sh | 30 ++
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 37caa18..c9f54d5 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' '
test_confirm --confirm=compose --compose
'
-test_expect_success $PREREQ 'confirm by default (due to cc)' '
+save_confirm () {
CONFIRM=$(git config --get sendemail.confirm)
+ test_when_finished git config sendemail.confirm ${CONFIRM:-never}
+}
+
+test_expect_success $PREREQ 'confirm by default (due to cc)' '
+ save_confirm
git config --unset sendemail.confirm
test_confirm
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm by default (due to --compose)' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config --unset sendemail.confirm
test_confirm --suppress-cc=all --compose
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config --unset sendemail.confirm
rm -fr outdir
git format-patch -2 -o outdir
@@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF
(inform assumes y)' '
--to=nob...@example.com \
--smtp-server=$(pwd)/fake.sendmail \
outdir/*.patch /dev/null
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
@@ -861,13 +857,10 @@ test_expect_success $PREREQ 'confirm detects EOF
(auto causes failure)' '
--to=nob...@example.com \
--smtp-server=$(pwd)/fake.sendmail \
$patches /dev/null
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm does not loop forever' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
@@ -876,9 +869,6 @@ test_expect_success $PREREQ 'confirm does not loop
forever' '
--to=nob...@example.com \
--smtp-server=$(pwd)/fake.sendmail \
$patches
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '
--
2.3.3.520.g3cfbb5d
--
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 18/25] t1301: use modern test_* helpers
On Wed, Mar 25, 2015 at 12:51:20AM +0100, SZEDER Gábor wrote: @@ -33,7 +32,7 @@ do git init --shared=1 test 1 = $(git config core.sharedrepository) ) -actual=$(ls -l sub/.git/HEAD) +actual=$(ls -l sub/.git/HEAD) case $actual in -rw-rw-r--*) : happy This hunk could go into the moderate -chain breakage patch. Doesn't really matter, what matters most is that it's fixed, but I really liked your classification of missing s in the early patches. Yeah, these later ones are a mish-mash of real fixes and just quieting --chain-lint. I pulled out ones that I felt needed a little more explanation, and generally kept changes for a file together (though I am not sure not always). I'm not sure it's worth the effort to go through another round of classifying. -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 21/25] t9001: use test_when_finished
On Wed, Mar 25, 2015 at 03:00:22AM +0100, SZEDER Gábor wrote: Instead, they can all use test_when_finished, and we can even make the code simpler by factoring out the shared lines. I think that saving the value of 'sendemail.confirm' is not necessary. There are two blocks of confirmation tests, this patch concerns only tests of the second block. The first block of confirmation tests is nearly at the beginning of the file in order to check the no confirm cases early. If any of those fails the remainig tests in the file are skipped because they might hang. The last of those tests sets 'sendemail.confirm' to 'never' and leaves it so to avoid unintentional prompting in the remaining tests and then its value is not modified until that second block of confirm tests are reached. This means that when those tests save the value of 'sendemail.confirm' they always save 'never'. Then why save it, just use test_when_finished to restore it to 'never' and all is well. Yeah, I suspected this while writing it the patch, but I preferred to keep it more obvious that there would be no accidental regression, since the series was already so long (and also because calling save_confirm is not any worse than test_when_finished). I don't mind a patch on top simplifying out save_confirm, if you're confident that's what we're always saving. -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: Sparse checkout not working as expected (colons in filenames on Windows)
On Wed, Mar 25, 2015 at 6:50 AM, Philip Oakley philipoak...@iee.org wrote: I've corrected the sparse-checkout, but won't the command line 'git update-index --skip-worktree' will still need it? (demo commands below) A git checkout (without arguments) or read-trree -mu should attach skip-worktree properly. You don't need to do it yourself. That said, the final error (which I'd missed in the earlier post) is: fatal: make_cache_entry failed for path 'ifcfg-eth0:0' This is on the Windows (pre-compiled msysgit at v1.9.5) Git bash, so could be a catch path in that code for make_cache_entry (I've not checked the code yet). So at the moment it doesn't look like sparse checkout can be used to avoid colons in windows on-disk files based on the current code. Both of your commands below fail by the same function, verify_path() because of this msysgit commit 2e2a2d1 (NTFS: Prevent problematic paths from being checked out - 2014-12-10). I guess that check is a bit too strong, it should apply when new index entries are created from worktree (not from a tree).. -- Philip Philip@PHILIPOAKLEY /d/Git_repos/colons $ cd tortoisegit-colons/ Philip@PHILIPOAKLEY /d/Git_repos/colons/tortoisegit-colons (test) $ git update-index --skip-worktree -- ifcfg-eth0\:0 Ignoring path ifcfg-eth0:0 Philip@PHILIPOAKLEY /d/Git_repos/colons/tortoisegit-colons (test) $ git reset error: Invalid path 'ifcfg-eth0:0' fatal: make_cache_entry failed for path 'ifcfg-eth0:0' -- 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] gc: save log from daemonized gc --auto and print it next time
On Wed, Mar 25, 2015 at 5:07 AM, Junio C Hamano gits...@pobox.com wrote: + LANG=C git gc --auto + sleep 1 # give it time to daemonize + while test -f .git/gc.pid; do sleep 1; done Yuck... Yeah.. it's hard to test daemon things. I'm not even sure if we should add a test, but I tried anyway. + grep too many unreachable loose objects .git/gc.log + LANG=C git gc --auto 2error + grep skipped error + grep too many unreachable loose objects error + ! test -f .git/gc.log + ) +' For that 17/ has very many loose objects that are still young and unreachable issue, I wonder if the right solution is somehow to flag the repository and prevent gc --auto from running until the situation improves. I checked at this time and found too many in 17/; upon finding that flag file (with a timestamp), if there are new files in 17/ or if there are other reasons to do a gc (perhaps there are too many packfiles to be consolidated?), then do the gc but otherwise quit silently before spending too many cycles on it, or something along that line? That's a separate problem that's being discussed in another thread. I think Jeff's idea of storing the number of estimated loose objects may be more reliable than timestamps.. -- 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 01/25] t/test-lib: introduce --chain-lint option
Quoting Jeff King p...@peff.net:
However, there are a number of places it cannot reach:
- it cannot find a failure to break out of loops on error,
like:
cmd1
for i in a b c; do
cmd2 $i
done
cmd3
which will not notice failures of cmd2 a or cmd b
s/cmd b/cmd2 b/ ?
- it cannot find a missing -chain inside a block or
subfunction, like:
foo () {
cmd1
cmd2
}
foo
bar
which will not notice a failure of cmd1.
And inside subshells. I think it's worth mentioning, too, because
subshells are used frequently when setting environment variables
( GIT_FOO=bar export GIT_FOO cmd1 ... ) test_cmp
or changing directory
( cd subdir cmd1 ... ) test_cmp
I was wondering whether we could do better here with helper functions,
something along the lines of:
# Set and export environment variable, automatically unsetting it after
# the test is over
test_setenv () {
eval $1=\$2
export $1
# sane_unset, to allow unsetting during the test
test_when_finished sane_unset '$1'
}
# Change to given directory, automatically return to current working
# directory after the test is over
test_cd () {
test_when_finished cd '$PWD'
cd $1
}
With these the above examples would become
test_setenv GIT_FOO bar cmd1 ... test_cmp
and
test_cd subdir cmd1 ... test_cmp
which means increased coverage for --chain-lint.
Thanks for working on this. I looked into this after seeing Jonathan's
patch back then, got quite far but never reached a chain-lint-clean
state, and only sent patches for the two most amusing breakages
(ddeaf7ef0d, 056f34bbcd).
I'm glad it's off my TODO list and I don't have to rebase a 1.5 year old
branch to current master :)
Gábor
- it only checks tests that you run; every platform will
have some tests skipped due to missing prequisites,
so it's impossible to say from one run that the test
suite is free of broken -chains. However, all tests get
run by _somebody_, so eventually we will notice problems.
- it does not operate on test_when_finished or prerequisite
blocks. It could, but these tends to be much shorter and
less of a problem, so I punted on them in this patch.
This patch was inspired by an earlier patch by Jonathan
Nieder:
http://article.gmane.org/gmane.comp.version-control.git/235913
This implementation and all bugs are mine.
Signed-off-by: Jeff King p...@peff.net
---
t/README | 10 ++
t/test-lib.sh | 16
2 files changed, 26 insertions(+)
diff --git a/t/README b/t/README
index d5bb0c9..35438bc 100644
--- a/t/README
+++ b/t/README
@@ -168,6 +168,16 @@ appropriately before running make.
Using this option with a RAM-based filesystem (such as tmpfs)
can massively speed up the test suite.
+--chain-lint::
+--no-chain-lint::
+ If --chain-lint is enabled, the test harness will check each
+ test to make sure that it properly -chains all commands (so
+ that a failure in the middle does not go unnoticed by the final
+ exit code of the test). This check is performed in addition to
+ running the tests themselves. You may also enable or disable
+ this feature by setting the GIT_TEST_CHAIN_LINT environment
+ variable to 1 or 0, respectively.
+
You can also set the GIT_TEST_INSTALLED environment variable to
the bindir of an existing git installation to test that installation.
You still need to have built this git sandbox, from which various
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c096778..50b3d3f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -232,6 +232,12 @@ do
--root=*)
root=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
+ --chain-lint)
+ GIT_TEST_CHAIN_LINT=1
+ shift ;;
+ --no-chain-lint)
+ GIT_TEST_CHAIN_LINT=0
+ shift ;;
-x)
trace=t
verbose=t
@@ -524,6 +530,16 @@ test_eval_ () {
test_run_ () {
test_cleanup=:
expecting_failure=$2
+
+ if test ${GIT_TEST_CHAIN_LINT:-0} != 0; then
+ # 117 is magic because it is unlikely to match the exit
+ # code of other programs
+ test_eval_ (exit 117) $1
+ if test $? != 117; then
+ error bug in the test script: broken -chain: $1
+ fi
+ fi
+
setup_malloc_check
test_eval_ $1
eval_ret=$?
--
2.3.3.520.g3cfbb5d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
Jakub Narębski jna...@gmail.com writes: On Tue, Mar 24, 2015 at 11:26 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: * jn/gitweb-utf8-in-links (2014-05-27) 1 commit - gitweb: Harden UTF-8 handling in generated links This has been lingering in my 'pu' branch without seeing any updates since $gmane/250758; is anybody still interested in resurrecting it and moving it forward? I can try to pick it up, but I am no longer sure that it is a good idea to solve the problem. After re-reading the discussion thread, I had the same impression. Let's drop the patch for now. It can be re-raised as/if needed. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] more -chaining test fixups
Here's what I found looking for loops like: for i in a b c; do something_important $i || break done something_else which presumably expect the chain to stop when something_important fails for any loop element. The solutions are one of (depending on the surrounding code): 1. Switch the break to return 1. The tests are all -chained, so the effect of a failed command is to exit the test immediately anyway. And we wrap our eval'd test snippets inside an extra layer of function call to explicitly allow early returns like this. 2. Switch the break to exit 1. Calling return from a subshell inside a function is a bit weird. It doesn't exit the function at all, but rather just the subshell (in both bash and dash). But if you are not in a function, calling return at all is an error in bash (subshell or no), and OK in dash (where it acts like exit). POSIX explicitly marks the outside of a function behavior as unspecified, but I couldn't find anything about the subshell behavior. So I'm loathe to depend on it, even though it does seem to do what we want, as I do not want to even think what some more obscure shells might do with it. And especially because we know that exit 1 portably does what we want (the only downside is that you have to recognize which situation you are in and use exit versus return). 3. Unroll the loops. In some cases the result is actually shorter, and (IMHO) more readable. These sites were all found with: git grep -E '(^|\|\|)[]*break' t/t*.sh (that's a space and a tab in the brackets). There are some matches there that I did not touch, because they were already fine. E.g., t5302 and t7700 use loops to assign a value to a variable and break out early, and then check the value of the variable. That's just the tip of the iceberg, though. Searching for git grep 'for .* in ' yields hundreds of hits. Most of which are probably fine (quite a few are outside -chains entirely). I focused on the ones that called break, because that indicated to me that the author was trying to address the -chain. Certainly anybody else is welcome to take a stab at the rest, but I'm also happy to fix them up as we touch nearby code and notice them. Most of the loops are in setup code that we do not expect to fail anyway, so examining them is a lot of work for a little gain. There were a few legitimate problems, though. I've ordered the patches below by descending severity. These apply on top of jk/test-chain-lint. [1/8]: perf-lib: fix ignored exit code inside loop [2/8]: t0020: fix ignored exit code inside loops [3/8]: t3305: fix ignored exit code inside loop [4/8]: t7701: fix ignored exit code inside loop These four are actual bugs. [5/8]: t: fix some trivial cases of ignored exit codes in loops These ones are in setup code, and so would almost certainly never fail. [6/8]: t: simplify loop exit-code status variables [7/8]: t0020: use test_* helpers instead of hand-rolled messages [8/8]: t9001: drop save_confirm helper These last three are pure cleanup, no behavior changes. The last two are not even strictly related to the same topic, but I noticed them while in the area. -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
[PATCH 4/8] t7701: fix ignored exit code inside loop
When checking a list of file mtimes, we use a loop and break
out early from the loop if any entry does not match.
However, the exit code of a loop exited via break is always
0, meaning that the test will fail to notice we had a
mismatch. Since the loop is inside a function, we can fix
this by doing an early return 1.
Signed-off-by: Jeff King p...@peff.net
---
t/t7701-repack-unpack-unreachable.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7701-repack-unpack-unreachable.sh
b/t/t7701-repack-unpack-unreachable.sh
index aad8a9c..b66e383 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -57,7 +57,7 @@ compare_mtimes ()
{
read tref rest
while read t rest; do
- test $tref = $t || break
+ test $tref = $t || return 1
done
}
--
2.3.4.635.gd6ffcfe
--
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/8] t0020: fix ignored exit code inside loops
A loop like:
for f in one two; do
something $f ||
break
done
will correctly break out of the loop when we see a failure
of one item, but the resulting exit code will always be
zero. We can fix that by putting the loop into a function or
subshell, but in this case it is simpler still to just
unroll the loop. We do add a helper function, which
hopefully makes the end result even more readable (in
addition to being shorter).
Reported-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Jeff King p...@peff.net
---
You can make each call site a one-liner that does the loop inside the
function (including the update-index call). But I tried to shoot for
readability rather than absolute minimum characters.
t/t0020-crlf.sh | 54 +++---
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 9fa26df..144fdcd 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -8,6 +8,13 @@ has_cr() {
tr '\015' Q $1 | grep Q /dev/null
}
+# add or remove CRs to disk file in-place
+# usage: munge_cr append|remove file
+munge_cr () {
+ ${1}_cr $2 tmp
+ mv tmp $2
+}
+
test_expect_success setup '
git config core.autocrlf false
@@ -100,14 +107,9 @@ test_expect_success 'update with autocrlf=input' '
rm -f tmp one dir/two three
git read-tree --reset -u HEAD
git config core.autocrlf input
-
- for f in one dir/two
- do
- append_cr $f tmp mv -f tmp $f
- git update-index -- $f ||
- break
- done
-
+ munge_cr append one
+ munge_cr append dir/two
+ git update-index -- one dir/two
differs=$(git diff-index --cached HEAD)
verbose test -z $differs
@@ -118,14 +120,9 @@ test_expect_success 'update with autocrlf=true' '
rm -f tmp one dir/two three
git read-tree --reset -u HEAD
git config core.autocrlf true
-
- for f in one dir/two
- do
- append_cr $f tmp mv -f tmp $f
- git update-index -- $f ||
- break
- done
-
+ munge_cr append one
+ munge_cr append dir/two
+ git update-index -- one dir/two
differs=$(git diff-index --cached HEAD)
verbose test -z $differs
@@ -136,13 +133,9 @@ test_expect_success 'checkout with autocrlf=true' '
rm -f tmp one dir/two three
git config core.autocrlf true
git read-tree --reset -u HEAD
-
- for f in one dir/two
- do
- remove_cr $f tmp mv -f tmp $f
- verbose git update-index -- $f ||
- break
- done
+ munge_cr remove one
+ munge_cr remove dir/two
+ git update-index -- one dir/two
test $one = $(git hash-object --stdin one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
@@ -154,18 +147,9 @@ test_expect_success 'checkout with autocrlf=input' '
rm -f tmp one dir/two three
git config core.autocrlf input
git read-tree --reset -u HEAD
-
- for f in one dir/two
- do
- if has_cr $f
- then
- echo Eh? $f
- false
- break
- else
- git update-index -- $f
- fi
- done
+ test_must_fail has_cr one
+ test_must_fail has_cr two
+ git update-index -- one dir/two
test $one = $(git hash-object --stdin one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
--
2.3.4.635.gd6ffcfe
--
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/8] t3305: fix ignored exit code inside loop
When we test deleting notes, we run git notes remove in a loop. However, the exit value of the loop will only reflect the final note we process. We should break out of the loop with a failing exit code as soon as we see a problem. Note that we can call exit 1 here without explicitly creating a subshell, because the while loop on the right-hand side of a pipe executes in its own implicit subshell. Note also that the break above does not suffer the same problem; it is meant to exit the loop early at a certain number of iterations. We can bump it into the conditional of the loop to make this more obvious. Signed-off-by: Jeff King p...@peff.net --- t/t3305-notes-fanout.sh | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh index b1ea64b..54460be 100755 --- a/t/t3305-notes-fanout.sh +++ b/t/t3305-notes-fanout.sh @@ -51,15 +51,12 @@ test_expect_success 'deleting most notes with git-notes' ' num_notes=250 i=0 git rev-list HEAD | - while read sha1 + while test $i -lt $num_notes read sha1 do i=$(($i + 1)) - if test $i -gt $num_notes - then - break - fi test_tick - git notes remove $sha1 + git notes remove $sha1 || + exit 1 done ' -- 2.3.4.635.gd6ffcfe -- 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/GSoC] Proposal: Make git-pull and git-am builtins
Hi, On Wed, Mar 25, 2015 at 2:37 AM, Junio C Hamano gits...@pobox.com wrote: Paul Tan pyoka...@gmail.com writes: ..., I propose the following requirements for the rewritten code: 1. No spawning of external git processes. This is to support systems with high ``fork()`` or process creation overhead, and to reduce redundant IO by taking advantage of the internal object, index and configuration cache. I suspect this may probably be too strict in practice. True, we should never say run_command_capture() just to to read from git rev-parse---we should just call get_sha1() instead. But for a complex command whose execution itself far outweighs the cost of forking, I do not think it is fair to say your project failed if you chose to run_command() it. For example, it may be perfectly OK to invoke git merge via run_command(). Yes, which is why I proposed writing a baseline using only the run-command APIs first. Any other optimizations can then be done selectively after that. I think it's still good to have the ideal in mind though (and whoops I forgot to put in the word ideal in the text). 3. The resulting builtin should not have wildly different behavior or bugs compared to the shell script. This on the other hand is way too loose. The original and the port must behave identically, unless the difference is fixing bugs in the original. I was considering that there may be slight behavioral changes when the rewritten code is modified to take greater advantage of the internal API, especially since some builtins due to historical issues, may have duplicated code from the internal API[1]. [1] I'm suspecting that the implementation of --merge-base in show-branch.c re-implements get_octopus_merge_bases(). Potential difficulties === Rewriting code may introduce bugs ... Yes, but that is a reasonable risk you need to manage to gain the benefit from this project. Of course, the downside of following this too strictly is that if there were any logical bugs in the original code, or if the original code is unclear, the rewritten code would inherit these problems too. I'd repeat my comment on the 3. above. Identifying and fixing bugs is great, but otherwise don't worry about this too much. Being bug-to-bug compatible with the original is way better than introducing new bugs of an unknown nature. Well yes, but I was thinking that if there are any glaring errors in the original source then it would be better to fix these errors during the rewrite than wasting time writing code that replicates these errors. Rewritten code may become harder to understand ... And also it may become harder to modify. That is the largest problem with any rewrite, and we should spend the most effort to avoid it. A new bugs introduced we can later fix as long as the result is understandable and maintainable. For the purpose of reducing git's dependencies, the rewritten C code should not depend on other libraries or executables other than what is already available to git builtins. Perhaps misphrased; see below. In this case I was thinking of making git depend on another project. (e.g, using an external regular expression library). Of course a balance has to be made in this aspect (thus the use of should not), but git-pull and git-am are relatively simple so there should be no need for that, We can see that the C version requires much more lines compared to the shell pipeline,... That is something you would solve by introducing reusable code in run_command API, isn't it? That is how various rewrites in the past did, and this project should do so too. You should aim to do this project by not just using what is already available, but adding what you discover is a useful reusable pattern into a set of new functions in the already available API set. Whoops, forgot to mention that here. (A brief mention was made on this kind of refactoring in the Development Approach). Thank you for your 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
[PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops
These are all cases where we do a setup step of the form: for i in $foo; do set_up $i || break done more_setup would not notice a failure in set_up (because break always returns a 0 exit code). These are just setup steps that we do not expect to fail, but it does not hurt to be defensive. Most can be fixed by converting the break to a return 1 (since we eval our tests inside a function for just this purpose). A few of the loops are inside subshells, so we can use just exit 1 to break out of the subshell. And a few can actually be made shorter by just unrolling the loop. Signed-off-by: Jeff King p...@peff.net --- t/t3010-ls-files-killed-modified.sh | 11 --- t/t3031-merge-criscross.sh | 2 +- t/t3202-show-branch-octopus.sh | 2 +- t/t4024-diff-optimize-common.sh | 2 +- t/t4046-diff-unmerged.sh| 8 t/t4151-am-abort.sh | 2 +- t/t5505-remote.sh | 8 t/t5514-fetch-multiple.sh | 4 ++-- t/t6026-merge-attr.sh | 6 +++--- t/t6040-tracking-info.sh| 7 +++ 10 files changed, 24 insertions(+), 28 deletions(-) diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 62fce10..580e158 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -55,13 +55,10 @@ test_expect_success 'git update-index --add to add various paths.' ' : path9 date path10 git update-index --add -- path0 path?/file? pathx/ju path7 path8 path9 path10 - for i in 1 2 - do - git init submod$i - ( - cd submod$i git commit --allow-empty -m empty $i - ) || break - done + git init submod1 + git -C submod1 commit --allow-empty -m empty 1 + git init submod2 + git -C submod2 commit --allow-empty -m empty 2 git update-index --add submod[12] ( cd submod1 diff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh index 7f41607..e59b0a3 100755 --- a/t/t3031-merge-criscross.sh +++ b/t/t3031-merge-criscross.sh @@ -32,7 +32,7 @@ test_expect_success 'setup repo with criss-cross history' ' do echo $n data/$n n=$(($n+1)) || - break + return 1 done # check them in diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh index 0a5d5e6..6adf478 100755 --- a/t/t3202-show-branch-octopus.sh +++ b/t/t3202-show-branch-octopus.sh @@ -19,7 +19,7 @@ test_expect_success 'setup' ' file$i git add file$i test_tick - git commit -m branch$i || break + git commit -m branch$i || return 1 done ' diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh index c4d733f..7e76018 100755 --- a/t/t4024-diff-optimize-common.sh +++ b/t/t4024-diff-optimize-common.sh @@ -139,7 +139,7 @@ test_expect_success setup ' ( printf C; zs $n ) file-c$n ( echo D; zs $n ) file-d$n - expect_pattern $n || break + expect_pattern $n || return 1 done expect ' diff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh index 25d50a6..d0f1447 100755 --- a/t/t4046-diff-unmerged.sh +++ b/t/t4046-diff-unmerged.sh @@ -8,7 +8,7 @@ test_expect_success setup ' do blob=$(echo $i | git hash-object --stdin) eval blob$i=$blob - eval m$i=\100644 \$blob$i $i\ || break + eval m$i=\100644 \$blob$i $i\ || return 1 done paths= for b in o x @@ -24,9 +24,9 @@ test_expect_success setup ' case $b in x) echo $m1$p ;; esac case $o in x) echo $m2$p ;; esac case $t in x) echo $m3$p ;; esac || - break - done || break - done || break + return 1 + done + done done ls-files-s.expect git update-index --index-info ls-files-s.expect git ls-files -s ls-files-s.actual diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 1176bcc..8d90634 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -20,7 +20,7 @@ test_expect_success setup ' echo $i otherfile-$i git add otherfile-$i test_tick - git commit -a -m $i || break + git commit -a -m $i || return 1 done git format-patch --no-numbered initial git checkout -b side initial diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 17c6330..7a8499c 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -579,7 +579,7 @@
[PATCH 6/8] t: simplify loop exit-code status variables
Since shell loops may drop the exit code of failed commands
inside the loop, some tests try to keep track of the status
by setting a variable. This can end up cumbersome and hard
to read; it is much simpler to just exit directly from the
loop using return 1 (since each case is either in a helper
function or inside a test snippet).
Signed-off-by: Jeff King p...@peff.net
---
t/t3060-ls-files-with-tree.sh | 13 -
t/t3901-i18n-patch.sh | 8 ++--
2 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index 61c1f53..36b10f7 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -25,15 +25,10 @@ test_expect_success setup '
do
num=00$n$m
sub/file-$num
- echo file-$num expected || {
- bad=t
- break
- }
- done test -z $bad || {
- bad=t
- break
- }
- done test -z $bad
+ echo file-$num expected ||
+ return 1
+ done
+ done
git add .
git commit -m add a bunch of files
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index a392f3d..75cf3ff 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -9,7 +9,7 @@ test_description='i18n settings and format-patch | am pipe'
check_encoding () {
# Make sure characters are not corrupted
- cnt=$1 header=$2 i=1 j=0 bad=0
+ cnt=$1 header=$2 i=1 j=0
while test $i -le $cnt
do
git format-patch --encoding=UTF-8 --stdout HEAD~$i..HEAD~$j |
@@ -20,14 +20,10 @@ check_encoding () {
grep ^encoding ISO8859-1 ;;
*)
grep ^encoding ISO8859-1; test $? != 0 ;;
- esac || {
- bad=1
- break
- }
+ esac || return 1
j=$i
i=$(($i+1))
done
- (exit $bad)
}
test_expect_success setup '
--
2.3.4.635.gd6ffcfe
--
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 8/8] t9001: drop save_confirm helper
The idea of this helper is that we want to save the current
value of a config variable and then restore it again after
the test completes. However, there's no point in actually
saving the value; it should always be restored to the string
never (which you can confirm by instrumenting
save_confirm to print the value it finds).
Let's just replace it with a single test_when_finished call.
Suggested-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Jeff King p...@peff.net
---
t/t9001-send-email.sh | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c9f54d5..7be14a4 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -817,25 +817,20 @@ test_expect_success $PREREQ '--confirm=compose' '
test_confirm --confirm=compose --compose
'
-save_confirm () {
- CONFIRM=$(git config --get sendemail.confirm)
- test_when_finished git config sendemail.confirm ${CONFIRM:-never}
-}
-
test_expect_success $PREREQ 'confirm by default (due to cc)' '
- save_confirm
+ test_when_finished git config sendemail.confirm never
git config --unset sendemail.confirm
test_confirm
'
test_expect_success $PREREQ 'confirm by default (due to --compose)' '
- save_confirm
+ test_when_finished git config sendemail.confirm never
git config --unset sendemail.confirm
test_confirm --suppress-cc=all --compose
'
test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
- save_confirm
+ test_when_finished git config sendemail.confirm never
git config --unset sendemail.confirm
rm -fr outdir
git format-patch -2 -o outdir
@@ -848,7 +843,7 @@ test_expect_success $PREREQ 'confirm detects EOF (inform
assumes y)' '
'
test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
- save_confirm
+ test_when_finished git config sendemail.confirm never
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
@@ -860,7 +855,7 @@ test_expect_success $PREREQ 'confirm detects EOF (auto
causes failure)' '
'
test_expect_success $PREREQ 'confirm does not loop forever' '
- save_confirm
+ test_when_finished git config sendemail.confirm never
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
--
2.3.4.635.gd6ffcfe
--
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/8] t0020: use test_* helpers instead of hand-rolled messages
These tests are not wrong, but it is much shorter and more idiomatic to say verbose or test_must_fail rather than printing our own messages on failure. Likewise, there is no need to say happy at the end of a test; the test suite takes care of that. Signed-off-by: Jeff King p...@peff.net --- I somehow missed these when doing 9157c5c in the earlier series. t/t0020-crlf.sh | 38 +- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh index 144fdcd..f94120a 100755 --- a/t/t0020-crlf.sh +++ b/t/t0020-crlf.sh @@ -35,9 +35,7 @@ test_expect_success setup ' for w in Some extra lines here; do echo $w; done one git diff patch.file patched=$(git hash-object --stdin one) - git read-tree --reset -u HEAD - - echo happy. + git read-tree --reset -u HEAD ' test_expect_success 'safecrlf: autocrlf=input, all CRLF' ' @@ -225,29 +223,9 @@ test_expect_success '.gitattributes says two is binary' ' git config core.autocrlf true git read-tree --reset -u HEAD - if has_cr dir/two - then - echo Huh? - false - else - : happy - fi - - if has_cr one - then - : happy - else - echo Huh? - false - fi - - if has_cr three - then - echo Huh? - false - else - : happy - fi + test_must_fail has_cr dir/two + verbose has_cr one + test_must_fail has_cr three ' test_expect_success '.gitattributes says two is input' ' @@ -256,13 +234,7 @@ test_expect_success '.gitattributes says two is input' ' echo two crlf=input .gitattributes git read-tree --reset -u HEAD - if has_cr dir/two - then - echo Huh? - false - else - : happy - fi + test_must_fail has_cr dir/two ' test_expect_success '.gitattributes says two and three are text' ' -- 2.3.4.635.gd6ffcfe -- 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/8] perf-lib: fix ignored exit code inside loop
When copying the test repository, we try to detect whether
the copy succeeded. However, most of the heavy lifting is
done inside a for loop, where our break will lose the exit
code of the failing cp. We can take advantage of the fact
that we are in a subshell, and just exit 1 to break out
with a code.
Signed-off-by: Jeff King p...@peff.net
---
t/perf/perf-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index a8c9574..5cf74ed 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -91,7 +91,7 @@ test_perf_create_repo_from () {
*/objects|*/hooks|*/config)
;;
*)
- cp -R $stuff . || break
+ cp -R $stuff . || exit 1
;;
esac
done
--
2.3.4.635.gd6ffcfe
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/GSoC] Proposal: Make git-pull and git-am builtins
Hi all, I'm applying for git in the Google Summer of Code this year. For my project, I propose to rewrite git-pull.sh and git-am.sh into fast optimized C builtins. I've already hacked up a prototype of a builtin git-pull in [1], and it showed a promising 8x improvement in execution time on Windows. Below is the full text of the proposal as submitted to google-melange for your review and feedback. It is marked up in reStructuredText. The latest (and rendered) version can be found at [2]. Regards, Paul. [1] http://thread.gmane.org/gmane.comp.version-control.git/265628 [2] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 (Thanks Matthieu for suggesting to post this on the mailing list. Will reply to your comments in a separate email). --8-- Make `git-pull` and `git-am` builtins :Abstract: `git-pull` and `git-am` are frequently used git subcommands. However, they are porcelain commands and implemented as shell scripts, which has some limitations which can cause poor performance, especially in non-POSIX environments like Windows. I propose to rewrite these scripts into low level C code and make them builtins. This will increase git's portability, and may improve the efficiency and performance of these commands. .. section-numbering:: Limitations of shell scripts = `git-pull` is a commonly executed command to check for new changes in the upstream repository and, if there are, fetch and integrate them into the current branch. `git-am` is another commonly executed command for applying a series of patches from a mailbox to the current branch. They are both git porcelain commands -- with no access to git's low level internal API. Currently, they are implemented by the shell scripts ``git-pull.sh`` and ``git-am.sh`` respectively. These shell scripts require a fully-functioning POSIX shell and utilities. As a result, these commands are difficult to port to non-POSIX environments like Windows. Since porcelain commands do not have access to git's internal API, performing any git-related function, no matter how trivial, requires git to be spawned in a separate process. This limitation leads to these git commands being relatively inefficient, and can cause long run times on certain platforms that do not have copy-on-write ``fork()`` semantics. Spawning processes can be slow --- Shell scripting, by itself, is severely limited in what it can do. Performing most operations in shell scripts require external executables to be called. For example, ``git-pull.sh`` spawns the git executable not only to perform git operations like `git-fetch` and `git-merge`, but it also spawns the git executable for trivial tasks such as retrieving configuration values with `git-config` and even quoting of command-line arguments with ``git rev-parse --sq-quote``. As a result, these shell scripts usually end up spawning a lot of processes. Process spawning is usually implemented as a ``fork()`` followed by an ``exec()`` by shells. This can be slow on systems that do not support copy-on-write semantics for ``fork()``, and thus needs to duplicate the memory of the parent process for every ``fork()`` call -- an expensive process. Furthermore, starting up processes on Windows is generally expensive as it performs `several extra steps`_ such as such as using an inter-process call to notify the Windows Client/Server Runtime Subsystem(CSRSS) about the process creation and checking for App Compatibility requirements. .. _`several extra steps`: http://www.microsoft.com/mspress/books/sampchap/4354a.aspx The official Windows port of git, Git for Windows, uses MSYS2 [#]_ to emulate ``fork()``. Since Windows does not support forking semantics natively, MSYS2 can only emulate ``fork()`` `without copy-on-write semantics`_. Coupled with Windows heavy process creation, this causes huge slowdowns of git on Windows. .. _`without copy-on-write semantics`: https://www.cygwin.com/faq.html#faq.api.fork A no-updates `git-pull`, for example, takes an average of 5.1s [#]_, as compared to Linux which only takes an average of 0.08s. 5 seconds, while seemingly short, would seem like an eternity to a user who just wants to quickly fetch and merge changes from upstream. `git-am`'s implementation reads each patch from the mailbox in a while loop, spawning many processes for each patch. Considering the cost of spawning each process, as well as the fact that runtime grows linearly with the number of patches, git-am takes a long time to process a seemingly small number of patches on Windows as compared to Linux. A quick benchmarks shows that `git-am` takes 7m 20.39s to apply 100 patches on Windows, compared to Linux, which took only 0.08s. Commands which call `git-am` are also affected as well. ``git-rebase--am.sh``, which implements the
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
Am 24.03.2015 um 17:06 schrieb Michael Haggerty: Parsing numbers is not rocket science, but there are a lot of pitfalls, especially around overflow. It's even harder to write such code via macros and the result is less readable. This patch series is mostly about finding a reasonable API and whipping the callers into shape. That seems ambitious enough for me. I'd rather stick with boring wrappers for now and lean on strtol()/strtoul() to do the dirty work. It will be easy for a motivated person to change the implementation later. The OpenBSD developers invented strtonum for that. Are you aware of it? Would it fit? This discussion may be useful: http://www.tedunangst.com/flak/post/the-design-of-strtonum 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: [RFC/GSoC] Proposal: Make git-pull and git-am builtins
On Tue, Mar 24, 2015 at 6:19 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: A few minor details: on operating systems with poor file system performance (i.e. Windows) = that's not only windows, I also commonly use a slow filesystem on Linux, just because it's NFS. Mentionning other cases of poor filesystem performance would show that the benefit is not limited to windows users, and would give less of a taste of windows-bashing. Ah right, I didn't think of network file systems. Thanks for the suggestion. About the timeline: I'd avoid too much parallelism. Usually, it's best to try to send a first patch to the mailing list as soon as possible, hence focus on one point first (I'd do that with pull, since that's the one which is already started). Then, you can parallelize coding on git am and the discussion on the pull patches. Whatever you plan, review and polishing takes more than that ;-). The risk is to end up with an almost good but not good enough to be mergeable code. That said, your timeline does plan patches and review early, so I'm not too worried. Well, I was thinking that after the full rewrite (2nd stage, halfway through the project), any optimizations made to the code will be done iteratively (and in separate small patches) so as to keep the patch series in an always almost mergeable state. This will hopefully make it much easier and shorter to do any final polishing and review for merging. A general advice: if time allows, try to contribute to discussions and review other than your own patches. It's nice to feel integrated in the community and not the GSoC student working alone at home ;-). Yeah I apologize for not participating in the list so actively because writing the git-pull prototype and the proposal took a fair chunk of my time. Also, my expertise with the code base is not that great yet so it takes quite a bit more effort for me to contribute constructively, but I expect that will improve in the future. Now that the proposal is more or less complete I can spend more time on discussions. Thanks, Paul -- 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 ignore help
On Tue, Mar 24, 2015 at 5:39 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Mar 24, 2015 at 8:55 AM, Eric Sunshine sunsh...@sunshineco.com wrote: e.g. db, reports or scripts, we could keep going for a while. I think I attempted to do this in the past and failed (don't remember exactly why). Maybe I'll try again some time in future. I also was pretty sure that you had attempted this, but couldn't find a reference to it, so I didn't mention it in my response. However, with some more digging, I finally located it[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/259870 Thank you. I only looked at my repo and no branch name suggested it (if only there is google search for a git repository..). So I gave up because of performance reasons again but that was for enabling it unconditionaly. If we enable it via a config variable and the user is made aware of the performance implications, I guess it would be ok. So it's back in my back log. How much does a config variable actually help? In a sense, one could argue that this is already an opt-in feature since it requires crafting gitignore in a particular fashion. Existing projects which have (properly) functioning gitignore rules won't trigger this behavior. In many cases, Git already allows people to shoot themselves in the foot if they desire, thus, as long as the potential performance impact is properly documented, this could be considered another such instance. -- 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] t1501: fix test with split index
t1501-worktree.sh does not copy the shared index in the relative $GIT_WORK_TREE and git subprocesses test, which makes the test fail when GIT_TEST_SPLIT_INDEX is set. Copy the shared index as well in order to fix this. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- This applies on top of nd/multiple-work-trees. Sorry for not catching it earlier, but I haven't tried to run the test-suite for the next branch then, where this appears. t/t1501-worktree.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index 4df7a2f..ce5c654 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -350,6 +350,7 @@ test_expect_success 'Multi-worktree setup' ' mkdir work mkdir -p repo.git/repos/foo cp repo.git/HEAD repo.git/index repo.git/repos/foo + ( cp repo.git/sharedindex.* repo.git/repos/foo 2/dev/null || : ) sane_unset GIT_DIR GIT_CONFIG GIT_WORK_TREE ' -- 2.1.0.264.g0463184.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 00/14] numparse module: systematically tighten up integer parsing
On 03/24/2015 04:58 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to enable it. What valid reason do you have to forbid it anywhere? Only because you do not accept it by default, you need to add to enable. I want to be able to plunge into this project without first auditing all call sites to see which features will turn out to be needed. So I'm erring on the side of flexibility. For now, I want to be able to prohibit '+' signs. Currently all of the flags cause additional features to be enabled. My guess was that most callers *won't* need most features, so it seemed easiest and most consistent to have all features be turned off by default and let the caller add the features that it wants to allow. Regarding specifically allowing/disallowing a leading '+': I saw a couple of callsites that explicitly check that the first character is a digit before calling strtol(). I assumed that is to disallow sign characters [1]. For example, diff.c: optarg() builtin/apply.c: parse_num() maybe date.c: match_multi_number() There are other callsites that call strtoul(), but store the result in a signed variable. Those would presumably not want to allow leading '-', but I'm not sure. I also imagine that there are places in protocols or file formats where signs should not be allowed (e.g., timestamps in commits?). Why is it a problem to allow git cmd --hexval=0x1234, even if git cmd --hexval=1234 would suffice? In some cases we would like to allow that flexibility; in some cases not. But the strtol()/strtoul() functions *always* allow it. The same issue. Whare are these some cases? I admit I'm not sure there are such places for hexadecimal numbers. I'm coming around to an alternate plan: Step 1: write a NUM_DEFAULT combination-of-options that gives the new functions behavior very like strtol()/strtoul() but without their insane features. Step 2: rewrite all callers to use that option (and usually endptr=NULL, meaning no trailing characters) unless it is manifestly clear that they are already trying to forbid some other features. This will already produce the largest benefit: avoiding overflows, missing error checking, etc. Steps 3 through ∞: as time allows, rewrite individual callsites to be stricter where appropriate. Hopefully steps 1 and 2 will not be too controversial. Michael [1] That assumption is based on a rather quick look over the code, because with well over 100 callsites, it is not practical to study each callsite carefully. -- Michael Haggerty mhag...@alum.mit.edu -- 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 0/3] protocol v2
Stefan Beller sbel...@google.com writes: So I started looking into extending the buffer size as another 'first step' towards the protocol version 2 again. But now I think the packed length limit of 64k is actually a good and useful thing to have and should be extended/fixed if and only if we run into serious trouble with too small packets later. I tend to agree. Too large a packet size would mean your latency would also suck, as pkt-line interface will not give you anything until you read the entire packet. The new protocol should be designed around a reasonably sized packets, using multiple packets to carry larger payload as necessary. -- 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] t1501: fix test with split index
Thomas Gummerer t.gumme...@gmail.com writes: t1501-worktree.sh does not copy the shared index in the relative $GIT_WORK_TREE and git subprocesses test, which makes the test fail when GIT_TEST_SPLIT_INDEX is set. Copy the shared index as well in order to fix this. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- This applies on top of nd/multiple-work-trees. Sorry for not catching it earlier, but I haven't tried to run the test-suite for the next branch then, where this appears. t/t1501-worktree.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index 4df7a2f..ce5c654 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -350,6 +350,7 @@ test_expect_success 'Multi-worktree setup' ' mkdir work mkdir -p repo.git/repos/foo cp repo.git/HEAD repo.git/index repo.git/repos/foo + ( cp repo.git/sharedindex.* repo.git/repos/foo 2/dev/null || : ) Is this a good place to use test-might-fail, e.g. test_might_fail cp repo.git/sharedindex.* repo.git/repos/foo or something? sane_unset GIT_DIR GIT_CONFIG GIT_WORK_TREE ' -- 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, RFC] checkout: Attempt to checkout submodules
On Mon, Mar 23, 2015 at 09:01:48PM +0100, Jens Lehmann wrote: Am 20.03.2015 um 01:13 schrieb Trevor Saunders: On Thu, Mar 19, 2015 at 02:15:19PM -0700, Junio C Hamano wrote: Trevor Saunders tbsau...@tbsaunde.org writes: I have a feeling that an optional feature that allows git submodule update to happen automatically from this codepath might be acceptable by the submodule folks, and they might even say it does not even have to be optional but should be enabled by default. ok, that seems fairly reasonable. I do kind of wonder though if it shouldn't be 'git submodule update --checkout' but that would get us kind of back to where we started. I guess since the default is checkout if you set the pref then you can be assumed to have some amount of idea what your doing. Me thinks it should be git checkout for those submodules that have their update setting set to 'checkout' (or not set at all). I'm not sure yet if it makes sense to attempt a rebase or merge here, but that can be added later if necessary. sgtm But I do not think it would fly well to unconditionally run checkout -f here. agreed Using -f here is ok when you extend the appropriate verify functions in unpack-trees.c to check that no modifications will be lost (unless the original checkout is used with -f). See the commit 76dbdd62 (submodule: teach unpack_trees() to update submodules) in my github repo at https://github.com/jlehmann/git-submod-enhancements for the basic concept (There is already a fixup! for that a bit further down the branch which handles submodule to file conversion, maybe one or two other changes will be needed when the test suite covers all relevant cases). ah, I see your already working a more complete solution to this sort of issue. I'll get out of your way then unless you want help. Trev -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/GSoC] Proposal: Make git-pull and git-am builtins
Paul Tan pyoka...@gmail.com writes: ..., I propose the following requirements for the rewritten code: 1. No spawning of external git processes. This is to support systems with high ``fork()`` or process creation overhead, and to reduce redundant IO by taking advantage of the internal object, index and configuration cache. I suspect this may probably be too strict in practice. True, we should never say run_command_capture() just to to read from git rev-parse---we should just call get_sha1() instead. But for a complex command whose execution itself far outweighs the cost of forking, I do not think it is fair to say your project failed if you chose to run_command() it. For example, it may be perfectly OK to invoke git merge via run_command(). 3. The resulting builtin should not have wildly different behavior or bugs compared to the shell script. This on the other hand is way too loose. The original and the port must behave identically, unless the difference is fixing bugs in the original. Potential difficulties === Rewriting code may introduce bugs ... Yes, but that is a reasonable risk you need to manage to gain the benefit from this project. Of course, the downside of following this too strictly is that if there were any logical bugs in the original code, or if the original code is unclear, the rewritten code would inherit these problems too. I'd repeat my comment on the 3. above. Identifying and fixing bugs is great, but otherwise don't worry about this too much. Being bug-to-bug compatible with the original is way better than introducing new bugs of an unknown nature. Rewritten code may become harder to understand ... And also it may become harder to modify. That is the largest problem with any rewrite, and we should spend the most effort to avoid it. A new bugs introduced we can later fix as long as the result is understandable and maintainable. For the purpose of reducing git's dependencies, the rewritten C code should not depend on other libraries or executables other than what is already available to git builtins. Perhaps misphrased; see below. We can see that the C version requires much more lines compared to the shell pipeline,... That is something you would solve by introducing reusable code in run_command API, isn't it? That is how various rewrites in the past did, and this project should do so too. You should aim to do this project by not just using what is already available, but adding what you discover is a useful reusable pattern into a set of new functions in the already available API set. -- 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] l10n: de.po: fix messages with abbreviated hashs
The three dots in messages where the hash is abbreviated were misinterpreted and are fixed with this commit. Noticed-by: Junio C Hamano gits...@pobox.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/po/de.po b/po/de.po index 7b30f62..f818350 100644 --- a/po/de.po +++ b/po/de.po @@ -1271,12 +1271,12 @@ msgstr Kann keine Commit-Beschreibung für %s bekommen #: sequencer.c:611 #, c-format msgid could not revert %s... %s -msgstr Konnte \revert\ nicht auf %s ausführen... %s +msgstr Konnte \revert\ nicht auf %s...(%s) ausführen #: sequencer.c:612 #, c-format msgid could not apply %s... %s -msgstr Konnte %s nicht anwenden... %s +msgstr Konnte %s...(%s) nicht anwenden #: sequencer.c:648 msgid empty commit set passed -- 2.3.3.434.g642b19b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] l10n: de.po: add space before ellipsis
From: Phillip Sz phillip.sze...@gmail.com Signed-off-by: Phillip Sz phillip.sze...@gmail.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/po/de.po b/po/de.po index 11fbd0f..7b30f62 100644 --- a/po/de.po +++ b/po/de.po @@ -616,7 +616,7 @@ msgstr #: help.c:373 #, c-format msgid in %0.1f seconds automatically... -msgstr Automatische Ausführung in %0.1f Sekunden... +msgstr Automatische Ausführung in %0.1f Sekunden ... #: help.c:380 #, c-format @@ -2436,7 +2436,7 @@ msgstr %s: Patch konnte nicht angewendet werden #: builtin/apply.c:3653 #, c-format msgid Checking patch %s... -msgstr Prüfe Patch %s... +msgstr Prüfe Patch %s ... #: builtin/apply.c:3746 builtin/checkout.c:231 builtin/reset.c:135 #, c-format @@ -4091,7 +4091,7 @@ msgstr Konnte zu klonenden Remote-Branch %s nicht finden. #: builtin/clone.c:561 #, c-format msgid Checking connectivity... -msgstr Prüfe Konnektivität... +msgstr Prüfe Konnektivität ... #: builtin/clone.c:564 msgid remote did not send all necessary objects @@ -4165,12 +4165,12 @@ msgstr Konnte Arbeitsverzeichnis '%s' nicht erstellen. #: builtin/clone.c:870 #, c-format msgid Cloning into bare repository '%s'...\n -msgstr Klone in Bare-Repository '%s'...\n +msgstr Klone in Bare-Repository '%s' ...\n #: builtin/clone.c:872 #, c-format msgid Cloning into '%s'...\n -msgstr Klone nach '%s'...\n +msgstr Klone nach '%s' ...\n #: builtin/clone.c:897 msgid --dissociate given, but there is no --reference @@ -4600,7 +4600,7 @@ msgstr #: builtin/commit.c:1199 msgid Clever... amending the last one with dirty index. msgstr -Klug... den letzten Commit mit einer geänderten Staging-Area nachbessern. +Klug ... den letzten Commit mit einer geänderten Staging-Area nachbessern. #: builtin/commit.c:1201 msgid Explicit paths specified without -i or -o; assuming --only paths... @@ -7335,7 +7335,7 @@ msgstr Aktualisiere %s..%s\n #: builtin/merge.c:1388 #, c-format msgid Trying really trivial in-index merge...\n -msgstr Probiere wirklich trivialen \in-index\-Merge...\n +msgstr Probiere wirklich trivialen \in-index\-Merge ...\n #: builtin/merge.c:1395 #, c-format @@ -7349,12 +7349,12 @@ msgstr Vorspulen nicht möglich, breche ab. #: builtin/merge.c:1450 builtin/merge.c:1529 #, c-format msgid Rewinding the tree to pristine...\n -msgstr Rücklauf des Verzeichnisses bis zum Ursprung...\n +msgstr Rücklauf des Verzeichnisses bis zum Ursprung ...\n #: builtin/merge.c:1454 #, c-format msgid Trying merge strategy %s...\n -msgstr Probiere Merge-Strategie %s...\n +msgstr Probiere Merge-Strategie %s ...\n #: builtin/merge.c:1520 #, c-format @@ -10450,7 +10450,7 @@ msgstr #: git-am.sh:166 msgid Falling back to patching base and 3-way merge... -msgstr Falle zurück zum Patchen der Basis und des 3-Wege-Merges... +msgstr Falle zurück zum Patchen der Basis und des 3-Wege-Merges ... #: git-am.sh:182 msgid Failed to merge in the changes. @@ -10943,7 +10943,7 @@ msgstr Änderungen von $mb zu $onto: msgid First, rewinding head to replay your work on top of it... msgstr Zunächst wird der Branch zurückgespult, um Ihre Änderungen\n -darauf neu anzuwenden... +darauf neu anzuwenden ... #: git-rebase.sh:620 #, sh-format -- 2.3.3.434.g642b19b -- 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] l10n: de.po: use bla … instead of bla...
Michael J Gruber g...@drmicha.warpmail.net wrote: Ralf Thielow venit, vidit, dixit 21.03.2015 22:21: Am 21. März 2015 um 13:52 schrieb Phillip Sz phillip.sze...@gmail.com: I think we should use it like this, as most open-source projects do. Also we should use a space before the three dots as per http://www.duden.de/sprachwissen/rechtschreibregeln/auslassungspunkte I don't think this rule of ellipsis applies here as the dots are meant to be a pattern to tell users that an argument can be passed multiple times. ... is used in (at least) 2 cases: * ellipsis * continuation arguments The patch changes both, but the Duden rule applies to the first case only (and is different from legacy rules, I think). Also, you might want to check the format of other commit messages and reword yours accordingly. Let's apply this instead. -- 8 -- From: Phillip Sz phillip.sze...@gmail.com Date: Sat, 21 Mar 2015 13:52:37 +0100 Subject: [PATCH] l10n: de.po: add space before ellipsis Signed-off-by: Phillip Sz phillip.sze...@gmail.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/po/de.po b/po/de.po index 11fbd0f..9fa3f4c 100644 --- a/po/de.po +++ b/po/de.po @@ -616,7 +616,7 @@ msgstr #: help.c:373 #, c-format msgid in %0.1f seconds automatically... -msgstr Automatische Ausführung in %0.1f Sekunden... +msgstr Automatische Ausführung in %0.1f Sekunden ... #: help.c:380 #, c-format @@ -1271,12 +1271,12 @@ msgstr Kann keine Commit-Beschreibung für %s bekommen #: sequencer.c:611 #, c-format msgid could not revert %s... %s -msgstr Konnte \revert\ nicht auf %s ausführen... %s +msgstr Konnte \revert\ nicht auf %s ausführen ... %s #: sequencer.c:612 #, c-format msgid could not apply %s... %s -msgstr Konnte %s nicht anwenden... %s +msgstr Konnte %s nicht anwenden ... %s #: sequencer.c:648 msgid empty commit set passed @@ -2436,7 +2436,7 @@ msgstr %s: Patch konnte nicht angewendet werden #: builtin/apply.c:3653 #, c-format msgid Checking patch %s... -msgstr Prüfe Patch %s... +msgstr Prüfe Patch %s ... #: builtin/apply.c:3746 builtin/checkout.c:231 builtin/reset.c:135 #, c-format @@ -4091,7 +4091,7 @@ msgstr Konnte zu klonenden Remote-Branch %s nicht finden. #: builtin/clone.c:561 #, c-format msgid Checking connectivity... -msgstr Prüfe Konnektivität... +msgstr Prüfe Konnektivität ... #: builtin/clone.c:564 msgid remote did not send all necessary objects @@ -4165,12 +4165,12 @@ msgstr Konnte Arbeitsverzeichnis '%s' nicht erstellen. #: builtin/clone.c:870 #, c-format msgid Cloning into bare repository '%s'...\n -msgstr Klone in Bare-Repository '%s'...\n +msgstr Klone in Bare-Repository '%s' ...\n #: builtin/clone.c:872 #, c-format msgid Cloning into '%s'...\n -msgstr Klone nach '%s'...\n +msgstr Klone nach '%s' ...\n #: builtin/clone.c:897 msgid --dissociate given, but there is no --reference @@ -4600,7 +4600,7 @@ msgstr #: builtin/commit.c:1199 msgid Clever... amending the last one with dirty index. msgstr -Klug... den letzten Commit mit einer geänderten Staging-Area nachbessern. +Klug ... den letzten Commit mit einer geänderten Staging-Area nachbessern. #: builtin/commit.c:1201 msgid Explicit paths specified without -i or -o; assuming --only paths... @@ -7335,7 +7335,7 @@ msgstr Aktualisiere %s..%s\n #: builtin/merge.c:1388 #, c-format msgid Trying really trivial in-index merge...\n -msgstr Probiere wirklich trivialen \in-index\-Merge...\n +msgstr Probiere wirklich trivialen \in-index\-Merge ...\n #: builtin/merge.c:1395 #, c-format @@ -7349,12 +7349,12 @@ msgstr Vorspulen nicht möglich, breche ab. #: builtin/merge.c:1450 builtin/merge.c:1529 #, c-format msgid Rewinding the tree to pristine...\n -msgstr Rücklauf des Verzeichnisses bis zum Ursprung...\n +msgstr Rücklauf des Verzeichnisses bis zum Ursprung ...\n #: builtin/merge.c:1454 #, c-format msgid Trying merge strategy %s...\n -msgstr Probiere Merge-Strategie %s...\n +msgstr Probiere Merge-Strategie %s ...\n #: builtin/merge.c:1520 #, c-format @@ -7682,7 +7682,7 @@ msgstr git notes copy [Optionen] von-Objekt nach-Objekt #: builtin/notes.c:51 msgid git notes copy --stdin [from-object to-object]... -msgstr git notes copy --stdin [von-Objekt nach-Objekt]... +msgstr git notes copy --stdin [von-Objekt nach-Objekt] ... #: builtin/notes.c:56 msgid git notes append [options] [object] @@ -8689,7 +8689,7 @@ msgstr git remote prune [Optionen] Name #: builtin/remote.c:64 msgid git remote update [options] [group | remote]... -msgstr git remote update [Optionen] [Gruppe | externesRepository]... +msgstr git remote update [Optionen] [Gruppe | externesRepository] ... #: builtin/remote.c:88 #, c-format @@ -9865,7 +9865,7 @@ msgstr fehlerhaftes Objekt bei '%s' #: builtin/tag.c:301 #, c-format msgid tag name too long: %.*s... -msgstr Tagname zu
Re: [PATCH 2/2] read-cache: fix reading of split index
On 03/24, Duy Nguyen wrote:
On Sat, Mar 21, 2015 at 4:43 AM, Thomas Gummerer t.gumme...@gmail.com wrote:
The split index extension uses ewah bitmaps to mark index entries as
deleted, instead of removing them from the index directly. This can
result in an on-disk index, in which entries of stage #0 and higher
stages appear, which are removed later when the index bases are merged.
15999d0 read_index_from(): catch out of order entries when reading an
index file introduces a check which checks if the entries are in order
after each index entry is read in do_read_index. This check may however
fail when a split index is read.
Fix this by moving checking the index after we know there is no split
index or after the split index bases are successfully merged instead.
Thank you for catching this. I was about to write would be nice to
point out what tests fail so the reviewer has easier time trying
themselves, but whoa.. quite a few of them!
May I suggest a slight modification. Even though stage info is messed
up before the index is merged, I think we should still check that both
front and base indexes have all the names sorted correctly (and even
stronger, the base index should never contain staged entries). If
Hmm I just tried adding another check for that, but the base index
does seem to include staged entries sometimes.
I've tried with this, but there are quite a few test failures. For
example in t3600-rm.sh test #52 fails, and test-dump-split-index shows
the submodule with stages 1, 2 and 3 in the index.
own 74cd8e14a8fcc5df52e5c47a3ba0c30b29e5075a
base 0ff6ae43b1caa039c2a6262f07678b88314a5b4f
16 6daff6d0fc4a9299deb0a51881e14cdbda16b88d 1 submod
16 ee8321115a919c0607236124af886df2c9f16e2f 2 submod
16 f3abce3ddcc2d68a8c113bd16767deeb376276f9 3 submod
replacements:
deletions: 3
diff --git a/read-cache.c b/read-cache.c
index 2ba67ce..b502290 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1528,6 +1528,7 @@ int do_read_index(struct index_state *istate, const char
*path, int must_exist,
struct cache_header *hdr;
void *mmap;
size_t mmap_size;
+ int fully_merged = 1;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
if (istate-initialized)
@@ -1580,6 +1581,10 @@ int do_read_index(struct index_state *istate, const char
*path, int must_exist,
ce = create_from_disk(disk_ce, consumed, previous_name);
set_index_entry(istate, i, ce);
+ if (ce_stage(ce)) {
+ fully_merged = 0;
+ }
+
if (i 0)
if (check_ce_order(istate-cache[i - 1], ce, 1) 0
multiple_stage_entries)
@@ -1610,6 +1615,8 @@ int do_read_index(struct index_state *istate, const char
*path, int must_exist,
src_offset += extsize;
}
munmap(mmap, mmap_size);
+ if (!fully_merged istate-split_index)
+ die(base index cannot contain staged entries);
return istate-cache_nr;
unmap:
sorting order is messed up, it could lead to other problems. So
instead of removing the test from do_read_index(), perhaps add a flag
in check_ce_order() to optionally detect the stage problem, but
print/do nothing, only set a flag so the caller know there may be a
problem. In the two new call sites you added, we still call the new
check_ce_order() again to make sure everything is in order. In the
call site where split index is not active, if the previous
check_ce_order() call from inside do_read_index() says everything is
ok, we could even skip the check.
--
Duy
--
Thomas Gummerer
--
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] l10n: de.po: use bla … instead of bla...
Ralf Thielow venit, vidit, dixit 21.03.2015 22:21: Am 21. März 2015 um 13:52 schrieb Phillip Sz phillip.sze...@gmail.com: I think we should use it like this, as most open-source projects do. Also we should use a space before the three dots as per http://www.duden.de/sprachwissen/rechtschreibregeln/auslassungspunkte I don't think this rule of ellipsis applies here as the dots are meant to be a pattern to tell users that an argument can be passed multiple times. ... is used in (at least) 2 cases: * ellipsis * continuation arguments The patch changes both, but the Duden rule applies to the first case only (and is different from legacy rules, I think). Also, you might want to check the format of other commit messages and reword yours accordingly. Michael -- 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 09/15] http: release the memory of a http pack request as well
On Sun, Mar 22, 2015 at 12:36 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
The cleanup function is used in 4 places now and it's always safe to
free up the memory as well.
Signed-off-by: Stefan Beller sbel...@google.com
---
http.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/http.c b/http.c
index 9c825af..4b179f6 100644
--- a/http.c
+++ b/http.c
@@ -1462,6 +1462,7 @@ void release_http_pack_request(struct
http_pack_request *preq)
}
preq-slot = NULL;
free(preq-url);
+ free(preq);
}
int finish_http_pack_request(struct http_pack_request *preq)
Freeing of preq in all the callers of this one looks sensible,
except for the one in finish_request() of http-push.c that pulls an
preq instance out of request-userData.
Can somebody help me follow the dataflow to convince me that this is
not leading to double-free in start_fetch_packed()?
I am not sure where you suspect the double free problem.
In start_fetch_packed we have a call to release_http_pack_request
2 times but just in an error-out-and-cleanup case, so either of one cases
is called.
In the latter place (http-push.c lines 335-347), we have code like
request-userData = preq;
if (!start_active_slot(preq-slot)) {
release_http_pack_request(preq);
repo-can_update_info_refs = 0;
release_request(request);
}
Do you mean that the release_http_pack_request and release_request may collide
as the `release_request(request);` has a pointer to preq via its userData field?
Well there is hope, as `release_request` only touches
free(request-url);
free(request);
and not the userData pointer.
I am a bit puzzled what you're trying to hint at.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
Junio C Hamano gits...@pobox.com writes: Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to enable it. What valid reason do you have to forbid it anywhere? Only because you do not accept it by default, you need to add to enable. Why is it a problem to allow git cmd --hexval=0x1234, even if git cmd --hexval=1234 would suffice? In some cases we would like to allow that flexibility; in some cases not. But the strtol()/strtoul() functions *always* allow it. The same issue. Whare are these some cases? And the same issue appears in the leading whitespace thing I did not mention in the earlier part of your message I responded to. I also notice you answered yourself that there may not be a valid reason to forbid end-user supplied 0x prefix to arguments we expect an integer for in your other message. In short, if it is not a clearly bogus input that indicates a typo or something (e.g. --size=48l? did the user meant 48, 48k, or 48m?), and if it is clear we can tell the user meant what the code would naturally interpret as (e.g. --hexval=0x1234), why forbid it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On 03/19/2015 07:22 AM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
* It allows leading whitespace.
This might be blessing in disguise. Our friends on MacOS may be
relying on that
git cmd --abbrev=$(wc -c foo)
to work as expected, even though their wc gives leading spaces,
for example.
Yuck. If you'd like, I can make sure to continue allowing leading
whitespace everywhere that it is allowed now. Alternatively, we could
make the parsing tighter and see if anybody squeals. What do you think?
* It allows arbitrary trailing characters.
Which is why we have introduced strtoul_ui() and such.
* It allows a leading sign character ('+' or '-'), even though the
result is unsigned.
I do not particularly see it a bad thing to accept --abbrev=+7.
Using unsigned type to accept a width and parsing --abbrev=-7 into
a large positive integer _is_ a problem, and we may want to fix it,
but I think that is still within the scope of the original better
strtol/strtoul, and I do not see it as a justification to introduce
a set of functions with completely unfamiliar name.
It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to
those call sites. Should I do so?
It would even be possible to allow --abbrev=-0, which is also a
non-negative integer. But it's more work and it seems pretty silly to me.
* If the string doesn't contain a number at all, it sets its endptr
argument to point at the start of the string but doesn't set errno.
Why is that a problem? A caller that cares is supposed to check
endptr and the string it gave, no? Now, if strtoul_ui() and friends
do not do so, I again think that is still within the scope of the
original better strtol/strtoul.
The text you are quoting was my argument for why we need wrappers around
strtol() and strtoul(), not for how the wrappers should be named.
The endptr convention for detecting errors is fine in theory, but in
practice a large majority of callers didn't check for errors correctly.
This is partly because the endptr convention is so awkward.
The existing strtoul_ui() and strtol_i() did check endptr correctly, but
there were only int-sized variants of the functions, and they didn't
give the caller the chance to capture endptr if the caller wanted to
allow trailing characters.
Why did I rename the wrapper functions?
* The old names, I think, emphasize that they take the long-sized
results of strtou?l() and convert them to integer size, which I think is
an implementation detail.
* The new functions will also have long versions. The obvious way to
generalize the existing function names for long variants (srtoul_ul()
and strtol_l()) seem kindof redundant.
* I wanted to change the signature and behavior of the functions, so
knowledge of the existing functions wouldn't be super helpful in
understanding the new ones.
* If the value is larger than fits in an unsigned long, it returns the
value clamped to the range 0..ULONG_MAX (setting errno to ERANGE).
Ditto.
* If the value is between -ULONG_MAX and 0, it returns the positive
integer with the same bit pattern, without setting errno(!) (I can
hardly think of a scenario where this would be useful.)
Ditto.
* For base=0 (autodetect base), it allows a base specifier prefix 0x
or 0 and parses the number accordingly. For base=16 it also allows
a 0x prefix.
Why is it a problem to allow git cmd --hexval=0x1234, even if git
cmd --hexval=1234 would suffice?
In some cases we would like to allow that flexibility; in some cases
not. But the strtol()/strtoul() functions *always* allow it.
When I looked around, I found scores of sites that call atoi(),
strtoul(), and strtol() carelessly. And it's understandable. Calling
any of these functions safely is so much work as to be completely
impractical in day-to-day code.
Yes, these burdens on the caller were exactly why strtoul_ui()
etc. wanted to reduce---and it will be a welcome change to do
necessary checks that are currently not done.
Please see the docstrings in numparse.h in the first commit for
detailed API docs. Briefly, there are eight new functions:
parse_{l,ul,i,ui}(const char *s, unsigned int flags,
T *result, char **endptr);
convert_{l,ul,i,ui}(const char *s, unsigned int flags, T *result);
The parse_*() functions are for parsing a number from the start of a
string; the convert_*() functions are for parsing a string that
consists of a single number.
I am not sure if I follow. Why not unify them into one 4-function
set, with optional endptr that could be NULL?
In the next version of this patch series I plan to include only four
functions, str_to_{i,ui,l,ul}(), named that way to make them more
reminiscent of the functions that they replace. They will take an entptr
argument, but if that argument is set to NULL, then the function will
error out if there are any trailing characters.
While we are on the topic of
Re: [PATCH 2/2] read-cache: fix reading of split index
Duy Nguyen pclo...@gmail.com writes: Thank you for catching this. I was about to write would be nice to point out what tests fail so the reviewer has easier time trying themselves, but whoa.. quite a few of them! May I suggest a slight modification. Even though stage info is messed up before the index is merged, I think we should still check that both front and base indexes have all the names sorted correctly (and even stronger, the base index should never contain staged entries). I smell a slight layering violation here, though. As far as the code to check the validity of the index is concerned, it is only about the in-core index other code uses at runtime, and how that in-core index is prepared, and most importantly, what is recorded in the istate before it gets ready to be used by other code, is not its concern. The state immediately after the base index is read but before it gets fixed up by the split-index code can have quirks specific to how split-index code does things and it is perfectly OK if it does not pass the check for the final shape. The above does not change if the current split-index code happens to promise certain properties in that intermediate state. It is fine if the split-index codepath wants to add its own validator to the intermediate state for added robustness, but the rules for the intermediate state and the rules for the final state can be different, and from the maintainability's point of view, it is better if we keep the validator for the final-shape oblivious to what split-index does. -- 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] t1501: fix test with split index
t1501-worktree.sh does not copy the shared index in the relative $GIT_WORK_TREE and git subprocesses test, which makes the test fail when GIT_TEST_SPLIT_INDEX is set. Copy the shared index as well in order to fix this. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- Is this a good place to use test-might-fail, e.g. test_might_fail cp repo.git/sharedindex.* repo.git/repos/foo or something? Yeah that makes sense, thanks. t/t1501-worktree.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index 4df7a2f..cc5b870 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -350,6 +350,7 @@ test_expect_success 'Multi-worktree setup' ' mkdir work mkdir -p repo.git/repos/foo cp repo.git/HEAD repo.git/index repo.git/repos/foo + test_might_fail cp repo.git/sharedindex.* repo.git/repos/foo sane_unset GIT_DIR GIT_CONFIG GIT_WORK_TREE ' -- 2.1.0.264.g0463184.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
macblame - al alterntive to 'git blame'
https://rubygems.org/gems/macblame/ check this out.. and you can also contribute to the developement at, https://github.com/praserocking/macblame-gem or https://github.com/praserocking/macblame .. hope this tool will be helpful to you all! Thanks, Prasanna -- 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 ignore help
Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Mar 24, 2015 at 5:39 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Mar 24, 2015 at 8:55 AM, Eric Sunshine sunsh...@sunshineco.com wrote: e.g. db, reports or scripts, we could keep going for a while. I think I attempted to do this in the past and failed (don't remember exactly why). Maybe I'll try again some time in future. I also was pretty sure that you had attempted this, but couldn't find a reference to it, so I didn't mention it in my response. However, with some more digging, I finally located it[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/259870 Thank you. I only looked at my repo and no branch name suggested it (if only there is google search for a git repository..). So I gave up because of performance reasons again but that was for enabling it unconditionaly. If we enable it via a config variable and the user is made aware of the performance implications, I guess it would be ok. So it's back in my back log. How much does a config variable actually help? In a sense, one could argue that this is already an opt-in feature since it requires crafting gitignore in a particular fashion. Existing projects which have (properly) functioning gitignore rules won't trigger this behavior. In many cases, Git already allows people to shoot themselves in the foot if they desire, thus, as long as the potential performance impact is properly documented, this could be considered another such instance. Yeah, as I re-read that old thread, I really do not think anything wrong with the reasoning expressed in the proposed log message. It is pointless to hunt for !do-not-exclude-me-please in D/.gitignore when D/ appears in .gitignore in the higher level, but if these two i.e. D/ !D/do-not-exclude-me-please appear together in .gitignore in the higher level, we can pay attention to that and pick up that single path. And doing so would be a lot more intuitive to the end user. My comment in the thread was only about the documentation being unclear and not about the feature, but somehow we failed to follow-up the topic to completion X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
On Tue, Mar 24, 2015 at 01:02:35PM -0700, Junio C Hamano wrote:
* jk/test-chain-lint (2015-03-22) 28 commits
[...]
What I queued here has fix to the issue J6t found in 15/25 squashed
in, and also has 26/25 and 27/25 follow-up fixes from Michael, plus
28/25 follow-up from Torsten. If everybody involved is happy with
it, we can just proceed with this copy, otherwise I'll let Peff
reroll. I am happy either way.
I'll merge this to 'next' soonish, unless I hear otherwise. I
double checked 15/25 (i.e. $feature{forks}{default} = [1];)
so I think we are in good shape.
Thanks, yeah, I think what you have queued is good.
-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: What's cooking in git.git (Mar 2015, #08; Mon, 23)
Junio C Hamano gits...@pobox.com writes: [Stalled] * mh/fdopen-with-retry (2015-03-06) 6 commits - buffer_fdinit(): use fdopen_with_retry() - update_info_file(): use fdopen_with_retry() - copy_to_log(): use fdopen_with_retry() - fdopen_lock_file(): use fdopen_with_retry() - SQUASH??? $gmane/264889 - xfdopen(): if first attempt fails, free memory and try again Various parts of the code where they call fdopen() can fail when they run out of memory; attempt to proceed by retrying the operation after freeing some resource. Waiting for further comments. Sorry, but I lost track. Is this one still viable, or have we decided that it is not worth doing it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)
Junio C Hamano gits...@pobox.com writes:
* jk/test-chain-lint (2015-03-22) 28 commits
- t6039: fix broken chain
- t9158, t9161: fix broken -chain in git-svn tests
- t9104: fix test for following larger parents
- t4104: drop hand-rolled error reporting
- t0005: fix broken -chains
- t7004: fix embedded single-quotes
- t0050: appease --chain-lint
- t9001: use test_when_finished
- t4117: use modern test_* helpers
- t6034: use modern test_* helpers
- t1301: use modern test_* helpers
- t0020: use modern test_* helpers
- t6030: use modern test_* helpers
- t9502: fix -chain breakage
- t7201: fix -chain breakage
- t3600: fix -chain breakage for setup commands
- t: avoid using : for comments
- t: wrap complicated expect_code users in a block
- t: use test_expect_code instead of hand-rolled comparison
- t: use test_might_fail for diff and grep
- t: fix -chaining issues around setup which might fail
- t: use test_must_fail instead of hand-rolled blocks
- t: use verbose instead of hand-rolled errors
- t: assume test_cmp produces verbose output
- t: fix trivial -chain breakage
- t: fix moderate -chain breakage
- t: fix severe -chain breakage
- t/test-lib: introduce --chain-lint option
People often forget to chain the commands in their test together
with , leaving a failure from an earlier command in the test go
unnoticed. The new GIT_TEST_CHAIN_LINT mechanism allows you to
catch such a mistake more easily.
What I queued here has fix to the issue J6t found in 15/25 squashed
in, and also has 26/25 and 27/25 follow-up fixes from Michael, plus
28/25 follow-up from Torsten. If everybody involved is happy with
it, we can just proceed with this copy, otherwise I'll let Peff
reroll. I am happy either way.
I'll merge this to 'next' soonish, unless I hear otherwise. I
double checked 15/25 (i.e. $feature{forks}{default} = [1];)
so I think we are in good shape.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] l10n: de.po: use bla … instead of bla...
Thanks a lot for fixing! Phillip Let's apply this instead. -- 8 -- From: Phillip Sz phillip.sze...@gmail.com Date: Sat, 21 Mar 2015 13:52:37 +0100 Subject: [PATCH] l10n: de.po: add space before ellipsis Signed-off-by: Phillip Sz phillip.sze...@gmail.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/po/de.po b/po/de.po index 11fbd0f..9fa3f4c 100644 --- a/po/de.po +++ b/po/de.po @@ -616,7 +616,7 @@ msgstr #: help.c:373 #, c-format msgid in %0.1f seconds automatically... -msgstr Automatische Ausführung in %0.1f Sekunden... +msgstr Automatische Ausführung in %0.1f Sekunden ... #: help.c:380 #, c-format @@ -1271,12 +1271,12 @@ msgstr Kann keine Commit-Beschreibung für %s bekommen #: sequencer.c:611 #, c-format msgid could not revert %s... %s -msgstr Konnte \revert\ nicht auf %s ausführen... %s +msgstr Konnte \revert\ nicht auf %s ausführen ... %s #: sequencer.c:612 #, c-format msgid could not apply %s... %s -msgstr Konnte %s nicht anwenden... %s +msgstr Konnte %s nicht anwenden ... %s #: sequencer.c:648 msgid empty commit set passed @@ -2436,7 +2436,7 @@ msgstr %s: Patch konnte nicht angewendet werden #: builtin/apply.c:3653 #, c-format msgid Checking patch %s... -msgstr Prüfe Patch %s... +msgstr Prüfe Patch %s ... #: builtin/apply.c:3746 builtin/checkout.c:231 builtin/reset.c:135 #, c-format @@ -4091,7 +4091,7 @@ msgstr Konnte zu klonenden Remote-Branch %s nicht finden. #: builtin/clone.c:561 #, c-format msgid Checking connectivity... -msgstr Prüfe Konnektivität... +msgstr Prüfe Konnektivität ... #: builtin/clone.c:564 msgid remote did not send all necessary objects @@ -4165,12 +4165,12 @@ msgstr Konnte Arbeitsverzeichnis '%s' nicht erstellen. #: builtin/clone.c:870 #, c-format msgid Cloning into bare repository '%s'...\n -msgstr Klone in Bare-Repository '%s'...\n +msgstr Klone in Bare-Repository '%s' ...\n #: builtin/clone.c:872 #, c-format msgid Cloning into '%s'...\n -msgstr Klone nach '%s'...\n +msgstr Klone nach '%s' ...\n #: builtin/clone.c:897 msgid --dissociate given, but there is no --reference @@ -4600,7 +4600,7 @@ msgstr #: builtin/commit.c:1199 msgid Clever... amending the last one with dirty index. msgstr -Klug... den letzten Commit mit einer geänderten Staging-Area nachbessern. +Klug ... den letzten Commit mit einer geänderten Staging-Area nachbessern. #: builtin/commit.c:1201 msgid Explicit paths specified without -i or -o; assuming --only paths... @@ -7335,7 +7335,7 @@ msgstr Aktualisiere %s..%s\n #: builtin/merge.c:1388 #, c-format msgid Trying really trivial in-index merge...\n -msgstr Probiere wirklich trivialen \in-index\-Merge...\n +msgstr Probiere wirklich trivialen \in-index\-Merge ...\n #: builtin/merge.c:1395 #, c-format @@ -7349,12 +7349,12 @@ msgstr Vorspulen nicht möglich, breche ab. #: builtin/merge.c:1450 builtin/merge.c:1529 #, c-format msgid Rewinding the tree to pristine...\n -msgstr Rücklauf des Verzeichnisses bis zum Ursprung...\n +msgstr Rücklauf des Verzeichnisses bis zum Ursprung ...\n #: builtin/merge.c:1454 #, c-format msgid Trying merge strategy %s...\n -msgstr Probiere Merge-Strategie %s...\n +msgstr Probiere Merge-Strategie %s ...\n #: builtin/merge.c:1520 #, c-format @@ -7682,7 +7682,7 @@ msgstr git notes copy [Optionen] von-Objekt nach-Objekt #: builtin/notes.c:51 msgid git notes copy --stdin [from-object to-object]... -msgstr git notes copy --stdin [von-Objekt nach-Objekt]... +msgstr git notes copy --stdin [von-Objekt nach-Objekt] ... #: builtin/notes.c:56 msgid git notes append [options] [object] @@ -8689,7 +8689,7 @@ msgstr git remote prune [Optionen] Name #: builtin/remote.c:64 msgid git remote update [options] [group | remote]... -msgstr git remote update [Optionen] [Gruppe | externesRepository]... +msgstr git remote update [Optionen] [Gruppe | externesRepository] ... #: builtin/remote.c:88 #, c-format @@ -9865,7 +9865,7 @@ msgstr fehlerhaftes Objekt bei '%s' #: builtin/tag.c:301 #, c-format msgid tag name too long: %.*s... -msgstr Tagname zu lang: %.*s... +msgstr Tagname zu lang: %.*s ... #: builtin/tag.c:306 #, c-format @@ -10450,7 +10450,7 @@ msgstr #: git-am.sh:166 msgid Falling back to patching base and 3-way merge... -msgstr Falle zurück zum Patchen der Basis und des 3-Wege-Merges... +msgstr Falle zurück zum Patchen der Basis und des 3-Wege-Merges ... #: git-am.sh:182 msgid Failed to merge in the changes. @@ -10943,7 +10943,7 @@ msgstr Änderungen von $mb zu $onto: msgid First, rewinding head to replay your work on top of it... msgstr Zunächst wird der Branch zurückgespult, um Ihre Änderungen\n -darauf neu anzuwenden... +darauf neu anzuwenden ... #: git-rebase.sh:620 #, sh-format -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo
Re: [RFC/GSoC] Proposal: Make git-pull and git-am builtins
Paul Tan pyoka...@gmail.com writes: On Tue, Mar 24, 2015 at 6:19 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: About the timeline: I'd avoid too much parallelism. Usually, it's best to try to send a first patch to the mailing list as soon as possible, hence focus on one point first (I'd do that with pull, since that's the one which is already started). Then, you can parallelize coding on git am and the discussion on the pull patches. Whatever you plan, review and polishing takes more than that ;-). The risk is to end up with an almost good but not good enough to be mergeable code. That said, your timeline does plan patches and review early, so I'm not too worried. Well, I was thinking that after the full rewrite (2nd stage, halfway through the project), any optimizations made to the code will be done iteratively (and in separate small patches) Yes, that's why I'm not too worried. But being able to say this part is done, it won't disturb me anymore ASAP is still good IMHO, even if this part is not so big. But again, I'm thinking out loudly, feel free to ignore. A general advice: if time allows, try to contribute to discussions and review other than your own patches. It's nice to feel integrated in the community and not the GSoC student working alone at home ;-). Yeah I apologize for not participating in the list so actively because writing the git-pull prototype and the proposal took a fair chunk of my time. Don't apologize, you're doing great. I'm only pointing out things that could be even better, but certainly not blaming you! -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/15] http: release the memory of a http pack request as well
Stefan Beller sbel...@google.com writes:
Well there is hope, as `release_request` only touches
free(request-url);
free(request);
and not the userData pointer.
OK.
I am a bit puzzled what you're trying to hint at.
The caller does this:
static void start_fetch_packed(struct transfer_request *request)
{
...
preq = new_http_pack_request(target, repo-url);
...
preq-slot-callback_func = process_response;
preq-slot-callback_data = request;
request-slot = preq-slot;
request-userData = preq;
/* Try to get the request started, abort the request on error */
request-state = RUN_FETCH_PACKED;
if (!start_active_slot(preq-slot)) {
fprintf(stderr, Unable to start GET request\n);
release_http_pack_request(preq);
repo-can_update_info_refs = 0;
release_request(request);
}
}
and start_active_slot() actually not just starts but calls
curl_multi_perform() to do things, like calling process_response(),
which has calls to release_{,http_pack_}request(). I didn't see
those releases and the releases we see in the above (i.e. when !start)
will not run at the same time (but I see it now ;-))
In short, not hinting at anything. I was genuinely having a hard time
following the codeflow.
--
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] l10n: de.po: use bla … instead of bla...
2015-03-24 18:32 GMT+01:00 Junio C Hamano gits...@pobox.com: Ralf Thielow ralf.thie...@gmail.com writes: diff --git a/po/de.po b/po/de.po index 11fbd0f..9fa3f4c 100644 --- a/po/de.po +++ b/po/de.po @@ -616,7 +616,7 @@ msgstr #: help.c:373 #, c-format msgid in %0.1f seconds automatically... -msgstr Automatische Ausführung in %0.1f Sekunden... +msgstr Automatische Ausführung in %0.1f Sekunden ... #: help.c:380 #, c-format @@ -1271,12 +1271,12 @@ msgstr Kann keine Commit-Beschreibung für %s bekommen #: sequencer.c:611 #, c-format msgid could not revert %s... %s -msgstr Konnte \revert\ nicht auf %s ausführen... %s +msgstr Konnte \revert\ nicht auf %s ausführen ... %s I do not read German, but aren't these two completely in different classes? The first one is not abbreviating any part of a word, but the second one's first ... is showing that %s is not giving the full word (it is fed the find-unique-abbrev result and adding ... to say it is not a full 40-hex). I do not read German, but I would not be surprised if the original were a mistranslation. Is it saying Could not revert THAT ONE ... THE SUBJECT OF THE COMMIT as if ... is some punctuation in the sentence (i.e. it could have been a ';' or ':' or '.' that ends the first part of the sentence, but ... is used to tell the reader to wait a bit before continuing with the rest of the sentence), not as part of THAT ONE? The ... in the original is a three-dot that means we say THAT ONE but it is not fully spelled out, there are more letters here. Please ignore the above if the convention in German is to have SP before three-dots for both cases. I do not read German. Still the placement of a word perform (ausführen) between %s and ... in the translation of the second one looks suspicious to me, though. @@ -9865,7 +9865,7 @@ msgstr fehlerhaftes Objekt bei '%s' #: builtin/tag.c:301 #, c-format msgid tag name too long: %.*s... -msgstr Tagname zu lang: %.*s... +msgstr Tagname zu lang: %.*s ... This is also We are not spelling it fully and there are actually some more letters in the original three-dots. You're right. I just misinterpreted the dots. It seems I'm chatting too much where ... means nothing but space. ;-) I'll fix it in this patch and will check other messages later. Thanks -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
Duy Nguyen pclo...@gmail.com writes: read-tree -m does not invoke diff, does it? If I went with my previous approach (modifying unpack-trees to ignore i-t-a entries) then this could be a problem, but because unpack-trees is untouched, merge operations should not be impacted by this patch. Theoretically yes, but not quite. I wouldn't be surprised if an enterprising soul saw an optimization opportunity in the read-tree -m A B codepath. When it finds that a tree in A and a valid cache-tree entry that corresponds to the tree matches, it could blow away all index entries covered by the cache-tree entry and replace them with B, either (1) unconditionally when -u is not given; or (2) as long as the working tree matches the index inside that directory when running with -u. And such an optimization used to be a valid thing to do in the old world; but (1) will break in the new world, if we drop that invalidation---the i-t-a entries will be discarded from the index. As i-t-a is not a norm but an abberration, I'd rather keep the pessimizing invalidation to keep the door open for such an optimization for a more common case, and there may be other cases in which our correctness around i-t-a depends on. -- 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] l10n: de.po: use bla … instead of bla...
2015-03-24 18:10 GMT+01:00 Ralf Thielow ralf.thie...@gmail.com: Let's apply this instead. -- 8 -- #: builtin/notes.c:51 msgid git notes copy --stdin [from-object to-object]... -msgstr git notes copy --stdin [von-Objekt nach-Objekt]... +msgstr git notes copy --stdin [von-Objekt nach-Objekt] ... #: builtin/remote.c:64 msgid git remote update [options] [group | remote]... -msgstr git remote update [Optionen] [Gruppe | externesRepository]... +msgstr git remote update [Optionen] [Gruppe | externesRepository] ... Oops. I'll remove the space in these two messages as the dots aren't ellipsis obviously. -- 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 0/3] protocol v2
On Tue, Mar 3, 2015 at 9:13 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: Junio pointed out in private that I didn't address the packet length limit (64k). I thought I could get away with a new capability (i.e. not worry about it now) but I finally admit that was a bad hack. So perhaps this on top. No, I didn't ;-) but I tend to agree that perhaps 4GB huge packet? is a bad idea. The problem I had with the version in your write-up was that it still assumed that all capabilities must come on one packet-line. So I started looking into extending the buffer size as another 'first step' towards the protocol version 2 again. But now I think the packed length limit of 64k is actually a good and useful thing to have and should be extended/fixed if and only if we run into serious trouble with too small packets later. I mean we can add the possibility now by introducing these special length 0x or 0xFFFE to mean we'd want to extend it in the future. But when doing this we need to be extra careful with buffer allocation. As it is easy to produce a denial of service attack if the receiving side blindly trusts the length and allocates as much memory. So having a 64k limit actually helps preventing this attack a bit as it is a very small number. -- 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] read-cache: tighten checks for do_read_index
03f15a7 read-cache: fix reading of split index moved the checks for the
correct order of index entries out of do_read_index. This loosens the
checks more than necessary. Re-introduce the checks for the order, but
don't error out when we have multiple stage-0 entries in the index.
Return a flag for the caller instead, if we have multiple stage-0
entries and let the caller decide if we need to error out.
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
This is a patch on top of my previous patch, as that one has already
been merged to next.
cache.h | 2 +-
read-cache.c| 54 -
test-dump-split-index.c | 2 +-
3 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/cache.h b/cache.h
index e7b24a2..3eaa258 100644
--- a/cache.h
+++ b/cache.h
@@ -487,7 +487,7 @@ struct lock_file;
extern int read_index(struct index_state *);
extern int read_index_preload(struct index_state *, const struct pathspec
*pathspec);
extern int do_read_index(struct index_state *istate, const char *path,
-int must_exist); /* for testting only! */
+int must_exist, int *multiple_stage_entries); /* for
testting only! */
extern int read_index_from(struct index_state *, const char *path);
extern int is_index_unborn(struct index_state *);
extern int read_index_unmerged(struct index_state *);
diff --git a/read-cache.c b/read-cache.c
index 36ff89f..2ba67ce 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1488,30 +1488,39 @@ static struct cache_entry *create_from_disk(struct
ondisk_cache_entry *ondisk,
return ce;
}
-static void check_ce_order(struct index_state *istate)
+static int check_ce_order(struct cache_entry *ce, struct cache_entry *next_ce,
+ int gentle_multiple_stage)
{
- unsigned int i;
-
- for (i = 1; i istate-cache_nr; i++) {
- struct cache_entry *ce = istate-cache[i - 1];
- struct cache_entry *next_ce = istate-cache[i];
- int name_compare = strcmp(ce-name, next_ce-name);
+ int name_compare = strcmp(ce-name, next_ce-name);
- if (0 name_compare)
- die(unordered stage entries in index);
- if (!name_compare) {
- if (!ce_stage(ce))
+ if (0 name_compare)
+ die(unordered stage entries in index);
+ if (!name_compare) {
+ if (!ce_stage(ce)) {
+ if (gentle_multiple_stage)
+ return 1;
+ else
die(multiple stage entries for merged file
'%s',
ce-name);
- if (ce_stage(ce) ce_stage(next_ce))
- die(unordered stage entries for '%s',
- ce-name);
}
+ if (ce_stage(ce) ce_stage(next_ce))
+ die(unordered stage entries for '%s',
+ ce-name);
}
+ return 0;
+}
+
+static void check_istate_order(struct index_state *istate)
+{
+ unsigned int i;
+
+ for (i = 1; i istate-cache_nr; i++)
+ check_ce_order(istate-cache[i - 1], istate-cache[i], 0);
}
/* remember to discard_cache() before reading a different cache! */
-int do_read_index(struct index_state *istate, const char *path, int must_exist)
+int do_read_index(struct index_state *istate, const char *path, int must_exist,
+ int *multiple_stage_entries)
{
int fd, i;
struct stat st;
@@ -1571,6 +1580,11 @@ int do_read_index(struct index_state *istate, const char
*path, int must_exist)
ce = create_from_disk(disk_ce, consumed, previous_name);
set_index_entry(istate, i, ce);
+ if (i 0)
+ if (check_ce_order(istate-cache[i - 1], ce, 1) 0
+ multiple_stage_entries)
+ *multiple_stage_entries |= 1;
+
src_offset += consumed;
}
strbuf_release(previous_name_buf);
@@ -1607,15 +1621,17 @@ int read_index_from(struct index_state *istate, const
char *path)
{
struct split_index *split_index;
int ret;
+ int multiple_stage_entries = 0;
/* istate-initialized covers both .git/index and .git/sharedindex.xxx
*/
if (istate-initialized)
return istate-cache_nr;
- ret = do_read_index(istate, path, 0);
+ ret = do_read_index(istate, path, 0, multiple_stage_entries);
split_index = istate-split_index;
if (!split_index || is_null_sha1(split_index-base_sha1)) {
- check_ce_order(istate);
+ if (multiple_stage_entries)
+ check_istate_order(istate);
return ret;
}
@@ -1625,7 +1641,7 @@ int
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
Michael Haggerty mhag...@alum.mit.edu writes: Regarding specifically allowing/disallowing a leading '+': I saw a couple of callsites that explicitly check that the first character is a digit before calling strtol(). I assumed that is to disallow sign characters [1]. For example, diff.c: optarg() This one I know is from if not a digit we know it is not a number; it is not an attempt to say we must forbid numbers to be spelled with '+', but more about we do not need it and this is easier to code without a full fledged str-to-num helper API sloppiness. builtin/apply.c: parse_num() This parses @@ -l,k +m,n @@@ after stripping the punctuation around the numbers, so this is a valid reason why you would want an optional feature CANNOT_HAVE_SIGN in the str-to-num parser and use it. maybe date.c: match_multi_number() The approxidate callers parse random garbage input and attempt to make best sense out of it, so they tend to try limitting the damage caused by incorrect guesses by insisting we do not consider +0 to be zero and such. So this is a valid reason why you would want an optional feature CANNOT_HAVE_SIGN in the str-to-num parser and use it. I'm coming around to an alternate plan: Step 1: write a NUM_DEFAULT combination-of-options that gives the new functions behavior very like strtol()/strtoul() but without their insane features. Step 2: rewrite all callers to use that option (and usually endptr=NULL, meaning no trailing characters) unless it is manifestly clear that they are already trying to forbid some other features. This will already produce the largest benefit: avoiding overflows, missing error checking, etc. Steps 3 through ∞: as time allows, rewrite individual callsites to be stricter where appropriate. Hopefully steps 1 and 2 will not be too controversial. All sounds sensible to me. -- 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: macblame - al alterntive to 'git blame'
On Tue, Mar 24, 2015 at 2:07 PM, Shenbaga Prasanna S prasann...@freshdesk.com wrote: https://rubygems.org/gems/macblame/ check this out.. and you can also contribute to the developement at, https://github.com/praserocking/macblame-gem or https://github.com/praserocking/macblame .. hope this tool will be helpful to you all! It would help if you pasted a sample output to see what it looks like. Although macblame script says macblame shows stats about the files tracked by git. It uses the output of 'git blame' and summarize it in a cleaner and intuitive format so it's not really an alternative to git-blame (you can't blame anybody by line with this). This is more like git blame --stat (if that option existed) -- 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 10/15] commit.c: fix a memory leak
Duy Nguyen pclo...@gmail.com writes:
On Sat, Mar 21, 2015 at 10:59 AM, Junio C Hamano gits...@pobox.com wrote:
A further tangent (Duy Cc'ed for this point). We might want to
rethink the interface to ce_path_match() and report_path_error()
so that we do not have to do a separate allocation of has this
pathspec been used? array. This was a remnant from the olden days
back when pathspec were mere const char ** where we did not have
any room to add per-item bit---these days pathspec is repreasented
as an array of struct pathspec and we can afford to add a bit
to the structure---which will make this kind of leak much less
likely to happen.
I just want to say noted (and therefore in my backlog). But no
promise that it will happen any time soon. Low hanging fruit, perhaps
some people may be interested..
OK, the other one I just did so that we won't forget. Otherwise we
will leave too many loose ends untied.
-- 8 --
From: Junio C Hamano gits...@pobox.com
Date: Tue, 24 Mar 2015 14:12:10 -0700
Subject: [PATCH] report_path_error(): move to dir.c
The expected call sequence is for the caller to use match_pathspec()
repeatedly on a set of pathspecs, accumulating the hits in a
separate array, and then call this function to diagnose a pathspec
that never matched anything, as that can indicate a typo from the
command line, e.g. git commit Maekfile.
Many builtin commands use this function from builtin/ls-files.c,
which is not a very healthy arrangement. ls-files might have been
the first command to feel the need for such a helper, but the need
is shared by everybody who uses the match and then report pattern.
Move it to dir.c where match_pathspec() is defined.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/ls-files.c | 43 ---
cache.h| 1 -
dir.c | 43 +++
dir.h | 1 +
4 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 47c3880..47d70b2 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -354,49 +354,6 @@ void overlay_tree_on_cache(const char *tree_name, const
char *prefix)
}
}
-int report_path_error(const char *ps_matched,
- const struct pathspec *pathspec,
- const char *prefix)
-{
- /*
-* Make sure all pathspec matched; otherwise it is an error.
-*/
- struct strbuf sb = STRBUF_INIT;
- int num, errors = 0;
- for (num = 0; num pathspec-nr; num++) {
- int other, found_dup;
-
- if (ps_matched[num])
- continue;
- /*
-* The caller might have fed identical pathspec
-* twice. Do not barf on such a mistake.
-* FIXME: parse_pathspec should have eliminated
-* duplicate pathspec.
-*/
- for (found_dup = other = 0;
-!found_dup other pathspec-nr;
-other++) {
- if (other == num || !ps_matched[other])
- continue;
- if (!strcmp(pathspec-items[other].original,
- pathspec-items[num].original))
- /*
-* Ok, we have a match already.
-*/
- found_dup = 1;
- }
- if (found_dup)
- continue;
-
- error(pathspec '%s' did not match any file(s) known to git.,
- pathspec-items[num].original);
- errors++;
- }
- strbuf_release(sb);
- return errors;
-}
-
static const char * const ls_files_usage[] = {
N_(git ls-files [options] [file...]),
NULL
diff --git a/cache.h b/cache.h
index f23fdbe..8ec0b65 100644
--- a/cache.h
+++ b/cache.h
@@ -1411,7 +1411,6 @@ extern int ws_blank_line(const char *line, int len,
unsigned ws_rule);
#define ws_tab_width(rule) ((rule) WS_TAB_WIDTH_MASK)
/* ls-files */
-int report_path_error(const char *ps_matched, const struct pathspec *pathspec,
const char *prefix);
void overlay_tree_on_cache(const char *tree_name, const char *prefix);
char *alias_lookup(const char *alias);
diff --git a/dir.c b/dir.c
index 797805d..5d6e102 100644
--- a/dir.c
+++ b/dir.c
@@ -377,6 +377,49 @@ int match_pathspec(const struct pathspec *ps,
return negative ? 0 : positive;
}
+int report_path_error(const char *ps_matched,
+ const struct pathspec *pathspec,
+ const char *prefix)
+{
+ /*
+* Make sure all pathspec matched; otherwise it is an error.
+*/
+ struct strbuf sb = STRBUF_INIT;
+ int num, errors = 0;
+ for (num = 0; num pathspec-nr; num++) {
+ int other, found_dup;
+
Re: Re* [PATCH 10/15] commit.c: fix a memory leak
On Tue, Mar 24, 2015 at 2:17 PM, Junio C Hamano gits...@pobox.com wrote: Move it to dir.c where match_pathspec() is defined. Signed-off-by: Junio C Hamano gits...@pobox.com Reviewed-by: Stefan Beller sbel...@google.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
