Re: [PATCH v4 4/4] mergetool: honor -O

2016-10-08 Thread Johannes Sixt
With this final fixup, the series looks good to me, and I have no 
further comments.


Reviewed-by: Johannes Sixt 

-- Hannes



Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

2016-10-08 Thread Johannes Schindelin
Hi Peff & Hannes,

On Fri, 7 Oct 2016, Jeff King wrote:

> On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote:
> 
> > Maybe it's time to aim for
> > 
> >   git config alias.d2u.shell \
> >'f() { git ls-files "$@" | xargs dos2unix; }; f'
> >   git config alias.d2u.cdup false
> >   git d2u *.c   # yada!
> 
> That would be nice. It would also allow "alias.foo_bar.shell"; right now
> "alias.foo_bar" is forbidden because of the config syntax, which does
> not allow underscores outside of the "subsection" name.

So what about this?

[alias]
d2u = !dos2unix
[alias "d2u"]
shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
exec = C:/cygwin64/bin/dos2unix.exe

You introduce all kinds of ambiguities here that did not exist before...

Ciao,
Dscho


Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data

2016-10-08 Thread Johannes Schindelin
Hi Junio,

On Thu, 6 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > If you prefer to accept such sloppy work, I will change it of course,
> > feeling dirty that it has my name on it.
> 
> I do want neither leaks nor sloppyness, and I thought that was
> understood by everybody, hence I thought the last round made it
> clear that _entrust() must not exist.

Well, you leave me with but one choice. With shame in my face I will send
out the next re-roll with this construct:

@@ -796,24 +1002,52 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
opts->allow_ff = git_config_bool_or_int(key, value, 
&error_flag);
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
-   else if (!strcmp(key, "options.strategy"))
-   git_config_string(&opts->strategy, key, value);
-   else if (!strcmp(key, "options.gpg-sign"))
-   git_config_string(&opts->gpg_sign, key, value);
+   else if (!strcmp(key, "options.strategy")) {
+   if (!value)
+   config_error_nonbool(key);
+   else {
+   free(opts->strategy);
+   opts->strategy = xstrdup(value);
+   }
+   }
+   else if (!strcmp(key, "options.gpg-sign")) {
+   if (!value)
+   config_error_nonbool(key);
+   else {
+   free(opts->gpg_sign);
+   opts->gpg_sign = xstrdup(value);
+   }
+   }
...
@@ -37,18 +26,32 @@ struct replay_opts {

int mainline;

-   const char *gpg_sign;
+   char *gpg_sign;

/* Merge strategy */
-   const char *strategy;
-   const char **xopts;
+   char *strategy;
+   char **xopts;

(this is not the complete diff, of course, but you get the idea.)

Ciao,
Dscho


Feature request: use relative path in worktree config files

2016-10-08 Thread Stéphane Klein
Hi,

"git worktree add" write absolute path in ".git/gitdir"

The code source is here
https://git.kernel.org/cgit/git/git.git/tree/builtin/worktree.c?h=v2.10.1#n256

Is it possible to use relative path in this config files:

* [main_worktree]/.git/worktrees/[worktree_foobar]/gitdir
* [worktree_foobar]/.git

Why:

1. I configure worktree on my host
2. next I use this git working copy in Docker with volume share
3. next I've some git error in Docker because config files use absolute path

Best regards,
Stéphane
-- 
Stéphane Klein 
blog: http://stephane-klein.info
cv : http://cv.stephane-klein.info
Twitter: http://twitter.com/klein_stephane


[PATCH v10 01/14] convert: quote filter names in error messages

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Git filter driver commands with spaces (e.g. `filter.sh foo`) are hard
to read in error messages. Quote them to improve the readability.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 convert.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index 077f5e6..986c239 100644
--- a/convert.c
+++ b/convert.c
@@ -412,7 +412,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)
child_process.out = out;
 
if (start_command(&child_process))
-   return error("cannot fork to run external filter %s", 
params->cmd);
+   return error("cannot fork to run external filter '%s'", 
params->cmd);
 
sigchain_push(SIGPIPE, SIG_IGN);
 
@@ -430,13 +430,13 @@ static int filter_buffer_or_fd(int in, int out, void 
*data)
if (close(child_process.in))
write_err = 1;
if (write_err)
-   error("cannot feed the input to external filter %s", 
params->cmd);
+   error("cannot feed the input to external filter '%s'", 
params->cmd);
 
sigchain_pop(SIGPIPE);
 
status = finish_command(&child_process);
if (status)
-   error("external filter %s failed %d", params->cmd, status);
+   error("external filter '%s' failed %d", params->cmd, status);
 
strbuf_release(&cmd);
return (write_err || status);
@@ -477,15 +477,15 @@ static int apply_filter(const char *path, const char 
*src, size_t len, int fd,
return 0;   /* error was already reported */
 
if (strbuf_read(&nbuf, async.out, len) < 0) {
-   error("read from external filter %s failed", cmd);
+   error("read from external filter '%s' failed", cmd);
ret = 0;
}
if (close(async.out)) {
-   error("read from external filter %s failed", cmd);
+   error("read from external filter '%s' failed", cmd);
ret = 0;
}
if (finish_async(&async)) {
-   error("external filter %s failed", cmd);
+   error("external filter '%s' failed", cmd);
ret = 0;
}
 
-- 
2.10.0



[PATCH v10 02/14] convert: modernize tests

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Use `test_config` to set the config, check that files are empty with
`test_must_be_empty`, compare files with `test_cmp`, and remove spaces
after ">" and "<".

Please note that the "rot13" filter configured in "setup" keeps using
`git config` instead of `test_config` because subsequent tests might
depend on it.

Reviewed-by: Stefan Beller 
Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 t/t0021-conversion.sh | 58 +--
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e799e59..dc50938 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -38,8 +38,8 @@ script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
 
 test_expect_success check '
 
-   cmp test.o test &&
-   cmp test.o test.t &&
+   test_cmp test.o test &&
+   test_cmp test.o test.t &&
 
# ident should be stripped in the repository
git diff --raw --exit-code :test :test.i &&
@@ -47,10 +47,10 @@ test_expect_success check '
embedded=$(sed -ne "$script" test.i) &&
test "z$id" = "z$embedded" &&
 
-   git cat-file blob :test.t > test.r &&
+   git cat-file blob :test.t >test.r &&
 
-   ./rot13.sh < test.o > test.t &&
-   cmp test.r test.t
+   ./rot13.sh test.t &&
+   test_cmp test.r test.t
 '
 
 # If an expanded ident ever gets into the repository, we want to make sure that
@@ -130,7 +130,7 @@ test_expect_success 'filter shell-escaped filenames' '
 
# delete the files and check them out again, using a smudge filter
# that will count the args and echo the command-line back to us
-   git config filter.argc.smudge "sh ./argc.sh %f" &&
+   test_config filter.argc.smudge "sh ./argc.sh %f" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
 
@@ -141,7 +141,7 @@ test_expect_success 'filter shell-escaped filenames' '
test_cmp expect "$special" &&
 
# do the same thing, but with more args in the filter expression
-   git config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
+   test_config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
 
@@ -154,9 +154,9 @@ test_expect_success 'filter shell-escaped filenames' '
 '
 
 test_expect_success 'required filter should filter data' '
-   git config filter.required.smudge ./rot13.sh &&
-   git config filter.required.clean ./rot13.sh &&
-   git config filter.required.required true &&
+   test_config filter.required.smudge ./rot13.sh &&
+   test_config filter.required.clean ./rot13.sh &&
+   test_config filter.required.required true &&
 
echo "*.r filter=required" >.gitattributes &&
 
@@ -165,17 +165,17 @@ test_expect_success 'required filter should filter data' '
 
rm -f test.r &&
git checkout -- test.r &&
-   cmp test.o test.r &&
+   test_cmp test.o test.r &&
 
./rot13.sh expected &&
git cat-file blob :test.r >actual &&
-   cmp expected actual
+   test_cmp expected actual
 '
 
 test_expect_success 'required filter smudge failure' '
-   git config filter.failsmudge.smudge false &&
-   git config filter.failsmudge.clean cat &&
-   git config filter.failsmudge.required true &&
+   test_config filter.failsmudge.smudge false &&
+   test_config filter.failsmudge.clean cat &&
+   test_config filter.failsmudge.required true &&
 
echo "*.fs filter=failsmudge" >.gitattributes &&
 
@@ -186,9 +186,9 @@ test_expect_success 'required filter smudge failure' '
 '
 
 test_expect_success 'required filter clean failure' '
-   git config filter.failclean.smudge cat &&
-   git config filter.failclean.clean false &&
-   git config filter.failclean.required true &&
+   test_config filter.failclean.smudge cat &&
+   test_config filter.failclean.clean false &&
+   test_config filter.failclean.required true &&
 
echo "*.fc filter=failclean" >.gitattributes &&
 
@@ -197,8 +197,8 @@ test_expect_success 'required filter clean failure' '
 '
 
 test_expect_success 'filtering large input to small output should use little 
memory' '
-   git config filter.devnull.clean "cat >/dev/null" &&
-   git config filter.devnull.required true &&
+   test_config filter.devnull.clean "cat >/dev/null" &&
+   test_config filter.devnull.required true &&
for i in $(test_seq 1 30); do printf "%1048576d" 1; done >30MB &&
echo "30MB filter=devnull" >.gitattributes &&
GIT_MMAP_LIMIT=1m GIT_ALLOC_LIMIT=1m git add 30MB
@@ -207,7 +207,7 @@ test_expect_success 'filtering large input to small output 
should use little mem
 test_expect_success 'filter that does not read is fine' '
test-genrandom foo $((128 * 1024 + 1)) >big &&
echo "big filter=epipe" >.gitattributes &&
-   git config f

[PATCH v10 00/14] Git filter protocol

2016-10-08 Thread larsxschneider
From: Lars Schneider 

The goal of this series is to avoid launching a new clean/smudge filter
process for each file that is filtered.

A short summary about v1 to v5 can be found here:
https://git.github.io/rev_news/2016/08/17/edition-18/

This series is also published on web:
https://github.com/larsxschneider/git/pull/14

Patches 1 and 2 are cleanups and not strictly necessary for the series.
Patches 3 to 12 are required preparation. Patch 13 is the main patch.
Patch 14 adds an example how to use the Git filter protocol in contrib.

Thanks a lot to
 Jakub, Junio, and Peff
for very helpful reviews,
Lars

## Changes since v9
  * replace the very specific "wait after close(stdin)" behavior with the
more flexible run-command "clean_on_exit_handler" flag to fix flaky t0021,
see discussion:
http://public-inbox.org/git/xmqq37k9mm7k@gitster.mtv.corp.google.com/
http://public-inbox.org/git/xmqq8tubitjs@gitster.mtv.corp.google.com/
  * run stop_multi_file_filter() for all filters on Git shutdown
  * actually kill filter in kill_multi_file_filter()
  * add new filter process to hashmap only if the process start was successful
  * remove superfluous fstat() call
  * avoid potential buffer overflow in packet_write_gently() packet size check
  * remove superfluous buffer in write_packetized_from_buf()
  * improve test name


Lars Schneider (14):
  convert: quote filter names in error messages
  convert: modernize tests
  run-command: move check_pipe() from write_or_die to run_command
  run-command: add clean_on_exit_handler
  pkt-line: rename packet_write() to packet_write_fmt()
  pkt-line: extract set_packet_header()
  pkt-line: add packet_write_fmt_gently()
  pkt-line: add packet_flush_gently()
  pkt-line: add packet_write_gently()
  pkt-line: add functions to read/write flush terminated packet streams
  convert: make apply_filter() adhere to standard Git error handling
  convert: prepare filter..process option
  convert: add filter..process option
  contrib/long-running-filter: add long running filter example

 Documentation/gitattributes.txt| 159 ++-
 builtin/archive.c  |   4 +-
 builtin/receive-pack.c |   4 +-
 builtin/remote-ext.c   |   4 +-
 builtin/upload-archive.c   |   4 +-
 connect.c  |   2 +-
 contrib/long-running-filter/example.pl | 127 +
 convert.c  | 372 +---
 daemon.c   |   2 +-
 http-backend.c |   2 +-
 pkt-line.c | 152 +-
 pkt-line.h |  12 +-
 run-command.c  |  36 ++-
 run-command.h  |   4 +-
 shallow.c  |   2 +-
 t/t0021-conversion.sh  | 505 ++---
 t/t0021/rot13-filter.pl| 191 +
 upload-pack.c  |  30 +-
 write_or_die.c |  13 -
 19 files changed, 1492 insertions(+), 133 deletions(-)
 create mode 100755 contrib/long-running-filter/example.pl
 create mode 100755 t/t0021/rot13-filter.pl



## Interdiff (v9..v10)

diff --git a/convert.c b/convert.c
index 88581d6..1d89632 100644
--- a/convert.c
+++ b/convert.c
@@ -516,23 +516,6 @@ static struct cmd2process 
*find_multi_file_filter_entry(struct hashmap *hashmap,
return hashmap_get(hashmap, &key, NULL);
 }

-static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process 
*entry)
-{
-   if (!entry)
-   return;
-   sigchain_push(SIGPIPE, SIG_IGN);
-   /*
-* We kill the filter most likely because an error happened already.
-* That's why we are not interested in any error code here.
-*/
-   close(entry->process.in);
-   close(entry->process.out);
-   sigchain_pop(SIGPIPE);
-   finish_command(&entry->process);
-   hashmap_remove(hashmap, entry, NULL);
-   free(entry);
-}
-
 static int packet_write_list(int fd, const char *line, ...)
 {
va_list args;
@@ -552,6 +535,49 @@ static int packet_write_list(int fd, const char *line, ...)
return packet_flush_gently(fd);
 }

