Git development
 help / color / mirror / Atom feed
* Re: How to simulate a real checkout to test a new smudge filter?
From: john smith @ 2016-09-10 16:31 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: Junio C Hamano, Torsten Bögershausen, git
In-Reply-To: <a00fa147-291c-536c-35da-56f88221e0f8@gmail.com>

On 9/10/16, Jakub Narębski <jnareb@gmail.com> wrote:
> You would need post-checkout hook together with clean / smudge filters
> (though you could get by without smudge filter, at least in theory...).
> The `post-checkout` hook could run e.g. "git checkout -- '*.conf'"
> to force use of smudge filter, after checking that it was a change
> of branch ("git checkout <commit-ish>").  See githooks(5) for details
> (last, third parameter passed to hook script is 1, rather than 0).

Neat idea.  I could still have an advantage of filter as git status
wouldn't show that files are changed and there would be no merge
conflicts and additionally filters would run after every checkout even
if files stayed the same.  But are sure that `git checkout -- <FILE>'
actually forces checkout and runs smudge filter?  It doesn't work for
me neither with 2.9.0 nor 2.10.0.85.gcda1bbd.  For a minimal example,
let's say that I have this in .gitattributes:

a     filter=test-filter

And this in .git/config:

smudge = touch /tmp/SMUDGE_RUN && cat

And also have a tracked file named `a' in the repository. Now this
should create /tmp/SMUDGE_RUN:

git checkout  -- a

but it doesn't.

This also doesn't work:

git checkout -f -- a

But this does:

git checkout HEAD^ && git co -

> Unfortunately I don't see a way to query .gitattributes files to
> find out all patterns that match specific attribute; you would need
> to duplicate configuration:
>
>   .gitattributes:
>   *.conf filter=transform
>
>   .git/config
>   [filter "transform"]
>   	clean  = replace-with-placeholder %f
>   	smudge = expand-with-branchname %f
>
>   .git/hooks/post-checkout
>   #!/bin/sh
>
>   test "$3" -eq "1" && git checkout -- '*.conf'

It's a secondary issue, but - in my repository I only have one .conf
file on every branch with real definitions of machine-specific
variables and a number of various config files for various programs
such as bash/.bashrc or screen/.screenrc so in my case it would be:

  .gitattributes:
  bash/.basrc filter=transform
  git/.gitconfig filter=transform
  (...)

  .git/config
  [filter "transform"]
        clean  = replace-with-placeholder %f
        smudge = expand-with-branchname %f

  .git/hooks/post-checkout
  #!/bin/sh

  test "$3" -eq "1" && (
       git checkout -- bash/.bashrc &&
       git checkout -- git/.gitconfig
       (...)
       )

Of course a list of files in .git/hooks/post-checkout could be
generated at runtime from first column of .gitattributes.

-- 
<wempwer@gmail.com>

^ permalink raw reply

* Re: [PATCH v7 10/10] convert: add filter.<driver>.process option
From: Torsten Bögershausen @ 2016-09-10 16:40 UTC (permalink / raw)
  To: larsxschneider
  Cc: git, peff, gitster, sbeller, Johannes.Schindelin, jnareb,
	mlbright, jacob.keller
In-Reply-To: <20160908182132.50788-11-larsxschneider@gmail.com>

[]

One general question up here, more comments inline.
The current order for a clean-filter is like this, I removed the error handling:

int convert_to_git()
{
	ret |= apply_filter(path, src, len, -1, dst, filter);
	if (ret && dst) {
		src = dst->buf;
		len = dst->len;
	}
	ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
	return ret | ident_to_git(path, src, len, dst, ca.ident);
}

The first step is the clean filter, the CRLF-LF conversion (if needed),
then ident.
The current implementation streams the whole file content to the filter,
(STDIN of the filter) and reads back STDOUT from the filter into a STRBUF.
This is to use the UNIX-like STDIN--STDOUT method for writing a filter.

However, int would_convert_to_git_filter_fd() and convert_to_git_filter_fd()
offer a sort of short-cut:
The filter reads from the file directly, and the output of the filter is
read into a STRBUF.

It looks as if the multi-filter approach can use this in a similar way:
Give the pathname to the filter, the filter opens the file for reading
and stream the result via the pkt-line protocol into Git.
This needs some more changes, and may be very well go into a separate patch
series. (and should).

What I am asking for:
When a multi-filter is used, the content is handled to the filter via pkt-line,
and the result is given to Git via pkt-line ?
Nothing wrong with it, I just wonder, if it should be mentioned somewhere.

> diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl
> new file mode 100755
> index 0000000..279fbfb
> --- /dev/null
> +++ b/contrib/long-running-filter/example.pl
> @@ -0,0 +1,111 @@
> +#!/usr/bin/perl
> +#
> +# Example implementation for the Git filter protocol version 2
> +# See Documentation/gitattributes.txt, section "Filter Protocol"
> +#
> +
> +use strict;
> +use warnings;
> +
> +my $MAX_PACKET_CONTENT_SIZE = 65516;
> +
> +sub packet_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 size '$bytes_read' field";
> +    }

This is half-kosher, I would say,
(And I really. really would like to see an implementation in C ;-)

A read function may look like this:

   ret = read(0, &buffer, 4);
   if (ret < 0) {
     /* Error, nothing we can do */
     exit(1);
   } else if (ret == 0) {
     /* EOF  */
     exit(0);
   } else if (ret < 4) {
     /* 
      * Go and read more, until we have 4 bytes or EOF or Error */
   } else {
     /* Good case, see below */
   }

