Thanks for fixing the missing SANITY prerequisite Stefan.
As for the error handling logic in setup.c: is_nonbare_repository_dir
(was clean.c: is_git_repository) my reasoning is as follows:
READ_GITFILE_ERR_STAT_FAILED
READ_GITFILE_ERR_NOT_A_FILE:
When checking random paths for .git files these a
On Mon, Aug 31, 2015 at 6:58 PM, Junio C Hamano wrote:
>
> Many of the constructs we see here shows clearly that this is an
> ancient part of the codebase ;-), as we would be using the one
> parameter form of "git init" and more test_* helpers if we were
> writing this script in today's Git codeba
On Mon, Aug 31, 2015 at 8:54 PM, Jeff King wrote:
> On Sun, Aug 30, 2015 at 11:18:09AM +0200, Erik Elfström wrote:
>
> Unfortunately CHAIN_LINT cannot reach inside a nested subshell. I cannot
> think of a way to make it do so, and besides, that is also the way to
> overri
While we are here, remove some boilerplate by using test_commit.
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 27557d6..86ceb38 100755
--- a/t/t7300-clean.sh
+++ b
On Fri, Jun 26, 2015 at 11:03 AM, Jeff King wrote:
> I happened to be playing with clang's static analyzer today, and it
> noticed that there is a subtle use-after-free here.
Doh, sorry about that. Thanks for fixing my bug.
/Erik
--
To unsubscribe from this list: send the line "unsubscribe git"
1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 30 ++
t/t7300-clean.sh | 10 --
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 6dcb72e..df53def 100644
This is v8 of this series. v7 can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/271220
Changes in v8:
* change name and type of file size limit variable in read_git_file_gently
Erik Elfström (5):
setup: add gentle version of read_gitfile
setup: sanity check file
The tests are run in dry-run mode to avoid having to restore the test
directories for each timed iteration. Using dry-run is an acceptable
compromise since we are mostly interested in the initial computation
of what to clean and not so much in the cleaning it self.
Signed-off-by: Erik Elfström
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 142 +++
1 file changed, 142 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..fbfdf2d 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,148
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
cache.h | 11 -
setup.c | 84 ++---
2 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/cache.h b/cache.h
index 571c98f..25578cb 100644
--- a/cache.h
+++ b/cache.h
fficiently unreasonable size that should never be exceeded by a
genuine .git file.
Signed-off-by: Erik Elfström
---
cache.h | 1 +
setup.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/cache.h b/cache.h
index 25578cb..858d9b3 100644
--- a/cache.h
+++ b/cache.h
@@ -454,6 +454,7 @@ ex
On Sun, Jun 14, 2015 at 5:42 AM, Eric Sunshine wrote:
>
> This variable name doesn't convey much about its purpose, and
> introduces a bit of maintenance burden if the limit is some day
> changed. Perhaps "sane_size_limit" or something even more descriptive
> (and/or terse) would be better.
>
Wou
1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 30 ++
t/t7300-clean.sh | 10 --
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 6dcb72e..df53def 100644
.
Erik Elfström (5):
setup: add gentle version of read_gitfile
setup: sanity check file size in read_gitfile_gently
t7300: add tests to document behavior of clean and nested git
p7300: add performance tests for clean
clean: improve performance when removing lots of directories
builtin
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 142 +++
1 file changed, 142 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..fbfdf2d 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,148
fficiently unreasonable size that should never be exceeded by a
genuine .git file.
Signed-off-by: Erik Elfström
---
cache.h | 1 +
setup.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/cache.h b/cache.h
index 25578cb..858d9b3 100644
--- a/cache.h
+++ b/cache.h
@@ -454,6 +454,7 @@ ex
The tests are run in dry-run mode to avoid having to restore the test
directories for each timed iteration. Using dry-run is an acceptable
compromise since we are mostly interested in the initial computation
of what to clean and not so much in the cleaning it self.
Signed-off-by: Erik Elfström
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
cache.h | 11 -
setup.c | 84 ++---
2 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/cache.h b/cache.h
index 571c98f..25578cb 100644
--- a/cache.h
+++ b/cache.h
1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 31 +++
t/t7300-clean.sh | 10 --
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..d739dcf
On Tue, Apr 28, 2015 at 8:24 AM, Jeff King wrote:
>
> This iteration looks reasonable overall to me.
>
> Should this is_git_repository() helper be available to other files? I
> think there are other calls to resolve_gitlink_ref() that would want the
> same treatment (e.g., I think "git status" may
On Tue, Apr 28, 2015 at 8:17 AM, Jeff King wrote:
>
> There was a discussion not too long ago on strategies for returning
> errors, and one of the suggestions was to return an "error strbuf"
> rather than a code[1]. That's less flexible, as the caller can't react
> differently based on the type of
On Tue, Apr 28, 2015 at 8:33 AM, Jeff King wrote:
>
> Do we actually need a large repo here? The real cost is coming from the
> directories we create. We could actually start with a totally empty
> repository if we wanted (though I don't think the t/perf system handles
> that right now). But if th
On Tue, Apr 28, 2015 at 8:02 AM, Jeff King wrote:
>
> My understanding is that PATH_MAX is set absurdly low on Windows
> systems (and doesn't actually represent the real limit of a path!).
> Since the value is picked arbitrarily anyway, could use something more
> independent (like 100K or somethin
tracked directory containing 10 sub
directories went from 61s to 1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 26 ++
t/t7300-clean.sh | 8 +++-
2 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/buil
The tests are run in dry-run mode to avoid having to restore the test
directories for each timed iteration. Using dry-run is an acceptable
compromise since we are mostly interested in the initial computation
of what to clean and not so much in the cleaning it self.
Signed-off-by: Erik Elfström
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 128 +++
1 file changed, 128 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..11f3a6d 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,134
fficiently unreasonable size that should never be exceeded by a
genuine .git file.
Signed-off-by: Erik Elfström
---
cache.h | 1 +
setup.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/cache.h b/cache.h
index 868e4d3..c9f1f8e 100644
--- a/cache.h
+++ b/cache.h
@@ -439,6 +439,7 @@ ex
Changes in v5:
* Added defines for read_gitfile_gently error codes.
This was a silly mistake, sorry about that.
Erik Elfström (5):
setup: add gentle version of read_gitfile
setup: sanity check file size in read_gitfile_gently
t7300: add tests to document behavior of clean and nested git
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
cache.h | 11 -
setup.c | 82 +++--
2 files changed, 75 insertions(+), 18 deletions(-)
diff --git a/cache.h b/cache.h
index 3d3244b..868e4d3 100644
--- a/cache.h
+++ b/cache.h
On Sat, Apr 25, 2015 at 6:47 PM, Junio C Hamano wrote:
> I do not think it is wrong per-se, but the changes in this patch
> shows why hardcoded values assigned to error_code without #define is
> not a good idea, as these values are now exposed to the callers of
> the new function. After we gain a
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 128 +++
1 file changed, 128 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..11f3a6d 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,134
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
cache.h | 3 ++-
setup.c | 82 +++--
2 files changed, 67 insertions(+), 18 deletions(-)
diff --git a/cache.h b/cache.h
index 3d3244b..6e29068 100644
--- a/cache.h
+++ b/cache.h
tracked directory containing 10 sub
directories went from 61s to 1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 26 ++
t/t7300-clean.sh | 8 +++-
2 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/buil
The tests are run in dry-run mode to avoid having to restore the test
directories for each timed iteration. Using dry-run is an acceptable
compromise since we are mostly interested in the initial computation
of what to clean and not so much in the cleaning it self.
Signed-off-by: Erik Elfström
fficiently unreasonable size that should never be exceeded by a
genuine .git file.
Signed-off-by: Erik Elfström
---
I'm not sure about this one but it felt like the safe thing to do.
This patch can be dropped if it is not desired.
I considered testing it using
"mkdir foo &&am
message in [5/5] to more clearly motivate
remaining behavioral changes of git clean.
Thanks to Junio C Hamano and Jeff King for comments and help on v3.
Erik Elfström (5):
setup: add gentle version of read_gitfile
setup: sanity check file size in read_gitfile_gently
t7300: add tests to
On Wed, Apr 22, 2015 at 9:46 PM, Jeff King wrote:
> On Wed, Apr 22, 2015 at 09:30:20PM +0200, erik elfström wrote:
>>
>> Yes, that is the problem. A dry run will spot this particular performance
>> issue but maybe we lose some value as a general performance test if
>>
On Tue, Apr 21, 2015 at 11:24 PM, Jeff King wrote:
>
> If I understand correctly, the reason that you need per-run setup is
> that your "git clean" command actually cleans things, and you need to
> restore the original state for each time-trial. Can you instead use "git
> clean -n" to do a dry-run
tests, either directly in this series or as a follow
up if that is preferred.
/Erik
On Tue, Apr 21, 2015 at 12:14 AM, Thomas Gummerer wrote:
> On 04/18, Erik Elfström wrote:
>> * Still have issues in the performance tests, see comments
>> from Thomas Gummerer on v2
>
&g
On Sun, Apr 19, 2015 at 3:14 AM, Junio C Hamano wrote:
> Erik Elfström writes:
>
>> Known Problems:
>> * Unsure about the setup.c:read_gitfile refactor, feels a bit
>> messy?
>
> The interface indeed feels somewhat messy. I suspect that a better
00 sub
directories went from 61s to 1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 25 +
t/t7300-clean.sh | 8 +++-
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 127 +++
1 file changed, 127 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..4b9a72a 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,133
Signed-off-by: Erik Elfström
---
t/perf/p7300-clean.sh | 37 +
1 file changed, 37 insertions(+)
create mode 100755 t/perf/p7300-clean.sh
diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
new file mode 100755
index 000..af50d5d
--- /dev/null
we won't clean either. How serious is this? Is there
an easy fix (preferebly to clean all bare repositories)?
* Still have issues in the performance tests, see comments
from Thomas Gummerer on v2
Thanks to Junio C Hamano and Jeff King for spotting fundamental
problems in v2 and suggesti
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
If this is going to be used for speculative probing should there
be a sanity check before:
buf = xmalloc(st.st_size + 1);
len = read_in_full(fd, buf, st.st_size);
Something like:
if (st.st_size > PATH_MA
On Wed, Apr 15, 2015 at 7:56 PM, Junio C Hamano wrote:
> Erik Elfström writes:
>
>> Before this change, clean used resolve_gitlink_ref to check for the
>> presence of nested git repositories. This had the drawback of creating
>> a ref_cache entry for every directory th
On Sat, Apr 11, 2015 at 7:59 PM, Thomas Gummerer wrote:
> On 04/11, Erik Elfström wrote:
>> Signed-off-by: Erik Elfström
>> ---
>> t/perf/p7300-clean.sh | 37 +
>> 1 file changed, 37 insertions(+)
>> create mode 100755 t/per
1.7s after this change.
Helped-by: Jeff King
Signed-off-by: Erik Elfström
---
builtin/clean.c | 24
t/t7300-clean.sh | 4 ++--
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..b679913 100644
--- a/builtin/cl
Signed-off-by: Erik Elfström
---
t/perf/p7300-clean.sh | 37 +
1 file changed, 37 insertions(+)
create mode 100755 t/perf/p7300-clean.sh
diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
new file mode 100755
index 000..af50d5d
--- /dev/null
Signed-off-by: Erik Elfström
---
t/t7300-clean.sh | 72
1 file changed, 72 insertions(+)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..58e6b4a 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,78
ray index
* use size_t instead of int for strbuf->len
fixes held back for cleanup patches:
* fixed existing broken && chains
* added assert in existing code to guard gainst
negative array index
Thanks to Eric Sunshine and Torsten Bögershausen for the very helpful
review!
Erik Elfströ
ray index
* use size_t instead of int for strbuf->len
fixes held back for cleanup patches:
* fixed existing broken && chains
* added assert in existing code to guard against
negative array index
Thanks to Eric Sunshine and Torsten Bögershausen for the very helpful
review!
Erik Elf
On Tue, Apr 7, 2015 at 12:10 AM, Eric Sunshine wrote:
> On Mon, Apr 6, 2015 at 7:48 AM, Erik Elfström wrote:
>> Before this change, clean used resolve_gitlink_ref to check for the
>> presence of nested git repositories. This had the drawback of creating
>> a ref_cache entr
p" option to be called be before each
iteration.
On Tue, Apr 7, 2015 at 12:09 AM, Eric Sunshine wrote:
> On Mon, Apr 6, 2015 at 4:40 PM, Torsten Bögershausen wrote:
>> On 2015-04-06 13.48, Erik Elfström wrote:
>>> Signed-off-by: Erik Elfström
>>> ---
>&g
will fix!
On Tue, Apr 7, 2015 at 12:06 AM, Eric Sunshine wrote:
> On Mon, Apr 6, 2015 at 7:48 AM, Erik Elfström wrote:
>> Signed-off-by: Erik Elfström
>> ---
>> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
>> index 99be5d9..cfdf6d4 100755
>> --- a/t/t7300-c
Signed-off-by: Erik Elfström
---
These tests were added so that I could understand the corner case
behaviors of clean and nested repositories and document the changes in
behavior that this series will cause. The ones marked as expect
failure will be changed to expect success later in the series
Signed-off-by: Erik Elfström
---
t/perf/p7300-clean.sh | 37 +
1 file changed, 37 insertions(+)
create mode 100755 t/perf/p7300-clean.sh
diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
new file mode 100755
index 000..3f56fb2
--- /dev/null
1.7s after this change.
Signed-off-by: Erik Elfström
Helped-by: Jeff King
---
builtin/clean.c | 23 +++
t/t7300-clean.sh | 4 ++--
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..e951bd9 100644
--- a/builtin/cl
when
reviewing.
Jeff King also expressed concerns that we may have similar performance
issues in other commands and that it could be good to unify these "is
this a repo?"-checks. This series only attempts to solve the git-clean
case.
Erik Elfström (3):
t7300: add tests to document behavi
/Erik
On Sat, Apr 4, 2015 at 9:55 PM, Jeff King wrote:
> On Sat, Apr 04, 2015 at 08:32:45PM +0200, erik elfström wrote:
>
>> In my scenario get_ref_cache will be called 1+ times, each time
>> with a new path. The final few calls will need to search through and
>> compa
Hi,
I'm having a performance issue with "git clean -qxfd" (note, not using
"-ff").
The performance issue shows up when trying to clean untracked
directories that themselves contain many sub directories. The
performance is highly non linear with the number of sub
directories. Some test numbers:
D
61 matches
Mail list logo