+static void read_multi_file_filter_status(int fd, struct strbuf *status) {
+   struct strbuf **pair;
+   char *line;
+   for (;;) {
+   line = packet_read_line(fd, NULL);
+   if (!line)
+   break;
+   pair = strbuf_split_str(line, '=', 2);
+   if (pair[0] && pair[0]->len && pair[1]) {
+   /* the last "status=" line wins */
+   if (!strcmp(pair[0]->buf, "status=")) {
+   strbuf_reset(status);
+   strbuf_addbuf(status, pair[1]);
+   }
+   }
+   strbuf_list_free(pair);
+   }
+}
+
+sta

[PATCH v10 10/14] pkt-line: add functions to read/write flush terminated packet streams

2016-10-08 Thread larsxschneider
From: Lars Schneider 

write_packetized_from_fd() and write_packetized_from_buf() write a
stream of packets. All content packets use the maximal packet size
except for the last one. After the last content packet a `flush` control
packet is written.

read_packetized_to_strbuf() reads arbitrary sized packets until it
detects a `flush` packet.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 pkt-line.c | 72 ++
 pkt-line.h |  8 +++
 2 files changed, 80 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index dca5a64..0b5125f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -197,6 +197,46 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, 
...)
va_end(args);
 }
 
+int write_packetized_from_fd(int fd_in, int fd_out)
+{
+   static char buf[LARGE_PACKET_DATA_MAX];
+   int err = 0;
+   ssize_t bytes_to_write;
+
+   while (!err) {
+   bytes_to_write = xread(fd_in, buf, sizeof(buf));
+   if (bytes_to_write < 0)
+   return COPY_READ_ERROR;
+   if (bytes_to_write == 0)
+   break;
+   err = packet_write_gently(fd_out, buf, bytes_to_write);
+   }
+   if (!err)
+   err = packet_flush_gently(fd_out);
+   return err;
+}
+
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)
+{
+   int err = 0;
+   size_t bytes_written = 0;
+   size_t bytes_to_write;
+
+   while (!err) {
+   if ((len - bytes_written) > LARGE_PACKET_DATA_MAX)
+   bytes_to_write = LARGE_PACKET_DATA_MAX;
+   else
+   bytes_to_write = len - bytes_written;
+   if (bytes_to_write == 0)
+   break;
+   err = packet_write_gently(fd_out, src_in + bytes_written, 
bytes_to_write);
+   bytes_written += bytes_to_write;
+   }
+   if (!err)
+   err = packet_flush_gently(fd_out);
+   return err;
+}
+
 static int get_packet_data(int fd, char **src_buf, size_t *src_size,
   void *dst, unsigned size, int options)
 {
@@ -306,3 +346,35 @@ char *packet_read_line_buf(char **src, size_t *src_len, 
int *dst_len)
 {
return packet_read_line_generic(-1, src, src_len, dst_len);
 }
+
+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out)
+{
+   int packet_len;
+
+   size_t orig_len = sb_out->len;
+   size_t orig_alloc = sb_out->alloc;
+
+   for (;;) {
+   strbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);
+   packet_len = packet_read(fd_in, NULL, NULL,
+   /* strbuf_grow() above always allocates one extra byte 
to
+* store a '\0' at the end of the string. packet_read()
+* writes a '\0' extra byte at the end, too. Let it know
+* that there is already room for the extra byte.
+*/
+   sb_out->buf + sb_out->len, LARGE_PACKET_DATA_MAX+1,
+   PACKET_READ_GENTLE_ON_EOF);
+   if (packet_len <= 0)
+   break;
+   sb_out->len += packet_len;
+   }
+
+   if (packet_len < 0) {
+   if (orig_alloc == 0)
+   strbuf_release(sb_out);
+   else
+   strbuf_setlen(sb_out, orig_len);
+   return packet_len;
+   }
+   return sb_out->len - orig_len;
+}
diff --git a/pkt-line.h b/pkt-line.h
index 3fa0899..18eac64 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -25,6 +25,8 @@ void packet_buf_flush(struct strbuf *buf);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
 int packet_flush_gently(int fd);
 int packet_write_fmt_gently(int fd, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
+int write_packetized_from_fd(int fd_in, int fd_out);
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
 
 /*
  * Read a packetized line into the buffer, which must be at least size bytes
@@ -77,8 +79,14 @@ char *packet_read_line(int fd, int *size);
  */
 char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);
 
+/*
+ * Reads a stream of variable sized packets until a flush packet is detected.
+ */
+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out);
+
 #define DEFAULT_PACKET_MAX 1000
 #define LARGE_PACKET_MAX 65520