> +    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 expected; $bytes_read read)";
> +        }
> +        return ( 0, $buffer );
> +    }
> +    else {
> +        die "invalid packet size";
> +    }
> +}
> +
> +sub packet_write {
> +    my ($packet) = @_;
> +    print STDOUT sprintf( "%04x", length($packet) + 4 );
> +    print STDOUT $packet;
> +    STDOUT->flush();
> +}
> +
> +sub packet_flush {
> +    print STDOUT sprintf( "%04x", 0 );
> +    STDOUT->flush();
> +}
> +
> +( packet_read() eq ( 0, "git-filter-client" ) ) || die "bad initialization";
> +( packet_read() eq ( 0, "version=2" ) )         || die "bad version";
> +( packet_read() eq ( 1, "" ) )                  || die "bad version end";
> +
> +packet_write("git-filter-server\n");
> +packet_write("version=2\n");
> +
> +( packet_read() eq ( 0, "clean=true" ) )  || die "bad capability";
> +( packet_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
> +( packet_read() eq ( 1, "" ) )            || die "bad capability end";
> +
> +packet_write( "clean=true\n" );
> +packet_write( "smudge=true\n" );
> +packet_flush();
> +
> +while (1) {
> +    my ($command) = packet_read() =~ /^command=([^=]+)\n$/;
> +    my ($pathname) = packet_read() =~ /^pathname=([^=]+)\n$/;
> +
> +    packet_read();
> +
> +    my $input = "";
> +    {
> +        binmode(STDIN);
> +        my $buffer;
> +        my $done = 0;
> +        while ( !$done ) {
> +            ( $done, $buffer ) = packet_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_write("status=success\n");
> +    packet_flush();
> +    while ( length($output) > 0 ) {
> +        my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
> +        packet_write($packet);
> +        if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
> +            $output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
> +        }
> +        else {
> +            $output = "";
> +        }
> +    }
> +    packet_flush(); # flush content!
> +    packet_flush(); # empty list!
> +}
> diff --git a/convert.c b/convert.c
> index a068df7..0ed48ed 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -3,6 +3,7 @@
>  #include "run-command.h"
>  #include "quote.h"
>  #include "sigchain.h"
> +#include "pkt-line.h"
>  
>  /*
>   * convert.c - convert a file when checking it out and checking it in.
> @@ -442,7 +443,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 +457,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 = &params;
> @@ -496,14 +491,318 @@ 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)

Is CAP_ too generic, and GIT_FILTER_CAP (or so) less calling for trouble ? 

> +
> +struct cmd2process {
> +	struct hashmap_entry ent; /* must be the first member! */
> +	int supported_capabilities;
> +	const char *cmd;
> +	struct child_process process;
> +};
> +
> +static int cmd_process_map_initialized;
> +static struct hashmap cmd_process_map;
> +
> +static int cmd2process_cmp(const struct cmd2process *e1,
> +                           const struct cmd2process *e2,
> +                           const void *unused)
> +{
> +	return strcmp(e1->cmd, e2->cmd);
> +}
> +
> +static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap, const char *cmd)
> +{
> +	struct cmd2process key;
> +	hashmap_entry_init(&key, strhash(cmd));
> +	key.cmd = cmd;
> +	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;
> +	int err;
> +	va_start(args, line);
> +	for (;;)
> +	{
> +		if (!line)
> +			break;
> +		if (strlen(line) > PKTLINE_DATA_MAXLEN)
> +			return -1;
> +		err = packet_write_fmt_gently(fd, "%s", line);
> +		if (err)
> +			return err;
> +		line = va_arg(args, const char*);
> +	}
> +	va_end(args);
> +	return packet_flush_gently(fd);
> +}
> +
> +static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)
> +{
> +	int err;
> +	struct cmd2process *entry;
> +	struct child_process *process;
> +	const char *argv[] = { cmd, NULL };
> +	struct string_list cap_list = STRING_LIST_INIT_NODUP;
> +	char *cap_buf;
> +	const char *cap_name;
> +
> +	entry = xmalloc(sizeof(*entry));
> +	hashmap_entry_init(entry, strhash(cmd));
> +	entry->cmd = cmd;
> +	entry->supported_capabilities = 0;
> +	process = &entry->process;
> +
> +	child_process_init(process);
> +	process->argv = argv;
> +	process->use_shell = 1;
> +	process->in = -1;
> +	process->out = -1;
> +
> +	if (start_command(process)) {
> +		error("cannot fork to run external filter '%s'", cmd);
> +		kill_multi_file_filter(hashmap, entry);
> +		return NULL;
> +	}
> +
> +	sigchain_push(SIGPIPE, SIG_IGN);
> +
> +	err = packet_write_list(process->in, "git-filter-client", "version=2", NULL);
> +	if (err)
> +		goto done;
> +
> +	err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
> +	if (err) {
> +		error("external filter '%s' does not support long running filter protocol", cmd);
> +		goto done;
> +	}
> +	err = strcmp(packet_read_line(process->out, NULL), "version=2");
> +	if (err)
> +		goto done;
> +
> +	err = packet_write_list(process->in, "clean=true", "smudge=true", NULL);
> +
> +	for (;;)
> +	{
> +		cap_buf = packet_read_line(process->out, NULL);
> +		if (!cap_buf)
> +			break;
> +		string_list_split_in_place(&cap_list, cap_buf, '=', 1);
> +
> +		if (cap_list.nr != 2 || strcmp(cap_list.items[1].string, "true"))
> +			continue;
> +
> +		cap_name = cap_list.items[0].string;
> +		if (!strcmp(cap_name, "clean")) {
> +			entry->supported_capabilities |= CAP_CLEAN;
> +		} else if (!strcmp(cap_name, "smudge")) {
> +			entry->supported_capabilities |= CAP_SMUDGE;
> +		} else {
> +			warning(
> +				"external filter '%s' requested unsupported filter capability '%s'",
> +				cmd, cap_name
> +			);
> +		}
> +
> +		string_list_clear(&cap_list, 0);
> +	}
> +
> +done:
> +	sigchain_pop(SIGPIPE);
> +
> +	if (err || errno == EPIPE) {
> +		error("initialization for external filter '%s' failed", cmd);
> +		kill_multi_file_filter(hashmap, entry);
> +		return NULL;
> +	}
> +
> +	hashmap_add(hashmap, entry);
> +	return entry;
> +}
> +
> +static void read_multi_file_filter_values(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]) {
> +			if (!strcmp(pair[0]->buf, "status=")) {
> +				strbuf_reset(status);
> +				strbuf_addbuf(status, pair[1]);
> +			}
> +		}
> +	}
> +}
> +
> +static int apply_multi_file_filter(const char *path, const char *src, size_t len,
> +                                   int fd, struct strbuf *dst, const char *cmd,
> +                                   const int wanted_capability)
> +{
> +	int err;
> +	struct cmd2process *entry;
> +	struct child_process *process;
> +	struct stat file_stat;
> +	struct strbuf nbuf = STRBUF_INIT;
> +	struct strbuf filter_status = STRBUF_INIT;
> +	char *filter_type;
> +
> +	if (!cmd_process_map_initialized) {
> +		cmd_process_map_initialized = 1;
> +		hashmap_init(&cmd_process_map, (hashmap_cmp_fn) cmd2process_cmp, 0);
> +		entry = NULL;
> +	} else {
> +		entry = find_multi_file_filter_entry(&cmd_process_map, cmd);
> +	}
> +
> +	fflush(NULL);
> +
> +	if (!entry) {
> +		entry = start_multi_file_filter(&cmd_process_map, cmd);
> +		if (!entry)
> +			return 0;
> +	}
> +	process = &entry->process;
> +
> +	if (!(wanted_capability & entry->supported_capabilities))
> +		return 0;
> +
> +	if (CAP_CLEAN & wanted_capability)
> +		filter_type = "clean";
> +	else if (CAP_SMUDGE & wanted_capability)
> +		filter_type = "smudge";
> +	else
> +		die("unexpected filter type");
> +
> +	if (fd >= 0 && !src) {
> +		if (fstat(fd, &file_stat) == -1)
> +			return 0;
> +		len = xsize_t(file_stat.st_size);
> +	}
> +
> +	sigchain_push(SIGPIPE, SIG_IGN);
> +
> +
> +	err = (strlen(filter_type) > PKTLINE_DATA_MAXLEN);

Extra () needed ?
More () in the code...

No more comments today (except that the kill is overkill)

^ permalink raw reply

* [PATCH 00/10] Another preparatory series for rename detection
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller

From: Stefan Beller <sbeller@google.com>

This applies on top of sb/diff-cleanup.

The long term strategy is to get the rename detection done, is to run
any output through emit_line as there we can either write it to the
output file or buffer it internally across file pairs and work on the
buffered output.

As I got stuck with the word diff as well as the correct implementation
of builtin_diff, I am sending this out as an intermediate state.
As the diff code is central to Git, I'd rather send these cleanups
early so I do not run into merge conflicts with other people down
the road.

Thanks,
Stefan

Stefan Beller (10):
  diff: move line ending check into emit_hunk_header
  diff: emit_{add, del, context}_line to increase {pre,post}image line
    count
  diff.c: drop tautologous condition in emit_line_0
  diff.c: rename diff_flush_patch to diff_flush_patch_filepair
  diff.c: reintroduce diff_flush_patch for all files
  diff.c: emit_line_0 can handle no color
  diff.c: convert fn_out_consume to use emit_line_*
  diff.c: convert emit_rewrite_diff to use emit_line_*
  diff.c: convert emit_rewrite_lines to use emit_line_0
  submodule.c: convert show_submodule_summary to use emit_line_fmt

 diff.c      | 114 ++++++++++++++++++++++++++++++++++++------------------------
 diff.h      |   3 ++
 submodule.c |  42 +++++++++-------------
 submodule.h |   3 +-
 4 files changed, 89 insertions(+), 73 deletions(-)

-- 
2.7.4


^ permalink raw reply

* [PATCH 01/10] diff: move line ending check into emit_hunk_header
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index cc8e812..aa50b2d 100644
--- a/diff.c
+++ b/diff.c
@@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
 	}
 
 	strbuf_add(&msgbuf, line + len, org_len - len);
