[PATCH v7 4/4] t1006: add tests for git cat-file --literally
Signed-off-by: Karthik Nayak --- t/t1006-cat-file.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index ab36b1e..5b74044 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -47,6 +47,18 @@ $content" test_cmp expect actual ' +test_expect_success "Type of $type is correct using --literally" ' + echo $type >expect && + git cat-file -t --literally $sha1 >actual && + test_cmp expect actual +' + +test_expect_success "Size of $type is correct using --literally" ' + echo $size >expect && + git cat-file -s --literally $sha1 >actual && + test_cmp expect actual +' + test -z "$content" || test_expect_success "Content of $type is correct" ' maybe_remove_timestamp "$content" $no_ts >expect && @@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed delta bases' ' } ' +bogus_type="bogus" +bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin expect && + git cat-file -t --literally $bogus_sha1 >actual && + test_cmp expect actual +' + +test_expect_success "Size of broken object is correct" ' + echo "0" >expect && + git cat-file -s --literally $bogus_sha1 >actual && + test_cmp expect actual +' + test_done -- 2.4.0.rc1.249.g9f2ee54 -- 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 v7 2/4] cat-file: teach cat-file a '--literally' option
Currently 'git cat-file' throws an error while trying to
print the type or size of a broken/corrupt object which is
created using 'git hash-object --literally'. This is
because these objects are usually of unknown types.
Teach git cat-file a '--literally' option where it prints
the type or size of a broken/corrupt object without throwing
an error.
Modify '-t' and '-s' options to call sha1_object_info_extended()
directly to support the '--literally' option.
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
Signed-off-by: Karthik Nayak
---
builtin/cat-file.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df99df4..91ceae0 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -9,13 +9,20 @@
#include "userdiff.h"
#include "streaming.h"
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+ int literally)
{
unsigned char sha1[20];
enum object_type type;
char *buf;
unsigned long size;
struct object_context obj_context;
+ struct object_info oi = {NULL};
+ struct strbuf sb = STRBUF_INIT;
+ unsigned flags = LOOKUP_REPLACE_OBJECT;
+
+ if (literally)
+ flags |= LOOKUP_LITERALLY;
if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
die("Not a valid object name %s", obj_name);
@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type,
const char *obj_name)
buf = NULL;
switch (opt) {
case 't':
- type = sha1_object_info(sha1, NULL);
- if (type > 0) {
- printf("%s\n", typename(type));
+ oi.typep = &type;
+ oi.typename = &sb;
+ if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+ die("git cat-file: could not get object info");
+ if (type >= 0 && sb.len) {
+ printf("%s\n", sb.buf);
+ strbuf_release(&sb);
return 0;
}
break;
case 's':
- type = sha1_object_info(sha1, &size);
- if (type > 0) {
+ oi.typep = &type;
+ oi.typename = &sb;
+ oi.sizep = &size;
+ if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+ die("git cat-file: could not get object info");
+ if (type >= 0 && sb.len) {
printf("%lu\n", size);
return 0;
}
@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)
}
static const char * const cat_file_usage[] = {
- N_("git cat-file (-t | -s | -e | -p | | --textconv) "),
+ N_("git cat-file (-t [--literally]|-s
[--literally]|-e|-p||--textconv) "),
N_("git cat-file (--batch | --batch-check) < "),
NULL
};
@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
struct batch_options batch = {0};
+ int literally = 0;
const struct option options[] = {
OPT_GROUP(N_(" can be one of: blob, tree, commit, tag")),
@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's
content"), 'p'),
OPT_SET_INT(0, "textconv", &opt,
N_("for blob objects, run textconv on object's
content"), 'c'),
+ OPT_BOOL( 0, "literally", &literally,
+ N_("get information about corrupt objects for
debugging Git")),
{ OPTION_CALLBACK, 0, "batch", &batch, "format",
N_("show info and content of objects fed from the
standard input"),
PARSE_OPT_OPTARG, batch_option_callback },
@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
git_config(git_cat_file_config, NULL);
- if (argc != 3 && argc != 2)
+ if (argc < 2 || argc > 4)
usage_with_options(cat_file_usage, options);
argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
@@ -405,5 +423,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
if (batch.enabled)
return batch_objects(&batch);
- return cat_one_file(opt, exp_type, obj_name);
+ if (literally && opt != 't' && opt != 's')
+ die("git cat-file --literally: use with -s or -t");
+ return cat_one_file(opt, exp_type, obj_name, literally);
}
--
2.4.0.rc1.249.g9f2ee54
--
To unsubscribe from this list: send the line "unsubscribe git" in
the bod
[PATCH v7 3/4] cat-file: add documentation for '--literally' option.
Signed-off-by: Karthik Nayak --- Documentation/git-cat-file.txt | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index f6a16f4..8bac7bd 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec SYNOPSIS [verse] -'git cat-file' (-t | -s | -e | -p | | --textconv ) +'git cat-file' (-t [--literally]| -s [--literally]| -e | -p | | --textconv ) 'git cat-file' (--batch | --batch-check) < DESCRIPTION @@ -69,6 +69,10 @@ OPTIONS not be combined with any other options or arguments. See the section `BATCH OUTPUT` below for details. +--literally:: + Print information of broken/corrupt objects of unknown type without + throwing an error. To be used combined with '-s' or '-t' option. + OUTPUT -- If '-t' is specified, one of the . -- 2.4.0.rc1.249.g9f2ee54 -- 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 v7 1/4] sha1_file.c: support reading from a loose object of unknown type
Update sha1_loose_object_info() to optionally allow it to read
from a loose object file of unknown/bogus type; as the function
usually returns the type of the object it read in the form of enum
for known types, add an optional "typename" field to receive the
name of the type in textual form and a flag to indicate the reading
of a loose object file of unknown/bogus type.
Add parse_sha1_header_extended() which acts as a wrapper around
parse_sha1_header() allowing more information to be obtained.
Add unpack_sha1_header_to_strbuf() to unpack sha1 headers of
unknown/corrupt objects which have a unknown sha1 header size to
a strbuf structure. This was written by Junio C Hamano but tested
by me.
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
Signed-off-by: Karthik Nayak
---
cache.h | 2 ++
sha1_file.c | 111
2 files changed, 91 insertions(+), 22 deletions(-)
diff --git a/cache.h b/cache.h
index c47323e..ea6faf0 100644
--- a/cache.h
+++ b/cache.h
@@ -881,6 +881,7 @@ extern int is_ntfs_dotgit(const char *name);
/* object replacement */
#define LOOKUP_REPLACE_OBJECT 1
+#define LOOKUP_LITERALLY 2
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)
{
@@ -1349,6 +1350,7 @@ struct object_info {
unsigned long *sizep;
unsigned long *disk_sizep;
unsigned char *delta_base_sha1;
+ struct strbuf *typename;
/* Response */
enum {
diff --git a/sha1_file.c b/sha1_file.c
index 980ce6b..5a1b904 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1564,6 +1564,34 @@ int unpack_sha1_header(git_zstream *stream, unsigned
char *map, unsigned long ma
return git_inflate(stream, 0);
}
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char
*map,
+ unsigned long mapsize,
+ struct strbuf *header)
+{
+ unsigned char buffer[32], *cp;
+ unsigned long bufsiz = sizeof(buffer);
+ int status;
+
+ status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+ if (status) {
+ strbuf_add(header, buffer, stream->next_out - buffer);
+ return status;
+ }
+
+ do {
+ status = git_inflate(stream, 0);
+ strbuf_add(header, buffer, stream->next_out - buffer);
+ for (cp = buffer; cp < stream->next_out; cp++)
+ if (!*cp)
+ /* Found the NUL at the end of the header */
+ return 0;
+ stream->next_out = buffer;
+ stream->avail_out = bufsiz;
+ } while (status != Z_STREAM_END);
+ return -1;
+}
+
static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long
size, const unsigned char *sha1)
{
int bytes = strlen(buffer) + 1;
@@ -1614,27 +1642,24 @@ static void *unpack_sha1_rest(git_zstream *stream, void
*buffer, unsigned long s
* too permissive for what we want to check. So do an anal
* object header parse by hand.
*/
-int parse_sha1_header(const char *hdr, unsigned long *sizep)
+int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
+ unsigned int flags)
{
- char type[10];
- int i;
+ struct strbuf typename = STRBUF_INIT;
unsigned long size;
+ int type;
/*
* The type can be at most ten bytes (including the
* terminating '\0' that we add), and is followed by
* a space.
*/
- i = 0;
for (;;) {
char c = *hdr++;
if (c == ' ')
break;
- type[i++] = c;
- if (i >= sizeof(type))
- return -1;
+ strbuf_addch(&typename, c);
}
- type[i] = 0;
/*
* The length must follow immediately, and be in canonical
@@ -1652,12 +1677,39 @@ int parse_sha1_header(const char *hdr, unsigned long
*sizep)
size = size * 10 + c;
}
}
- *sizep = size;
+ type = type_from_string_gently(typename.buf, typename.len, 1);
+ if (oi->sizep)
+ *oi->sizep = size;
+ if (oi->typename)
+ strbuf_addbuf(oi->typename, &typename);
+ strbuf_release(&typename);
+
+ /*
+* Set type to 0 if its an unknown object and
+* we're obtaining the type using '--literally'
+* option.
+*/
+ if ((flags & LOOKUP_LITERALLY) && (type == -1))
+ type = 0;
+ else if (type == -1)
+ die("invalid object type");
+ if (oi->typep)
+ *oi->typep = type;
/*
* The length
[PATCH v7 0/4] cat-file: teach cat-file a '--literally' option
Changes made : * Fixed the problem with packed types, which failed a few tests. * Misspelled email. Thanks for the suggestions and comments on version 6. -- 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 v6 0/4] cat-file: add a '--literally' option
On 04/03/2015 02:05 AM, Junio C Hamano wrote: When merged to 'pu', this seems to break at least 5309 and 5300 tests (there might be others, but I didn't check). There was a problem with packed object types. I have it fixed, will send a re-roll. -- 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
[RFCv5 PATCH] daemon: add systemd support
systemd supports git-daemon's existing --inetd mode as well. Signed-off-by: Shawn Landden --- Documentation/git-daemon.txt | 41 +++- Makefile | 10 ++ daemon.c | 45 ++-- 3 files changed, 85 insertions(+), 11 deletions(-) Respond to review in http://article.gmane.org/gmane.comp.version-control.git/266650 I did not indent the example documents as that was for inetd, and that would break copy/paste. These are all documentation changes, no functional differences. (Well, the example gained StandardError=null to match --inetd) v5: do not change whitespace of Makefile diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt index a69b361..a273565 100644 --- a/Documentation/git-daemon.txt +++ b/Documentation/git-daemon.txt @@ -18,7 +18,7 @@ SYNOPSIS [--allow-override=] [--forbid-override=] [--access-hook=] [--[no-]informative-errors] [--inetd | - [--listen=] [--port=] + [--listen=] [--port=] [--systemd] [--user= [--group=]]] [...] @@ -81,8 +81,8 @@ OPTIONS --inetd:: Have the server run as an inetd service. Implies --syslog. - Incompatible with --detach, --port, --listen, --user and --group - options. + Incompatible with --systemd, --detach, --port, --listen, --user and + --group options. --listen=:: Listen on a specific IP address or hostname. IP addresses can @@ -146,8 +146,8 @@ OPTIONS the option are given to `getpwnam(3)` and `getgrnam(3)` and numeric IDs are not supported. + -Giving these options is an error when used with `--inetd`; use -the facility of inet daemon to achieve the same before spawning +Giving these options is an error when used with `--inetd` or `--systemd`; use +the facility of systemd or the inet daemon to achieve the same before spawning 'git daemon' if needed. + Like many programs that switch user id, the daemon does not reset @@ -180,6 +180,14 @@ Git configuration files in that directory are readable by ``. errors are not enabled, all errors report "access denied" to the client. The default is --no-informative-errors. +--systemd:: + For running git-daemon under systemd(1) which will pass + an open connection. This is similar to --inetd, except + that more than one address/port can be listened to at once + both through systemd and through --listen, and git-daemon doesn't get + invoked for every connection. For more details see systemd.socket(5). + Incompatible with --inetd, --detach, --user and --group options. + --access-hook=:: Every time a client connects, first run an external command specified by the with service name (e.g. "upload-pack"), @@ -304,7 +312,30 @@ selectively enable/disable services per repository:: uploadpack = false uploadarch = true ++ + +systemd configuration example:: ++ + +# /etc/systemd/system/git-daemon.socket +[Unit] +Description=Git Daemon socket +[Socket] +ListenStream=9418 + +[Install] +WantedBy=sockets.target + +# /etc/systemd/system/git-daemon.service +[Unit] +Description=Git Daemon + +[Service] +ExecStart=/usr/lib/git-core/git-daemon --systemd --reuseaddr --base-path=/var/lib /var/lib/git +User=git-daemon +StandardError=null + ENVIRONMENT --- diff --git a/Makefile b/Makefile index 5f3987f..83f5d8e 100644 --- a/Makefile +++ b/Makefile @@ -42,6 +42,9 @@ all:: # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). # +# Define NO_SYSTEMD to prevent systemd socket activation support from being +# built into git-daemon. +# # Define EXPATDIR=/foo/bar if your expat header and library files are in # /foo/bar/include and /foo/bar/lib directories. # @@ -995,6 +998,13 @@ ifeq ($(uname_S),Darwin) PTHREAD_LIBS = endif +ifndef NO_SYSTEMD + ifeq ($(shell echo "\#include " | $(CC) -E - -o /dev/null && echo y),y) + BASIC_CFLAGS += -DHAVE_SYSTEMD + EXTLIBS += -lsystemd + endif +endif + ifndef CC_LD_DYNPATH ifdef NO_R_TO_GCC_LINKER # Some gcc does not accept and pass -R to the linker to specify diff --git a/daemon.c b/daemon.c index 9ee2187..ad8a79a 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,7 @@ +#ifdef HAVE_SYSTEMD +# include +#endif + #include "cache.h" #include "pkt-line.h" #include "exec_cmd.h" @@ -29,6 +33,9 @@ static const char daemon_usage[] = " [--access-hook=]\n" " [--inetd | [--listen=] [--port=]\n" " [--detach] [--user= [--
[RFCv4 PATCH] daemon: add systemd support
systemd supports git-daemon's existing --inetd mode as well. Signed-off-by: Shawn Landden --- Documentation/git-daemon.txt | 41 +++- Makefile | 14 -- daemon.c | 45 ++-- 3 files changed, 87 insertions(+), 13 deletions(-) Respond to review in http://article.gmane.org/gmane.comp.version-control.git/266650 I did not indent the example documents as that was for inetd, and that would break copy/paste. These are all documentation changes, no functional differences. (Well, the example gained StandardError=null to match --inetd) diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt index a69b361..a273565 100644 --- a/Documentation/git-daemon.txt +++ b/Documentation/git-daemon.txt @@ -18,7 +18,7 @@ SYNOPSIS [--allow-override=] [--forbid-override=] [--access-hook=] [--[no-]informative-errors] [--inetd | - [--listen=] [--port=] + [--listen=] [--port=] [--systemd] [--user= [--group=]]] [...] @@ -81,8 +81,8 @@ OPTIONS --inetd:: Have the server run as an inetd service. Implies --syslog. - Incompatible with --detach, --port, --listen, --user and --group - options. + Incompatible with --systemd, --detach, --port, --listen, --user and + --group options. --listen=:: Listen on a specific IP address or hostname. IP addresses can @@ -146,8 +146,8 @@ OPTIONS the option are given to `getpwnam(3)` and `getgrnam(3)` and numeric IDs are not supported. + -Giving these options is an error when used with `--inetd`; use -the facility of inet daemon to achieve the same before spawning +Giving these options is an error when used with `--inetd` or `--systemd`; use +the facility of systemd or the inet daemon to achieve the same before spawning 'git daemon' if needed. + Like many programs that switch user id, the daemon does not reset @@ -180,6 +180,14 @@ Git configuration files in that directory are readable by ``. errors are not enabled, all errors report "access denied" to the client. The default is --no-informative-errors. +--systemd:: + For running git-daemon under systemd(1) which will pass + an open connection. This is similar to --inetd, except + that more than one address/port can be listened to at once + both through systemd and through --listen, and git-daemon doesn't get + invoked for every connection. For more details see systemd.socket(5). + Incompatible with --inetd, --detach, --user and --group options. + --access-hook=:: Every time a client connects, first run an external command specified by the with service name (e.g. "upload-pack"), @@ -304,7 +312,30 @@ selectively enable/disable services per repository:: uploadpack = false uploadarch = true ++ + +systemd configuration example:: ++ + +# /etc/systemd/system/git-daemon.socket +[Unit] +Description=Git Daemon socket +[Socket] +ListenStream=9418 + +[Install] +WantedBy=sockets.target + +# /etc/systemd/system/git-daemon.service +[Unit] +Description=Git Daemon + +[Service] +ExecStart=/usr/lib/git-core/git-daemon --systemd --reuseaddr --base-path=/var/lib /var/lib/git +User=git-daemon +StandardError=null + ENVIRONMENT --- diff --git a/Makefile b/Makefile index 5f3987f..644db71 100644 --- a/Makefile +++ b/Makefile @@ -42,6 +42,9 @@ all:: # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). # +# Define NO_SYSTEMD to prevent systemd socket activation support from being +# built into git-daemon. +# # Define EXPATDIR=/foo/bar if your expat header and library files are in # /foo/bar/include and /foo/bar/lib directories. # @@ -995,6 +998,13 @@ ifeq ($(uname_S),Darwin) PTHREAD_LIBS = endif +ifndef NO_SYSTEMD + ifeq ($(shell echo "\#include " | $(CC) -E - -o /dev/null && echo y),y) + BASIC_CFLAGS += -DHAVE_SYSTEMD + EXTLIBS += -lsystemd + endif +endif + ifndef CC_LD_DYNPATH ifdef NO_R_TO_GCC_LINKER # Some gcc does not accept and pass -R to the linker to specify @@ -1403,8 +1413,8 @@ ifdef NATIVE_CRLF endif ifdef USE_NED_ALLOCATOR - COMPAT_CFLAGS += -Icompat/nedmalloc - COMPAT_OBJS += compat/nedmalloc/nedmalloc.o +COMPAT_CFLAGS += -Icompat/nedmalloc +COMPAT_OBJS += compat/nedmalloc/nedmalloc.o endif ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT diff --git a/daemon.c b/daemon.c index 9ee2187..ad8a79a 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,7 @@ +#ifdef HAVE_S
Re: gitk drawing bug
On 15-04-03 07:05 PM, Alex Henrie wrote: 2015-02-18 12:27 GMT-07:00 Martin d'Anjou : It appears I have uncovered inconsistent behaviour in gitk. Looks like a bug. I have a picture here: https://docs.google.com/document/d/19TTzGD94B9EEIrVU5mRMjfJFvF5Ar3MlPblRJfP5OdQ/edit?usp=sharing Essentially, when I hit shift-F5, it sometimes draw the history differently (still valid, but drawn differently). There is no change in the repository between the shift-F5 keystrokes. Did you ever contact the gitk maintainer, Paul Mackerras , about this bug? -Alex I do not remember contacting Paul Mackerras, but thank you for doing so. Martin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] Documentation: add git-log --merges= option and log.merges config. var
Helped-by: Eric Sunshine
Signed-off-by: Koosha Khajehmoogahi
---
Documentation/git-log.txt | 3 +++
Documentation/rev-list-options.txt | 18 +++---
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 18bc716..e16f0f8 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -190,6 +190,9 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
+log.merges::
+Default for `--merges=` option. Defaults to `show`.
+
mailmap.*::
See linkgit:git-shortlog[1].
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index f620ee4..0bb2390 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -96,12 +96,24 @@ if it is part of the log message.
--remove-empty::
Stop when a given path disappears from the tree.
+--merges={show|hide|only}::
++
+--
+`show`: show both merge and non-merge commits
+
+`hide`: only show non-merge commits; same as `--max-parents=1`
+
+`only`: only show merge commits; same as `--min-parents=2`
+
+If `--merges=` is not specified, default value is `show`.
+--
++
+
--merges::
- Print only merge commits. This is exactly the same as `--min-parents=2`.
+ Shorthand for `--merges=only`.
--no-merges::
- Do not print commits with more than one parent. This is
- exactly the same as `--max-parents=1`.
+ Shorthand for `--merges=hide`.
--min-parents=::
--max-parents=::
--
2.3.3.263.g095251d.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
[PATCH v2 2/5] log: honor log.merges= option
From: Junio C Hamano
[kk: wrote commit message]
Helped-by: Eris Sunshine
Signed-off-by: Koosha Khajehmoogahi
---
builtin/log.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..c7a7aad 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -36,6 +36,7 @@ static int decoration_given;
static int use_mailmap_config;
static const char *fmt_patch_subject_prefix = "PATCH";
static const char *fmt_pretty;
+static const char *log_merges;
static const char * const builtin_log_usage[] = {
N_("git log [] [] [[--] ...]"),
@@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char
*value, void *cb)
decoration_style = 0; /* maybe warn? */
return 0;
}
+ if (!strcmp(var, "log.merges")) {
+ return git_config_string(&log_merges, var, value);
+ }
if (!strcmp(var, "log.showroot")) {
default_show_root = git_config_bool(var, value);
return 0;
@@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
init_revisions(&rev, prefix);
rev.always_show_header = 1;
+ if (log_merges && parse_merges_opt(&rev, log_merges))
+ die("unknown config value for log.merges: %s", log_merges);
memset(&opt, 0, sizeof(opt));
opt.def = "HEAD";
opt.revarg_opt = REVARG_COMMITTISH;
--
2.3.3.263.g095251d.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
[PATCH v2 4/5] t4202-log: add tests for --merges=
Helped-by: Eric Sunshine
Signed-off-by: Koosha Khajehmoogahi
---
t/t4202-log.sh | 84 ++
1 file changed, 84 insertions(+)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..ceaaf4e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -270,6 +270,90 @@ cat > expect <<\EOF
* initial
EOF
+test_expect_success 'setup --merges=' '
+ git log --min-parents=2 --pretty=tformat:%s >expect_only &&
+ git log --max-parents=1 --pretty=tformat:%s >expect_hide &&
+ git log --min-parents=-1 --pretty=tformat:%s >expect_show
+'
+
+test_expect_success 'log with config log.merges=show' '
+ test_config log.merges show &&
+ git log --pretty=tformat:%s >actual &&
+ test_cmp expect_show actual
+'
+
+test_expect_success 'log with config log.merges=only' '
+ test_config log.merges only &&
+ git log --pretty=tformat:%s >actual &&
+ test_cmp expect_only actual
+'
+
+test_expect_success 'log with config log.merges=hide' '
+ test_config log.merges hide &&
+ git log --pretty=tformat:%s >actual &&
+ test_cmp expect_hide actual
+'
+
+test_expect_success 'log with config log.merges=show with git log --no-merges'
'
+ test_config log.merges show &&
+ git log --no-merges --pretty=tformat:%s >actual &&
+ test_cmp expect_hide actual
+'
+
+test_expect_success 'log with config log.merges=hide with git log ---merges' '
+ test_config log.merges hide &&
+ git log --merges --pretty=tformat:%s >actual &&
+ test ! -s actual
+'
+
+test_expect_success 'log --merges=show' '
+ test_unconfig log.merges &&
+ git log --merges=show --pretty=tformat:%s >actual &&
+ test_cmp expect_show actual
+'
+
+test_expect_success 'log --merges=only' '
+ test_unconfig log.merges &&
+ git log --merges=only --pretty=tformat:%s >actual &&
+ test_cmp expect_only actual
+'
+
+test_expect_success 'log --merges=hide' '
+ test_unconfig log.merges &&
+ git log --merges=hide --pretty=tformat:%s >actual &&
+ test_cmp expect_hide actual
+'
+
+test_log_merges() {
+ test_config log.merges $2 &&
+ git log --merges=$1 --pretty=tformat:%s >actual &&
+ test_cmp $3 actual
+}
+
+test_expect_success 'log --merges=show with config log.merges=hide' '
+ test_log_merges show hide expect_show
+'
+
+test_expect_success 'log --merges=show with config log.merges=only' '
+ test_log_merges show only expect_show
+'
+
+test_expect_success 'log --merges=only with config log.merges=show' '
+ test_log_merges only show expect_only
+'
+
+test_expect_success 'log --merges=only with config log.merges=hide' '
+ test_log_merges only hide expect_only
+'
+
+test_expect_success 'log --merges=hide with config log.merges=show' '
+ test_log_merges hide show expect_hide
+'
+
+test_expect_success 'log --merges=hide with config log.merges=only' '
+ test_log_merges hide only expect_hide
+'
+
test_expect_success 'log --graph with merge' '
git log --graph --date-order --pretty=tformat:%s |
sed "s/ *\$//" >actual &&
--
2.3.3.263.g095251d.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
[PATCH v2 5/5] bash-completion: add support for git-log --merges= and log.merges
Helped-by: Eric Sunshine
Signed-off-by: Koosha Khajehmoogahi
---
contrib/completion/git-completion.bash | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index fbe5972..a75d7f5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1406,7 +1406,7 @@ _git_ls_tree ()
__git_log_common_options="
--not --all
--branches --tags --remotes
- --first-parent --merges --no-merges
+ --first-parent --merges --merges= --no-merges
--max-count=
--max-age= --since= --after=
--min-age= --until= --before=
@@ -1451,7 +1451,11 @@ _git_log ()
__gitcomp "long short" "" "${cur##--decorate=}"
return
;;
- --*)
+ --merges=*)
+ __gitcomp "show hide only" "" "${cur##--merges=}"
+ return
+ ;;
+ --*)
__gitcomp "
$__git_log_common_options
$__git_log_shortlog_options
@@ -1861,6 +1865,10 @@ _git_config ()
__gitcomp "$__git_log_date_formats"
return
;;
+ log.merges)
+ __gitcomp "show hide only"
+ return
+ ;;
sendemail.aliasesfiletype)
__gitcomp "mutt mailrc pine elm gnus"
return
@@ -2150,6 +2158,7 @@ _git_config ()
interactive.singlekey
log.date
log.decorate
+ log.merges
log.showroot
mailmap.file
man.
--
2.3.3.263.g095251d.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
[PATCH v2 1/5] revision: add --merges={show|only|hide} option
From: Junio C Hamano
revision: add a new option 'merges=' with
possible values of 'only', 'show' and 'hide'.
The option is used when showing the list of commits.
The value 'only' lists only merges. The value 'show'
is the default behavior which shows the commits as well
as merges and the value 'hide' makes it just list commit
items.
[kk: chose names for options; wrote commit message]
Helped-by: Eric Sunshine
Signed-off-by: Koosha Khajehmoogahi
---
revision.c | 20
revision.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/revision.c b/revision.c
index 6399a04..c3c3dcc 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs,
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
}
+int parse_merges_opt(struct rev_info *revs, const char *param)
+{
+ if (!strcmp(param, "show")) {
+ revs->min_parents = 0;
+ revs->max_parents = -1;
+ } else if (!strcmp(param, "only")) {
+ revs->min_parents = 2;
+ revs->max_parents = -1;
+ } else if (!strcmp(param, "hide")) {
+ revs->min_parents = 0;
+ revs->max_parents = 1;
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
int *unkc, const char **unkv)
{
@@ -1800,6 +1817,9 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
revs->show_all = 1;
} else if (!strcmp(arg, "--remove-empty")) {
revs->remove_empty_trees = 1;
+ } else if (starts_with(arg, "--merges=")) {
+ if (parse_merges_opt(revs, arg + 9))
+ die("unknown option: %s", arg);
} else if (!strcmp(arg, "--merges")) {
revs->min_parents = 2;
} else if (!strcmp(arg, "--no-merges")) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..f9df58c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv,
struct rev_info *revs,
extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t
*ctx,
const struct option *options,
const char * const usagestr[]);
+extern int parse_merges_opt(struct rev_info *, const char *);
#define REVARG_CANNOT_BE_FILENAME 01
#define REVARG_COMMITTISH 02
extern int handle_revision_arg(const char *arg, struct rev_info *revs,
--
2.3.3.263.g095251d.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: Freeing struct lock_file?
On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote: > David Turner writes: > > > Why is it impossible to free struct lock_files? I understand that they > > become part of a linked list, and that there's an atexit handler that > > goes over that list. But couldn't we just remove them from the linked > > list and then free them? > > I suspect that the code is worried about getting a signal, while it > is manipulating the linked list, and then cause the atexit handler > to walk a list that is in a broken state. This is technically possible, but practically unlikely: aligned pointer-sized writes are atomic on very nearly every processor, and that is all that is required to remove an item from a linked list safely in this case (though not, of course, in the general multi-threaded case). But I can see why git wouldn't want to depend on that behavior. C11 has a way to do this safely, but AIUI, git doesn't want to move to C99 let alone C11. So I guess this will just have to remain the way it is. -- 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 2.3.4, ssh: Could not resolve hostname
Thanks for keeping me in the loop! I have two thoughts on handling input: As a coder I want to know exactly what's going on in my code. If I've given erroneous input I'd like to know about it in the most useful and quickest way, never glossed over, liberally accepted, nor fixed for me even if the input is non-ambigous. I won't learn the right way unless I'm told. I enjoy that when I've typo'd a command in GIT it gives useful suggestions to what I might have meant. But, most of the coding *I* do is for the non-coder or the general end user. These might be people that would reasonably yell at their computer screen "you know what I meant!" So I try to be more liberal in the input I write code to accept by filtering it, cleaning it, etc. I'll even filter input by keystroke when possible. I have the philosophy: don't tell the user that they input something bad, just prevent them from inputting it to begin with. I know, this is appropriate when building a GUI and not for CLI. thanks for listening Reid Woodbury > On Apr 3, 2015, at 2:32 PM, Kyle J. McKay wrote: > > On Apr 2, 2015, at 17:02, Torsten Bögershausen wrote: > >> On 2015-04-02 21.35, Jeff King wrote: >>> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote: >>> Ah, understand. Here's my project URL for 'remote "origin"' with a more meaningful representation of their internal FQDN: url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git The "online" is their literal internal TLD. >>> >>> Thanks. The problem is the extra ":" after "online"; your URL is >>> malformed. You can just drop that colon entirely. >>> >>> I do not think we need to support this syntax going forward (the colon >>> is meaningless here, and our documentation is clear that it should go >>> with a port number), but on the other hand, it might be nice to be more >>> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see >>> whether supporting that would hurt some of the other cases, or whether >>> it would make the code too awkward. >>> >>> -Peff >> >> Thanks for digging. >> >> This makes my think that it is >> a) non-standard to have the extra colon > > It's not. See RFC 3986 appendix A: > > authority = [ userinfo "@" ] host [ ":" port ] > > port = *DIGIT > > "*DIGIT" means (see RFC 2234 section 3.6) zero or more digits. > >> b) The error message could be better >> c) We don't have a test case >> d) This reminds my of an improvement from Linus: >> 608d48b2207a61528 >> .. >> So when somebody passes me a "please pull" request pointing to something >> like the following >> >> git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git >> >> (note the extraneous colon at the end of the host name), git would happily >> try to connect to port 0, which would generally just cause the remote to >> not even answer, and the "connect()" will take a long time to time out. >> . >> >> Sorry guys for the regression, the old parser handled the extra colon as >> "port 0", >> the new one looks for the "/" as the end of the hostname (and the beginning >> of the path) >> >> Either we accept the extra colon as before, or the parser puts out a better >> error message, > > [...] > >> Spontaneously I would say that a trailing ':' at the end of a hostname in >> the ssh:// scheme >> can be safely ignored, what do you think ? > > You know, there is a "url_normalize" routine in urlmatch.h/urlmatch.c that > checks for a lot of these things and provides a translated error message if > there's a problem as well as normalizing and separating out the various parts > of the URL. It does not currently handle default ports for anything other > than http[s] but it would be simple enough to add support for ssh, git, > ftp[s] and rsync default ports too. > > -Kyle -- 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] standardize usage strings that were missed the first time
On Fri, Apr 3, 2015 at 3:47 PM, Alex Henrie wrote:
> 2015-04-02 15:56 GMT-06:00 Junio C Hamano :
>> Thanks, but please no more _("string") changes for the rest of the
>> cycle, as that would impact i18n folks who will be starting from
>> tagged -rc releases.
>>
>> Please hold them off, and resend them after 2.4.0 final.
>
> I thought that during a code freeze, you held onto patches or
> committed them to a staging branch.
Yes, but that is done as time-permits basis, and it is much more
efficient to spread the load of remembering the topics that are
needed in future ;-)
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: gitk drawing bug
2015-02-18 12:27 GMT-07:00 Martin d'Anjou : > It appears I have uncovered inconsistent behaviour in gitk. Looks like > a bug. I have a picture here: > https://docs.google.com/document/d/19TTzGD94B9EEIrVU5mRMjfJFvF5Ar3MlPblRJfP5OdQ/edit?usp=sharing > > Essentially, when I hit shift-F5, it sometimes draw the history > differently (still valid, but drawn differently). There is no change > in the repository between the shift-F5 keystrokes. Did you ever contact the gitk maintainer, Paul Mackerras , about this bug? -Alex -- 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: [v3RFC] systemd socket activation support
On Fri, Apr 3, 2015 at 6:30 PM, Shawn Landden wrote: > systemd supports git-daemon's existing --inetd mode as well. > > v2: actually test... > v3: make optional, switch to libsystemd Every issue raised by my review[1] of v2 still applies to v3, so I won't bother repeating them here, however, there is one comment (below) new to this version of the patch. [1]: http://article.gmane.org/gmane.comp.version-control.git/266650 > shawn@zephyr:~/git/git$ ldd /lib/x86_64-linux-gnu/libsystemd.so.0 > linux-vdso.so.1 (0x7ffeba7ec000) > libcap.so.2 => /lib/x86_64-linux-gnu/libcap.so.2 (0x7fea158fe000) > libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7fea155f9000) > librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x7fea153f) > libselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 > (0x7fea151cb000) > liblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 > (0x7fea14fa8000) > libgcrypt.so.20 => /lib/x86_64-linux-gnu/libgcrypt.so.20 > (0x7fea14cc5000) > libresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 > (0x7fea14aae000) > libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7fea148aa000) > libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 > (0x7fea1468b000) > libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7fea142e7000) > /lib64/ld-linux-x86-64.so.2 (0x7fea15d5b000) > libattr.so.1 => /lib/x86_64-linux-gnu/libattr.so.1 > (0x7fea140e2000) > libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 > (0x7fea13e73000) > libgpg-error.so.0 => /lib/x86_64-linux-gnu/libgpg-error.so.0 > (0x7fea13c61000) > > ew...and only for two tiny functions. > > Signed-off-by: Shawn Landden > --- > diff --git a/Makefile b/Makefile > index 5f3987f..362af94 100644 > --- a/Makefile > +++ b/Makefile > @@ -1403,8 +1413,8 @@ ifdef NATIVE_CRLF > endif > > ifdef USE_NED_ALLOCATOR > - COMPAT_CFLAGS += -Icompat/nedmalloc > - COMPAT_OBJS += compat/nedmalloc/nedmalloc.o > + COMPAT_CFLAGS += -Icompat/nedmalloc > + COMPAT_OBJS += compat/nedmalloc/nedmalloc.o You're sneaking in (whitespace) changes unrelated to the rest of the patch. Typically, such unrelated cleanups should be relegated to separate preparatory patches. (In this case, however, the change is probably unwarranted and likely accidental, but serves as a reminder to review patches before sending them out.) > endif > > ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT > diff --git a/daemon.c b/daemon.c > index 9ee2187..16b9eda 100644 > --- a/daemon.c > +++ b/daemon.c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: Fetch the proper revision of utf16 files
*note* I tried to send this to the list earlier but forgot to set the
mode to plain-text so it got rejected. Resending. Apologies for any
duplicates.
I think the context of the patch doesn't make this clear, but if you
look at git-p4.py in this spot you'll see that this is in a block that
begins:
if type_base == "utf16":
Where "type_base" is extracted, by the script, from the filetype
provided by p4 (i.e. metadata provided by p4). In the particular
scenario I encountered, P4 said that the file was of type xutf16 from
which the split_p4_type() method extracts "utf16" as the type_base.
The essence of the issue here has nothing to do with UTF16, its just
coincidental that it happens for files of that type. The problem is
that the script wasn't requesting the proper revision from P4 and thus
would always get the *latest* revision. This corrects that by having
the script request the appropriate revision from P4.
Daniel
On Fri, Apr 3, 2015 at 2:46 PM, Eric Sunshine wrote:
> On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham wrote:
>> git-p4 always fetches the latest revision of UTF16
>> files from P4 rather than the revision at the commit being sync'd.
>>
>> The print command should, instead, specify the revision number from the
>> commit in question using the file#revision syntax.
>>
>> The file#revision syntax is preferable over file@changelist for
>> consistency with how streamP4Files formats the fileArgs list.
>
> As a non-Perforce reader trying to understand this patch, there are a
> couple issues which are unclear or inadequately explained. Perhaps you
> could provide a bit more detail or cite relevant sources.
>
> First, does "UTF16 file" refer to the content or the filename?
>
> Second, I may be entirely missing it, but the commit message doesn't
> seem to explain why this impacts only "UTF16 files", and why this
> solution is the appropriate fix.
>
> If the answer to the first question is that the filename is UTF-16,
> then would an alternate fix be to convert the value of
> file['depotFile'] to have the same encoding as the "print -q -o - ..."
> command-line? (Again, please excuse my Perforce-ignorance if I'm
> completely off the mark.)
>
>> Signed-off-by: Daniel Bingham
>> ---
>> git-p4.py | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index ff132b2..156f3a4 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2101,7 +2101,8 @@ class P4Sync(Command, P4UserMap):
>> # them back too. This is not needed to the cygwin windows
>> version,
>> # just the native "NT" type.
>> #
>> -text = p4_read_pipe(['print', '-q', '-o', '-',
>> file['depotFile']])
>> +ufile = "%s#%s" % (file['depotFile'], file['rev'])
>> +text = p4_read_pipe(['print', '-q', '-o', '-', ufile])
>> if p4_version_string().find("/NT") >= 0:
>> text = text.replace("\r\n", "\n")
>> contents = [ text ]
>> --
>> 2.3.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: Fetch the proper revision of utf16 files
[Daniel: Your HTML-formatted response probably did not make it to the
Git mailing list since only plain text is accepted. Also, please
respond inline on this list rather than top-posting.]
On Fri, Apr 3, 2015 at 5:54 PM, Domain Admin wrote:
> I think the context of the patch doesn't make this clear, but if you look at
> git-p4.py in this spot you'll see that this is in a block that begins:
>
> if type_base == "utf16":
>
> Where "type_base" is extracted, by the script, from the filetype provided by
> p4 (i.e. metadata provided by p4). In the particular scenario I encountered,
> P4 said that the file was of type xutf16 from which the split_p4_type()
> method extracts "utf16" as the type_base.
So, the patch's mention of "UTF16 file" indeed refers to the content
rather than the filename.
Now that I've studied up on Perforce a bit and read through some of
the git-p4.py code, I understand that there is a special-case for
files with utf-16 content in which git-p4 re-reads the content in
order to work around a problem with ASCII text saved with type utf-16
getting mangled. The work-around, 55aa5714 (git-p4: handle utf16
filetype properly; 2011-10-17), is buggy in that it neglects to
specify a particular revision of the file, and your patch fixes that.
Perhaps your commit message could make that a bit clearer by
mentioning the special-case and citing 55aa5714? Maybe something like:
The special-case handling of files with UTF-16 content done by
55aa5714 (git-p4: handle utf16 filetype properly; 2011-10-17)
neglects to specify a revision when re-reading the file content.
As a result, the latest revision of the UTF-16 file is always
fetched rather than the desired one.
Also, is it possible to craft a test for this issue and place it in
one of the t98xx scripts?
> Daniel
>
> On Fri, Apr 3, 2015 at 2:46 PM, Eric Sunshine
> wrote:
>>
>> On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham
>> wrote:
>> > git-p4 always fetches the latest revision of UTF16
>> > files from P4 rather than the revision at the commit being sync'd.
>> >
>> > The print command should, instead, specify the revision number from the
>> > commit in question using the file#revision syntax.
>> >
>> > The file#revision syntax is preferable over file@changelist for
>> > consistency with how streamP4Files formats the fileArgs list.
>>
>> As a non-Perforce reader trying to understand this patch, there are a
>> couple issues which are unclear or inadequately explained. Perhaps you
>> could provide a bit more detail or cite relevant sources.
>>
>> First, does "UTF16 file" refer to the content or the filename?
>>
>> Second, I may be entirely missing it, but the commit message doesn't
>> seem to explain why this impacts only "UTF16 files", and why this
>> solution is the appropriate fix.
>>
>> If the answer to the first question is that the filename is UTF-16,
>> then would an alternate fix be to convert the value of
>> file['depotFile'] to have the same encoding as the "print -q -o - ..."
>> command-line? (Again, please excuse my Perforce-ignorance if I'm
>> completely off the mark.)
>>
>> > Signed-off-by: Daniel Bingham
>> > ---
>> > git-p4.py | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/git-p4.py b/git-p4.py
>> > index ff132b2..156f3a4 100755
>> > --- a/git-p4.py
>> > +++ b/git-p4.py
>> > @@ -2101,7 +2101,8 @@ class P4Sync(Command, P4UserMap):
>> > # them back too. This is not needed to the cygwin windows
>> > version,
>> > # just the native "NT" type.
>> > #
>> > -text = p4_read_pipe(['print', '-q', '-o', '-',
>> > file['depotFile']])
>> > +ufile = "%s#%s" % (file['depotFile'], file['rev'])
>> > +text = p4_read_pipe(['print', '-q', '-o', '-', ufile])
>> > if p4_version_string().find("/NT") >= 0:
>> > text = text.replace("\r\n", "\n")
>> > contents = [ text ]
>> > --
>> > 2.3.5
--
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] standardize usage strings that were missed the first time
2015-04-02 15:56 GMT-06:00 Junio C Hamano :
> Thanks, but please no more _("string") changes for the rest of the
> cycle, as that would impact i18n folks who will be starting from
> tagged -rc releases.
>
> Please hold them off, and resend them after 2.4.0 final.
I thought that during a code freeze, you held onto patches or
committed them to a staging branch. But it's OK, I will resend after
2.4 final is released. Thanks for the clarification.
-Alex
--
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
[v3RFC] systemd socket activation support
systemd supports git-daemon's existing --inetd mode as well. v2: actually test... v3: make optional, switch to libsystemd shawn@zephyr:~/git/git$ ldd /lib/x86_64-linux-gnu/libsystemd.so.0 linux-vdso.so.1 (0x7ffeba7ec000) libcap.so.2 => /lib/x86_64-linux-gnu/libcap.so.2 (0x7fea158fe000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7fea155f9000) librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x7fea153f) libselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 (0x7fea151cb000) liblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 (0x7fea14fa8000) libgcrypt.so.20 => /lib/x86_64-linux-gnu/libgcrypt.so.20 (0x7fea14cc5000) libresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 (0x7fea14aae000) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7fea148aa000) libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x7fea1468b000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7fea142e7000) /lib64/ld-linux-x86-64.so.2 (0x7fea15d5b000) libattr.so.1 => /lib/x86_64-linux-gnu/libattr.so.1 (0x7fea140e2000) libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x7fea13e73000) libgpg-error.so.0 => /lib/x86_64-linux-gnu/libgpg-error.so.0 (0x7fea13c61000) ew...and only for two tiny functions. Signed-off-by: Shawn Landden --- Documentation/git-daemon.txt | 25 Makefile | 14 -- daemon.c | 46 ++-- 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt index a69b361..898e01f 100644 --- a/Documentation/git-daemon.txt +++ b/Documentation/git-daemon.txt @@ -20,6 +20,7 @@ SYNOPSIS [--inetd | [--listen=] [--port=] [--user= [--group=]]] + [--systemd] [...] DESCRIPTION @@ -190,6 +191,12 @@ Git configuration files in that directory are readable by ``. exiting with a zero status). It can also look at the $REMOTE_ADDR and $REMOTE_PORT environment variables to learn about the requestor when making this decision. +--systemd:: + For running git-daemon under systemd(1) which will pass + an open connection. This is similar to --inetd, except + that more than one address/port can be listened to at once + both through systemd and through --listen, and git-daemon doesn't get + invoked for every connection. For more details see systemd.socket(5). + The external command can optionally write a single line to its standard output to be sent to the requestor as an error message when @@ -304,7 +311,25 @@ selectively enable/disable services per repository:: uploadpack = false uploadarch = true ++ +systemd configuration example: + +# /etc/systemd/system/git-daemon.socket +[Unit] +Description=Git Daemon socket + +[Socket] +ListenStream=9418 + +[Install] +WantedBy=sockets.target +# /etc/systemd/system/git-daemon.service +[Unit] +Description=Git Daemon +[Service] +ExecStart=/usr/lib/git-core/git-daemon --systemd --reuseaddr --base-path=/var/lib /var/lib/git +User=gitdaemon ENVIRONMENT --- diff --git a/Makefile b/Makefile index 5f3987f..362af94 100644 --- a/Makefile +++ b/Makefile @@ -42,6 +42,9 @@ all:: # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). # +# Define NO_SYSTEMD to prevent systemd socket activation support from being +# built into git-daemon. +# # Define EXPATDIR=/foo/bar if your expat header and library files are in # /foo/bar/include and /foo/bar/lib directories. # @@ -995,6 +998,13 @@ ifeq ($(uname_S),Darwin) PTHREAD_LIBS = endif +ifndef NO_SYSTEMD + ifeq ($(shell echo "\#include " | $(CC) -E - -o /dev/null && echo y),y) + BASIC_CFLAGS += -DHAVE_SYSTEMD + EXTLIBS += -lsystemd + endif +endif + ifndef CC_LD_DYNPATH ifdef NO_R_TO_GCC_LINKER # Some gcc does not accept and pass -R to the linker to specify @@ -1403,8 +1413,8 @@ ifdef NATIVE_CRLF endif ifdef USE_NED_ALLOCATOR - COMPAT_CFLAGS += -Icompat/nedmalloc - COMPAT_OBJS += compat/nedmalloc/nedmalloc.o + COMPAT_CFLAGS += -Icompat/nedmalloc + COMPAT_OBJS += compat/nedmalloc/nedmalloc.o endif ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT diff --git a/daemon.c b/daemon.c index 9ee2187..16b9eda 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,7 @@ +#ifdef HAVE_SYSTEMD +# include +#endif + #include "cache.h" #include "pkt-line.h" #include "exec_cmd.h" @@ -29,6 +33,9 @@ static const char daemon_usage[] = " [--access-hook=]\n" " [--inetd | [--listen=] [-
Re: [PATCH] diff-highlight: Fix broken multibyte string
On Apr 3, 2015, at 15:08, Jeff King wrote:
Doing:
diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-
highlight/diff-highlight
index 08c88bb..1c4b599 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -165,7 +165,7 @@ sub highlight_pair {
sub split_line {
local $_ = shift;
return map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR*)/;
+ split /($COLOR+)/;
}
sub highlight_line {
gives me a 25% speed improvement, and the same output processing
git.git's entire "git log -p" output.
I thought that meant we could also optimize out the "map" call
entirely,
and just use the first split (with "*") to end up with a list of
$COLOR
chunks and single characters, but it does not seem to work. So maybe I
am misreading something about what is going on.
I think our emails crossed in flight...
Using just the first split (with "*") produces useless empty elements
which I think ends up causing problems. I suppose you could surround
it with a grep /./ to remove them but that would defeat the point of
the optimization.
-Kyle
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] diff-highlight: do not split multibyte characters
When the input is UTF-8 and Perl is operating on bytes instead of
characters, a diff that changes one multibyte character to another
that shares an initial byte sequence will result in a broken diff
display as the common byte sequence prefix will be separated from
the rest of the bytes in the multibyte character.
For example, if a single line contains only the unicode character
U+C9C4 (encoded as UTF-8 0xEC, 0xA7, 0x84) and that line is then
changed to the unicode character U+C9C0 (encoded as UTF-8 0xEC,
0xA7, 0x80), when operating on bytes diff-highlight will show only
the single byte change from 0x84 to 0x80 thus creating invalid UTF-8
and a broken diff display.
Fix this by putting Perl into character mode when splitting the line
and then back into byte mode after the split is finished.
The utf8::xxx functions require Perl 5.8 so we require that as well.
Also, since we are mucking with code in the split_line function, we
change a '*' quantifier to a '+' quantifier when matching the $COLOR
expression which has the side effect of speeding everything up while
eliminating useless '' elements in the returned array.
Reported-by: Yi EungJun
Signed-off-by: Kyle J. McKay
---
On Apr 2, 2015, at 19:19, Yi, EungJun wrote:
>> I timed this one versus the existing diff-highlight. It's about 7%
>> slower. That's not great, but is acceptable to me. The
>> String::Multibyte
>> version was a lot faster, which was nice (but I'm still unclear on
>> _why_).
>
> I think the reason is here:
>
>> sub split_line {
>> local $_ = shift;
>> return map { /$COLOR/ ? $_ : ($mbcs ? $mbcs->strsplit('', $_) :
>> split //) }
>> split /($COLOR)/;
>> }
>
> I removed "*" from "split /($COLOR*)/". Actually I don't know why "*"
> was required but I need to remove it to make my patch works correctly.
>
> On Fri, Apr 3, 2015 at 10:24 AM, Jeff King wrote:
>> EungJun, does this version meet your needs?
This version differs from the former as follows:
1) Slightly faster code that eliminates the need for Encode::is_utf8.
2) The '*' quantifier is changed to '+' in the split_line regexs which
was probably the original intent anyway as using '*' generates useless
empty elements. This has the side effect of greatly increasing the
speed so the tiny speed penalty for the UTF-8 checking is vastly
overwhelmed by the overall speed up. :)
3) The 'use 5.008;' line has been added since the utf8::xxx functions
require Perl 5.8
-Kyle
contrib/diff-highlight/diff-highlight | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index 08c88bbc..ffefc31a 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -1,5 +1,6 @@
#!/usr/bin/perl
+use 5.008;
use warnings FATAL => 'all';
use strict;
@@ -164,8 +165,12 @@ sub highlight_pair {
sub split_line {
local $_ = shift;
- return map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR*)/;
+ return utf8::decode($_) ?
+ map { utf8::encode($_); $_ }
+ map { /$COLOR/ ? $_ : (split //) }
+ split /($COLOR+)/ :
+ map { /$COLOR/ ? $_ : (split //) }
+ split /($COLOR+)/;
}
sub highlight_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] diff-highlight: Fix broken multibyte string
On Fri, Apr 03, 2015 at 11:19:24AM +0900, Yi, EungJun wrote:
> > I timed this one versus the existing diff-highlight. It's about 7%
> > slower. That's not great, but is acceptable to me. The String::Multibyte
> > version was a lot faster, which was nice (but I'm still unclear on
> > _why_).
>
> I think the reason is here:
>
> > sub split_line {
> >local $_ = shift;
> >return map { /$COLOR/ ? $_ : ($mbcs ? $mbcs->strsplit('', $_) : split
> > //) }
> > split /($COLOR)/;
> > }
>
> I removed "*" from "split /($COLOR*)/". Actually I don't know why "*"
> was required but I need to remove it to make my patch works correctly.
Ah, OK, that makes more sense. The "*" was meant to handle the case of
multiple groups of ANSI colors in a row. But I think it should have been
"+" in that case, as we would otherwise split on the empty field, which
would mean character-by-character. And the second "split" in the map
would then be superfluous, which would break your patch (we've already
split the multi-byte characters before we even hit $mbcs->strsplit).
Kyle's patch does not care, because it tweaks the string so that normal
split works. Which means there is an easy speedup here. :)
Doing:
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index 08c88bb..1c4b599 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -165,7 +165,7 @@ sub highlight_pair {
sub split_line {
local $_ = shift;
return map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR*)/;
+ split /($COLOR+)/;
}
sub highlight_line {
gives me a 25% speed improvement, and the same output processing
git.git's entire "git log -p" output.
I thought that meant we could also optimize out the "map" call entirely,
and just use the first split (with "*") to end up with a list of $COLOR
chunks and single characters, but it does not seem to work. So maybe I
am misreading something about what is going on.
-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: RFC: git status --amend
On Fri, Apr 03, 2015 at 02:57:48PM -0700, David Aguilar wrote: > > But I discarded it as a useless suggestion before writing it down, > > primarily because I couldn't come up with an explanation _why_ being > > able to say "git status --relative-to=next Makefile" is useful when > > on the 'master' branch. > > Aesthetically it's appealing because it mirrors commands like > "git diff HEAD^", etc. > > I can see it being useful for script writers but it's a minority > case that's already handled by having "status --amend" for the > common case of needing to mimic "commit --amend". > > Beyond that use case, someone could use it to write a butchery > tool that gets a quick high-level diff of changes for both index > and worktree against an arbitrary ref, and then apply those > changes selectively using other git tools. Hmm. What if you had a tool that created commits out of an alternate working tree and index, and then committed directly to a branch without touching HEAD? Then you might run: GIT_WORK_TREE=... GIT_INDEX_FILE=... git status --relative-to=mybranch right before running: old=$(git rev-parse refs/heads/mybranch) && tree=$(GIT_INDEX_FILE=... git commit-tree) && commit=$(echo whatever | git commit-tree -p $old $tree) && git update-ref refs/heads/mybranch $old or similar. That is basically "git-new-workdir", but with no per-workdir HEAD. Which is probably crazy, but maybe useful for a one-off commit to another branch or something. I dunno. I do not have such a tool or plan to work on one, but it is at least plausible to me. -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: Freeing struct lock_file?
David Turner writes: > Why is it impossible to free struct lock_files? I understand that they > become part of a linked list, and that there's an atexit handler that > goes over that list. But couldn't we just remove them from the linked > list and then free them? I suspect that the code is worried about getting a signal, while it is manipulating the linked list, and then cause the atexit handler to walk a list that is in a broken state. -- 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: git status --amend
On Wed, Apr 01, 2015 at 10:16:22AM -0700, Junio C Hamano wrote: > David Aguilar writes: > > > Would generalizing "status" to have a more gittish syntax make > > you feel less torn? > > One of my early draft responses included a one whose punch line was > "Why limit the comparison to HEAD and HEAD^ but no other point of > reference?" > > But I discarded it as a useless suggestion before writing it down, > primarily because I couldn't come up with an explanation _why_ being > able to say "git status --relative-to=next Makefile" is useful when > on the 'master' branch. Aesthetically it's appealing because it mirrors commands like "git diff HEAD^", etc. I can see it being useful for script writers but it's a minority case that's already handled by having "status --amend" for the common case of needing to mimic "commit --amend". Beyond that use case, someone could use it to write a butchery tool that gets a quick high-level diff of changes for both index and worktree against an arbitrary ref, and then apply those changes selectively using other git tools. status is superior to the other tools (diff-index, diff-files, ls-files) because we can get all of the information in a single git invocation, which is more of a perf. concern but worth considering. > Surely, I may have changes in the Makefile relative to my index > because I am preparing for the next rc release, and the Makefile in > the index may be different from that of the 'next' branch because I > am on my 'master' branch. The potential output can be "explained" > in such a mechanical sense (e.g. "we generated the output this > way"). > > But I do not see an easy-to-understand explanation of the _meaning_ > of the output, i.e. "What does it mean that the working tree file > has been modified since the checkout and the index is different > relative to that other branch? How does that information help me > after I learn it? What would I do differently with that information > at hand?" > > Compared to that, "Show me what damage I would inflict if I did > 'commit' now. By the way, I may want to see that information > limited to these paths" is a question whose utility is easily > explained, and so is the same question with 'commit' replaced by > 'commit --amend'. Yeah, ergonomically it would still make sense to have "status --amend" (even if it also were to also understand "status ") for symmetry. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/4] git-p4: update locked test case
Eric Sunshine writes: > On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand wrote: >> The add-new-file and copy-existing-file tests from >> t9816-git-p4-locked.sh now pass. Update the test >> case accordingly. > > This patch should be folded into patch 3/4, shouldn't it? If the fix and tests were done by the same person, we strongly prefer to see the code fix and test expectation update in the same commit. What Luke is doing here is to spread the credits/blame to two people. If I were playing Luke's role, I might have squashed them into a single commit, kept the authorship of the whole thing under Blair Holloway's name, and extend the log message by mentioning that I flipped the test expectations, but I'd say either is fine for this change. >> Signed-off-by: Luke Diamand >> --- >> t/t9816-git-p4-locked.sh | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh >> index 464f10b..d048bd3 100755 >> --- a/t/t9816-git-p4-locked.sh >> +++ b/t/t9816-git-p4-locked.sh >> @@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' ' >> ) >> ' >> >> -test_expect_failure 'add with lock not taken' ' >> +test_expect_success 'add with lock not taken' ' >> test_when_finished cleanup_git && >> git p4 clone --dest="$git" //depot && >> ( >> @@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' ' >> ) >> ' >> >> -test_expect_failure 'copy with lock taken' ' >> +test_expect_success 'copy with lock taken' ' >> lock_in_another_client && >> test_when_finished cleanup_git && >> test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" && >> -- >> 2.3.0.rc1.30.g76afe74 -- 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] diff-highlight: Fix broken multibyte string
On Thu, Apr 02, 2015 at 06:59:50PM -0700, Kyle J. McKay wrote: > It should work as well as the original did for any 1-byte encoding. That > is, if it's not valid UTF-8 it should pass through unchanged and any single > byte encoding should just work. But, as you point out, multibyte encodings > other than UTF-8 won't work, but they should behave the same as they did > before. Yeah, sorry, I should have been more clear that I meant multibyte encodings. UTF-8 is the only common multibyte encoding I run across, but that's because Latin1 served most of my pre-UTF-8 needs. I suspect things are very different for people in Asia. I don't know how badly they would want support for other encodings. I'm happy to go with a UTF-8 solution for now, and see if anybody wants to expand it further later. > >I timed this one versus the existing diff-highlight. It's about 7% > >slower. > > I'd expect that, we're doing extra work we weren't doing before. I was worried would be 200% or something. :) > >Makes sense. I'm happy enough listing perl 5.8 as a dependency. > > Maybe that should be added. The rest of Git's perl code seems to have a > 'use 5.008;' already, so I figured that was a reasonable dependency. :) I shouldn't have said "listing". I just meant "have" as a dependency. I am also happy with adding "use 5.008", but I agree it's probably not necessary at this point. It was released in 2002 (wow, has it really been that long?). -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] git-p4: Fetch the proper revision of utf16 files
On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham wrote:
> git-p4 always fetches the latest revision of UTF16
> files from P4 rather than the revision at the commit being sync'd.
>
> The print command should, instead, specify the revision number from the
> commit in question using the file#revision syntax.
>
> The file#revision syntax is preferable over file@changelist for
> consistency with how streamP4Files formats the fileArgs list.
As a non-Perforce reader trying to understand this patch, there are a
couple issues which are unclear or inadequately explained. Perhaps you
could provide a bit more detail or cite relevant sources.
First, does "UTF16 file" refer to the content or the filename?
Second, I may be entirely missing it, but the commit message doesn't
seem to explain why this impacts only "UTF16 files", and why this
solution is the appropriate fix.
If the answer to the first question is that the filename is UTF-16,
then would an alternate fix be to convert the value of
file['depotFile'] to have the same encoding as the "print -q -o - ..."
command-line? (Again, please excuse my Perforce-ignorance if I'm
completely off the mark.)
> Signed-off-by: Daniel Bingham
> ---
> git-p4.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index ff132b2..156f3a4 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2101,7 +2101,8 @@ class P4Sync(Command, P4UserMap):
> # them back too. This is not needed to the cygwin windows
> version,
> # just the native "NT" type.
> #
> -text = p4_read_pipe(['print', '-q', '-o', '-',
> file['depotFile']])
> +ufile = "%s#%s" % (file['depotFile'], file['rev'])
> +text = p4_read_pipe(['print', '-q', '-o', '-', ufile])
> if p4_version_string().find("/NT") >= 0:
> text = text.replace("\r\n", "\n")
> contents = [ text ]
> --
> 2.3.5
--
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
Freeing struct lock_file?
Why is it impossible to free struct lock_files? I understand that they become part of a linked list, and that there's an atexit handler that goes over that list. But couldn't we just remove them from the linked list and then free them? Even if we couldn't free all lock_files, maybe we could free some? Anyway, just curious why that decision was made. -- 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 2.3.4, ssh: Could not resolve hostname
On Apr 2, 2015, at 17:02, Torsten Bögershausen wrote: On 2015-04-02 21.35, Jeff King wrote: On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote: Ah, understand. Here's my project URL for 'remote "origin"' with a more meaningful representation of their internal FQDN: url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git The "online" is their literal internal TLD. Thanks. The problem is the extra ":" after "online"; your URL is malformed. You can just drop that colon entirely. I do not think we need to support this syntax going forward (the colon is meaningless here, and our documentation is clear that it should go with a port number), but on the other hand, it might be nice to be more liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see whether supporting that would hurt some of the other cases, or whether it would make the code too awkward. -Peff Thanks for digging. This makes my think that it is a) non-standard to have the extra colon It's not. See RFC 3986 appendix A: authority = [ userinfo "@" ] host [ ":" port ] port = *DIGIT "*DIGIT" means (see RFC 2234 section 3.6) zero or more digits. b) The error message could be better c) We don't have a test case d) This reminds my of an improvement from Linus: 608d48b2207a61528 .. So when somebody passes me a "please pull" request pointing to something like the following git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git (note the extraneous colon at the end of the host name), git would happily try to connect to port 0, which would generally just cause the remote to not even answer, and the "connect()" will take a long time to time out. . Sorry guys for the regression, the old parser handled the extra colon as "port 0", the new one looks for the "/" as the end of the hostname (and the beginning of the path) Either we accept the extra colon as before, or the parser puts out a better error message, [...] Spontaneously I would say that a trailing ':' at the end of a hostname in the ssh:// scheme can be safely ignored, what do you think ? You know, there is a "url_normalize" routine in urlmatch.h/urlmatch.c that checks for a lot of these things and provides a translated error message if there's a problem as well as normalizing and separating out the various parts of the URL. It does not currently handle default ports for anything other than http[s] but it would be simple enough to add support for ssh, git, ftp[s] and rsync default ports too. -Kyle-- 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 (Apr 2015, #01; Thu, 2)
"Kyle J. McKay" writes: > On Apr 2, 2015, at 15:09, Junio C Hamano wrote: > >> * jc/show-branch (2014-03-24) 5 commits >> - show-branch: use commit slab to represent bitflags of arbitrary >> width >> - show-branch.c: remove "all_mask" >> - show-branch.c: abstract out "flags" operation >> - show-branch.c: lift all_mask/all_revs to a global static >> - show-branch.c: update comment style >> >> Waiting for the final step to lift the hard-limit before sending it >> out. > > > May I ask, now that this topic is over 1-year old, please, what is the > final step? It would be to "lift the hard-limit"; all the existing changes are to allow the limit to be lifted by moving from "bits in a word-sized field" to "commit-slab that can hold arbitrary number of bits". > > Perhaps someone might submit a patch... ;) > > -Kyle -- 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] clone: Warn if clone lacks LICENSE or COPYING file
Ævar Arnfjörð Bjarmason writes: > As others have indicated here this feature is really specific to a > single lint-like use-case and doesn't belong in clone as a built-in > feature. > > However perhaps an interesting generalization of this would be > something like a post-clone hook, obviously you couldn't store that in > .git/hooks/ like other githooks(5) since there's no repo yet, Yes, and these things come from templates, and you can specify the template source location when running "git clone". So I do not think anything is needed on our side and it's all doable with what the users already have, as long as we are talking about making it only an opt-in feature. Which means > You could also just have a shell alias that wrapped git-clone... is also perfectly acceptable, I would think. -- 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] clone: Warn if clone lacks LICENSE or COPYING file
On Sun, 22 Mar 2015 18:56:52 +0100, Ævar Arnfjörð Bjarmason wrote: > However perhaps an interesting generalization of this would be > something like a post-clone hook, obviously you couldn't store that in > .git/hooks/ like other githooks(5) since there's no repo yet, but > having it configured via the user/system config might be an > interesting feature. Would that be acceptable to the wider group? --- David A. Wheeler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: Fetch the proper revision of utf16 files
git-p4 always fetches the latest revision of UTF16
files from P4 rather than the revision at the commit being sync'd.
The print command should, instead, specify the revision number from the
commit in question using the file#revision syntax.
The file#revision syntax is preferable over file@changelist for
consistency with how streamP4Files formats the fileArgs list.
Signed-off-by: Daniel Bingham
---
git-p4.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index ff132b2..156f3a4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2101,7 +2101,8 @@ class P4Sync(Command, P4UserMap):
# them back too. This is not needed to the cygwin windows version,
# just the native "NT" type.
#
-text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+ufile = "%s#%s" % (file['depotFile'], file['rev'])
+text = p4_read_pipe(['print', '-q', '-o', '-', ufile])
if p4_version_string().find("/NT") >= 0:
text = text.replace("\r\n", "\n")
contents = [ text ]
--
2.3.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: Fetch the proper revision of utf16 files
I discovered what appears to be a bug with the handling of utf16 type files when cloning from P4. It appears that it always fetches the content of the latest version rather than getting the revision specified in the changelist being processed. This patch is an attempt to resolve that by formatting the print command using the revision number as is done in streamP4Files(). Daniel Bingham (1): git-p4: Fetch the proper revision of utf16 files git-p4.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.3.5 -- 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 2.3.4, ssh: Could not resolve hostname
On Fri, Apr 03, 2015 at 02:01:55PM -0700, Junio C Hamano wrote: > Torsten Bögershausen writes: > > > This makes my think that it is > > a) non-standard to have the extra colon > > b) The error message could be better > > For that, perhaps > > -ssh: Could not resolve hostname :: nodename nor servname provided, or > not known > +ssh: Could not resolve hostname ":": nodename nor servname provided, or > not known > > would be something we would want to do, no matter what other fixes > we would apply. That message comes from the ssh client. So the "we" here would have to submit a patch to OpenSSH". The easier way to diagnose inside git is to set GIT_TRACE, which makes it more clear: $ GIT_TRACE=1 git clone ssh://bogosity:/repo.git ... 17:05:00.734019 run-command.c:347 trace: run_command: 'ssh' 'bogosity:' 'git-upload-pack '\''/repo.git'\''' -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: git 2.3.4, ssh: Could not resolve hostname
Torsten Bögershausen writes: > This makes my think that it is > a) non-standard to have the extra colon > b) The error message could be better For that, perhaps -ssh: Could not resolve hostname :: nodename nor servname provided, or not known +ssh: Could not resolve hostname ":": nodename nor servname provided, or not known would be something we would want to do, no matter what other fixes we would apply. > Spontaneously I would say that a trailing ':' at the end of a > hostname in the ssh:// scheme can be safely ignored, what do you > think? If it is not too much hassle to make the current code do so, I'd say that is a good way forward. Giving a warning that lets the user know that the input has an extra and unwanted colon in it may be a plus, too. 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: GSoC 2015: students proposals submitted, selection started
On Wed, Apr 01, 2015 at 06:19:00PM +0200, Matthieu Moy wrote: > Latest news from the google summer of code: students have completed > their proposals. We have 2 proposals for "convert scripts to > builtins", and 4 for "unify git branch, git tag and git for-each-ref" > (plus some out-of-scope proposals). See > http://git.github.io/SoC-2015-Ideas.html for more details about the > projects. > > We have to request slots before April 13th. Thanks, Matthieu, for keeping our GSoC activities moving forward. And thank you to all of the folks who have been helping review microprojects and proposals. If there are any other long-time community members who want to read and comment on the proposals, you can sign up through https://www.google-melange.com and request to be a mentor for git (you do not have to mentor, but that is the access level where you get to see the proposals). -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: [PATCHv2 4/4] git-p4: update locked test case
On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand wrote: > The add-new-file and copy-existing-file tests from > t9816-git-p4-locked.sh now pass. Update the test > case accordingly. This patch should be folded into patch 3/4, shouldn't it? > Signed-off-by: Luke Diamand > --- > t/t9816-git-p4-locked.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh > index 464f10b..d048bd3 100755 > --- a/t/t9816-git-p4-locked.sh > +++ b/t/t9816-git-p4-locked.sh > @@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' ' > ) > ' > > -test_expect_failure 'add with lock not taken' ' > +test_expect_success 'add with lock not taken' ' > test_when_finished cleanup_git && > git p4 clone --dest="$git" //depot && > ( > @@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' ' > ) > ' > > -test_expect_failure 'copy with lock taken' ' > +test_expect_success 'copy with lock taken' ' > lock_in_another_client && > test_when_finished cleanup_git && > test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" && > -- > 2.3.0.rc1.30.g76afe74 -- 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: GSoC 2015: students proposals submitted, selection started
On Wed, Apr 1, 2015 at 9:19 AM, Matthieu Moy wrote: > Hi, > > Latest news from the google summer of code: students have completed their > proposals. We have 2 proposals for "convert scripts to builtins", and 4 for > "unify git branch, git tag and git for-each-ref" (plus some out-of-scope > proposals). See http://git.github.io/SoC-2015-Ideas.html for more details > about the projects. > > We have to request slots before April 13th. > > My guess is that we'll request 2 slots, and from previous discussions I don't > think we will have difficulties to find enough co-mentors. Still, if you're > interested in mentoring a project, now would be a good time to say so. > > Without committing to mentor a student, you can also join the Git > organisation on https://www.google-melange.com/ if you want to > see/review/rate/discuss students proposals. > > Stefan: you said you were interested in mentoring. If it's still the case, > then please create an account on melange and request the "mentor" role for > Git. I did say that though I'd guess a Co-mentor role would fit me better. I've seen Jeff wants to mentor the "Unifying git branch -l, git tag -l, and git for-each-ref" where there are 4 applicants for this topic. Though I guess we'd only accept one applicant here as the topic is quite the same all of them proposed. There are 2 proposals to rewrite shell scripts to builtins, which Dscho is interested in mentoring one of them, which I'd like to co-mentor then. The other proposals do not look as convincing to me as I'd step up as a main mentor. Stefan > > Regards, > > -- > 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: [PATCHv2 1/4] git-p4: fix small bug in locked test scripts
On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand wrote: > Test script t9816-git-p4-locked.sh test #4 tests for > adding a file that is locked by Perforce automatially. s/automatially/automatically/ > This is currently not supported by git-p4 and so is > expected to fail. > > However, a small typo meant it always failed, even with > a fixed git-p4. Fix the typo to resolve this. > > Signed-off-by: Luke Diamand > --- > t/t9816-git-p4-locked.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh > index e71e543..ce0eb22 100755 > --- a/t/t9816-git-p4-locked.sh > +++ b/t/t9816-git-p4-locked.sh > @@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' ' > ( > cd "$git" && > echo line1 >>add-lock-not-taken && > - git add file2 && > + git add add-lock-not-taken && > git commit -m "add add-lock-not-taken" && > git config git-p4.skipSubmitEdit true && > git p4 submit --verbose > -- > 2.3.0.rc1.30.g76afe74 -- 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
[PATCHv2 3/4] git-p4: fix filetype detection on files opened exclusively
From: "Holloway, Blair"
If a Perforce server is configured to automatically set +l (exclusive lock) on
add of certain file types, git p4 submit will fail during getP4OpenedType, as
the regex doesn't expect the trailing '*exclusive*' from p4 opened:
//depot/file.png#1 - add default change (binary+l) *exclusive*
Signed-off-by: Blair Holloway
Acked-by: Luke Diamand
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index ff132b2..d43482a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -368,7 +368,7 @@ def getP4OpenedType(file):
# Returns the perforce file type for the given file.
result = p4_read_pipe(["opened", wildcard_encode(file)])
-match = re.match(".*\((.+)\)\r?$", result)
+match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result)
if match:
return match.group(1)
else:
--
2.3.0.rc1.30.g76afe74
--
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
[PATCHv2 1/4] git-p4: fix small bug in locked test scripts
Test script t9816-git-p4-locked.sh test #4 tests for adding a file that is locked by Perforce automatially. This is currently not supported by git-p4 and so is expected to fail. However, a small typo meant it always failed, even with a fixed git-p4. Fix the typo to resolve this. Signed-off-by: Luke Diamand --- t/t9816-git-p4-locked.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh index e71e543..ce0eb22 100755 --- a/t/t9816-git-p4-locked.sh +++ b/t/t9816-git-p4-locked.sh @@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' ' ( cd "$git" && echo line1 >>add-lock-not-taken && - git add file2 && + git add add-lock-not-taken && git commit -m "add add-lock-not-taken" && git config git-p4.skipSubmitEdit true && git p4 submit --verbose -- 2.3.0.rc1.30.g76afe74 -- 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
[PATCHv2 4/4] git-p4: update locked test case
The add-new-file and copy-existing-file tests from t9816-git-p4-locked.sh now pass. Update the test case accordingly. Signed-off-by: Luke Diamand --- t/t9816-git-p4-locked.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh index 464f10b..d048bd3 100755 --- a/t/t9816-git-p4-locked.sh +++ b/t/t9816-git-p4-locked.sh @@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' ' ) ' -test_expect_failure 'add with lock not taken' ' +test_expect_success 'add with lock not taken' ' test_when_finished cleanup_git && git p4 clone --dest="$git" //depot && ( @@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' ' ) ' -test_expect_failure 'copy with lock taken' ' +test_expect_success 'copy with lock taken' ' lock_in_another_client && test_when_finished cleanup_git && test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" && -- 2.3.0.rc1.30.g76afe74 -- 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
[PATCHv2 0/4] git-p4: fix filetype detection on files opened exclusively
This is a followup series to Blair's patch to fix filetype detection on files opened exclusively. It updates the git-p4 unit tests to catchup with that fix, fixing a couple of small bugs in the original tests. Holloway, Blair (1): git-p4: fix filetype detection on files opened exclusively Luke Diamand (3): git-p4: fix small bug in locked test scripts git-p4: small fix for locked-file-move-test git-p4: update locked test case git-p4.py| 2 +- t/t9816-git-p4-locked.sh | 10 +- 2 files changed, 6 insertions(+), 6 deletions(-) -- 2.3.0.rc1.30.g76afe74 -- 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
[PATCHv2 2/4] git-p4: small fix for locked-file-move-test
The test for handling of failure when trying to move a file that is locked by another client was not quite correct - it failed early on because the target file in the move already existed. The test now fails because git-p4 does not properly detect that p4 has rejected the move, and instead just crashes. At present, git-p4 has no support for detecting that a file has been locked and reporting it to the user, so this is the expected outcome. Signed-off-by: Luke Diamand --- t/t9816-git-p4-locked.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh index ce0eb22..464f10b 100755 --- a/t/t9816-git-p4-locked.sh +++ b/t/t9816-git-p4-locked.sh @@ -130,8 +130,8 @@ test_expect_failure 'move with lock taken' ' git p4 clone --dest="$git" //depot && ( cd "$git" && - git mv file1 file2 && - git commit -m "mv file1 to file2" && + git mv file1 file3 && + git commit -m "mv file1 to file3" && git config git-p4.skipSubmitEdit true && git config git-p4.detectRenames true && git p4 submit --verbose -- 2.3.0.rc1.30.g76afe74 -- 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/4] diff --cc: relax path filtering
On Thu, Apr 02, 2015 at 05:13:10PM -0400, Jeff King wrote:
> On Thu, Apr 02, 2015 at 11:34:09PM +0300, Max Kirillov wrote:
>
>> For diff --cc, paths fitering used to select only paths which have
>> changed in all parents, while diffing itself output hunks which are
>> changed in as few as 2 parents.
>
> I'm confused about "used to" here. Is this a regression due to the
> combine-diff rewriting that happened in 2.0, or do you just mean "before
> this patch series, we used to do this other thing".
As far as I can see it was "always", at least since 1.8.0;
the test script did not work before that.
>> Fix intersect_paths() to add paths which have at least 2 changed
>> parents.
>
> I'd worry a little that this is increasing the cost to do "log --cc", as
> it means we will have to open and look at extra files, and we may find
> in many cases that there aren't any interesting hunks. Which would imply
> we might want to put it behind a flag, rather than as the default
> ("--cc-me-harder").
>
> But if I'm understanding the issue correctly, this should only matter
> for octopus merges. That is, the old rule for looking at a path was "is
> there at least one parent whose content we took verbatim", but the new
> one is "are there are at least 2 parents whose content we did not take
> verbatim". With only two parents, those would be the same thing, I
> think.
Yes, I hope so. I tried to reproduce benchamrk which is in 8518ff8fabc
(git log --raw --no-abbrev --no-renames (-c|--cc) v3.10..v3.11),
and saw no difference. But my times was about 3 seconds, not
20 as there, andI cannot say my computer is very fast, so
probably I've done something wrong.
--
Max
--
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 v6 2/4] cat-file: teach cat-file a '--literally' option
On 04/03/2015 12:38 AM, Junio C Hamano wrote:
Karthik Nayak writes:
Currently 'git cat-file' throws an error while trying to
print the type or size of a broken/corrupt object which is
created using 'git hash-object --literally'. This is
because these objects are usually of unknown types.
Teach git cat-file a '--literally' option where it prints
the type or size of a broken/corrupt object without throwing
an error.
Modify '-t' and '-s' options to call sha1_object_info_extended()
directly to support the '--literally' option.
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
s/$/.com/ perhaps?
Definitely!
Signed-off-by: Karthik Nayak
---
builtin/cat-file.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df99df4..91ceae0 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -9,13 +9,20 @@
#include "userdiff.h"
#include "streaming.h"
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+ int literally)
{
unsigned char sha1[20];
enum object_type type;
char *buf;
unsigned long size;
struct object_context obj_context;
+ struct object_info oi = {NULL};
+ struct strbuf sb = STRBUF_INIT;
+ unsigned flags = LOOKUP_REPLACE_OBJECT;
+
+ if (literally)
+ flags |= LOOKUP_LITERALLY;
if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
die("Not a valid object name %s", obj_name);
@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type,
const char *obj_name)
buf = NULL;
switch (opt) {
case 't':
- type = sha1_object_info(sha1, NULL);
- if (type > 0) {
- printf("%s\n", typename(type));
+ oi.typep = &type;
+ oi.typename = &sb;
+ if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+ die("git cat-file: could not get object info");
+ if (type >= 0 && sb.len) {
+ printf("%s\n", sb.buf);
+ strbuf_release(&sb);
return 0;
}
break;
case 's':
- type = sha1_object_info(sha1, &size);
- if (type > 0) {
+ oi.typep = &type;
+ oi.typename = &sb;
+ oi.sizep = &size;
+ if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+ die("git cat-file: could not get object info");
+ if (type >= 0 && sb.len) {
printf("%lu\n", size);
return 0;
}
@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)
}
static const char * const cat_file_usage[] = {
- N_("git cat-file (-t | -s | -e | -p | | --textconv) "),
+ N_("git cat-file (-t [--literally]|-s [--literally]|-e|-p||--textconv)
"),
N_("git cat-file (--batch | --batch-check) < "),
NULL
};
@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
struct batch_options batch = {0};
+ int literally = 0;
const struct option options[] = {
OPT_GROUP(N_(" can be one of: blob, tree, commit, tag")),
@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's
content"), 'p'),
OPT_SET_INT(0, "textconv", &opt,
N_("for blob objects, run textconv on object's
content"), 'c'),
+ OPT_BOOL( 0, "literally", &literally,
+ N_("get information about corrupt objects for debugging
Git")),
{ OPTION_CALLBACK, 0, "batch", &batch, "format",
N_("show info and content of objects fed from the standard
input"),
PARSE_OPT_OPTARG, batch_option_callback },
@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
git_config(git_cat_file_config, NULL);
- if (argc != 3 && argc != 2)
+ if (argc < 2 || argc > 4)
usage_with_options(cat_file_usage, options);
argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
@@ -405,5 +423,7 @@ int cmd_cat_file(int argc, const char **argv, const char
*prefix)
if (batch.enabled)
return batch_objects(&batch);
- return cat_one_file(opt, exp_type, obj_name);
+ if (literally && opt != 't' && opt != 's')
+ die("git cat-file --literally: use with -s or -t");
+ return cat_one_file(opt, exp_type, obj_name, literally
[PATCH v2 4/4] t4059: rewrite to be adaptive to hunk filtering
Looks like there is no exact specification what `diff -c` and
`diff --cc` should output. Considering this it is not reasonable to
hardcode any specific output in test. Rather, it should verify that file
selection behaves the same as hunk selection.
Rewrite test so that it makes sure that a "short" file is shown if and
only if the corresponding "long" file's contains the changed hunk.
Signed-off-by: Max Kirillov
---
t/t4059-diff-cc-not-affected-by-path-filtering.sh | 84 ---
1 file changed, 59 insertions(+), 25 deletions(-)
diff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh
b/t/t4059-diff-cc-not-affected-by-path-filtering.sh
index ab3dbd2..f4232b0 100755
--- a/t/t4059-diff-cc-not-affected-by-path-filtering.sh
+++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh
@@ -6,11 +6,20 @@ test_description='combined diff filtering is not affected by
preliminary path fi
# spot changes which were discarded during conflict resolution.
. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+# history is:
+# (mergebase) --> (branch1) --\
+# | V
+# \ --> (branch2) --(merge)
+# there are files in 2 subdirectories, "long" and "short"
+# each file in "long" subdirecty has exactly same history as same file in
"short" one,
+# but it has added lines with changes in both branches which merge without
conflict
+# so the long files are always selected at path filtering
test_expect_success setup '
mkdir short &&
mkdir long &&
- for fn in win1 win2 merge delete base only1 only2
+ for fn in win1 win2 merge delete base only1 only2 only1discard
only2discard
do
test_seq 3 >short/$fn &&
git add short/$fn &&
@@ -29,9 +38,12 @@ test_expect_success setup '
git add $dir/$fn || return $?
done || return $?
done &&
- sed -e "s/^7/7change1/" long/only2 >sed.new &&
- mv sed.new long/only2 &&
- git add long/only2 &&
+ for fn in only2 only2discard
+ do
+ sed -e "s/^7/7change1/" long/$fn >sed.new &&
+ mv sed.new long/$fn &&
+ git add long/$fn || return $?
+ done &&
git commit -m branch1 &&
git branch branch1 &&
@@ -45,9 +57,12 @@ test_expect_success setup '
git add $dir/$fn || return $?
done || return $?
done &&
- sed -e "s/^11/11change2/" long/only1 >sed.new &&
- mv sed.new long/only1 &&
- git add long/only1 &&
+ for fn in only1 only1discard
+ do
+ sed -e "s/^11/11change2/" long/$fn >sed.new &&
+ mv sed.new long/$fn &&
+ git add long/$fn || return $?
+ done &&
git commit -m branch2 &&
git branch branch2 &&
@@ -55,6 +70,10 @@ test_expect_success setup '
git checkout mergebase -- . &&
test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" >long/base
&&
git add long/base &&
+ test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/"
>long/only1discard &&
+ git add long/only1discard &&
+ test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/"
>long/only2discard &&
+ git add long/only2discard &&
test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e
"s/^2/2change1/" >long/win1 &&
git add long/win1 &&
test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e
"s/^2/2change2/" >long/win2 &&
@@ -69,6 +88,10 @@ test_expect_success setup '
git add long/only2 &&
test_seq 3 >short/base &&
git add short/base &&
+ test_seq 3 >short/only1discard &&
+ git add short/only1discard &&
+ test_seq 3 >short/only2discard &&
+ git add short/only2discard &&
test_seq 3 | sed -e "s/^2/2change1/" >short/win1 &&
git add short/win1 &&
test_seq 3 | sed -e "s/^2/2change2/" >short/win2 &&
@@ -85,24 +108,35 @@ test_expect_success setup '
git branch merge
'
-test_expect_success 'diff with mergebase shows discarded change from parent 2
in merged file' '
- git diff --cc merge branch1 branch2 mergebase -- long/win1 >actual &&
- test -s actual
-'
-
-test_expect_success 'diff with mergebase shows discarded change from parent 1
in merged file' '
- git diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &&
- test -s actual
-'
+# the difference in short file must be returned if and only if it is shown in
long file
+for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do
+ if git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q
'^[ +-]\{3\}2\(change[12]|merge\)\?$'
+ then
+ test_expect_success "diff --cc contains short/$fn" '
+ git diff --cc merge branch1 branch2 mergebase --
short/'"$fn"' >actual &&
+ test -s actual
+ '
+ else
+ t
Re: [PATCH 1/4] Add test for showing discarded changes with diff --cc
On Thu, Apr 02, 2015 at 01:55:58PM -0700, Junio C Hamano wrote: > So I'll give a preliminary nitpicks first ;-) Thank you; fixed the issues you mentioned. >> + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e >> "s/^2/2change2/" >long/only2 && >> +git add long/only2 && > > Patch is line-wrapped around here? I hope it was you email program which wrapped it (maybe at quoting). It looks ok in my mutt and in source, and I uses same git format-patch+git send-email as usual. -- Max -- 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 2/4] combine-diff.c: refactor: extract insert_path()
Signed-off-by: Max Kirillov
---
combine-diff.c | 43 ---
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 8eb7278..a236bb5 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -22,6 +22,28 @@ static int compare_paths(const struct combine_diff_path *one,
two->path, strlen(two->path), two->mode);
}
+static void insert_path(struct combine_diff_path **pos, const char* path, int
n, int num_parent, struct diff_filepair *queue_item)
+{
+ int len;
+ struct combine_diff_path *p;
+
+ len = strlen(path);
+ p = xmalloc(combine_diff_path_size(num_parent, len));
+ p->path = (char *) &(p->parent[num_parent]);
+ memcpy(p->path, path, len);
+ p->path[len] = 0;
+ p->next = *pos;
+ memset(p->parent, 0,
+ sizeof(p->parent[0]) * num_parent);
+
+ hashcpy(p->oid.hash, queue_item->two->sha1);
+ p->mode = queue_item->two->mode;
+ hashcpy(p->parent[n].oid.hash, queue_item->one->sha1);
+ p->parent[n].mode = queue_item->one->mode;
+ p->parent[n].status = queue_item->status;
+ *pos = p;
+}
+
static struct combine_diff_path *intersect_paths(struct combine_diff_path
*curr, int n, int num_parent)
{
struct diff_queue_struct *q = &diff_queued_diff;
@@ -30,27 +52,10 @@ static struct combine_diff_path *intersect_paths(struct
combine_diff_path *curr,
if (!n) {
for (i = 0; i < q->nr; i++) {
- int len;
- const char *path;
if (diff_unmodified_pair(q->queue[i]))
continue;
- path = q->queue[i]->two->path;
- len = strlen(path);
- p = xmalloc(combine_diff_path_size(num_parent, len));
- p->path = (char *) &(p->parent[num_parent]);
- memcpy(p->path, path, len);
- p->path[len] = 0;
- p->next = NULL;
- memset(p->parent, 0,
- sizeof(p->parent[0]) * num_parent);
-
- hashcpy(p->oid.hash, q->queue[i]->two->sha1);
- p->mode = q->queue[i]->two->mode;
- hashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);
- p->parent[n].mode = q->queue[i]->one->mode;
- p->parent[n].status = q->queue[i]->status;
- *tail = p;
- tail = &p->next;
+ insert_path(tail, q->queue[i]->two->path, n,
num_parent, q->queue[i]);
+ tail = &(*tail)->next;
}
return curr;
}
--
2.3.4.2801.g3d0809b
--
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 1/4] t4059: test 'diff --cc' with a change from only few parents
If `git diff --cc` is used with 2 or more parents, then it shows all hunks which have changed compared to at least 2 parents. Which is reasonable, because those places are likely places for conflicts, and it should be displayed how they were resolved. But, preliminary path filtering passes a path only it was changed compared to each parent. So, if a hunk which has not changed compared to some of parents is shown if there are more changed hunks in the file, but not shown if it is the only change. This looks inconsistent and for some scenarios it is desirable to show such changes. Add the test which demonstrates the issue. Signed-off-by: Max Kirillov --- t/t4059-diff-cc-not-affected-by-path-filtering.sh | 108 ++ 1 file changed, 108 insertions(+) create mode 100755 t/t4059-diff-cc-not-affected-by-path-filtering.sh diff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh b/t/t4059-diff-cc-not-affected-by-path-filtering.sh new file mode 100755 index 000..3e6e59b --- /dev/null +++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh @@ -0,0 +1,108 @@ +#!/bin/sh + +test_description='combined diff filtering is not affected by preliminary path filtering' +# Since diff --cc allows use not only real parents but any commits, use merge +# base here as the 3rd "parent". The trick was suggested in $gmane/191557 to +# spot changes which were discarded during conflict resolution. + +. ./test-lib.sh + +test_expect_success setup ' + mkdir short && + mkdir long && + for fn in win1 win2 merge delete base only1 only2 + do + test_seq 3 >short/$fn && + git add short/$fn && + test_seq 11 >long/$fn && + git add long/$fn || return $? + done && + git commit -m mergebase && + git branch mergebase && + + for fn in win1 win2 merge delete base only1 + do + for dir in short long + do + sed -e "s/^2/2change1/" -e "s/^7/7change1/" $dir/$fn >sed.new && + mv sed.new $dir/$fn && + git add $dir/$fn || return $? + done || return $? + done && + sed -e "s/^7/7change1/" long/only2 >sed.new && + mv sed.new long/only2 && + git add long/only2 && + git commit -m branch1 && + git branch branch1 && + + git reset --hard mergebase && + for fn in win1 win2 merge delete base only2 + do + for dir in short long + do + sed -e "s/^2/2change2/" -e "s/^11/11change2/" $dir/$fn >sed.new && + mv sed.new $dir/$fn && + git add $dir/$fn || return $? + done || return $? + done && + sed -e "s/^11/11change2/" long/only1 >sed.new && + mv sed.new long/only1 && + git add long/only1 && + git commit -m branch2 && + git branch branch2 && + + test_must_fail git merge branch1 && + git checkout mergebase -- . && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" >long/base && + git add long/base && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "s/^2/2change1/" >long/win1 && + git add long/win1 && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "s/^2/2change2/" >long/win2 && + git add long/win2 && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "s/^2/2merged/" >long/merge && + git add long/merge && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "/^2/d" >long/delete && + git add long/delete && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "s/^2/2change1/" >long/only1 && + git add long/only1 && + test_seq 11 | sed -e "s/^7/7change1/" -e "s/^11/11change2/" -e "s/^2/2change2/" >long/only2 && + git add long/only2 && + test_seq 3 >short/base && + git add short/base && + test_seq 3 | sed -e "s/^2/2change1/" >short/win1 && + git add short/win1 && + test_seq 3 | sed -e "s/^2/2change2/" >short/win2 && + git add short/win2 && + test_seq 3 | sed -e "s/^2/2merged/" >short/merge && + git add short/merge && + test_seq 3 | sed -e "/^2/d" >short/delete && + git add short/delete && + test_seq 3 | sed -e "s/^2/2change1/" >short/only1 && + git add short/only1 && + test_seq 3 | sed -e "s/^2/2change2/" >short/only2 && + git add short/only2 && + git commit -m merge && + git branch merge +' + +test_expect_success 'diff with mergebase shows discarded change from parent 2 in merged file' ' + git diff --cc merge branch1 branch2 mergebase -- long/win1 >actual && + test -s actual +' + +test_expect_success 'diff with mergebase shows discarded change from parent 1 in merged file' ' + git diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &
[PATCH v2 0/4] diff --cc: relax path filtering
Fixes: * test renamed and commit message rewritten, so that it mentions 3-parent merge rather than other uses * test: fix && chaining and do at new line * use postincrement in C code Max Kirillov (4): t4059: test 'diff --cc' with a change from only few parents combine-diff.c: refactor: extract insert_path() diff --cc: relax too strict paths picking t4059: rewrite to be adaptive to hunk filtering combine-diff.c| 95 +-- t/t4059-diff-cc-not-affected-by-path-filtering.sh | 142 ++ 2 files changed, 203 insertions(+), 34 deletions(-) create mode 100755 t/t4059-diff-cc-not-affected-by-path-filtering.sh -- 2.3.4.2801.g3d0809b -- 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 3/4] diff --cc: relax too strict paths picking
For diff --cc, paths fitering used to select only paths which have
changed in all parents, while diffing itself output hunks which are
changed in as few as 2 parents.
Fix intersect_paths() to add paths which have at least 2 changed
parents. In this new functionality does not require special treatment of
first pass, because path can be added from any parent, not just the
first one.
Signed-off-by: Max Kirillov
---
combine-diff.c| 56 ---
t/t4059-diff-cc-not-affected-by-path-filtering.sh | 4 +-
2 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index a236bb5..2285c7c 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -25,6 +25,7 @@ static int compare_paths(const struct combine_diff_path *one,
static void insert_path(struct combine_diff_path **pos, const char* path, int
n, int num_parent, struct diff_filepair *queue_item)
{
int len;
+ int parent_idx;
struct combine_diff_path *p;
len = strlen(path);
@@ -41,43 +42,64 @@ static void insert_path(struct combine_diff_path **pos,
const char* path, int n,
hashcpy(p->parent[n].oid.hash, queue_item->one->sha1);
p->parent[n].mode = queue_item->one->mode;
p->parent[n].status = queue_item->status;
+ for (parent_idx = 0; parent_idx < n; parent_idx++) {
+ hashcpy(p->parent[parent_idx].oid.hash, p->oid.hash);
+ p->parent[parent_idx].mode = p->mode;
+ p->parent[parent_idx].status = ' ';
+ }
*pos = p;
}
+static int changed_parents(struct combine_diff_path *p, int n)
+{
+ int parent_idx;
+ int result = 0;
+
+ for (parent_idx = 0; parent_idx < n; parent_idx++) {
+ if (p->parent[parent_idx].status != ' ')
+ result++;
+ }
+
+ return result;
+}
+
static struct combine_diff_path *intersect_paths(struct combine_diff_path
*curr, int n, int num_parent)
{
struct diff_queue_struct *q = &diff_queued_diff;
struct combine_diff_path *p, **tail = &curr;
int i, cmp;
- if (!n) {
- for (i = 0; i < q->nr; i++) {
- if (diff_unmodified_pair(q->queue[i]))
- continue;
- insert_path(tail, q->queue[i]->two->path, n,
num_parent, q->queue[i]);
- tail = &(*tail)->next;
- }
- return curr;
- }
-
/*
* paths in curr (linked list) and q->queue[] (array) are
* both sorted in the tree order.
*/
i = 0;
- while ((p = *tail) != NULL) {
- cmp = ((i >= q->nr)
- ? -1 : compare_paths(p, q->queue[i]->two));
+ while ((p = *tail) != NULL || i < q->nr) {
+ cmp = (i >= q->nr) ? -1
+ : (p == NULL) ? 1
+ : compare_paths(p, q->queue[i]->two);
if (cmp < 0) {
- /* p->path not in q->queue[]; drop it */
- *tail = p->next;
- free(p);
+ /* p->path not in q->queue[] */
+ if (num_parent > 2 && 2 - changed_parents(p, n) <=
num_parent - n - 1) {
+ /* still can get 2 changed parents */
+ hashcpy(p->parent[n].oid.hash, p->oid.hash);
+ p->parent[n].mode = p->mode;
+ p->parent[n].status = ' ';
+ tail = &p->next;
+ } else {
+ *tail = p->next;
+ free(p);
+ }
continue;
}
if (cmp > 0) {
- /* q->queue[i] not in p->path; skip it */
+ /* q->queue[i] not in p->path */
+ if (1 <= num_parent - n - 1) {
+ insert_path(tail, q->queue[i]->two->path, n,
num_parent, q->queue[i]);
+ tail = &(*tail)->next;
+ }
i++;
continue;
}
diff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh
b/t/t4059-diff-cc-not-affected-by-path-filtering.sh
index 3e6e59b..ab3dbd2 100755
--- a/t/t4059-diff-cc-not-affected-by-path-filtering.sh
+++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh
@@ -95,12 +95,12 @@ test_expect_success 'diff with mergebase shows discarded
change from parent 1 in
test -s actual
'
-test_expect_failure 'diff with mergebase shows fully discarded file from
parent 2' '
+test_expect_success 'diff with mergebase shows fully discarded file from
parent 2' '
git diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&
test -s actual
'
-test_expect_failure 'd
Re: [PATCH] improve documentation for some commands that use pathspecs
On Tue, Mar 31, 2015 at 10:22 PM, Joey Hess wrote: > After being surprised that git-ls-files expands pathspecs, here's a patch > that would have saved me. > --- > Documentation/git-ls-files.txt | 9 + > Documentation/git-ls-tree.txt | 8 > 2 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > index e26f01f..f7a3039 100644 > --- a/Documentation/git-ls-files.txt > +++ b/Documentation/git-ls-files.txt > @@ -17,7 +17,7 @@ SYNOPSIS > [--exclude-per-directory=] > [--exclude-standard] > [--error-unmatch] [--with-tree=] > - [--full-name] [--abbrev] [--] [...] > + [--full-name] [--abbrev] [--] [...] Just a note that "git ls-files -h" still says "", from ls_files_usage in builtin/ls-files.c -- 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] improve documentation for some commands that use pathspecs
On Tue, Mar 31, 2015 at 10:22 PM, Joey Hess wrote: > After being surprised that git-ls-files expands pathspecs, here's a patch > that would have saved me. > --- > Documentation/git-ls-files.txt | 9 + > Documentation/git-ls-tree.txt | 8 ls-tree only supports straight file or directory paths, no globbing or other magic (except ":/") so I'm not sure if we should change its man page now. On the bright side, if you use unsupported pathspec feature on ls-tree, it'll reject, so not so bad. -- 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] t1509: update prepare script to be able to run t1509 in chroot again
On Fri, Apr 3, 2015 at 7:01 PM, Jeff King wrote: > Aside from the nits above, I did get it to run t1509 with this. I can't > say I'm incredibly excited about the whole thing, if only because it is > clear that nobody is going to run it regularly (so regressions will > likely go unnoticed, which is the whole point of the test script). But > perhaps it is better than nothing, and it is not hurting anyone to sit > there and bitrot again. ;) To be honest, I didn't run it often in the last 5 years. The code it protects seems not broken during this time and probably so for a foreseeable future. So I don't mind if you just kill t1509 and related scripts. -- 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] t1509: update prepare script to be able to run t1509 in chroot again
On Fri, Apr 03, 2015 at 05:08:57PM +0700, Nguyễn Thái Ngọc Duy wrote:
> diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
> index 6269117..c61afbf 100755
> --- a/t/t1509/prepare-chroot.sh
> +++ b/t/t1509/prepare-chroot.sh
> @@ -14,25 +14,42 @@ xmkdir() {
>
> R="$1"
>
> +[ "$UID" -eq 0 ] && die "This script should not be run as root, what if it
> does rm -rf /?"
This line complains for me. $UID is set in my bash login shell, but not
in the dash shell for this script. Maybe "id -u" instead?
> xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
> -[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1
> 3 && chmod 666 $R/dev/null"
> +touch "$R/dev/null"
It is nice not to mknod here, as it requires root. But making /dev/null
a regular file seems a bit flaky. We will constantly overwrite it with
the output of each test, and then pipe that output back into the next
test. The current set of tests in t1509 does not seem to mind, though.
> +# Fake perl to reduce dependency, t1509 does not use perl, but some
> +# env might slip through, see test-lib.sh, unset.*PERL_PATH
> +sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS >
> "$R$(pwd)/GIT-BUILD-OPTIONS"
Re-preparing an already-made chroot makes this:
PERL_PATH=/bin/true'/usr/bin/perl'
Did you want "PERL_PATH=.*" as your regex?
> -echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
> +cat < +Execute this in root:
> +chroot $R /bin/su - $(id -nu)
> +IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i
> +EOF
I found the "in root" here (and in the original) confusing. Do you mean
"as root"? I wonder if it would make sense to just show:
sudo chroot $R /bin/su - $(id -nu)
as the sample command.
Aside from the nits above, I did get it to run t1509 with this. I can't
say I'm incredibly excited about the whole thing, if only because it is
clear that nobody is going to run it regularly (so regressions will
likely go unnoticed, which is the whole point of the test script). But
perhaps it is better than nothing, and it is not hurting anyone to sit
there and bitrot again. ;)
-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 v3] config.c: split some variables to $GIT_DIR/config.worktree
On Thu, Apr 2, 2015 at 3:56 AM, Max Kirillov wrote:
> On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> The general principle is like in the last mail: .git/config is for
>> both shared and private keys of main worktree (i.e. nothing is
>> changed from today). .git/worktrees/xx/config.worktree is for
>> private keys only (and private keys in .git/config are ignored)
>>
>> With this we don't have to bump core.repository_format_version for
>> main worktree because nothing is changed. There will be problems
>> with info/config.worktree:
>>
>> - it's customizable, so expect the user to break it (*)
>
> I would rather say it's manual tuning rather than a break.
But if the user accidentally deletes core.worktree then something will break.
>> - if we add new stuff to the template, we'll need to help migrate
>> current info/core.worktree (which does not have new stuff).
>> Auto updating this file could be risky. I'm tend to just
>> warn the user that this and that keys should be included and let
>> them modify the file.
>
> I don't think there even should be warning. Just don't
> change the info/config.worktree in the existing repositories
> and let users extend it as they feel a need for it.
>
> Later there could be a tool (an option to git config for
> example) which adds a variable or pattern to the
> info/core.worktree and copied existing variable(s) from
> commong config to worktree-specific ones.
I was thinking "git checkout --to" would do this. It checks if the
main worktree has info/core.worktree, if not, re-init the repo to add
this file from default template.
>> @@ -1932,6 +2002,23 @@ int git_config_set_multivar_in_file(const char
>> *config_filename,
>>
>> store.multi_replace = multi_replace;
>>
>> + if (git_common_dir_env && is_config_local(key)) {
>> + if (!config_filename)
>> + config_filename = filename_buf =
>> git_pathdup("config.worktree");
>> + /* cheap trick, but should work 90% of time */
>> + else if (!ends_with(config_filename, ".worktree"))
>> + die("%s can only be stored in %s",
>> + key, git_path("config.worktree"));
>
> Is `config_filename` set only for cases when config file is
> explicitly set for "git config" command with "--file"
> option? Then probably here should not be any intelligence at
> all; if user resort to manual picking the file he must be
> allowed to do stupid things.
Yes. It may make sense in the previous versions where this feature
could work in a normal worktree, but when it becomes
multiworktree-specific, I guess we can drop this.
--
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] t1509: update prepare script to be able to run t1509 in chroot again
Tested on Gentoo and OpenSUSE 13.1, both x86-64
Signed-off-by: Nguyễn Thái Ngọc Duy
---
On Wed, Apr 1, 2015 at 2:14 AM, Jonathan Nieder wrote:
> Jeff King wrote:
>
>> No tests, as we would need to be able to write to "/" to do
>> so.
>
> t1509-root-worktree.sh is supposed to test the repository-at-/ case.
> But I wouldn't be surprised if it's bitrotted, since people don't set
> up a throwaway chroot or VM for tests too often.
Can't leave it rotting. Either fix it or kill it. This is the first
option. Good news is the test passes, nothing else is broken. Bad
news is it does not detect the core.worktree breakage, but this on
top would verify that Jeff's patch works
diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
index b6977d4..17ff4ce 100755
--- a/t/t1509-root-worktree.sh
+++ b/t/t1509-root-worktree.sh
@@ -224,6 +224,10 @@ test_expect_success 'setup' '
test_cmp expected result
'
+test_expect_success 'no core.worktree after git init' '
+ test "`git config core.worktree`" = ""
+'
+
test_vars 'auto gitdir, root' ".git" "/" ""
test_foobar_root
t/t1509/prepare-chroot.sh | 35 ++-
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
index 6269117..c61afbf 100755
--- a/t/t1509/prepare-chroot.sh
+++ b/t/t1509/prepare-chroot.sh
@@ -14,25 +14,42 @@ xmkdir() {
R="$1"
+[ "$UID" -eq 0 ] && die "This script should not be run as root, what if it
does rm -rf /?"
[ -n "$R" ] || die "usage: prepare-chroot.sh "
[ -x git ] || die "This script needs to be executed at git source code's top
directory"
-[ -x /bin/busybox ] || die "You need busybox"
+if [ -x /bin/busybox ]; then
+ BB=/bin/busybox
+elif [ -x /usr/bin/busybox ]; then
+ BB=/usr/bin/busybox
+else
+ die "You need busybox"
+fi
xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
-[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3
&& chmod 666 $R/dev/null"
+touch "$R/dev/null"
echo "root:x:0:0:root:/:/bin/sh" > "$R/etc/passwd"
echo "$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh" >> "$R/etc/passwd"
echo "root::0:root" > "$R/etc/group"
echo "$(id -ng)::$(id -g):$(id -nu)" >> "$R/etc/group"
-[ -x "$R/bin/busybox" ] || cp /bin/busybox "$R/bin/busybox"
-[ -x "$R/bin/sh" ] || ln -s /bin/busybox "$R/bin/sh"
-[ -x "$R/bin/su" ] || ln -s /bin/busybox "$R/bin/su"
+[ -x "$R$BB" ] || cp $BB "$R/bin/busybox"
+for cmd in sh su ls expr tr basename rm mkdir mv id uname dirname cat true sed
diff; do
+ ln -f -s /bin/busybox "$R/bin/$cmd"
+done
mkdir -p "$R$(pwd)"
rsync --exclude-from t/t1509/excludes -Ha . "$R$(pwd)"
-ldd git | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
- mkdir -p "$R$(dirname $i)"
- cp "$i" "$R/$i"
+# Fake perl to reduce dependency, t1509 does not use perl, but some
+# env might slip through, see test-lib.sh, unset.*PERL_PATH
+sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS >
"$R$(pwd)/GIT-BUILD-OPTIONS"
+for cmd in git $BB;do
+ ldd $cmd | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
+ mkdir -p "$R$(dirname $i)"
+ cp "$i" "$R/$i"
+ done
done
-echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
+cat
