[PATCH 0/3] t0000 cleanups
When I want to debug a failing test, I often end up doing: cd t ./t4107-tab -v -i cd tratab The test names are long, so tab-completing on the trash directory is very helpful. Lately I've noticed that there are a bunch of crufty trash directories in my t/ directory, which makes my tab-completion more annoying. It turns out they're leftovers from t running, due to a bad interaction with some other fixes from last April. The first patch fixes that. The second patch is a follow-on cleanup enabled by the first. The third is an unrelated cleanup that I noticed when I ran t 47 times in a row. :) [1/3]: t: set TEST_OUTPUT_DIRECTORY for sub-tests [2/3]: t: simplify HARNESS_ACTIVE hack [3/3]: t: drop known breakage test t/t-basic.sh | 19 +++ t/test-lib.sh| 2 -- 2 files changed, 7 insertions(+), 14 deletions(-) -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 2/3] t0000: simplify HARNESS_ACTIVE hack
Commit 517cd55 set HARNESS_ACTIVE unconditionally in
sub-tests, because that value affects the output of
--verbose. t needs stable output from its sub-tests,
and we may or may not be running under a TAP harness.
That commit made the decision to always set the variable,
since it has another useful side effect, which is
suppressing writes to t/test-results by the sub-tests (which
would just pollute the real results).
Since the last commit, though, the sub-tests have their own
test-results directories, so this is no longer an issue. We
can now update a few comments that are no longer accurate
nor necessary.
We can also revisit the choice of HARNESS_ACTIVE. Since we
must choose one value for stability, it's probably saner to
have it off. This means that future patches could test
things like the test-results writing, or the --quiet
option, which is currently ignored when run under a harness.
Signed-off-by: Jeff King p...@peff.net
---
I do not have any plans to write such tests, and I'd be OK if we wanted
to stop this just at fixing up the comments. But it took me a while to
figure out what is going on, and I believe unsetting HARNESS_ACTIVE in
the sub-tests is the choice that is least likely to cause somebody in
the future to have to re-figure it out. :)
t/t-basic.sh | 14 +-
t/test-lib.sh| 2 --
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index bc4e3e2..e6c5b63 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -50,11 +50,11 @@ run_sub_test_lib_test () {
shift 2
mkdir $name
(
- # Pretend we're a test harness. This prevents
- # test-lib from writing the counts to a file that will
- # later be summarized, showing spurious failed tests
- HARNESS_ACTIVE=t
- export HARNESS_ACTIVE
+ # Pretend we're not running under a test harness, whether we
+ # are or not. The test-lib output depends on the setting of
+ # this variable, so we need a stable setting under which to run
+ # the sub-test.
+ sane_unset HARNESS_ACTIVE
cd $name
cat $name.sh -EOF
#!$SHELL_PATH
@@ -235,16 +235,13 @@ test_expect_success 'test --verbose' '
grep -v ^Initialized empty test-verbose/out+ test-verbose/out
check_sub_test_lib_test test-verbose -\EOF
expecting success: true
-Z
ok 1 - passing test
Z
expecting success: echo foo
foo
-Z
ok 2 - test with output
Z
expecting success: false
-Z
not ok 3 - failing test
# false
Z
@@ -267,7 +264,6 @@ test_expect_success 'test --verbose-only' '
Z
expecting success: echo foo
foo
-Z
ok 2 - test with output
Z
not ok 3 - failing test
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1cf78d5..1531c24 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -481,8 +481,6 @@ test_at_end_hook_ () {
test_done () {
GIT_EXIT_OK=t
- # Note: t relies on $HARNESS_ACTIVE disabling the .counts
- # output file
if test -z $HARNESS_ACTIVE
then
test_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
--
1.8.5.1.399.g900e7cd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] t0000: drop known breakage test
Having a simulated known breakage test means that the test
suite will always tell us there is a bug to be fixed, even
though it is only simulated.
The right way to test this is in a sub-test, that can also
check that we provide the correct exit status and output.
Fortunately, we already have such a test (added much later
by 5ebf89e).
We could arguably get rid of the simulated success test
immediately above, as well, as it is also redundant with the
tests added in 5ebf89e. However, it does not have the
annoying behavior of the known breakage test. It may also
be easier to debug if the test suite is truly broken, since
it is not a test-within-a-test, as the later tests are.
Signed-off-by: Jeff King p...@peff.net
---
I am not _that_ bothered by the known breakage, but AFAICT there is
zero benefit to keeping this redundant test. But maybe I am missing
something.
t/t-basic.sh | 3 ---
1 file changed, 3 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index e6c5b63..a2bb63c 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -41,9 +41,6 @@ test_expect_success '.git/objects should have 3
subdirectories' '
test_expect_success 'success is reported like this' '
:
'
-test_expect_failure 'pretend we have a known breakage' '
- false
-'
run_sub_test_lib_test () {
name=$1 descr=$2 # stdin is the body of the test code
--
1.8.5.1.399.g900e7cd
--
To unsubscribe from this list: send the line 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:// protocol over SSL/TLS
On Fri, Dec 27, 2013 at 08:47:54PM +0600, Sergey Sharybin wrote: The web server software has nothing to do with HTTP[S] used by Git being smart, I think, it just has to be set up properly. Misunderstood git doc then which says it has to be Apache, currently - other CGI servers don't work, last I checked. Do you happen to remember where you saw that claim? If the manual in git's Documentation/ directory says that, I'd like to fix it. I added sample lighttpd config to git help http-backend a while back. I tested it at the time, but I do not currently run a lighttpd git server at all. -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 02/12] Convert starts_with() to skip_prefix() for option parsing
On Thu, Dec 26, 2013 at 11:27:10AM -0800, Junio C Hamano wrote: I still need consensus on the name here guys, parse_prefix. remove_prefix or strip_prefix? If no other opinions i'll go with strip_prefix (Jeff's comment before parse_prefix() also uses strip) Yup, that comment is where I took strip from. When you name your thing as X, using too generic a word X, and then need to explain what X does using a bit more specific word Y, you are often better off naming it after Y. FWIW, the reason I shied away from strip is that I did not want to imply that the function mutates the string. But since nobody else seems concerned with that, I think strip is fine. -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 v2] git-svn: workaround for a bug in svn serf backend
2013/12/28 Junio C Hamano gits...@pobox.com: Eric Wong normalper...@yhbt.net writes: git://git.bogomips.org/git-svn.git master for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace: git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +) Roman Kagan (1): git-svn: workaround for a bug in svn serf backend perl/Git/SVN/Editor.pm | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Thanks. I almost missed this pull-request, though. Will pull. Thanks! I'd like to note that it's IMO worth including in the 'maint' branch as it's a crasher. Especially so since the real fix has been merged in the subversion upstream and nominated for 1.8 branch, so the workaround may soon lose its relevance. Roman. -- To unsubscribe from this list: send the line 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] merge-base: fix duplicates and not best ancestors in output
Hi there!
First of all: I'm new to mailing-lists, sorry if I'm doing it wrong.
I've found a bug in git merge-base, causing it to show not best common
ancestors and duplicates under some circumstances (example is given in
attached test case).
Problem cause is algorithm used in get_octopus_merge_bases(), it
iteratively concatenates merge bases, and don't care if there are
duplicates or decsendants/ancestors in result.
What I suggest as a solution is to simply reduce bases list after
get_octopus_merge_bases().
Here is the fix:
---
builtin/merge-base.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e88eb93..d6ad330 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -44,19 +44,19 @@ static struct commit *get_commit_reference(const char
*arg)
return r;
}
-static int handle_octopus(int count, const char **args, int reduce, int
show_all)
+static int handle_octopus(int count, const char **args, int reduce_only,
int show_all)
{
struct commit_list *revs = NULL;
struct commit_list *result;
int i;
- if (reduce)
+ if (reduce_only)
show_all = 1;
for (i = count - 1; i = 0; i--)
commit_list_insert(get_commit_reference(args[i]), revs);
- result = reduce ? reduce_heads(revs) : get_octopus_merge_bases(revs);
+ result = reduce_heads(reduce_only ? revs : get_octopus_merge_bases(revs));
if (!result)
return 1;
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats
From: Junio C Hamano gits...@pobox.com
Christian Couder christian.cou...@gmail.com writes:
Ok, so would you prefer the following:
- NAME_ONLY_REPLACE_FMT and --format=name_only instead of
SHORT_REPLACE_FMT and --format=short
- NAME_AND_VALUE_REPLACE_FMT and --format=name_and_value instead of
MEDIUM_REPLACE_FMT and --format=medium
- DEBUG_REPLACE_FMT and --format=debug instead of FULL _REPLACE_FMT
and --format=full
The end-user facing names are probably fine with short, medium,
full, as long as what they show are clearly explained in the
end-user documentation (patch 10/10 covers this).
Ok, I will try to improve on that.
I have a hunch that we may later regret full when somebody wants
to add even fuller information, though. It might be better spelled
long instead;
Ok, I will use long instead.
I'd rather see REPLACE_FMT_ as a prefix, not suffix. Do we use
common suffix for enum values elsewhere?
I don't see common suffix, but we have the following enums about
formats:
* in builtin/commit.c:
static enum status_format {
STATUS_FORMAT_NONE = 0,
STATUS_FORMAT_LONG,
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN,
STATUS_FORMAT_UNSPECIFIED
} status_format = STATUS_FORMAT_UNSPECIFIED;
* in builtin/help.c:
enum help_format {
HELP_FORMAT_NONE,
HELP_FORMAT_MAN,
HELP_FORMAT_INFO,
HELP_FORMAT_WEB
};
* in commit.h
enum cmit_fmt {
CMIT_FMT_RAW,
CMIT_FMT_MEDIUM,
CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM,
CMIT_FMT_SHORT,
CMIT_FMT_FULL,
CMIT_FMT_FULLER,
CMIT_FMT_ONELINE,
CMIT_FMT_EMAIL,
CMIT_FMT_USERFORMAT,
CMIT_FMT_UNSPECIFIED
};
To conform to the above and what you suggest, I will send a new series
using the following:
enum replace_format {
REPLACE_FORMAT_SHORT,
REPLACE_FORMAT_MEDIUM,
REPLACE_FORMAT_LONG
};
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 05/10] t6050: show that git cat-file --batch fails with replace objects
When --batch is passed to git cat-file, the sha1_object_info_extended() function is used to get information about the objects passed to git cat-file. Unfortunately sha1_object_info_extended() doesn't take care of object replacement properly, so it will often fail with a message like this: $ echo a3fb2e1845a1aaf129b7975048973414dc172173 | git cat-file --batch a3fb2e1845a1aaf129b7975048973414dc172173 commit 231 fatal: object a3fb2e1845a1aaf129b7975048973414dc172173 change size!? The goal of this patch is to show this breakage. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 7d47984..b90dbdc 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -276,6 +276,11 @@ test_expect_success '-f option bypasses the type check' ' git replace -f HEAD^ $BLOB ' +test_expect_failure 'git cat-file --batch works on replace objects' ' + git replace | grep $PARA3 + echo $PARA3 | git cat-file --batch +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.8.4.1.616.g07f5c81 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/10] Rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT
The READ_SHA1_FILE_REPLACE flag is more related to using the
lookup_replace_object() function rather than the
read_sha1_file() function.
We also need such a flag to be used with sha1_object_info()
instead of read_sha1_file().
The name LOOKUP_REPLACE_OBJECT is therefore better for this
flag.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
cache.h | 4 ++--
sha1_file.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index ce377e1..873a6b5 100644
--- a/cache.h
+++ b/cache.h
@@ -760,11 +760,11 @@ int daemon_avoid_alias(const char *path);
int offset_1st_component(const char *path);
/* object replacement */
-#define READ_SHA1_FILE_REPLACE 1
+#define LOOKUP_REPLACE_OBJECT 1
extern void *read_sha1_file_extended(const unsigned char *sha1, enum
object_type *type, unsigned long *size, unsigned flag);
static inline void *read_sha1_file(const unsigned char *sha1, enum object_type
*type, unsigned long *size)
{
- return read_sha1_file_extended(sha1, type, size,
READ_SHA1_FILE_REPLACE);
+ return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
}
extern const unsigned char *do_lookup_replace_object(const unsigned char
*sha1);
static inline const unsigned char *lookup_replace_object(const unsigned char
*sha1)
diff --git a/sha1_file.c b/sha1_file.c
index daacc0c..76e9f32 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2591,7 +2591,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
void *data;
char *path;
const struct packed_git *p;
- const unsigned char *repl = (flag READ_SHA1_FILE_REPLACE)
+ const unsigned char *repl = (flag LOOKUP_REPLACE_OBJECT)
? lookup_replace_object(sha1) : sha1;
errno = 0;
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/10] builtin/replace: teach listing using short, medium or long formats
By default when listing replace refs, only the sha1 of the
replaced objects are shown.
In many cases, it is much nicer to be able to list all the
sha1 of the replaced objects along with the sha1 of the
replacment objects.
And in other cases it might be interesting to also show the
types of the replaced and replacement objects.
This patch introduce a new --format=fmt option where
fmt can be any of the following:
'short': this is the same as when no --format
option is used, that is only the sha1 of
the replaced objects are shown
'medium': this also lists the sha1 of the
replacement objects
'long': this shows the sha1 and the type of both
the replaced and the replacement objects
Some documentation and some tests will follow.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 65 +--
1 file changed, 58 insertions(+), 7 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index b1bd3ef..b93d204 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -16,27 +16,69 @@
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
N_(git replace -d object...),
- N_(git replace -l [pattern]),
+ N_(git replace [--format=format] [-l [pattern]]),
NULL
};
+enum replace_format {
+ REPLACE_FORMAT_SHORT,
+ REPLACE_FORMAT_MEDIUM,
+ REPLACE_FORMAT_LONG
+};
+
+struct show_data {
+ const char *pattern;
+ enum replace_format format;
+};
+
static int show_reference(const char *refname, const unsigned char *sha1,
int flag, void *cb_data)
{
- const char *pattern = cb_data;
+ struct show_data *data = cb_data;
+
+ if (!fnmatch(data-pattern, refname, 0)) {
+ if (data-format == REPLACE_FORMAT_SHORT)
+ printf(%s\n, refname);
+ else if (data-format == REPLACE_FORMAT_MEDIUM)
+ printf(%s - %s\n, refname, sha1_to_hex(sha1));
+ else { /* data-format == REPLACE_FORMAT_LONG */
+ unsigned char object[20];
+ enum object_type obj_type, repl_type;
+
+ if (get_sha1(refname, object))
+ return error(Failed to resolve '%s' as a valid
ref., refname);
+
+ obj_type = sha1_object_info(object, NULL);
+ repl_type = sha1_object_info(sha1, NULL);
- if (!fnmatch(pattern, refname, 0))
- printf(%s\n, refname);
+ printf(%s (%s) - %s (%s)\n, refname,
typename(obj_type),
+ sha1_to_hex(sha1), typename(repl_type));
+ }
+ }
return 0;
}
-static int list_replace_refs(const char *pattern)
+static int list_replace_refs(const char *pattern, const char *format)
{
+ struct show_data data;
+
if (pattern == NULL)
pattern = *;
+ data.pattern = pattern;
+
+ if (format == NULL || *format == '\0' || !strcmp(format, short))
+ data.format = REPLACE_FORMAT_SHORT;
+ else if (!strcmp(format, medium))
+ data.format = REPLACE_FORMAT_MEDIUM;
+ else if (!strcmp(format, long))
+ data.format = REPLACE_FORMAT_LONG;
+ else
+ die(invalid replace format '%s'\n
+ valid formats are 'short', 'medium' and 'long'\n,
+ format);
- for_each_replace_ref(show_reference, (void *) pattern);
+ for_each_replace_ref(show_reference, (void *) data);
return 0;
}
@@ -127,10 +169,12 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int list = 0, delete = 0, force = 0;
+ const char *format = NULL;
struct option options[] = {
OPT_BOOL('l', list, list, N_(list replace refs)),
OPT_BOOL('d', delete, delete, N_(delete replace refs)),
OPT_BOOL('f', force, force, N_(replace the ref if it
exists)),
+ OPT_STRING(0, format, format, N_(format), N_(use this
format)),
OPT_END()
};
@@ -140,6 +184,10 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
usage_msg_opt(-l and -d cannot be used together,
git_replace_usage, options);
+ if (format delete)
+ usage_msg_opt(--format and -d cannot be used together,
+ git_replace_usage, options);
+
if (force (list || delete))
usage_msg_opt(-f cannot be used with -d or -l,
git_replace_usage, options);
@@ -157,6 +205,9 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
[PATCH v4 03/10] Introduce lookup_replace_object_extended() to pass flags
Currently, there is only one caller to lookup_replace_object()
that can benefit from passing it some flags, but we expect
that there could be more.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
cache.h | 6 ++
sha1_file.c | 3 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 873a6b5..563f85f 100644
--- a/cache.h
+++ b/cache.h
@@ -773,6 +773,12 @@ static inline const unsigned char
*lookup_replace_object(const unsigned char *sh
return sha1;
return do_lookup_replace_object(sha1);
}
+static inline const unsigned char *lookup_replace_object_extended(const
unsigned char *sha1, unsigned flag)
+{
+ if (! (flag LOOKUP_REPLACE_OBJECT))
+ return sha1;
+ return lookup_replace_object(sha1);
+}
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/sha1_file.c b/sha1_file.c
index 76e9f32..4fb2f17 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2591,8 +2591,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
void *data;
char *path;
const struct packed_git *p;
- const unsigned char *repl = (flag LOOKUP_REPLACE_OBJECT)
- ? lookup_replace_object(sha1) : sha1;
+ const unsigned char *repl = lookup_replace_object_extended(sha1, flag);
errno = 0;
data = read_object(repl, type, size);
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/10] replace_object: don't check read_replace_refs twice
Since ecef (inline lookup_replace_object() calls, May 15 2011) the read_replace_refs global variable is checked twice, once in lookup_replace_object() and once again in do_lookup_replace_object(). As do_lookup_replace_object() is called only from lookup_replace_object(), we can remove the check in do_lookup_replace_object(). Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- replace_object.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/replace_object.c b/replace_object.c index d0b1548..cdcaf8c 100644 --- a/replace_object.c +++ b/replace_object.c @@ -97,9 +97,6 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1) int pos, depth = MAXREPLACEDEPTH; const unsigned char *cur = sha1; - if (!read_replace_refs) - return sha1; - prepare_replace_object(); /* Try to recursively replace the object */ -- 1.8.4.1.616.g07f5c81 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/10] teach replace objects to sha1_object_info_extended()
Here is version 4 of a patch series to improve the way sha1_object_info_extended() behaves when it is passed a replaced object. The idea is to add a flags argument to it in the same way as what has been done to read_sha1_file(). This patch series was inspired by a sub thread in this discussion: http://thread.gmane.org/gmane.comp.version-control.git/238118 The only changes compared to version 3 are the following: - the name of the 'full' format is now 'long' - the names of the replace_format enum fields have been prepended with 'REPLACE_FORMAT_'. This avoids a compilation conflict on Windows where SHORT is predefined. Thanks to Karsten for reporting this problem. These changes only affect patches 7/10, 8/10, 9/10 and 10/10 that add a new --format option to list replace refs. Christian Couder (10): Rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT replace_object: don't check read_replace_refs twice Introduce lookup_replace_object_extended() to pass flags Add an unsigned flags parameter to sha1_object_info_extended() t6050: show that git cat-file --batch fails with replace objects sha1_file: perform object replacement in sha1_object_info_extended() builtin/replace: teach listing using short, medium or long formats t6050: add tests for listing with --format builtin/replace: unset read_replace_refs Documentation/git-replace: describe --format option Documentation/git-replace.txt | 19 +++- builtin/cat-file.c| 2 +- builtin/replace.c | 67 ++- cache.h | 12 ++-- replace_object.c | 3 -- sha1_file.c | 20 ++--- streaming.c | 2 +- t/t6050-replace.sh| 42 +++ 8 files changed, 141 insertions(+), 26 deletions(-) -- 1.8.4.1.616.g07f5c81 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/10] Documentation/git-replace: describe --format option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index f373ab4..0a02f70 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' -d object... -'git replace' -l [pattern] +'git replace' [--format=format] [-l [pattern]] DESCRIPTION --- @@ -70,6 +70,23 @@ OPTIONS Typing git replace without arguments, also lists all replace refs. +--format=format:: + When listing, use the specified format, which can be one of + 'short', 'medium' and 'long'. When omitted, the format + defaults to 'short'. + +FORMATS +--- + +The following format are available: + +* 'short': + replaced sha1 +* 'medium': + replaced sha1 - replacement sha1 +* 'long': + replaced sha1 (replaced type) - replacement sha1 (replacement type) + CREATING REPLACEMENT OBJECTS -- 1.8.4.1.616.g07f5c81 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/10] sha1_file: perform object replacement in sha1_object_info_extended()
sha1_object_info_extended() should perform object replacement
if it is needed.
The simplest way to do that is to make it call
lookup_replace_object_extended().
And now its unsigned flags parameter is used as it is passed
to lookup_replace_object_extended().
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
sha1_file.c| 13 +++--
t/t6050-replace.sh | 2 +-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 482037e..ee224e4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2448,8 +2448,9 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi,
struct cached_object *co;
struct pack_entry e;
int rtype;
+ const unsigned char *real = lookup_replace_object_extended(sha1, flags);
- co = find_cached_object(sha1);
+ co = find_cached_object(real);
if (co) {
if (oi-typep)
*(oi-typep) = co-type;
@@ -2461,23 +2462,23 @@ int sha1_object_info_extended(const unsigned char
*sha1, struct object_info *oi,
return 0;
}
- if (!find_pack_entry(sha1, e)) {
+ if (!find_pack_entry(real, e)) {
/* Most likely it's a loose object. */
- if (!sha1_loose_object_info(sha1, oi)) {
+ if (!sha1_loose_object_info(real, oi)) {
oi-whence = OI_LOOSE;
return 0;
}
/* Not a loose object; someone else may have just packed it. */
reprepare_packed_git();
- if (!find_pack_entry(sha1, e))
+ if (!find_pack_entry(real, e))
return -1;
}
rtype = packed_object_info(e.p, e.offset, oi);
if (rtype 0) {
- mark_bad_packed_object(e.p, sha1);
- return sha1_object_info_extended(sha1, oi, 0);
+ mark_bad_packed_object(e.p, real);
+ return sha1_object_info_extended(real, oi, 0);
} else if (in_delta_base_cache(e.p, e.offset)) {
oi-whence = OI_DBCACHED;
} else {
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index b90dbdc..bb785ec 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,7 +276,7 @@ test_expect_success '-f option bypasses the type check' '
git replace -f HEAD^ $BLOB
'
-test_expect_failure 'git cat-file --batch works on replace objects' '
+test_expect_success 'git cat-file --batch works on replace objects' '
git replace | grep $PARA3
echo $PARA3 | git cat-file --batch
'
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 04/10] Add an unsigned flags parameter to sha1_object_info_extended()
This parameter is not used yet, but it will be used to tell
sha1_object_info_extended() if it should perform object
replacement or not.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/cat-file.c | 2 +-
cache.h| 2 +-
sha1_file.c| 6 +++---
streaming.c| 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..b15c064 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -238,7 +238,7 @@ static int batch_one_object(const char *obj_name, struct
batch_options *opt,
return 0;
}
- if (sha1_object_info_extended(data-sha1, data-info) 0) {
+ if (sha1_object_info_extended(data-sha1, data-info,
LOOKUP_REPLACE_OBJECT) 0) {
printf(%s missing\n, obj_name);
fflush(stdout);
return 0;
diff --git a/cache.h b/cache.h
index 563f85f..701e42c 100644
--- a/cache.h
+++ b/cache.h
@@ -1104,7 +1104,7 @@ struct object_info {
} packed;
} u;
};
-extern int sha1_object_info_extended(const unsigned char *, struct object_info
*);
+extern int sha1_object_info_extended(const unsigned char *, struct object_info
*, unsigned flags);
/* Dumb servers support */
extern int update_server_info(int);
diff --git a/sha1_file.c b/sha1_file.c
index 4fb2f17..482037e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2443,7 +2443,7 @@ static int sha1_loose_object_info(const unsigned char
*sha1,
return 0;
}
-int sha1_object_info_extended(const unsigned char *sha1, struct object_info
*oi)
+int sha1_object_info_extended(const unsigned char *sha1, struct object_info
*oi, unsigned flags)
{
struct cached_object *co;
struct pack_entry e;
@@ -2477,7 +2477,7 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi)
rtype = packed_object_info(e.p, e.offset, oi);
if (rtype 0) {
mark_bad_packed_object(e.p, sha1);
- return sha1_object_info_extended(sha1, oi);
+ return sha1_object_info_extended(sha1, oi, 0);
} else if (in_delta_base_cache(e.p, e.offset)) {
oi-whence = OI_DBCACHED;
} else {
@@ -2499,7 +2499,7 @@ int sha1_object_info(const unsigned char *sha1, unsigned
long *sizep)
oi.typep = type;
oi.sizep = sizep;
- if (sha1_object_info_extended(sha1, oi) 0)
+ if (sha1_object_info_extended(sha1, oi, LOOKUP_REPLACE_OBJECT) 0)
return -1;
return type;
}
diff --git a/streaming.c b/streaming.c
index debe904..9659f18 100644
--- a/streaming.c
+++ b/streaming.c
@@ -113,7 +113,7 @@ static enum input_source istream_source(const unsigned char
*sha1,
oi-typep = type;
oi-sizep = size;
- status = sha1_object_info_extended(sha1, oi);
+ status = sha1_object_info_extended(sha1, oi, 0);
if (status 0)
return stream_error;
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/10] builtin/replace: unset read_replace_refs
When checking to see if some objects are of the same type
and when displaying the type of objects, git replace uses
the sha1_object_info() function.
Unfortunately this function by default respects replace
refs, so instead of the type of a replaced object, it
gives the type of the replacement object which might
be different.
To fix this bug, and because git replace should work at a
level before replacement takes place, let's unset the
read_replace_refs global variable at the beginning of
cmd_replace().
Suggested-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 2 ++
t/t6050-replace.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index b93d204..2336325 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -178,6 +178,8 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
OPT_END()
};
+ read_replace_refs = 0;
+
argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0);
if (list delete)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 9d05101..719a116 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -306,7 +306,7 @@ test_expect_success 'test --format medium' '
test_cmp expected actual
'
-test_expect_failure 'test --format long' '
+test_expect_success 'test --format long' '
{
echo $H1 (commit) - $BLOB (blob)
echo $BLOB (blob) - $REPLACED (blob)
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/10] t6050: add tests for listing with --format
This patch adds tests for git replace -l --format=fmt.
'short', 'medium' and 'long' are the only allowed values
for fmt.
'short' is the same as with no --format option.
Tests for 'medium' and 'long' are the most needed.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 37 +
1 file changed, 37 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index bb785ec..9d05101 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -281,6 +281,43 @@ test_expect_success 'git cat-file --batch works on replace
objects' '
echo $PARA3 | git cat-file --batch
'
+test_expect_success 'test --format bogus' '
+ test_must_fail git replace --format bogus /dev/null 21
+'
+
+test_expect_success 'test --format short' '
+ git replace --format=short actual
+ git replace expected
+ test_cmp expected actual
+'
+
+test_expect_success 'test --format medium' '
+ H1=$(git --no-replace-objects rev-parse HEAD~1)
+ HT=$(git --no-replace-objects rev-parse HEAD^{tree})
+ MYTAG=$(git --no-replace-objects rev-parse mytag)
+ {
+ echo $H1 - $BLOB
+ echo $BLOB - $REPLACED
+ echo $HT - $H1
+ echo $PARA3 - $S
+ echo $MYTAG - $HASH1
+ } | sort expected
+ git replace -l --format medium | sort actual
+ test_cmp expected actual
+'
+
+test_expect_failure 'test --format long' '
+ {
+ echo $H1 (commit) - $BLOB (blob)
+ echo $BLOB (blob) - $REPLACED (blob)
+ echo $HT (tree) - $H1 (commit)
+ echo $PARA3 (commit) - $S (commit)
+ echo $MYTAG (tag) - $HASH1 (commit)
+ } | sort expected
+ git replace --format=long | sort actual
+ test_cmp expected actual
+'
+
test_expect_success 'replace ref cleanup' '
test -n $(git replace)
git replace -d $(git replace)
--
1.8.4.1.616.g07f5c81
--
To unsubscribe from this list: send the line 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] t6010: add test to git merge-base --all --octopus
And here is the test: with git 1.8.5.2 this test don't pass because of git merge-base may return duplicates and incorrect set of bases (not best common ancestors) --- t/t6010-merge-base.sh | 39 +++ 1 file changed, 39 insertions(+) diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh index f80bba8..8652c12 100755 --- a/t/t6010-merge-base.sh +++ b/t/t6010-merge-base.sh @@ -230,4 +230,43 @@ test_expect_success 'criss-cross merge-base for octopus-step' ' test_cmp expected.sorted actual.sorted ' +test_expect_success 'merge-base --octopus --all for complex tree' ' + # Best common ancestor for JE, JAA and JDD is JC + # JE + #/ | + # / | + # / | + # JAA/| + # |\ / | + # | \ | JDD | + # | \ |/ | | + # | JC JD | + # || /| | + # ||/ | | + # JA| | | + # |\ /| | | + # | JB | | | + # \ \ | / / + #\__\|/___/ + #J + test_commit J + test_commit JB + git reset --hard J + test_commit JC + git reset --hard J + test_commit JTEMP1 + test_merge JA JB + test_merge JAA JC + git reset --hard J + test_commit JTEMP2 + test_merge JD JB + test_merge JDD JC + git reset --hard J + test_commit JTEMP3 + test_merge JE JC + git rev-parse JC expected + git merge-base --all --octopus JAA JDD JE actual + test_cmp expected actual +' + test_done -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: german translation bug
Jonathan Nieder jrnie...@gmail.com writes: Hi, Wolfgang Rohdewald wrote: Am Mittwoch, 25. Dezember 2013, 22:53:29 schrieb Wolfgang Rohdewald: I suppose I should open a KDE bug report? I meant a ubuntu bug report of course. Yes, please. Feel free to cc me if doing so. In generally, I'm a bit uncomfortable lately at how Ubuntu's translation system works for Git. They are trying to solve a real problem: old Ubuntu releases stick to old versions of git that do not have as complete translations as the latest version. But their solution to this problem does not seem to work well and creates a lot of confusion. Worse, it creates duplicated effort, as their custom translations don't seem to have been submitted upstream for review or inclusion. Even worse, the German one under discussion uses an entirely different vocabulary to describe git concepts: ubuntu (from OP): wr@s5:~/src/linux$ git status # Auf Zweig drm-intel-testing Nichts zum Einreichen, Arbeitsverzeichnis leer de.po current: msgid nothing to commit, working directory clean\n msgstr nichts zu committen, Arbeitsverzeichnis unverändert\n de.po before the big vocabulary change: msgid nothing to commit, working directory clean\n msgstr nichts einzutragen, Arbeitsverzeichnis sauber\n So their word for commit (v.) is einreichen, whereas we had committen and before that eintragen. As if there weren't enough confusion around German terminology yet. (FWIW I also think it's a terrible choice because it suggests a transaction between multiple people, which to me sounds like it should mean push.) -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: Runaway git remote if group definition contains a remote by the same name
FWIW, the issue is still present. -- Forwarded message -- From: Alex Riesen raa.l...@gmail.com Date: Wed, Nov 17, 2010 at 6:10 PM Subject: Runaway git remote if group definition contains a remote by the same name To: Git Mailing List git@vger.kernel.org Hi, it is also a way to create a fork bomb out of the innocent tool on platforms where pressing Ctrl-C does not terminate subprocesses of the foreground process (like, of course, Windows). To reproduce, run git -c remotes.origin='origin other' remote update origin I just cannot look at it right now, and have to resolve to only reporting the problem to warn people. Something seems to resolve the remotes group definition over and over again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 23/23] compat/mingw.h: Fix the MinGW and msvc builds
On 28/12/13 10:00, Jeff King wrote:
On Wed, Dec 25, 2013 at 11:08:57PM +0100, Erik Faye-Lund wrote:
On Sat, Dec 21, 2013 at 3:00 PM, Jeff King p...@peff.net wrote:
From: Ramsay Jones ram...@ramsay1.demon.co.uk
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Jeff King p...@peff.net
---
compat/mingw.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/compat/mingw.h b/compat/mingw.h
index 92cd728..8828ede 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -345,6 +345,7 @@ static inline char *mingw_find_last_dir_sep(const char
*path)
#define PATH_SEP ';'
#define PRIuMAX I64u
#define PRId64 I64d
+#define PRIx64 I64x
Please, move this before patch #8, and adjust the commit message.
Yeah, that makes sense. Though I think we can do one better and simply
remove the need for it entirely. The only use of PRIx64 is in a
debugging function that does not get called.
How about squashing the patch below into patch 8 (ewah: compressed
bitmap implementation):
diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
index f104b87..9ced2da 100644
--- a/ewah/ewah_bitmap.c
+++ b/ewah/ewah_bitmap.c
@@ -381,18 +381,6 @@ void ewah_iterator_init(struct ewah_iterator *it, struct
ewah_bitmap *parent)
read_new_rlw(it);
}
-void ewah_dump(struct ewah_bitmap *self)
-{
- size_t i;
- fprintf(stderr, %PRIuMAX bits | %PRIuMAX words | ,
- (uintmax_t)self-bit_size, (uintmax_t)self-buffer_size);
-
- for (i = 0; i self-buffer_size; ++i)
- fprintf(stderr, %016PRIx64 , (uint64_t)self-buffer[i]);
-
- fprintf(stderr, \n);
-}
-
void ewah_not(struct ewah_bitmap *self)
{
size_t pointer = 0;
diff --git a/ewah/ewok.h b/ewah/ewok.h
index 619afaa..43adeb5 100644
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -193,8 +193,6 @@ void ewah_and(
struct ewah_bitmap *ewah_j,
struct ewah_bitmap *out);
-void ewah_dump(struct ewah_bitmap *self);
-
/**
* Direct word access
*/
I'm always in favour of removing unused (or unwanted) code! :-D
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line 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:// protocol over SSL/TLS
On Fri, Dec 27, 2013 at 02:21:31PM -0800, Junio C Hamano wrote: Konstantin Khomoutov flatw...@users.sourceforge.net writes: The Git protocol does not implement it itself but you can channel it over a TLS tunnel (via stunnel for instance). Unfortunately, this means a specialized software and setup on both ends so if the question was about a general client using stock Git then the answer is no, it's impossible. Hmph, I somehow had an impression that you wouldn't need anything more complex than a simple helper that uses git-remote-ext on the client side. On the remote end, you'd need to have something that terminates the incoming SSL/TLS and plugs it to your git daemon. If you have some tool that can do cleartext I/O from stdin/stdout and establishes ciphertext connection itself, you can use it with git-remote-ext. It was written for cases exactly like that. To do git:// inside, use the %G pseudo-argument. -Ilari -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] t0000: drop known breakage test
Jeff King wrote: I am not _that_ bothered by the known breakage, but AFAICT there is zero benefit to keeping this redundant test. Devil's advocate: it ensures that anyone wrapping git's tests (like the old smoketest infrastructure experiment) is able to handle an expected failure. But in practice I don't mind the behavior before or after this patch. If the test harness is that broken, we'll know. And people writing code that wraps git's tests can write their own custom sanity-checks. -- To unsubscribe from this list: send the line 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:// protocol over SSL/TLS
Yeah, i understand this. We can not protect self from every single possible attack.. On Fri, Dec 27, 2013 at 10:26 PM, Bernhard R. Link brl+...@mail.brlink.eu wrote: * Sergey Sharybin sergey@gmail.com [131227 15:25]: Security in this case is about being sure everyone gets exactly the same repository as stored on the server, without any modifications to the sources cased by MITM. Note that ssl (and thus https) only helps here against a resource-less man-in-the-middle. Getting catch-all CA-signed certificates is said to no longer available to everyone as easily as it was some years ago, but unless you allow only one private CA (and even there clients often fail) you still should assume everyone resourceful enough to still be able to do MITM. Bernhard R. Link -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests
Hi, Jeff King wrote: Once upon a time, the test-lib library would create trash directories in the current working directory, unless we were explicitly told to put it elsewhere via --root. As a result, t created the sub-test trash directories inside its own trash directory. However, we noticed that this did not cover all cases, since we would need to respect $TEST_OUTPUT_DIRECTORY even if --root is not given (or is relative). Commit 38b074d fixed this to consistently use the full path. So the idea if I am reading correctly is Instead of relying on the implicit output directory chosen with chdir, which doesn't even work any more, set TEST_OUTPUT_DIRECTORY to decide where output for the sub-tests used by t's sanity checks for the test harness go. I'm not sure I completely understand the regression caused by 38b074d. Is the idea that before that commit, TEST_OUTPUT_DIRECTORY was only used for the test-results/ directory so the only harm done was some mixing of test results? What is the symptom this patch alleviates? As a result, t's sub-tests are now created in git's original test output directory rather than in our trash directory. This might be the source of my confusion. Is sub-tests an abbreviation for sub-test trash directories here? Furthermore, since some of the sub-tests simulate failures, the trash directories do not get cleaned up, and the cruft is left in the t/ directory. We could fix this by passing a new --root=$TRASH_DIRECTORY option to the sub-test. However, we do not want the sub-tests to write anything at all to git's directory (e.g., they should not be writing to t/test-results, either, although this is already handled by separate code). Ah, HARNESS_ACTIVE prevents output of test-results. Does the git test harness write something else to TEST_OUTPUT_DIRECTORY? Is the idea that using --root would be functionally equivalent but (1) more confusing and (2) less futureproof? So the best solution is to simply reset $TEST_OUTPUT_DIRECTORY entirely in the sub-test, which covers this case, as well as any future ones. So, to sum up: if I understand correctly - git used to only use TEST_OUTPUT_DIRECTORY to decide where test results go. You'd have to use --root to set a custom location for trash directories. - in that old setup, t leaves around extra trash directories with --root, since the sub-tests inherit the parent test's $root and put trash directories there. - after 38b074d, that old problem still exists and furthermore t leaves around extra trash directories even when --root is not in use, since the sub-tests inherit the value of TEST_OUTPUT_DIRECTORY from the parent test. - this patch fixes the TEST_OUTPUT_DIRECTORY problem (but not the $root problem) by setting TEST_OUTPUT_DIRECTORY explicitly Does that sound right? If so, should sub-tests unset $root, too? Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t0000: simplify HARNESS_ACTIVE hack
Jeff King wrote:
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -50,11 +50,11 @@ run_sub_test_lib_test () {
shift 2
mkdir $name
(
- # Pretend we're a test harness. This prevents
- # test-lib from writing the counts to a file that will
- # later be summarized, showing spurious failed tests
- HARNESS_ACTIVE=t
- export HARNESS_ACTIVE
+ # Pretend we're not running under a test harness, whether we
+ # are or not. The test-lib output depends on the setting of
+ # this variable, so we need a stable setting under which to run
+ # the sub-test.
+ sane_unset HARNESS_ACTIVE
Makes sense.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests
Jonathan Nieder wrote: - git used to only use TEST_OUTPUT_DIRECTORY to decide where test results go. You'd have to use --root to set a custom location for trash directories. - in that old setup, t leaves around extra trash directories with --root, since the sub-tests inherit the parent test's $root and put trash directories there. Nope, since sub-tests are run with fork + exec, which loses $root... - after 38b074d, that old problem still exists and furthermore t leaves around extra trash directories even when --root is not in use, since the sub-tests inherit the value of TEST_OUTPUT_DIRECTORY from the parent test. ... meaning the TEST_OUTPUT_DIRECTORY problem is the only problem - this patch fixes the TEST_OUTPUT_DIRECTORY problem (but not the $root problem) by setting TEST_OUTPUT_DIRECTORY explicitly Does that sound right? If so, should sub-tests unset $root, too? ... and there's no need to 'unset root'. So the patch itself looks right. I think describing the symptoms up front would probably be enough to make the commit message less confusing to read. Jonathan -- To unsubscribe from this list: send the line 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 0/3] t0000 cleanups
Jeff King wrote: When I want to debug a failing test, I often end up doing: cd t ./t4107-tab -v -i cd tratab The test names are long, so tab-completing on the trash directory is very helpful. Lately I've noticed that there are a bunch of crufty trash directories in my t/ directory, which makes my tab-completion more annoying. Ah, and if I'd read this then I wouldn't have had to be confused at all. Would it work to replace the commit message with something like this? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests
On Sat, Dec 28, 2013 at 02:13:13PM -0800, Jonathan Nieder wrote: So the idea if I am reading correctly is Instead of relying on the implicit output directory chosen with chdir, which doesn't even work any more, set TEST_OUTPUT_DIRECTORY to decide where output for the sub-tests used by t's sanity checks for the test harness go. Right. I'm not sure I completely understand the regression caused by 38b074d. Is the idea that before that commit, TEST_OUTPUT_DIRECTORY was only used for the test-results/ directory so the only harm done was some mixing of test results? $TEST_OUTPUT_DIRECTORY was actually used in $TRASH_DIRECTORY, but some code paths properly used $TRASH_DIRECTORY, and some used another variable that (sometimes) contained a relative form of $TRASH_DIRECTORY. The creation of the repo was one such code-path. So there were already potentially problems before 38b074d (any sub-test looking at its $TRASH_DIRECTORY variable would find the wrong path), but I do not know offhand if it could trigger any bugs. Post-38b074d, we consistently use $TRASH_DIRECTORY (and therefore respect $TEST_OUTPUT_DIRECTORY) everywhere. What is the symptom this patch alleviates? As a result, t's sub-tests are now created in git's original test output directory rather than in our trash directory. This might be the source of my confusion. Is sub-tests an abbreviation for sub-test trash directories here? Yes, I should have said sub-test trash directories. And I think that answers your what is the symptom question. We could fix this by passing a new --root=$TRASH_DIRECTORY option to the sub-test. However, we do not want the sub-tests to write anything at all to git's directory (e.g., they should not be writing to t/test-results, either, although this is already handled by separate code). Ah, HARNESS_ACTIVE prevents output of test-results. Yes. My original notion was Oh, and this fixes broken test-results, too!. But then I noticed that it is already handled in a different way. :) Does the git test harness write something else to TEST_OUTPUT_DIRECTORY? Is the idea that using --root would be functionally equivalent but (1) more confusing and (2) less futureproof? Exactly. I do not think TEST_OUTPUT_DIRECTORY is used for anything else, but if someone were to ever add a new use, the sub-tests would almost certainly want that use to affect only the t trash directory. So, to sum up: if I understand correctly You answered these yourself in your follow-up. :) So the patch itself looks right. I think describing the symptoms up front would probably be enough to make the commit message less confusing to read. Would adding the missing trash directories wording above be sufficient? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] t0000: drop known breakage test
On Sat, Dec 28, 2013 at 12:51:04PM -0800, Jonathan Nieder wrote: Jeff King wrote: I am not _that_ bothered by the known breakage, but AFAICT there is zero benefit to keeping this redundant test. Devil's advocate: it ensures that anyone wrapping git's tests (like the old smoketest infrastructure experiment) is able to handle an expected failure. Thanks. One of the things I love about open source is that as soon as I say I can't see how..., the answer is crowd-sourced for me. :) That being said, even if the test has a non-zero possible value... But in practice I don't mind the behavior before or after this patch. If the test harness is that broken, we'll know. And people writing code that wraps git's tests can write their own custom sanity-checks. ...I think for these reasons that the value is smaller than the disruption caused by the test, and the patch is a net win. -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: Fwd: Runaway git remote if group definition contains a remote by the same name
On Sat, Dec 28, 2013 at 03:56:55PM +0100, Alex Riesen wrote: it is also a way to create a fork bomb out of the innocent tool on platforms where pressing Ctrl-C does not terminate subprocesses of the foreground process (like, of course, Windows). To reproduce, run git -c remotes.origin='origin other' remote update origin Hmm. This is a pretty straightforward reference cycle. We expand origin to contain itself, so it recurses forever on expansion. As with most such problems, the cycle path may be longer than one: git -c remotes.foo='bar baz' -c remotes.bar='foo baz' fetch foo Detecting the cycle can be done by keeping track of which names we've seen, or just by putting in a depth limit that no sane setup would hit. In either case, it's complicated slightly by the fact that we pass the expanded list to a sub-process (which then recurses on the expansion). So we'd have to communicate the depth (or the list of seen remotes) via the command line or the environment. One alternative would be to have the parent git fetch recursively expand the list itself down to scalar entries, and then invoke sub-processes on the result (and tell them not to expand at all). That would also let us cull duplicates if a remote is found via multiple groups. Interestingly, the problem does not happen with this: git -c remotes.foo=foo fetch foo Fetch sees that foo expands only to a single item and says oh, that must not be a group. And then treats it like a regular remote, rather than recursing. So it's not clear to me whether groups are meant to be recursive or not. They are in some cases: # fetch remotes 1-4 git -c remotes.parent='child1 child2' \ -c remotes.child1='remote1 remote2' \ -c remotes.child2='remote3 remote4' \ fetch parent but not in others: # foo should be an alias for bar, but it's not git -c remotes.foo=bar \ -c remotes.bar='remote1 remote2' \ fetch foo If they are not allowed to recurse, the problem is much easier; the parent fetch simply tells all of the sub-invocations not to expand the arguments further. However, whether it was planned or not, it has been this way for a long time. I would not be surprised if somebody is relying on the recursion to help organize their groups. So I think the sanest thing is probably: 1. Teach fetch to expand recursively in a single process, and then tell sub-processes (via a new command-line option) not to expand any further. 2. Teach fetch to detect cycles (probably just by a simple depth counter). 3. Teach the group-reading code to detect groups more robustly, so that a single-item group like remotes.foo=bar correctly recurses to bar. 4. (Optional) Teach the expansion code from step 1 to cull duplicates, so that we do not try to fetch from the same remote twice (e.g., if it is mentioned as part of two groups, and both are specified on the command line). I do not plan to work on this myself in the immediate future, but perhaps it is an interesting low-hanging fruit for somebody else. -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