+	if (line[org_len - 1] != '\n')
+		strbuf_addch(&msgbuf, '\n');
+
 	emit_line(ecbdata->opt, "", "", msgbuf.buf, msgbuf.len);
 	strbuf_release(&msgbuf);
 }
@@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		len = sane_truncate_line(ecbdata, line, len);
 		find_lno(line, ecbdata);
 		emit_hunk_header(ecbdata, line, len);
-		if (line[len-1] != '\n')
-			putc('\n', o->file);
 		return;
 	}
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

At all call sites of emit_{add, del, context}_line we increment the line
count, so move it into the respective functions to make the code at the
calling site a bit clearer.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index aa50b2d..156c2aa 100644
--- a/diff.c
+++ b/diff.c
@@ -536,6 +536,7 @@ static void emit_add_line(const char *reset,
 			  struct emit_callback *ecbdata,
 			  const char *line, int len)
 {
+	ecbdata->lno_in_postimage++;
 	emit_line_checked(reset, ecbdata, line, len,
 			  DIFF_FILE_NEW, WSEH_NEW, '+');
 }
@@ -544,6 +545,7 @@ static void emit_del_line(const char *reset,
 			  struct emit_callback *ecbdata,
 			  const char *line, int len)
 {
+	ecbdata->lno_in_preimage++;
 	emit_line_checked(reset, ecbdata, line, len,
 			  DIFF_FILE_OLD, WSEH_OLD, '-');
 }
@@ -552,6 +554,8 @@ static void emit_context_line(const char *reset,
 			      struct emit_callback *ecbdata,
 			      const char *line, int len)
 {
+	ecbdata->lno_in_postimage++;
+	ecbdata->lno_in_preimage++;
 	emit_line_checked(reset, ecbdata, line, len,
 			  DIFF_CONTEXT, WSEH_CONTEXT, ' ');
 }
@@ -662,13 +666,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 
 		endp = memchr(data, '\n', size);
 		len = endp ? (endp - data + 1) : size;
-		if (prefix != '+') {
-			ecb->lno_in_preimage++;
+		if (prefix != '+')
 			emit_del_line(reset, ecb, data, len);
-		} else {
-			ecb->lno_in_postimage++;
+		else
 			emit_add_line(reset, ecb, data, len);
-		}
 		size -= len;
 		data += len;
 	}
@@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 
 	switch (line[0]) {
 	case '+':
-		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);
 		break;
 	case '-':
-		ecbdata->lno_in_preimage++;
 		emit_del_line(reset, ecbdata, line + 1, len - 1);
 		break;
 	case ' ':
-		ecbdata->lno_in_postimage++;
-		ecbdata->lno_in_preimage++;
 		emit_context_line(reset, ecbdata, line + 1, len - 1);
 		break;
 	default:
-- 
2.7.4


^ permalink raw reply related

* [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

When `first` equals '\r', then it cannot equal '\n', which makes
`!has_trailing_newline` always true if `first` is '\r'.
Drop that check.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 156c2aa..9d2e704 100644
--- a/diff.c
+++ b/diff.c
@@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
 
 	if (len == 0) {
 		has_trailing_newline = (first == '\n');
-		has_trailing_carriage_return = (!has_trailing_newline &&
-						(first == '\r'));
+		has_trailing_carriage_return = (first == '\r');
 		nofirst = has_trailing_newline || has_trailing_carriage_return;
 	} else {
 		has_trailing_newline = (len > 0 && line[len-1] == '\n');
-- 
2.7.4


^ permalink raw reply related

* [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

---
 diff.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 85fb887..87b1bb2 100644
--- a/diff.c
+++ b/diff.c
@@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
 		warning(rename_limit_advice, varname, needed);
 }
 
+static void diff_flush_patch(struct diff_options *o)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		if (check_pair_status(p))
+			diff_flush_patch_filepair(p, o);
+	}
+}
+
 void diff_flush(struct diff_options *options)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
@@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)
 			}
 		}
 
-		for (i = 0; i < q->nr; i++) {
-			struct diff_filepair *p = q->queue[i];
-			if (check_pair_status(p))
-				diff_flush_patch_filepair(p, options);
-		}
+		diff_flush_patch(options);
 	}
 
 	if (output_format & DIFF_FORMAT_CALLBACK)
-- 
2.7.4


^ permalink raw reply related

* [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 9d2e704..85fb887 100644
--- a/diff.c
+++ b/diff.c
@@ -4176,7 +4176,7 @@ int diff_unmodified_pair(struct diff_filepair *p)
 	return 0;
 }
 
-static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
+static void diff_flush_patch_filepair(struct diff_filepair *p, struct diff_options *o)
 {
 	if (diff_unmodified_pair(p))
 		return;
@@ -4672,7 +4672,7 @@ void diff_flush(struct diff_options *options)
 	    DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
 	    DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
 		/*
-		 * run diff_flush_patch for the exit status. setting
+		 * run diff_flush_patch_filepair for the exit status. setting
 		 * options->file to /dev/null should be safe, because we
 		 * aren't supposed to produce any output anyway.
 		 */
@@ -4685,7 +4685,7 @@ void diff_flush(struct diff_options *options)
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
 			if (check_pair_status(p))
-				diff_flush_patch(p, options);
+				diff_flush_patch_filepair(p, options);
 			if (options->found_changes)
 				break;
 		}
@@ -4705,7 +4705,7 @@ void diff_flush(struct diff_options *options)
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
 			if (check_pair_status(p))
-				diff_flush_patch(p, options);
+				diff_flush_patch_filepair(p, options);
 		}
 	}
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

In the coming patches we want to eliminate writes that do not go
though emit_line_* as we later want to hook into the emit_line_*
functions to add a buffered output. To make the conversion easy,
add a function that takes a format string.

Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
---
 diff.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 2aefd0f..7dcef73 100644
--- a/diff.c
+++ b/diff.c
@@ -493,6 +493,19 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset
 	emit_line_0(o, set, reset, line[0], line+1, len-1);
 }
 