+#define LARGE_PACKET_DATA_MAX (LARGE_PACKET_MAX - 4)
 extern char packet_buffer[LARGE_PACKET_MAX];
 
 #endif
-- 
2.10.0



[PATCH v10 14/14] contrib/long-running-filter: add long running filter example

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Add a simple pass-thru filter as example implementation for the Git
filter protocol version 2. See Documentation/gitattributes.txt, section
"Filter Protocol" for more info.

Signed-off-by: Lars Schneider 
---
 Documentation/gitattributes.txt|   4 +-
 contrib/long-running-filter/example.pl | 127 +
 2 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100755 contrib/long-running-filter/example.pl

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 5868f00..a182ef2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -514,7 +514,9 @@ the next "key=value" list containing a command. Git will 
close
 the command pipe on exit. The filter is expected to detect EOF
 and exit gracefully on its own.
 
-If you develop your own long running filter
+A long running filter demo implementation can be found in
+`contrib/long-running-filter/example.pl` located in the Git
+core repository. If you develop your own long running filter
 process then the `GIT_TRACE_PACKET` environment variables can be
 very helpful for debugging (see linkgit:git[1]).
 
diff --git a/contrib/long-running-filter/example.pl 
b/contrib/long-running-filter/example.pl
new file mode 100755
index 000..f4102d2
--- /dev/null
+++ b/contrib/long-running-filter/example.pl
@@ -0,0 +1,127 @@
+#!/usr/bin/perl
+#
+# Example implementation for the Git filter protocol version 2
+# See Documentation/gitattributes.txt, section "Filter Protocol"
+#
+# Please note, this pass-thru filter is a minimal skeleton. No proper
+# error handling was implemented.
+#
+
+use strict;
+use warnings;
+
+my $MAX_PACKET_CONTENT_SIZE = 65516;
+
+sub packet_bin_read {
+   my $buffer;
+   my $bytes_read = read STDIN, $buffer, 4;
+   if ( $bytes_read == 0 ) {
+
+   # EOF - Git stopped talking to us!
+   exit();
+   }
+   elsif ( $bytes_read != 4 ) {
+   die "invalid packet: '$buffer'";
+   }
+   my $pkt_size = hex($buffer);
+   if ( $pkt_size == 0 ) {
+   return ( 1, "" );
+   }
+   elsif ( $pkt_size > 4 ) {
+   my $content_size = $pkt_size - 4;
+   $bytes_read = read STDIN, $buffer, $content_size;
+   if ( $bytes_read != $content_size ) {
+   die "invalid packet ($content_size bytes expected; 
$bytes_read bytes read)";
+   }
+   return ( 0, $buffer );
+   }
+   else {
+   die "invalid packet size: $pkt_size";
+   }
+}
+
+sub packet_txt_read {
+   my ( $res, $buf ) = packet_bin_read();
+   unless ( $buf =~ s/\n$// ) {
+   die "A non-binary line MUST be terminated by an LF.";
+   }
+   return ( $res, $buf );
+}
+
+sub packet_bin_write {
+   my $buf = shift;
+   print STDOUT sprintf( "%04x", length($buf) + 4 );
+   print STDOUT $buf;
+   STDOUT->flush();
+}
+
+sub packet_txt_write {
+   packet_bin_write( $_[0] . "\n" );
+}
+
+sub packet_flush {
+   print STDOUT sprintf( "%04x", 0 );
+   STDOUT->flush();
+}
+
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) )  || die "bad version end";
+
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
+
+( packet_txt_read() eq ( 0, "clean=true" ) )  || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) )|| die "bad capability end";
+
+packet_txt_write("clean=true");
+packet_txt_write("smudge=true");
+packet_flush();
+
+while (1) {
+   my ($command)  = packet_txt_read() =~ /^command=([^=]+)$/;
+   my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
+
+   packet_bin_read();
+
+   my $input = "";
+   {
+   binmode(STDIN);
+   my $buffer;
+   my $done = 0;
+   while ( !$done ) {
+   ( $done, $buffer ) = packet_bin_read();
+   $input .= $buffer;
+   }
+   }
+
+   my $output;
+   if ( $command eq "clean" ) {
+   ### Perform clean here ###
+   $output = $input;
+   }
+   elsif ( $command eq "smudge" ) {
+   ### Perform smudge here ###
+   $output = $input;
+   }
+   else {
+   die "bad command '$command'";
+   }
+
+   packet_txt_write("status=success");
+   packet_flush();
+   while ( length($output) > 0 ) {
+   my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
+   packet_bin_write($packet);
+   if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
+   $output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
+   }
+

[PATCH v10 11/14] convert: make apply_filter() adhere to standard Git error handling

2016-10-08 Thread larsxschneider
From: Lars Schneider 

apply_filter() returns a boolean that tells the caller if it
"did convert or did not convert". The variable `ret` was used throughout
the function to track errors whereas `1` denoted success and `0`
failure. This is unusual for the Git source where `0` denotes success.

Rename the variable and flip its value to make the function easier
readable for Git developers.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 convert.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/convert.c b/convert.c
index 986c239..597f561 100644
--- a/convert.c
+++ b/convert.c
@@ -451,7 +451,7 @@ static int apply_filter(const char *path, const char *src, 
size_t len, int fd,
 *
 * (child --> cmd) --> us
 */
-   int ret = 1;
+   int err = 0;
struct strbuf nbuf = STRBUF_INIT;
struct async async;
struct filter_params params;
@@ -477,23 +477,20 @@ static int apply_filter(const char *path, const char 
*src, size_t len, int fd,
return 0;   /* error was already reported */
 
if (strbuf_read(&nbuf, async.out, len) < 0) {
-   error("read from external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("read from external filter '%s' failed", cmd);
}
if (close(async.out)) {
-   error("read from external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("read from external filter '%s' failed", cmd);
}
if (finish_async(&async)) {
-   error("external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("external filter '%s' failed", cmd);
}
 
-   if (ret) {
+   if (!err) {
strbuf_swap(dst, &nbuf);
}
strbuf_release(&nbuf);
-   return ret;
+   return !err;
 }
 
 static struct convert_driver {
-- 
2.10.0



[PATCH v10 13/14] convert: add filter..process option

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Git's clean/smudge mechanism invokes an external filter process for
every single blob that is affected by a filter. If Git filters a lot of
blobs then the startup time of the external filter processes can become
a significant part of the overall Git execution time.

In a preliminary performance test this developer used a clean/smudge
filter written in golang to filter 12,000 files. This process took 364s
with the existing filter mechanism and 5s with the new mechanism. See
details here: https://github.com/github/git-lfs/pull/1382

This patch adds the `filter..process` string option which, if
used, keeps the external filter process running and processes all blobs
with the packet format (pkt-line) based protocol over standard input and
standard output. The full protocol is explained in detail in
`Documentation/gitattributes.txt`.

A few key decisions:

* The long running filter process is referred to as filter protocol
  version 2 because the existing single shot filter invocation is
  considered version 1.
* Git sends a welcome message and expects a response right after the
  external filter process has started. This ensures that Git will not
  hang if a version 1 filter is incorrectly used with the
  filter..process option for version 2 filters. In addition,
  Git can detect this kind of error and warn the user.
* The status of a filter operation (e.g. "success" or "error) is set
  before the actual response and (if necessary!) re-set after the
  response. The advantage of this two step status response is that if
  the filter detects an error early, then the filter can communicate
  this and Git does not even need to create structures to read the
  response.
* All status responses are pkt-line lists terminated with a flush
  packet. This allows us to send other status fields with the same
  protocol in the future.

Helped-by: Martin-Louis Bright 
Reviewed-by: Jakub Narebski 
Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 Documentation/gitattributes.txt | 157 +-
 convert.c   | 297 +-
 t/t0021-conversion.sh   | 447 +++-
 t/t0021/rot13-filter.pl | 191 +
 4 files changed, 1082 insertions(+), 10 deletions(-)
 create mode 100755 t/t0021/rot13-filter.pl

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 7aff940..5868f00 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -293,7 +293,13 @@ checkout, when the `smudge` command is specified, the 
command is
 fed the blob object from its standard input, and its standard
 output is used to update the worktree file.  Similarly, the
 `clean` command is used to convert the contents of worktree file
-upon checkin.
+upon checkin. By default these commands process only a single
+blob and terminate.  If a long running `process` filter is used
+in place of `clean` and/or `smudge` filters, then Git can process
+all blobs with a single filter command invocation for the entire
+life of a single Git command, for example `git add --all`.  See
+section below for the description of the protocol used to
+communicate with a `process` filter.
 
 One use of the content filtering is to massage the content into a shape
 that is more convenient for the platform, filesystem, and the user to use.
@@ -373,6 +379,155 @@ not exist, or may have different contents. So, smudge and 
clean commands
 should not try to access the file on disk, but only act as filters on the
 content provided to them on standard input.
 
+Long Running Filter Process
+^^^
+
+If the filter command (a string value) is defined via
+`filter..process` then Git can process all blobs with a
+single filter invocation for the entire life of a single Git
+command. This is achieved by using a packet format (pkt-line,
+see technical/protocol-common.txt) based protocol over standard
+input and standard output as follows. All packets, except for the
+"*CONTENT" packets and the "" flush packet, are considered
+text and therefore are terminated by a LF.
+
+Git starts the filter when it encounters the first file
+that needs to be cleaned or smudged. After the filter started
+Git sends a welcome message ("git-filter-client"), a list of
+supported protocol version numbers, and a flush packet. Git expects
+to read a welcome response message ("git-filter-server") and exactly
+one protocol version number from the previously sent list. All further
+communication will be based on the selected version. The remaining
+protocol description below documents "version=2". Please note that
+"version=42" in the example below does not exist and is only there
+to illustrate how the protocol would look like with more than one
+version.
+
+After the version negotiation Git sends a list of all capabilities that
+it supports and a flush packet. Git expects to read a list of desired
+capabilities, whi

[PATCH v10 03/14] run-command: move check_pipe() from write_or_die to run_command

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Move check_pipe() to run_command and make it public. This is necessary
to call the function from pkt-line in a subsequent patch.

While at it, make async_exit() static to run_command.c as it is no
longer used from outside.

Signed-off-by: Lars Schneider 
Signed-off-by: Ramsay Jones 
Signed-off-by: Junio C Hamano 
---
 run-command.c  | 17 +++--
 run-command.h  |  2 +-
 write_or_die.c | 13 -
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/run-command.c b/run-command.c
index 5a4dbb6..3269362 100644
--- a/run-command.c
+++ b/run-command.c
@@ -634,7 +634,7 @@ int in_async(void)
return !pthread_equal(main_thread, pthread_self());
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
pthread_exit((void *)(intptr_t)code);
 }
@@ -684,13 +684,26 @@ int in_async(void)
return process_is_async;
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
exit(code);
 }
 
 #endif
 
+void check_pipe(int err)
+{
+   if (err == EPIPE) {
+   if (in_async())
+   async_exit(141);
+
+   signal(SIGPIPE, SIG_DFL);
+   raise(SIGPIPE);
+   /* Should never happen, but just in case... */
+   exit(141);
+   }
+}
+
 int start_async(struct async *async)
 {
int need_in, need_out;
diff --git a/run-command.h b/run-command.h
index 5066649..cf29a31 100644
--- a/run-command.h
+++ b/run-command.h
@@ -139,7 +139,7 @@ struct async {
 int start_async(struct async *async);
 int finish_async(struct async *async);
 int in_async(void);
-void NORETURN async_exit(int code);
+void check_pipe(int err);
 
 /**
  * This callback should initialize the child process and preload the
diff --git a/write_or_die.c b/write_or_die.c
index 0734432..eab8c8d 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,19 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
 
-static void check_pipe(int err)
-{
-   if (err == EPIPE) {
-   if (in_async())
-   async_exit(141);
-
-   signal(SIGPIPE, SIG_DFL);
-   raise(SIGPIPE);
-   /* Should never happen, but just in case... */
-   exit(141);
-   }
-}
-
 /*
  * Some cases use stdio, but want to flush after the write
  * to get error handling (and to get better interactive
-- 
2.10.0



[PATCH v10 09/14] pkt-line: add packet_write_gently()

2016-10-08 Thread larsxschneider
From: Lars Schneider 

packet_write_fmt_gently() uses format_packet() which lets the caller
only send string data via "%s". That means it cannot be used for
arbitrary data that may contain NULs.

Add packet_write_gently() which writes arbitrary data and does not die
in case of an error. The function is used by other pkt-line functions in
a subsequent patch.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 pkt-line.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index 286eb09..dca5a64 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -171,6 +171,23 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
return status;
 }
 
+static int packet_write_gently(const int fd_out, const char *buf, size_t size)
+{
+   static char packet_write_buffer[LARGE_PACKET_MAX];
+   size_t packet_size;
+
+   if (size > sizeof(packet_write_buffer) - 4)
+   return error("packet write failed - data exceeds max packet 
size");
+
+   packet_trace(buf, size, 1);
+   packet_size = size + 4;
+   set_packet_header(packet_write_buffer, packet_size);
+   memcpy(packet_write_buffer + 4, buf, size);
+   if (write_in_full(fd_out, packet_write_buffer, packet_size) == 
packet_size)
+   return 0;
+   return error("packet write failed");
+}
+
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
 {
va_list args;
-- 
2.10.0



[PATCH v10 05/14] pkt-line: rename packet_write() to packet_write_fmt()

2016-10-08 Thread larsxschneider
From: Lars Schneider 

packet_write() should be called packet_write_fmt() because it is a
printf-like function that takes a format string as first parameter.

packet_write_fmt() should be used for text strings only. Arbitrary
binary data should use a new packet_write() function that is introduced
in a subsequent patch.

Suggested-by: Junio C Hamano 
Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 builtin/archive.c|  4 ++--
 builtin/receive-pack.c   |  4 ++--
 builtin/remote-ext.c |  4 ++--
 builtin/upload-archive.c |  4 ++--
 connect.c|  2 +-
 daemon.c |  2 +-
 http-backend.c   |  2 +-
 pkt-line.c   |  2 +-
 pkt-line.h   |  2 +-
 shallow.c|  2 +-
 upload-pack.c| 30 +++---
 11 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/archive.c b/builtin/archive.c
index a1e3b94..49f4914 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -47,10 +47,10 @@ static int run_remote_archiver(int argc, const char **argv,
if (name_hint) {
const char *format = archive_format_from_filename(name_hint);
if (format)
-   packet_write(fd[1], "argument --format=%s\n", format);
+   packet_write_fmt(fd[1], "argument --format=%s\n", 
format);
}
for (i = 1; i < argc; i++)
-   packet_write(fd[1], "argument %s\n", argv[i]);
+   packet_write_fmt(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
 
buf = packet_read_line(fd[0], NULL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 011db00..1ce7682 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -218,7 +218,7 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
 static void show_ref(const char *path, const unsigned char *sha1)
 {
if (sent_capabilities) {
-   packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
+   packet_write_fmt(1, "%s %s\n", sha1_to_hex(sha1), path);
} else {
struct strbuf cap = STRBUF_INIT;
 
@@ -233,7 +233,7 @@ static void show_ref(const char *path, const unsigned char 
*sha1)
if (advertise_push_options)
strbuf_addstr(&cap, " push-options");
strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
-   packet_write(1, "%s %s%c%s\n",
+   packet_write_fmt(1, "%s %s%c%s\n",
 sha1_to_hex(sha1), path, 0, cap.buf);
strbuf_release(&cap);
sent_capabilities = 1;
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 88eb8f9..11b48bf 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -128,9 +128,9 @@ static void send_git_request(int stdin_fd, const char 
*serv, const char *repo,
const char *vhost)
 {
if (!vhost)
-   packet_write(stdin_fd, "%s %s%c", serv, repo, 0);
+   packet_write_fmt(stdin_fd, "%s %s%c", serv, repo, 0);
else
-   packet_write(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
+   packet_write_fmt(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
 vhost, 0);
 }
 
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2caedf1..dc872f6 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -88,11 +88,11 @@ int cmd_upload_archive(int argc, const char **argv, const 
char *prefix)
writer.git_cmd = 1;
if (start_command(&writer)) {
int err = errno;
-   packet_write(1, "NACK unable to spawn subprocess\n");
+   packet_write_fmt(1, "NACK unable to spawn subprocess\n");
die("upload-archive: %s", strerror(err));
}
 
-   packet_write(1, "ACK\n");
+   packet_write_fmt(1, "ACK\n");
packet_flush(1);
 
while (1) {
diff --git a/connect.c b/connect.c
index 722dc3f..5330d9c 100644
--- a/connect.c
+++ b/connect.c
@@ -730,7 +730,7 @@ struct child_process *git_connect(int fd[2], const char 
*url,
 * Note: Do not add any other headers here!  Doing so
 * will cause older git-daemon servers to crash.
 */
-   packet_write(fd[1],
+   packet_write_fmt(fd[1],
 "%s %s%chost=%s%c",
 prog, path, 0,
 target_host, 0);
diff --git a/daemon.c b/daemon.c
index 425aad0..afce1b9 100644
--- a/daemon.c
+++ b/daemon.c
@@ -281,7 +281,7 @@ static int daemon_error(const char *dir, const char *msg)
 {
if (!informative_errors)
msg = "access denied or repository not exported";
-   packet_write(1, "ERR %s: %s", msg, dir);
+   packet_write_fmt(1, "ERR %s: %s", msg, dir);
return -1;
 }
 
diff --git a/http-backen

[PATCH v10 08/14] pkt-line: add packet_flush_gently()

2016-10-08 Thread larsxschneider
From: Lars Schneider 

packet_flush() would die in case of a write error even though for some
callers an error would be acceptable. Add packet_flush_gently() which
writes a pkt-line flush packet like packet_flush() but does not die in
case of an error. The function is used in a subsequent patch.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 pkt-line.c | 8 
 pkt-line.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index 56915f0..286eb09 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -91,6 +91,14 @@ void packet_flush(int fd)
write_or_die(fd, "", 4);
 }
 
+int packet_flush_gently(int fd)
+{
+   packet_trace("", 4, 1);
+   if (write_in_full(fd, "", 4) == 4)
+   return 0;
+   return error("flush packet write failed");
+}
+
 void packet_buf_flush(struct strbuf *buf)
 {
packet_trace("", 4, 1);
diff --git a/pkt-line.h b/pkt-line.h
index 3caea77..3fa0899 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
 void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format 
(printf, 2, 3)));
 void packet_buf_flush(struct strbuf *buf);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
+int packet_flush_gently(int fd);
 int packet_write_fmt_gently(int fd, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
 
 /*
-- 
2.10.0



[PATCH v10 07/14] pkt-line: add packet_write_fmt_gently()

2016-10-08 Thread larsxschneider
From: Lars Schneider 

packet_write_fmt() would die in case of a write error even though for
some callers an error would be acceptable. Add packet_write_fmt_gently()
which writes a formatted pkt-line like packet_write_fmt() but does not
die in case of an error. The function is used in a subsequent patch.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 pkt-line.c | 34 ++
 pkt-line.h |  1 +
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index e8adc0f..56915f0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -125,16 +125,42 @@ static void format_packet(struct strbuf *out, const char 
*fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
 }
 
+static int packet_write_fmt_1(int fd, int gently,
+ const char *fmt, va_list args)
+{
+   struct strbuf buf = STRBUF_INIT;
+   ssize_t count;
+
+   format_packet(&buf, fmt, args);
+   count = write_in_full(fd, buf.buf, buf.len);
+   if (count == buf.len)
+   return 0;
+
+   if (!gently) {
+   check_pipe(errno);
+   die_errno("packet write with format failed");
+   }
+   return error("packet write with format failed");
+}
+
 void packet_write_fmt(int fd, const char *fmt, ...)
 {
-   static struct strbuf buf = STRBUF_INIT;
va_list args;
 
-   strbuf_reset(&buf);
va_start(args, fmt);
-   format_packet(&buf, fmt, args);
+   packet_write_fmt_1(fd, 0, fmt, args);
+   va_end(args);
+}
+
+int packet_write_fmt_gently(int fd, const char *fmt, ...)
+{
+   int status;
+   va_list args;
+
+   va_start(args, fmt);
+   status = packet_write_fmt_1(fd, 1, fmt, args);
va_end(args);
-   write_or_die(fd, buf.buf, buf.len);
+   return status;
 }
 
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/pkt-line.h b/pkt-line.h
index 1902fb3..3caea77 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
 void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format 
(printf, 2, 3)));
 void packet_buf_flush(struct strbuf *buf);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
+int packet_write_fmt_gently(int fd, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
 
 /*
  * Read a packetized line into the buffer, which must be at least size bytes
-- 
2.10.0



[PATCH v10 12/14] convert: prepare filter..process option

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Refactor the existing 'single shot filter mechanism' and prepare the
new 'long running filter mechanism'.

Signed-off-by: Lars Schneider 
---
 convert.c | 60 ++--
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/convert.c b/convert.c
index 597f561..71e11ff 100644
--- a/convert.c
+++ b/convert.c
@@ -442,7 +442,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)
return (write_err || status);
 }
 
-static int apply_filter(const char *path, const char *src, size_t len, int fd,
+static int apply_single_file_filter(const char *path, const char *src, size_t 
len, int fd,
 struct strbuf *dst, const char *cmd)
 {
/*
@@ -456,12 +456,6 @@ static int apply_filter(const char *path, const char *src, 
size_t len, int fd,
struct async async;
struct filter_params params;
 
-   if (!cmd || !*cmd)
-   return 0;
-
-   if (!dst)
-   return 1;
-
memset(&async, 0, sizeof(async));
async.proc = filter_buffer_or_fd;
async.data = ¶ms;
@@ -493,6 +487,9 @@ static int apply_filter(const char *path, const char *src, 
size_t len, int fd,
return !err;
 }
 
+#define CAP_CLEAN(1u<<0)
+#define CAP_SMUDGE   (1u<<1)
+
 static struct convert_driver {
const char *name;
struct convert_driver *next;
@@ -501,6 +498,29 @@ static struct convert_driver {
int required;
 } *user_convert, **user_convert_tail;
 
+static int apply_filter(const char *path, const char *src, size_t len,
+   int fd, struct strbuf *dst, struct convert_driver *drv,
+   const unsigned int wanted_capability)
+{
+   const char *cmd = NULL;
+
+   if (!drv)
+   return 0;
+
+   if (!dst)
+   return 1;
+
+   if ((CAP_CLEAN & wanted_capability) && drv->clean)
+   cmd = drv->clean;
+   else if ((CAP_SMUDGE & wanted_capability) && drv->smudge)
+   cmd = drv->smudge;
+
+   if (cmd && *cmd)
+   return apply_single_file_filter(path, src, len, fd, dst, cmd);
+
+   return 0;
+}
+
 static int read_convert_config(const char *var, const char *value, void *cb)
 {
const char *key, *name;
@@ -839,7 +859,7 @@ int would_convert_to_git_filter_fd(const char *path)
if (!ca.drv->required)
return 0;
 
-   return apply_filter(path, NULL, 0, -1, NULL, ca.drv->clean);
+   return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);
 }
 
 const char *get_convert_attr_ascii(const char *path)
@@ -872,18 +892,12 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
struct strbuf *dst, enum safe_crlf checksafe)
 {
int ret = 0;
-   const char *filter = NULL;
-   int required = 0;
struct conv_attrs ca;
 
convert_attrs(&ca, path);
-   if (ca.drv) {
-   filter = ca.drv->clean;
-   required = ca.drv->required;
-   }
 
-   ret |= apply_filter(path, src, len, -1, dst, filter);
-   if (!ret && required)
+   ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);
+   if (!ret && ca.drv && ca.drv->required)
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
if (ret && dst) {
@@ -907,7 +921,7 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
assert(ca.drv);
assert(ca.drv->clean);
 
-   if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
+   if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
@@ -919,15 +933,9 @@ static int convert_to_working_tree_internal(const char 
*path, const char *src,
int normalizing)
 {
int ret = 0, ret_filter = 0;
-   const char *filter = NULL;
-   int required = 0;
struct conv_attrs ca;
 
convert_attrs(&ca, path);
-   if (ca.drv) {
-   filter = ca.drv->smudge;
-   required = ca.drv->required;
-   }
 
ret |= ident_to_worktree(path, src, len, dst, ca.ident);
if (ret) {
@@ -938,7 +946,7 @@ static int convert_to_working_tree_internal(const char 
*path, const char *src,
 * CRLF conversion can be skipped if normalizing, unless there
 * is a smudge filter.  The filter might expect CRLFs.
 */
-   if (filter || !normalizing) {
+   if ((ca.drv && ca.drv->smudge) || !normalizing) {
ret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);
if (ret) {
src = dst->buf;
@@ -946,8 +954,8 @@ static int convert_to_working_tree_internal(const char 
*path, const char *src,
}
}
 
-   ret_

[PATCH v10 06/14] pkt-line: extract set_packet_header()

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Extracted set_packet_header() function converts an integer to a 4 byte
hex string. Make this function locally available so that other pkt-line
functions could use it.

Signed-off-by: Lars Schneider 
Signed-off-by: Junio C Hamano 
---
 pkt-line.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index 0a9b61c..e8adc0f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -97,10 +97,20 @@ void packet_buf_flush(struct strbuf *buf)
strbuf_add(buf, "", 4);
 }
 
-#define hex(a) (hexchar[(a) & 15])
-static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+static void set_packet_header(char *buf, const int size)
 {
static char hexchar[] = "0123456789abcdef";
+
+   #define hex(a) (hexchar[(a) & 15])
+   buf[0] = hex(size >> 12);
+   buf[1] = hex(size >> 8);
+   buf[2] = hex(size >> 4);
+   buf[3] = hex(size);
+   #undef hex
+}
+
+static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+{
size_t orig_len, n;
 
orig_len = out->len;
@@ -111,10 +121,7 @@ static void format_packet(struct strbuf *out, const char 
*fmt, va_list args)
if (n > LARGE_PACKET_MAX)
die("protocol error: impossibly long line");
 
-   out->buf[orig_len + 0] = hex(n >> 12);
-   out->buf[orig_len + 1] = hex(n >> 8);
-   out->buf[orig_len + 2] = hex(n >> 4);
-   out->buf[orig_len + 3] = hex(n);
+   set_packet_header(&out->buf[orig_len], n);
packet_trace(out->buf + orig_len + 4, n - 4, 1);
 }
 
-- 
2.10.0



[PATCH v10 04/14] run-command: add clean_on_exit_handler

2016-10-08 Thread larsxschneider
From: Lars Schneider 

Some processes might want to perform cleanup tasks before Git kills them
due to the 'clean_on_exit' flag. Let's give them an interface for doing
this. The feature is used in a subsequent patch.

Please note, that the cleanup callback is not executed if Git dies of a
signal. The reason is that only "async-signal-safe" functions would be
allowed to be call in that case. Since we cannot control what functions
the callback will use, we will not support the case. See 507d7804 for
more details.

Helped-by: Johannes Sixt 
Signed-off-by: Lars Schneider 
---
 run-command.c | 19 +++
 run-command.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/run-command.c b/run-command.c
index 3269362..e5fd6ff 100644
--- a/run-command.c
+++ b/run-command.c
@@ -21,6 +21,7 @@ void child_process_clear(struct child_process *child)
 
 struct child_to_clean {
pid_t pid;
+   struct child_process *process;
struct child_to_clean *next;
 };
 static struct child_to_clean *children_to_clean;
@@ -31,6 +32,15 @@ static void cleanup_children(int sig, int in_signal)
while (children_to_clean) {
struct child_to_clean *p = children_to_clean;
children_to_clean = p->next;
+
+   if (p->process && !in_signal) {
+   struct child_process *process = p->process;
+   if (process->clean_on_exit_handler) {
+   trace_printf("trace: run_command: running exit 
handler for pid %d", p->pid);
+   process->clean_on_exit_handler(process);
+   }
+   }
+
kill(p->pid, sig);
if (!in_signal)
free(p);
@@ -49,10 +59,11 @@ static void cleanup_children_on_exit(void)
cleanup_children(SIGTERM, 0);
 }
 
-static void mark_child_for_cleanup(pid_t pid)
+static void mark_child_for_cleanup(pid_t pid, struct child_process *process)
 {
struct child_to_clean *p = xmalloc(sizeof(*p));
p->pid = pid;
+   p->process = process;
p->next = children_to_clean;
children_to_clean = p;
 
@@ -422,7 +433,7 @@ int start_command(struct child_process *cmd)
if (cmd->pid < 0)
error_errno("cannot fork() for %s", cmd->argv[0]);
else if (cmd->clean_on_exit)
-   mark_child_for_cleanup(cmd->pid);
+   mark_child_for_cleanup(cmd->pid, cmd);
 
/*
 * Wait for child's execvp. If the execvp succeeds (or if fork()
@@ -483,7 +494,7 @@ int start_command(struct child_process *cmd)
if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
error_errno("cannot spawn %s", cmd->argv[0]);
if (cmd->clean_on_exit && cmd->pid >= 0)
-   mark_child_for_cleanup(cmd->pid);
+   mark_child_for_cleanup(cmd->pid, cmd);
 
argv_array_clear(&nargv);
cmd->argv = sargv;
@@ -765,7 +776,7 @@ int start_async(struct async *async)
exit(!!async->proc(proc_in, proc_out, async->data));
}
 
-   mark_child_for_cleanup(async->pid);
+   mark_child_for_cleanup(async->pid, NULL);
 
if (need_in)
close(fdin[0]);
diff --git a/run-command.h b/run-command.h
index cf29a31..dd1c78c 100644
--- a/run-command.h
+++ b/run-command.h
@@ -43,6 +43,8 @@ struct child_process {
unsigned stdout_to_stderr:1;
unsigned use_shell:1;
unsigned clean_on_exit:1;
+   void (*clean_on_exit_handler)(struct child_process *process);
+   void *clean_on_exit_handler_cbdata;
 };
 
 #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
-- 
2.10.0



Re: [PATCH] remote.c: free previous results when looking for a ref match

2016-10-08 Thread René Scharfe

Am 08.10.2016 um 01:58 schrieb Stefan Beller:

Signed-off-by: Stefan Beller 
---
 remote.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/remote.c b/remote.c
index ad6c542..5f9afb4 100644
--- a/remote.c
+++ b/remote.c
@@ -833,6 +833,8 @@ static int match_name_with_pattern(const char *key, const 
char *name,
strbuf_add(&sb, value, vstar - value);
strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
strbuf_addstr(&sb, vstar + 1);
+   if (*result)
+   free(*result);


free(3) can handle NULL pointers; this check is not necessary.

Is it wise to release memory for callers?  I'd expect them to be 
responsible for that.  Some of them can pass uninitialized pointers; 
this is not allowed anymore after the change.



*result = strbuf_detach(&sb, NULL);
}
return ret;
@@ -1262,6 +1264,8 @@ static char *get_ref_match(const struct refspec *rs, int 
rs_nr, const struct ref
 */
if (!send_mirror && !starts_with(ref->name, "refs/heads/"))
return NULL;
+   if (name)
+   free(name);


Again, this check is not necessary.  If I read the code correctly the 
pointer could be uninitialized at that point, though, causing free(3) to 
crash.



name = xstrdup(ref->name);
}
if (ret_pat)





[PATCH] remove unnecessary NULL check before free(3)

2016-10-08 Thread René Scharfe
free(3) handles NULL pointers just fine.  Add a semantic patch for
removing unnecessary NULL checks before calling this function, and
apply it on the code base.

Signed-off-by: Rene Scharfe 
---
 contrib/coccinelle/free.cocci | 5 +
 parse-options-cb.c| 3 +--
 2 files changed, 6 insertions(+), 2 deletions(-)
 create mode 100644 contrib/coccinelle/free.cocci

diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
new file mode 100644
index 000..e282131
--- /dev/null
+++ b/contrib/coccinelle/free.cocci
@@ -0,0 +1,5 @@
+@@
+expression E;
+@@
+- if (E)
+  free(E);
diff --git a/parse-options-cb.c b/parse-options-cb.c
index b5d9209..b7d8f7d 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -211,8 +211,7 @@ int parse_opt_passthru(const struct option *opt, const char 
*arg, int unset)
if (recreate_opt(&sb, opt, arg, unset) < 0)
return -1;
 
-   if (*opt_value)
-   free(*opt_value);
+   free(*opt_value);
 
*opt_value = strbuf_detach(&sb, NULL);
 
-- 
2.10.0



[PATCH] use strbuf_add_unique_abbrev() for adding short hashes, part 3

2016-10-08 Thread René Scharfe
Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
instead of taking detours through find_unique_abbrev() and its static
buffer.  This is shorter in most cases and a bit more efficient.

The changes here are not easily handled by a semantic patch because
they involve removing temporary variables and deconstructing format
strings for strbuf_addf().

Signed-off-by: Rene Scharfe 
---
 merge-recursive.c |  6 +++---
 pretty.c  | 12 +---
 submodule.c   |  7 +++
 wt-status.c   | 10 --
 4 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 5200d5c..9041c2f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -202,9 +202,9 @@ static void output_commit_title(struct merge_options *o, 
struct commit *commit)
strbuf_addf(&o->obuf, "virtual %s\n",
merge_remote_util(commit)->name);
else {
-   strbuf_addf(&o->obuf, "%s ",
-   find_unique_abbrev(commit->object.oid.hash,
-   DEFAULT_ABBREV));
+   strbuf_add_unique_abbrev(&o->obuf, commit->object.oid.hash,
+DEFAULT_ABBREV);
+   strbuf_addch(&o->obuf, ' ');
if (parse_commit(commit) != 0)
strbuf_addstr(&o->obuf, _("(bad commit)\n"));
else {
diff --git a/pretty.c b/pretty.c
index 25efbca..0c31495 100644
--- a/pretty.c
+++ b/pretty.c
@@ -544,15 +544,13 @@ static void add_merge_info(const struct 
pretty_print_context *pp,
strbuf_addstr(sb, "Merge:");
 
while (parent) {
-   struct commit *p = parent->item;
-   const char *hex = NULL;
+   struct object_id *oidp = &parent->item->object.oid;
+   strbuf_addch(sb, ' ');
if (pp->abbrev)
-   hex = find_unique_abbrev(p->object.oid.hash, 
pp->abbrev);
-   if (!hex)
-   hex = oid_to_hex(&p->object.oid);
+   strbuf_add_unique_abbrev(sb, oidp->hash, pp->abbrev);
+   else
+   strbuf_addstr(sb, oid_to_hex(oidp));
parent = parent->next;
-
-   strbuf_addf(sb, " %s", hex);
}
strbuf_addch(sb, '\n');
 }
diff --git a/submodule.c b/submodule.c
index 2de06a3..476f60b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -392,10 +392,9 @@ static void show_submodule_header(FILE *f, const char 
*path,
}
 
 output_header:
-   strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
-   find_unique_abbrev(one->hash, DEFAULT_ABBREV));
-   if (!fast_backward && !fast_forward)
-   strbuf_addch(&sb, '.');
+   strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
+   strbuf_add_unique_abbrev(&sb, one->hash, DEFAULT_ABBREV);
+   strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
strbuf_add_unique_abbrev(&sb, two->hash, DEFAULT_ABBREV);
if (message)
strbuf_addf(&sb, " %s%s\n", message, reset);
diff --git a/wt-status.c b/wt-status.c
index 99d1b0a..ca5c45f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1110,7 +1110,6 @@ static void abbrev_sha1_in_line(struct strbuf *line)
split = strbuf_split_max(line, ' ', 3);
if (split[0] && split[1]) {
unsigned char sha1[20];
-   const char *abbrev;
 
/*
 * strbuf_split_max left a space. Trim it and re-add
@@ -1118,9 +1117,10 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 */
strbuf_trim(split[1]);
if (!get_sha1(split[1]->buf, sha1)) {
-   abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
strbuf_reset(split[1]);
-   strbuf_addf(split[1], "%s ", abbrev);
+   strbuf_add_unique_abbrev(split[1], sha1,
+DEFAULT_ABBREV);
+   strbuf_addch(split[1], ' ');
strbuf_reset(line);
for (i = 0; split[i]; i++)
strbuf_addbuf(line, split[i]);
@@ -1343,10 +1343,8 @@ static char *get_branch(const struct worktree *wt, const 
char *path)
else if (starts_with(sb.buf, "refs/"))
;
else if (!get_sha1_hex(sb.buf, sha1)) {
-   const char *abbrev;
-   abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
strbuf_reset(&sb);
-   strbuf_addstr(&sb, abbrev);
+   strbuf_add_unique_abbrev(&sb, sha1, DEFAULT_ABBREV);
} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
goto got_nothing;
else/* bisect */
-- 
2.10.1



Re: Feature request: use relative path in worktree config files

2016-10-08 Thread Stéphane Klein
I've write a small tool in Golang to fix this issue:
https://github.com/harobed/fix-git-worktree

2016-10-08 11:35 GMT+02:00 Stéphane Klein :
> Hi,
>
> "git worktree add" write absolute path in ".git/gitdir"
>
> The code source is here
> https://git.kernel.org/cgit/git/git.git/tree/builtin/worktree.c?h=v2.10.1#n256
>
> Is it possible to use relative path in this config files:
>
> * [main_worktree]/.git/worktrees/[worktree_foobar]/gitdir
> * [worktree_foobar]/.git
>
> Why:
>
> 1. I configure worktree on my host
> 2. next I use this git working copy in Docker with volume share
> 3. next I've some git error in Docker because config files use absolute path
>
> Best regards,
> Stéphane
> --
> Stéphane Klein 
> blog: http://stephane-klein.info
> cv : http://cv.stephane-klein.info
> Twitter: http://twitter.com/klein_stephane



-- 
Stéphane Klein 
blog: http://stephane-klein.info
cv : http://cv.stephane-klein.info
Twitter: http://twitter.com/klein_stephane


Re: [PATCH v10 13/14] convert: add filter..process option

2016-10-08 Thread Jakub Narębski
Part 1 of review, starting with the protocol v2 itself.

W dniu 08.10.2016 o 13:25, larsxschnei...@gmail.com pisze:
> From: Lars Schneider 
> 
> Git's clean/smudge mechanism invokes an external filter process for
> every single blob that is affected by a filter. If Git filters a lot of
> blobs then the startup time of the external filter processes can become
> a significant part of the overall Git execution time.
> 
> In a preliminary performance test this developer used a clean/smudge
> filter written in golang to filter 12,000 files. This process took 364s
> with the existing filter mechanism and 5s with the new mechanism. See
> details here: https://github.com/github/git-lfs/pull/1382
> 
> This patch adds the `filter..process` string option which, if
> used, keeps the external filter process running and processes all blobs
> with the packet format (pkt-line) based protocol over standard input and
> standard output. The full protocol is explained in detail in
> `Documentation/gitattributes.txt`.
> 
> A few key decisions:
> 
> * The long running filter process is referred to as filter protocol
>   version 2 because the existing single shot filter invocation is
>   considered version 1.
> * Git sends a welcome message and expects a response right after the
>   external filter process has started. This ensures that Git will not
>   hang if a version 1 filter is incorrectly used with the
>   filter..process option for version 2 filters. In addition,
>   Git can detect this kind of error and warn the user.
> * The status of a filter operation (e.g. "success" or "error) is set
>   before the actual response and (if necessary!) re-set after the
>   response. The advantage of this two step status response is that if
>   the filter detects an error early, then the filter can communicate
>   this and Git does not even need to create structures to read the
>   response.
> * All status responses are pkt-line lists terminated with a flush
>   packet. This allows us to send other status fields with the same
>   protocol in the future.

Looks good to me.

> 
> Helped-by: Martin-Louis Bright 
> Reviewed-by: Jakub Narebski 
> Signed-off-by: Lars Schneider 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/gitattributes.txt | 157 +-
>  convert.c   | 297 +-
>  t/t0021-conversion.sh   | 447 
> +++-
>  t/t0021/rot13-filter.pl | 191 +
>  4 files changed, 1082 insertions(+), 10 deletions(-)
>  create mode 100755 t/t0021/rot13-filter.pl
> 
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 7aff940..5868f00 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -293,7 +293,13 @@ checkout, when the `smudge` command is specified, the 
> command is
>  fed the blob object from its standard input, and its standard
>  output is used to update the worktree file.  Similarly, the
>  `clean` command is used to convert the contents of worktree file
> -upon checkin.
> +upon checkin. By default these commands process only a single
> +blob and terminate.  If a long running `process` filter is used
> +in place of `clean` and/or `smudge` filters, then Git can process
> +all blobs with a single filter command invocation for the entire
> +life of a single Git command, for example `git add --all`.  See
> +section below for the description of the protocol used to
> +communicate with a `process` filter.

I don't remember how this part looked like in previous versions
of this patch series, but "... is used in place of `clean` ..."
does not tell explicitly about the precedence of those 
configuration variables.  I think it should be stated explicitly
that `process` takes precedence over any `clean` and/or `smudge`
settings for the same `filter.` (regardless of whether
the long running `process` filter support "clean" and/or "smudge"
operations or not).

>  
>  One use of the content filtering is to massage the content into a shape
>  that is more convenient for the platform, filesystem, and the user to use.
> @@ -373,6 +379,155 @@ not exist, or may have different contents. So, smudge 
> and clean commands
>  should not try to access the file on disk, but only act as filters on the
>  content provided to them on standard input.
>  
> +Long Running Filter Process
> +^^^
> +
> +If the filter command (a string value) is defined via
> +`filter..process` then Git can process all blobs with a
> +single filter invocation for the entire life of a single Git
> +command. This is achieved by using a packet format (pkt-line,
> +see technical/protocol-common.txt) based protocol over standard
> +input and standard output as follows. All packets, except for the
> +"*CONTENT" packets and the "" flush packet, are considered
> +text and therefore are terminated by a LF.

Maybe s/standard input and output/\& of filter process,/ (that is,
add 

Bug? git worktree fails with master on bare repo

2016-10-08 Thread Michael Tutty
Hey all,
I'm working on some server-side software to do a merge. By using git
worktree it's possible to check out a given branch for a bare repo and
merge another branch into it. It's very fast, even with large
repositories.

The only exception seems to be merging to master. When I do git
worktree add /tmp/path/to/worktree master I get an error:

[fatal: 'master' is already checked out at '/path/to/bare/repo']

But this is clearly not true, git worktree list gives:

[/path/to/bare/repo (bare)]

...and of course, there is no work tree at that path, just the bare
repo files you'd expect.


Re: [PATCH v10 13/14] convert: add filter..process option

2016-10-08 Thread Torsten Bögershausen
On 09.10.16 01:06, Jakub Narębski wrote:
>> +
>> > +packet:  git< status=abort
>> > +packet:  git< 
>> > +
>> > +
>> > +After the filter has processed a blob it is expected to wait for
>> > +the next "key=value" list containing a command. Git will close
>> > +the command pipe on exit. The filter is expected to detect EOF
>> > +and exit gracefully on its own.
> Any "kill filter" solutions should probably be put here.  I guess
> that filter exiting means EOF on its standard output when read
> by Git command, isn't it?
>
Isn't it that Git closes the command pipe, then filter sees EOF on it's stdin

and does a graceful exit.





Re: [PATCH v10 14/14] contrib/long-running-filter: add long running filter example

2016-10-08 Thread Torsten Bögershausen
On 08.10.16 13:25, larsxschnei...@gmail.com wrote:
> From: Lars Schneider 
> 
> Add a simple pass-thru filter as example implementation for the Git
> filter protocol version 2. See Documentation/gitattributes.txt, section
> "Filter Protocol" for more info.
> 

Nothing wrong with code in contrib.
I may have missed parts of the discussion, was there a good reason to
drop the test case completely?

>When adding a new feature, make sure that you have new tests to show
>the feature triggers the new behavior when it should, and to show the
>feature does not trigger when it shouldn't.  After any code change, make
>sure that the entire test suite passes.

Or is there a plan to add them later ?



%C(auto) not working as expected

2016-10-08 Thread Tom Hale

$ ~/repo/git/git --version
git version 2.10.0.GIT

The `git-log` man page says:

`auto` alone (i.e.  %C(auto)) will turn on auto coloring on the next 
placeholders until the color is switched again.


In this example:

http://i.imgur.com/y3yLxk7.png

I turn on auto colouring for green, but it seems that this is not being 
respected when piped through `cat`.


Am I misunderstanding the manual?

--
Tom Hale


Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

2016-10-08 Thread Jeff King
On Sun, Oct 09, 2016 at 02:01:49AM -0400, Jeff King wrote:

> > So what about this?
> > 
> > [alias]
> > d2u = !dos2unix
> > [alias "d2u"]
> > shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
> > exec = C:/cygwin64/bin/dos2unix.exe
> > 
> > You introduce all kinds of ambiguities here that did not exist before...
> 
> If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> then yes, I think that's an unavoidable part of the transition.  IMHO,
> the new should take precedence over the old, and people will gradually
> move from one to the other.
> 
> If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> example, the problem is that you have keys with overlapping meanings.
> One solution is "don't do that" (so have a key like "cmd", and another
> to select "shell or git-cmd", etc). Another is to define some rule, like
> "last one wins" (so "exec" overrides "shell" in your example).
> 
> I'd prefer the "don't do that" path. The config you showed is
> nonsensical, and it doesn't really matter that much how we behave. But
> it is better still if we have a config scheme that makes it hard to
> write nonsensical things in the first place.

Just to be clear on my "don't do that", I don't mean "the user should
not do that", but "do not design a config scheme with keys that have
overlapping meanings".

-Peff


Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

2016-10-08 Thread Jeff King
On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote:

> > > Maybe it's time to aim for
> > > 
> > >   git config alias.d2u.shell \
> > >'f() { git ls-files "$@" | xargs dos2unix; }; f'
> > >   git config alias.d2u.cdup false
> > >   git d2u *.c   # yada!
> > 
> > That would be nice. It would also allow "alias.foo_bar.shell"; right now
> > "alias.foo_bar" is forbidden because of the config syntax, which does
> > not allow underscores outside of the "subsection" name.
> 
> So what about this?
> 
>   [alias]
>   d2u = !dos2unix
>   [alias "d2u"]
>   shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
>   exec = C:/cygwin64/bin/dos2unix.exe
> 
> You introduce all kinds of ambiguities here that did not exist before...

If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
then yes, I think that's an unavoidable part of the transition.  IMHO,
the new should take precedence over the old, and people will gradually
move from one to the other.

If you mean the ambiguity between alias.X.shell and alias.X.exec in your
example, the problem is that you have keys with overlapping meanings.
One solution is "don't do that" (so have a key like "cmd", and another
to select "shell or git-cmd", etc). Another is to define some rule, like
"last one wins" (so "exec" overrides "shell" in your example).

I'd prefer the "don't do that" path. The config you showed is
nonsensical, and it doesn't really matter that much how we behave. But
it is better still if we have a config scheme that makes it hard to
write nonsensical things in the first place.

-Peff


A head scratcher, clone results in modified files (tested linux and cygwin) - .gitattributes file?

2016-10-08 Thread Jason Pyeron
Does anyone have any ideas as to why this clone resulted in modified files and 
how to prevent it?

There is a .gitattributes in the tree, which says:

*text=auto
*.java   text diff=java
*.html   text diff=html
*.csstext
*.js text
*.sqltext

But even then the *.bin files full of non-ascii garbage are impacted?! But I 
cannot find a difference:

root@YYY /projects/commons-io
$ git cat-file -p 7c9cd59c8a00e0bd3f18da42b32cd40024ad1505 | hexdump -C
  a9 fa bf e9 a4 6c a8 ca  0d 0a c1 63 c5 e9 a4 a4  |.l.c|
0010  a4 e5 0d 0a   ||
0014

root@YYY /projects/commons-io
$ git cat-file -p 7c9cd59c8a00e0bd3f18da42b32cd40024ad1505 | sha1sum.exe
d69820e1282801ccd627e35fb213e8832949c6ac *-

root@YYY /projects/commons-io
$ hexdump.exe -C src/test/resources/test-file-x-windows-950.bin
  a9 fa bf e9 a4 6c a8 ca  0d 0a c1 63 c5 e9 a4 a4  |.l.c|
0010  a4 e5 0d 0a   ||
0014

root@YYY /projects/commons-io
$ sha1sum.exe src/test/resources/test-file-x-windows-950.bin
d69820e1282801ccd627e35fb213e8832949c6ac 
*src/test/resources/test-file-x-windows-950.bin

Deleting the .gitattributes and the checkout -- did not help. No luck deleting 
the file then restoring it either.

Not even git clone git://git.apache.org/commons-io.git --config 
core.attributesFile=/dev/null fixed it. Details below.

-Jason

Cygwin test:

root@YYY /projects
$ git clone git://git.apache.org/commons-io.git
Cloning into 'commons-io'...
remote: Counting objects: 21203, done.
remote: Compressing objects: 100% (3454/3454), done.
remote: Total 21203 (delta 10822), reused 21129 (delta 10794)
Receiving objects: 100% (21203/21203), 2.51 MiB | 607.00 KiB/s, done.
Resolving deltas: 100% (10822/10822), done.
Checking connectivity... done.

root@YYY /projects
$ cd commons-io/

root@YYY /projects/commons-io
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   
src/main/java/org/apache/commons/io/serialization/package.html
modified:   src/site/xdoc/upgradeto2_3.xml
modified:   
src/test/resources/org/apache/commons/io/FileUtilsTestDataCRLF.dat
modified:   src/test/resources/test-file-gbk.bin
modified:   
src/test/resources/test-file-iso8859-1-shortlines-win-linebr.bin
modified:   src/test/resources/test-file-utf8-win-linebr.bin
modified:   src/test/resources/test-file-windows-31j.bin
modified:   src/test/resources/test-file-x-windows-949.bin
modified:   src/test/resources/test-file-x-windows-950.bin

no changes added to commit (use "git add" and/or "git commit -a")

root@YYY /projects/commons-io
$ git rev-parse HEAD
c5f2e40e7a8234fe48e08d451d3152ba58a03ac6

root@YYY /projects/commons-io
$ git version
git version 2.8.3

root@YYY /projects/commons-io
$ git config --list
user.email=jpye...@pdinc.us
user.name=Jason Pyeron
credential.helper=cache --timeout=99
push.default=simple
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
core.ignorecase=true
remote.origin.url=git://git.apache.org/commons-io.git
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
branch.master.remote=origin
branch.master.merge=refs/heads/master

$ uname -a
CYGWIN_NT-6.1-WOW YYY 2.5.2(0.297/5/3) 2016-06-23 14:27 i686 Cygwin

root@YYY /projects/commons-io
$

Linux test:

root@XX /tmp
# git clone git://git.apache.org/commons-io.git
Cloning into 'commons-io'...
remote: Counting objects: 21203, done.
remote: Compressing objects: 100% (3454/3454), done.
remote: Total 21203 (delta 10822), reused 21129 (delta 10794)
Receiving objects: 100% (21203/21203), 2.51 MiB | 176 KiB/s, done.
Resolving deltas: 100% (10822/10822), done.

root@XX /tmp
# cd commons-io/

root@XX /tmp/commons-io
# git status
# On branch master
# Changes not staged for commit:
#   (use "git add ..." to update what will be committed)
#   (use "git checkout -- ..." to discard changes in working directory)
#
#   modified:   
src/main/java/org/apache/commons/io/serialization/package.html
#   modified:   src/site/xdoc/upgradeto2_3.xml
#   modified:   
src/test/resources/org/apache/commons/io/FileUtilsTestDataCRLF.dat
#   modified:   src/test/resources/test-file-gbk.bin
#   modified:   
src/test/resources/test-file-iso8859-1-shortlines-win-linebr.bin
#   modified:   src/test/resources/test-file-utf8-win-linebr.bin
#   modified:   src/test/resources/test-file-windows-31j.bin
#   modified:   src/test/resources/test-file-x-windows-949.bin
#   modified:   src/test/resources/test-file-x-windows-950.bin
#
no changes added to commit (use "git add" and/or "git commit -a")

root@XX /tmp/commons-i

Re: %C(auto) not working as expected

2016-10-08 Thread René Scharfe

Am 09.10.2016 um 07:43 schrieb Tom Hale:

$ ~/repo/git/git --version
git version 2.10.0.GIT

The `git-log` man page says:

`auto` alone (i.e.  %C(auto)) will turn on auto coloring on the next
placeholders until the color is switched again.

In this example:

http://i.imgur.com/y3yLxk7.png

I turn on auto colouring for green, but it seems that this is not being
respected when piped through `cat`.

Am I misunderstanding the manual?


So this colors the ref name decoration and the short hash:

$ git log -n1 --format="%C(auto)%d %C(auto)%Cgreen%h"

And this only colors the short hash:

$ git log -n1 --format="%C(auto)%d %C(auto)%Cgreen%h" | cat

%C(auto) respects the color-related configuration settings; that's 
mentioned on the man page for git log in the section on %C(...).  You 
probably have color.ui=auto or color.diff=auto in your config, which 
means that output to terminals is to be colored, but output to files and 
pipes isn't.  You could override that setting e.g. by adding the command 
line option --color=always.


%Cgreen emits color codes unconditionally.  %C(auto,green) would respect 
the config settings.


Your second %C(auto) has no effect because it is overridden by the 
immediately following %Cgreen.


You may want to add a %Creset at the end to avoid colors bleeding out 
over the end of the line.  You can see that happening e.g. with:


$ git log -n3 --format="normal %C(green)green" | cat

Without cat bash seems to add a reset automatically.

René



Re: Bug? git worktree fails with master on bare repo

2016-10-08 Thread Kevin Daudt
On Sat, Oct 08, 2016 at 07:30:36PM -0500, Michael Tutty wrote:
> Hey all,
> I'm working on some server-side software to do a merge. By using git
> worktree it's possible to check out a given branch for a bare repo and
> merge another branch into it. It's very fast, even with large
> repositories.
> 
> The only exception seems to be merging to master. When I do git
> worktree add /tmp/path/to/worktree master I get an error:
> 
> [fatal: 'master' is already checked out at '/path/to/bare/repo']
> 
> But this is clearly not true, git worktree list gives:
> 
> [/path/to/bare/repo (bare)]
> 
> ...and of course, there is no work tree at that path, just the bare
> repo files you'd expect.

A bare repo still has a HEAD, which by default points to
refs/heads/master. That's what's it complaining about.

So the question is, should there be an exception for the branch 'checked
out' on a bare reposity.