+static void emit_line_fmt(struct diff_options *o,
+			  const char *set, const char *reset,
+			  const char *fmt, ...)
+{
+	struct strbuf sb = STRBUF_INIT;
+	va_list ap;
+	va_start(ap, fmt);
+	strbuf_vaddf(&sb, fmt, ap);
+	va_end(ap);
+
+	emit_line(o, set, reset, sb.buf, sb.len);
+}
+
 static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
 {
 	if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
@@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
 	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
 	struct diff_options *o = ecbdata->opt;
-	const char *line_prefix = diff_line_prefix(o);
 
 	o->found_changes = 1;
 
@@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
 		name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
 
-		fprintf(o->file, "%s%s--- %s%s%s\n",
-			line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
-		fprintf(o->file, "%s%s+++ %s%s%s\n",
-			line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
+		emit_line_fmt(o, meta, reset, "--- %s%s\n",
+			      ecbdata->label_path[0], name_a_tab);
+
+		emit_line_fmt(o, meta, reset, "+++ %s%s\n",
+			      ecbdata->label_path[1], name_b_tab);
+
 		ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
 	}
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 10/10] submodule.c: convert show_submodule_summary to use emit_line_fmt
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

---
 diff.c      |  5 ++---
 diff.h      |  3 +++
 submodule.c | 42 +++++++++++++++++-------------------------
 submodule.h |  3 +--
 4 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/diff.c b/diff.c
index 255fbf3..c00306b 100644
--- a/diff.c
+++ b/diff.c
@@ -493,7 +493,7 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset
 	emit_line_0(o, set, reset, line[0], line+1, len-1);
 }
 
-static void emit_line_fmt(struct diff_options *o,
+void emit_line_fmt(struct diff_options *o,
 			  const char *set, const char *reset,
 			  const char *fmt, ...)
 {
@@ -2310,8 +2310,7 @@ static void builtin_diff(const char *name_a,
 			(!two->mode || S_ISGITLINK(two->mode))) {
 		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
 		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
-		show_submodule_summary(o->file, one->path ? one->path : two->path,
-				line_prefix,
+		show_submodule_summary(o, one->path ? one->path : two->path,
 				one->oid.hash, two->oid.hash,
 				two->dirty_submodule,
 				meta, del, add, reset);
diff --git a/diff.h b/diff.h
index 7883729..1699d9c 100644
--- a/diff.h
+++ b/diff.h
@@ -180,6 +180,9 @@ struct diff_options {
 	int diff_path_counter;
 };
 
+void emit_line_fmt(struct diff_options *o, const char *set, const char *reset,
+		   const char *fmt, ...);
+
 enum color_diff {
 	DIFF_RESET = 0,
 	DIFF_CONTEXT = 1,
diff --git a/submodule.c b/submodule.c
index 1b5cdfb..c817b46 100644
--- a/submodule.c
+++ b/submodule.c
@@ -304,8 +304,8 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
 	return prepare_revision_walk(rev);
 }
 
-static void print_submodule_summary(struct rev_info *rev, FILE *f,
-		const char *line_prefix,
+static void print_submodule_summary(struct rev_info *rev,
+		struct diff_options *o,
 		const char *del, const char *add, const char *reset)
 {
 	static const char format[] = "  %m %s";
@@ -317,24 +317,16 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
 		ctx.date_mode = rev->date_mode;
 		ctx.output_encoding = get_log_output_encoding();
 		strbuf_setlen(&sb, 0);
-		strbuf_addstr(&sb, line_prefix);
-		if (commit->object.flags & SYMMETRIC_LEFT) {
-			if (del)
-				strbuf_addstr(&sb, del);
-		}
-		else if (add)
-			strbuf_addstr(&sb, add);
 		format_commit_message(commit, format, &sb, &ctx);
-		if (reset)
-			strbuf_addstr(&sb, reset);
-		strbuf_addch(&sb, '\n');
-		fprintf(f, "%s", sb.buf);
+		if (commit->object.flags & SYMMETRIC_LEFT)
+			emit_line_fmt(o, del, reset, "%s\n", sb.buf);
+		else if (add)
+			emit_line_fmt(o, add, reset, "%s\n", sb.buf);
 	}
 	strbuf_release(&sb);
 }
 
-void show_submodule_summary(FILE *f, const char *path,
-		const char *line_prefix,
+void show_submodule_summary(struct diff_options *o, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule, const char *meta,
 		const char *del, const char *add, const char *reset)
@@ -359,30 +351,30 @@ void show_submodule_summary(FILE *f, const char *path,
 		message = "(revision walker failed)";
 
 	if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-		fprintf(f, "%sSubmodule %s contains untracked content\n",
-			line_prefix, path);
+		emit_line_fmt(o, 0, 0,
+			      "Submodule %s contains untracked content\n", path);
 	if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
-		fprintf(f, "%sSubmodule %s contains modified content\n",
-			line_prefix, path);
+		emit_line_fmt(o, 0, 0,
+			      "Submodule %s contains modified content\n", path);
 
 	if (!hashcmp(one, two)) {
 		strbuf_release(&sb);
 		return;
 	}
 
-	strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
-			find_unique_abbrev(one, DEFAULT_ABBREV));
+	strbuf_addf(&sb, "Submodule %s %s..", path,
+		    find_unique_abbrev(one, DEFAULT_ABBREV));
 	if (!fast_backward && !fast_forward)
 		strbuf_addch(&sb, '.');
 	strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
 	if (message)
-		strbuf_addf(&sb, " %s%s\n", message, reset);
+		strbuf_addf(&sb, " %s\n", message);
 	else
-		strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
-	fwrite(sb.buf, sb.len, 1, f);
+		strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
+	emit_line_fmt(o, meta, reset, "%s", sb.buf);
 
 	if (!message) /* only NULL if we succeeded in setting up the walk */
-		print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+		print_submodule_summary(&rev, o, del, add, reset);
 	if (left)
 		clear_commit_marks(left, ~0);
 	if (right)
diff --git a/submodule.h b/submodule.h
index 2af9390..fefc0ab 100644
--- a/submodule.h
+++ b/submodule.h
@@ -41,8 +41,7 @@ int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
 const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
-void show_submodule_summary(FILE *f, const char *path,
-		const char *line_prefix,
+void show_submodule_summary(struct diff_options *o, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule, const char *meta,
 		const char *del, const char *add, const char *reset);
-- 
2.7.4


^ permalink raw reply related

* [PATCH 09/10] diff.c: convert emit_rewrite_lines to use emit_line_0
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 98d6655..255fbf3 100644
--- a/diff.c
+++ b/diff.c
@@ -690,7 +690,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 	if (!endp) {
 		const char *context = diff_get_color(ecb->color_diff,
 						     DIFF_CONTEXT);
-		putc('\n', ecb->opt->file);
+		emit_line_0(ecb->opt, 0, 0, 0, "\n", 1);
 		emit_line_0(ecb->opt, context, reset, '\\',
 			    nneof, strlen(nneof));
 	}
-- 
2.7.4


^ permalink raw reply related

* [PATCH 08/10] diff.c: convert emit_rewrite_diff to use emit_line_*
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/diff.c b/diff.c
index 7dcef73..98d6655 100644
--- a/diff.c
+++ b/diff.c
@@ -653,17 +653,17 @@ static void remove_tempfile(void)
 	}
 }
 
-static void print_line_count(FILE *file, int count)
+static void add_line_count(struct strbuf *out, int count)
 {
 	switch (count) {
 	case 0:
-		fprintf(file, "0,0");
+		strbuf_addstr(out, "0,0");
 		break;
 	case 1:
-		fprintf(file, "1");
+		strbuf_addstr(out, "1");
 		break;
 	default:
-		fprintf(file, "1,%d", count);
+		strbuf_addf(out, "1,%d", count);
 		break;
 	}
 }
@@ -714,7 +714,7 @@ static void emit_rewrite_diff(const char *name_a,
 	char *data_one, *data_two;
 	size_t size_one, size_two;
 	struct emit_callback ecbdata;
-	const char *line_prefix = diff_line_prefix(o);
+	struct strbuf out = STRBUF_INIT;
 
 	if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		a_prefix = o->b_prefix;
@@ -752,20 +752,23 @@ static void emit_rewrite_diff(const char *name_a,
 	ecbdata.lno_in_preimage = 1;
 	ecbdata.lno_in_postimage = 1;
 
+	emit_line_fmt(o, metainfo, reset, "--- %s%s\n", a_name.buf, name_a_tab);
+	emit_line_fmt(o, metainfo, reset, "+++ %s%s\n", b_name.buf, name_b_tab);
+
 	lc_a = count_lines(data_one, size_one);
 	lc_b = count_lines(data_two, size_two);
-	fprintf(o->file,
-		"%s%s--- %s%s%s\n%s%s+++ %s%s%s\n%s%s@@ -",
-		line_prefix, metainfo, a_name.buf, name_a_tab, reset,
-		line_prefix, metainfo, b_name.buf, name_b_tab, reset,
-		line_prefix, fraginfo);
+
+	strbuf_addstr(&out, "@@ -");
 	if (!o->irreversible_delete)
-		print_line_count(o->file, lc_a);
+		add_line_count(&out, lc_a);
 	else
-		fprintf(o->file, "?,?");
-	fprintf(o->file, " +");
-	print_line_count(o->file, lc_b);
-	fprintf(o->file, " @@%s\n", reset);
+		strbuf_addstr(&out, "?,?");
+	strbuf_addstr(&out, " +");
+	add_line_count(&out, lc_b);
+	strbuf_addstr(&out, " @@\n");
+	emit_line(o, fraginfo, reset, out.buf, out.len);
+	strbuf_release(&out);
+
 	if (lc_a && !o->irreversible_delete)
 		emit_rewrite_lines(&ecbdata, '-', data_one, size_one);
 	if (lc_b)
-- 
2.7.4


^ permalink raw reply related

* [PATCH 06/10] diff.c: emit_line_0 can handle no color
From: Stefan Beller @ 2016-09-11  5:42 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-1-git-send-email-stefanbeller@gmail.com>

From: Stefan Beller <sbeller@google.com>

---
 diff.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 87b1bb2..2aefd0f 100644
--- a/diff.c
+++ b/diff.c
@@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
 	}
 
 	if (len || !nofirst) {
-		fputs(set, file);
+		if (set)
+			fputs(set, file);
 		if (!nofirst)
 			fputc(first, file);
 		fwrite(line, len, 1, file);
-		fputs(reset, file);
+		if (reset)
+			fputs(reset, file);
 	}
 	if (has_trailing_carriage_return)
 		fputc('\r', file);
-- 
2.7.4


^ permalink raw reply related

* [PATCH v2 0/5] Pull out require_clean_work_tree() functionality from builtin/pull.c
From: Johannes Schindelin @ 2016-09-11  8:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1472137582.git.johannes.schindelin@gmx.de>

This is the 5th last patch series of my work to accelerate interactive
rebases in particular on Windows.

Basically, all it does is to make reusable some functions that were
ported over from git-pull.sh but made private to builtin/pull.c.

Changes since v1:

- skipped patch that tries to make require_clean_work_tree() smart
  enough to read the index if it was not read yet.

- added a code-comment clarifying that it is the duty of
  require_clean_work_tree()'s caller to ensure that the index has been
  read.

- made the action in require_clean_work_tree() translateable. This
  amazingly easy without complexifying the code, simply by using N_()
  and _() as indicated by Jakub.


Johannes Schindelin (5):
  pull: drop confusing prefix parameter of die_on_unclean_work_tree()
  pull: make code more similar to the shell script again
  Make the require_clean_work_tree() function truly reusable
  Export also the has_un{staged,committed}_changed() functions
  wt-status: teach has_{unstaged,uncommitted}_changes() about submodules

 builtin/pull.c | 71 +++--------------------------------------------------
 wt-status.c    | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 wt-status.h    |  6 +++++
 3 files changed, 86 insertions(+), 68 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/require-clean-work-tree-v2
Fetch-It-Via: git fetch https://github.com/dscho/git require-clean-work-tree-v2

Interdiff vs v1:

 diff --git a/builtin/pull.c b/builtin/pull.c
 index 843ff19..c639167 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -809,7 +809,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
  			die(_("Updating an unborn branch with changes added to the index."));
  
  		if (!autostash)
 -			require_clean_work_tree("pull with rebase",
 +			require_clean_work_tree(N_("pull with rebase"),
  				"Please commit or stash them.", 1, 0);
  
  		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 diff --git a/wt-status.c b/wt-status.c
 index 129b054..086ae79 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -2258,20 +2258,13 @@ int require_clean_work_tree(const char *action, const char *hint, int ignore_sub
  	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
  	int err = 0;
  
 -	if (read_cache() < 0) {
 -		error(_("Could not read index"));
 -		if (gently)
 -			return -1;
 -		exit(1);
 -	}
 -
  	hold_locked_index(lock_file, 0);
  	refresh_cache(REFRESH_QUIET);
  	update_index_if_able(&the_index, lock_file);
  	rollback_lock_file(lock_file);
  
  	if (has_unstaged_changes(ignore_submodules)) {
 -		error(_("Cannot %s: You have unstaged changes."), action);
 +		error(_("Cannot %s: You have unstaged changes."), _(action));
  		err = 1;
  	}
  
 @@ -2279,7 +2272,8 @@ int require_clean_work_tree(const char *action, const char *hint, int ignore_sub
  		if (err)
  			error(_("Additionally, your index contains uncommitted changes."));
  		else
 -			error(_("Cannot %s: Your index contains uncommitted changes."), action);
 +			error(_("Cannot %s: Your index contains uncommitted changes."),
 +			      _(action));
  		err = 1;
  	}
  
 diff --git a/wt-status.h b/wt-status.h
 index f0e66c4..54fec77 100644
 --- a/wt-status.h
 +++ b/wt-status.h
 @@ -128,6 +128,7 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, .
  __attribute__((format (printf, 3, 4)))
  void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
  
 +/* The following functions expect that the caller took care of reading the index. */
  int has_unstaged_changes(int ignore_submodules);
  int has_uncommitted_changes(int ignore_submodules);
  int require_clean_work_tree(const char *action, const char *hint,

-- 
2.10.0.windows.1.10.g803177d

base-commit: cda1bbd474805e653dda8a71d4ea3790e2a66cbb

^ permalink raw reply

* [PATCH v2 1/5] pull: drop confusing prefix parameter of die_on_unclean_work_tree()
From: Johannes Schindelin @ 2016-09-11  8:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

In cmd_pull(), when verifying that there are no changes preventing a
rebasing pull, we diligently pass the prefix parameter to the
die_on_unclean_work_tree() function which in turn diligently passes it
to the has_unstaged_changes() and has_uncommitted_changes() functions.

The casual reader might now be curious (as this developer was) whether
that means that calling `git pull --rebase` in a subdirectory will
ignore unstaged changes in other parts of the working directory. And be
puzzled that `git pull --rebase` (correctly) complains about those
changes outside of the current directory.

The puzzle is easily resolved: while we take pains to pass around the
prefix and even pass it to init_revisions(), the fact that no paths are
passed to init_revisions() ensures that the prefix is simply ignored.

That, combined with the fact that we will *always* want a *full* working
directory check before running a rebasing pull, is reason enough to
simply do away with the actual prefix parameter and to pass NULL
instead, as if we were running this from the top-level working directory
anyway.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pull.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 398aae1..d4bd635 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -328,12 +328,12 @@ static int git_pull_config(const char *var, const char *value, void *cb)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(const char *prefix)
+static int has_unstaged_changes(void)
 {
 	struct rev_info rev_info;
 	int result;
 
-	init_revisions(&rev_info, prefix);
+	init_revisions(&rev_info, NULL);
 	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
 	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
 	diff_setup_done(&rev_info.diffopt);
@@ -344,7 +344,7 @@ static int has_unstaged_changes(const char *prefix)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(const char *prefix)
+static int has_uncommitted_changes(void)
 {
 	struct rev_info rev_info;
 	int result;
@@ -352,7 +352,7 @@ static int has_uncommitted_changes(const char *prefix)
 	if (is_cache_unborn())
 		return 0;
 
-	init_revisions(&rev_info, prefix);
+	init_revisions(&rev_info, NULL);
 	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
 	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
 	add_head_to_pending(&rev_info);
@@ -365,7 +365,7 @@ static int has_uncommitted_changes(const char *prefix)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(const char *prefix)
+static void die_on_unclean_work_tree(void)
 {
 	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
 	int do_die = 0;
@@ -375,12 +375,12 @@ static void die_on_unclean_work_tree(const char *prefix)
 	update_index_if_able(&the_index, lock_file);
 	rollback_lock_file(lock_file);
 
-	if (has_unstaged_changes(prefix)) {
+	if (has_unstaged_changes()) {
 		error(_("Cannot pull with rebase: You have unstaged changes."));
 		do_die = 1;
 	}
 
-	if (has_uncommitted_changes(prefix)) {
+	if (has_uncommitted_changes()) {
 		if (do_die)
 			error(_("Additionally, your index contains uncommitted changes."));
 		else
@@ -875,7 +875,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			die(_("Updating an unborn branch with changes added to the index."));
 
 		if (!autostash)
-			die_on_unclean_work_tree(prefix);
+			die_on_unclean_work_tree();
 
 		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 			hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.10.g803177d



^ permalink raw reply related

* [PATCH v2 2/5] pull: make code more similar to the shell script again
From: Johannes Schindelin @ 2016-09-11  8:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

When converting the pull command to a builtin, the
require_clean_work_tree() function was renamed and the pull-specific
parts hard-coded.

This makes it impossible to reuse the code, so let's modify the code to
make it more similar to the original shell script again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pull.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index d4bd635..a3ed054 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -365,10 +365,11 @@ static int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(void)
+static int require_clean_work_tree(const char *action, const char *hint,
+		int gently)
 {
 	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-	int do_die = 0;
+	int err = 0;
 
 	hold_locked_index(lock_file, 0);
 	refresh_cache(REFRESH_QUIET);
@@ -376,20 +377,27 @@ static void die_on_unclean_work_tree(void)
 	rollback_lock_file(lock_file);
 
 	if (has_unstaged_changes()) {
-		error(_("Cannot pull with rebase: You have unstaged changes."));
-		do_die = 1;
+		error(_("Cannot %s: You have unstaged changes."), _(action));
+		err = 1;
 	}
 
 	if (has_uncommitted_changes()) {
-		if (do_die)
+		if (err)
 			error(_("Additionally, your index contains uncommitted changes."));
 		else
-			error(_("Cannot pull with rebase: Your index contains uncommitted changes."));
-		do_die = 1;
+			error(_("Cannot %s: Your index contains uncommitted changes."),
+			      _(action));
+		err = 1;
 	}
 
-	if (do_die)
-		exit(1);
+	if (err) {
+		if (hint)
+			error("%s", hint);
+		if (!gently)
+			exit(err);
+	}
+
+	return err;
 }
 
 /**
@@ -875,7 +883,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			die(_("Updating an unborn branch with changes added to the index."));
 
 		if (!autostash)
-			die_on_unclean_work_tree();
+			require_clean_work_tree(N_("pull with rebase"),
+				"Please commit or stash them.", 0);
 
 		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 			hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.10.g803177d



^ permalink raw reply related

* [PATCH v2 3/5] Make the require_clean_work_tree() function truly reusable
From: Johannes Schindelin @ 2016-09-11  8:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

It is remarkable that libgit.a did not sport this function yet... Let's
move it into a more prominent (and into an actually reusable) spot:
wt-status.[ch].

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pull.c | 76 +---------------------------------------------------------
 wt-status.c    | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 wt-status.h    |  3 +++
 3 files changed, 79 insertions(+), 75 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a3ed054..14ef8b5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -17,6 +17,7 @@
 #include "revision.h"
 #include "tempfile.h"
 #include "lockfile.h"
+#include "wt-status.h"
 
 enum rebase_type {
 	REBASE_INVALID = -1,
@@ -326,81 +327,6 @@ static int git_pull_config(const char *var, const char *value, void *cb)
 }
 
 /**
- * Returns 1 if there are unstaged changes, 0 otherwise.
- */
-static int has_unstaged_changes(void)
-{
-	struct rev_info rev_info;
-	int result;
-
-	init_revisions(&rev_info, NULL);
-	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
-	diff_setup_done(&rev_info.diffopt);
-	result = run_diff_files(&rev_info, 0);
-	return diff_result_code(&rev_info.diffopt, result);
-}
-
-/**
- * Returns 1 if there are uncommitted changes, 0 otherwise.
- */
-static int has_uncommitted_changes(void)
-{
-	struct rev_info rev_info;
-	int result;
-
-	if (is_cache_unborn())
-		return 0;
-
-	init_revisions(&rev_info, NULL);
-	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
-	add_head_to_pending(&rev_info);
-	diff_setup_done(&rev_info.diffopt);
-	result = run_diff_index(&rev_info, 1);
-	return diff_result_code(&rev_info.diffopt, result);
-}
-
-/**
- * If the work tree has unstaged or uncommitted changes, dies with the
- * appropriate message.
- */
-static int require_clean_work_tree(const char *action, const char *hint,
-		int gently)
-{
-	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-	int err = 0;
-
-	hold_locked_index(lock_file, 0);
-	refresh_cache(REFRESH_QUIET);
-	update_index_if_able(&the_index, lock_file);
-	rollback_lock_file(lock_file);
-
-	if (has_unstaged_changes()) {
-		error(_("Cannot %s: You have unstaged changes."), _(action));
-		err = 1;
-	}
-
-	if (has_uncommitted_changes()) {
-		if (err)
-			error(_("Additionally, your index contains uncommitted changes."));
-		else
-			error(_("Cannot %s: Your index contains uncommitted changes."),
-			      _(action));
-		err = 1;
-	}
-
-	if (err) {
-		if (hint)
-			error("%s", hint);
-		if (!gently)
-			exit(err);
-	}
-
-	return err;
-}
-
-/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
diff --git a/wt-status.c b/wt-status.c
index 539aac1..9ab9adc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 #include "worktree.h"
+#include "lockfile.h"
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";
@@ -2209,3 +2210,77 @@ void wt_status_print(struct wt_status *s)
 		break;
 	}
 }
+
+/**
+ * Returns 1 if there are unstaged changes, 0 otherwise.
+ */
+static int has_unstaged_changes(void)
+{
+	struct rev_info rev_info;
+	int result;
+
+	init_revisions(&rev_info, NULL);
+	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+	diff_setup_done(&rev_info.diffopt);
+	result = run_diff_files(&rev_info, 0);
+	return diff_result_code(&rev_info.diffopt, result);
+}
+
+/**
+ * Returns 1 if there are uncommitted changes, 0 otherwise.
+ */
+static int has_uncommitted_changes(void)
+{
+	struct rev_info rev_info;
+	int result;
+
+	if (is_cache_unborn())
+		return 0;
+
+	init_revisions(&rev_info, NULL);
+	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+	add_head_to_pending(&rev_info);
+	diff_setup_done(&rev_info.diffopt);
+	result = run_diff_index(&rev_info, 1);
+	return diff_result_code(&rev_info.diffopt, result);
+}
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+int require_clean_work_tree(const char *action, const char *hint, int gently)
+{
+	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+	int err = 0;
+
+	hold_locked_index(lock_file, 0);
+	refresh_cache(REFRESH_QUIET);
+	update_index_if_able(&the_index, lock_file);
+	rollback_lock_file(lock_file);
+
+	if (has_unstaged_changes()) {
+		error(_("Cannot %s: You have unstaged changes."), _(action));
+		err = 1;
+	}
+
+	if (has_uncommitted_changes()) {
+		if (err)
+			error(_("Additionally, your index contains uncommitted changes."));
+		else
+			error(_("Cannot %s: Your index contains uncommitted changes."),
+			      _(action));
+		err = 1;
+	}
+
+	if (err) {
+		if (hint)
+			error("%s", hint);
+		if (!gently)
+			exit(err);
+	}
+
+	return err;
+}
diff --git a/wt-status.h b/wt-status.h
index e401837..03ecf53 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,4 +128,7 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, .
 __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
 
+/* The following function expect that the caller took care of reading the index. */
+int require_clean_work_tree(const char *action, const char *hint, int gently);
+
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.10.g803177d



^ permalink raw reply related

* [PATCH v2 4/5] Export also the has_un{staged,committed}_changed() functions
From: Johannes Schindelin @ 2016-09-11  8:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

They will be used in the upcoming rebase helper.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 wt-status.c | 4 ++--
 wt-status.h | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9ab9adc..f1120e6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,7 +2214,7 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(void)
+int has_unstaged_changes(void)
 {
 	struct rev_info rev_info;
 	int result;
@@ -2230,7 +2230,7 @@ static int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(void)
+int has_uncommitted_changes(void)
 {
 	struct rev_info rev_info;
 	int result;
diff --git a/wt-status.h b/wt-status.h
index 03ecf53..68e367a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,7 +128,9 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, .
 __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
 
-/* The following function expect that the caller took care of reading the index. */
+/* The following functions expect that the caller took care of reading the index. */
+int has_unstaged_changes(void);
+int has_uncommitted_changes(void);
 int require_clean_work_tree(const char *action, const char *hint, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.10.g803177d



^ permalink raw reply related

* [PATCH v2 5/5] wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
From: Johannes Schindelin @ 2016-09-11  8:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

Sometimes we are *actually* interested in those changes... For
example when an interactive rebase wants to continue with a staged
submodule update.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pull.c |  2 +-
 wt-status.c    | 16 +++++++++-------
 wt-status.h    |  7 ++++---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 14ef8b5..c639167 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 		if (!autostash)
 			require_clean_work_tree(N_("pull with rebase"),
-				"Please commit or stash them.", 0);
+				"Please commit or stash them.", 1, 0);
 
 		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 			hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index f1120e6..086ae79 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,13 +2214,14 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-int has_unstaged_changes(void)
+int has_unstaged_changes(int ignore_submodules)
 {
 	struct rev_info rev_info;
 	int result;
 
 	init_revisions(&rev_info, NULL);
-	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+	if (ignore_submodules)
+		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
 	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_files(&rev_info, 0);
@@ -2230,7 +2231,7 @@ int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-int has_uncommitted_changes(void)
+int has_uncommitted_changes(int ignore_submodules)
 {
 	struct rev_info rev_info;
 	int result;
@@ -2239,7 +2240,8 @@ int has_uncommitted_changes(void)
 		return 0;
 
 	init_revisions(&rev_info, NULL);
-	DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+	if (ignore_submodules)
+		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
 	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
 	add_head_to_pending(&rev_info);
 	diff_setup_done(&rev_info.diffopt);
@@ -2251,7 +2253,7 @@ int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-int require_clean_work_tree(const char *action, const char *hint, int gently)
+int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently)
 {
 	struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
 	int err = 0;
@@ -2261,12 +2263,12 @@ int require_clean_work_tree(const char *action, const char *hint, int gently)
 	update_index_if_able(&the_index, lock_file);
 	rollback_lock_file(lock_file);
 
-	if (has_unstaged_changes()) {
+	if (has_unstaged_changes(ignore_submodules)) {
 		error(_("Cannot %s: You have unstaged changes."), _(action));
 		err = 1;
 	}
 
-	if (has_uncommitted_changes()) {
+	if (has_uncommitted_changes(ignore_submodules)) {
 		if (err)
 			error(_("Additionally, your index contains uncommitted changes."));
 		else
diff --git a/wt-status.h b/wt-status.h
index 68e367a..54fec77 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -129,8 +129,9 @@ __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
 
 /* The following functions expect that the caller took care of reading the index. */
-int has_unstaged_changes(void);
-int has_uncommitted_changes(void);
-int require_clean_work_tree(const char *action, const char *hint, int gently);
+int has_unstaged_changes(int ignore_submodules);
+int has_uncommitted_changes(int ignore_submodules);
+int require_clean_work_tree(const char *action, const char *hint,
+	int ignore_submodules, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.10.g803177d

^ permalink raw reply related

* Re: [PATCH v3 16/17] lib'ify checkout_fast_forward_to()
From: Johannes Schindelin @ 2016-09-11  8:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Sunshine
In-Reply-To: <xmqqzinhaq5p.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Instead of dying there, let the caller high up in the callchain
> > notice the error and handle it (by dying, still).
> >
> > The only callers of checkout_fast_forward_to(), cmd_merge(),
> > pull_into_void(), cmd_pull() and sequencer's fast_forward_to(),
> > already check the return value and handle it appropriately. With this
> > step, we make it notice an error return from this function.
> >
> > So this is a safe conversion to make checkout_fast_forward_to()
> > callable from new callers that want it not to die, without changing
> > the external behaviour of anything existing.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> I'll retitle this to
> 
>     sequencer: lib'ify chckout_fast_forward()
> 
> and checkout_fast_forward_to() in the second paragraph to match the
> reality.  Other than that, the above analysis matches what I see in
> the code and the libification done here looks correct.

Good catch. Please also fix it in the third paragraph. (I changed it also
locally, in case a v4 is required.)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 22/22] sequencer: refactor write_message()
From: Johannes Schindelin @ 2016-09-11  8:26 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: git, Junio C Hamano
In-Reply-To: <2e15ae99-59cf-7fac-517b-30b59d6d574d@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]

Hi Kuba,

On Fri, 9 Sep 2016, Jakub Narębski wrote:

> W dniu 09.09.2016 o 16:40, Johannes Schindelin napisał:
> > On Fri, 2 Sep 2016, Jakub Narębski wrote:
> >> W dniu 01.09.2016 o 16:20, Johannes Schindelin pisze:
> >>> On Thu, 1 Sep 2016, Jakub Narębski wrote: 
> >>>> W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:
> >>
> >>>>>  	if (commit_lock_file(&msg_file) < 0)
> >>>>>  		return error(_("Error wrapping up %s."), filename);
> >>>>
> >>>> Another "while at it"... though the one that can be safely postponed
> >>>> (well, the make message easier to understand part, not the quote
> >>>> filename part):
> >>>>
> >>>>   		return error(_("Error wrapping up writing to '%s'."), filename);
> >>>
> >>> As I inherited this message, I'll keep it.
> >>
> >> Well, please then add quotes while at it, at least, for consistency
> >>
> >>   		return error(_("Error wrapping up '%s'."), filename);
> > 
> > I may do that as a final patch, once all the other concerns are addressed.
> > I really do not want to change the error message during the conversion.
> 
> Is not wanting to change error messages during conversion because of
> your use of Scientist tool to catch errors in conversion process?

It is more out of an aversion to mix unrelated purposes in the same patch.
You will see that I inserted an extra patch with the purpose of fixing the
style, that touches all the relevant error messages in sequencer.c.

> BTW. could you tell us what were those three regression caught by the
> cross-validation?

Sure!

The first one was that my original version of the rebase-i-extra patches
did not reorder patches correctly when there was more than one space after
the fixup!. I fixed it, and added this test:

cbcd2cb (rebase -i: we allow extra spaces after fixup!/squash!, 2016-07-07)

The second one was that `git commit --fixup` unwraps the commit subject
into one long line and rebase -i *still* manages to find the correct
commit to fix up. The test is part of the rebase-i-extra patches (and
therefore you will find this commit only in my fork):

9fc25ce (t3415: test fixup with wrapped oneline, 2016-07-24)

The third one was an obscure one: when I marked a commit as 'edit' and
there was a merge conflict cherry-picking that particular commit, rebase
--continue would squash the resolved changes *into the previous* commit,
but with the cherry-picked commit's message. It was a simple, stupid
oversight to write the "amend" file in the "edit" code path even if the
cherry-pick failed.

All fixed, of course.

Ciao,
Dscho

^ permalink raw reply

* Git garden shears, was Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file
From: Johannes Schindelin @ 2016-09-11  8:33 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: Junio C Hamano, git
In-Reply-To: <5b707a0d-6c10-abb5-3213-d13490e9b9de@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3006 bytes --]

Hi Kuba,

On Fri, 9 Sep 2016, Jakub Narębski wrote:

> Hello Johannes,
> 
> W dniu 09.09.2016 o 17:12, Johannes Schindelin napisał:
> > On Thu, 1 Sep 2016, Junio C Hamano wrote: 
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I was sort of expecting that, when you do the preserve-merges mode
> >> of "rebase -i", you would need to jump around, doing "we have
> >> reconstructed the side branch on a new 'onto', let's give the result
> >> this temporary name ':1', and then switch to the trunk (which would
> >> call for 'reset <commit>' instruction) and merge that thing (which
> >> would be 'merge :1' or perhaps called 'pick :1')", and at that point
> >> you no longer validate the object references upfront.
> > 
> > Except that is not how --preserve-merges works: it *still* uses the SHA-1s
> > as identifiers, even when the SHA-1 may have changed in the meantime.
> > 
> > That is part of why it was a bad design.
> 
> When preserving merges, there are (as far as I understand it), two
> problems:
>  - what it means to preserve changes (which change to pick,
>    that is what is the mainline changes rebase is re-applying)
>  - what are parents of the merge commit (at least one parent
>    would be usually rewritten)
> 
> Maybe the internal (and perhaps also user-visible) representation
> of merge in instruction sheet could use the notation of filter-branch,
> that is 'map(<sha-1>)'... it could also imply the mainline.
> 
> That is the instruction in the internal instruction sheet could
> look like this:
> 
>   merge -m 1 map(2fd4e1c67a2d28fced849ee1bb76e7391b93eb12) da39a3ee5e6b4b0d3255bfef95601890afd80709 \t Merge 'foo' into master  
> 
> 
> Note that it has nothing to do with this series!

Right. But I did solve that already. In the Git garden shears [*1*]
(essentially my New And Improved attempt at recreating branch structures
while rebasing), I generate and process scripts like this:

	mark onto

	# Branch: super-cool-feature
	rewind onto
	pick 00001 feature
	pick 00002 documentation
	mark super-cool-feature

	# Branch: typo-fix
	rewind onto
	pick 0000a fix a tyop

	rewind onto
	merge -C cafebabe super-cool-feature
	merge -C babecafe typo-fix

	cleanup super-cool-feature typo-fix

Of course this will change a little, still, once I get around to implement
this on top of the rebase--helper.

For example, I am not so hot about the "merge -C ..." syntax. I'll
probably split that into a "remerge <SHA-1> <mark>" and a new "merge
<mark>" command (the latter asking interactively for the merge commit
message).

And also: the cleanup stage should not be necessary, as the "mark"
commands can accumulate the known marks into a file in the state
directory.

But you get the idea.

No :1 or some such. That's machine readable. But it's utter nonsense for
user-facing UIs.

Ciao,
Dscho

Footnote *1*:
https://github.com/git-for-windows/build-extra/blob/master/shears.sh

^ permalink raw reply

* Draft of Git Rev News edition 19
From: Christian Couder @ 2016-09-11 10:25 UTC (permalink / raw)
  To: git
  Cc: Thomas Ferris Nicolaisen, Junio C Hamano, Johannes Schindelin,
	Josh Triplett, Kevin Willford

Hi,

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-19.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/179

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas and myself plan to publish this edition on Wednesday
September 14.

Thanks,
Christian.

^ permalink raw reply

* [PATCH v2 0/4] git add --chmod: always change the file
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160904113954.21697-1-t.gummerer@gmail.com>

Changes since v1:
Added missing x in the documentation.

The changes since v1 are only minor, but as it's been a week, this is
as much a ping as a fixup of my error :)

Thomas Gummerer (4):
  add: document the chmod option
  update-index: use the same structure for chmod as add
  read-cache: introduce chmod_index_entry
  add: modify already added files when --chmod is given

 Documentation/git-add.txt |  7 +++++-
 builtin/add.c             | 36 +++++++++++++++++++++----------
 builtin/checkout.c        |  2 +-
 builtin/commit.c          |  2 +-
 builtin/update-index.c    | 55 ++++++++++++++++++-----------------------------
 cache.h                   | 12 ++++++-----
 read-cache.c              | 33 +++++++++++++++++++++-------
 t/t3700-add.sh            | 21 ++++++++++++++++++
 8 files changed, 107 insertions(+), 61 deletions(-)

-- 
2.10.0.304.gf2ff484


^ permalink raw reply

* [PATCH v2 1/4] add: document the chmod option
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>

The git add --chmod option was introduced in 4e55ed3 ("add: add
--chmod=+x / --chmod=-x options", 2016-05-31), but was never
documented.  Document the feature.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-add.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6a96a66..7ed63dc 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
 	  [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
-	  [--] [<pathspec>...]
+	  [--chmod=(+|-)x] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -165,6 +165,11 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 	be ignored, no matter if they are already present in the work
 	tree or not.
 
+--chmod=(+|-)x::
+	Override the executable bit of the added files.  The executable
+	bit is only changed in the index, the files on disk are left
+	unchanged.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
2.10.0.304.gf2ff484


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox