All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Charles Bailey <charles@hashpling.org>
Cc: Hannu Koivisto <azure@iki.fi>,
	git@vger.kernel.org, Theodore Tso <tytso@mit.edu>
Subject: Re: [PATCH] mergetool: respect autocrlf by using checkout-index
Date: Mon, 26 Jan 2009 14:08:06 -0800	[thread overview]
Message-ID: <7vskn53em1.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <7v7i4h4v19.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Mon, 26 Jan 2009 13:28:02 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Charles Bailey <charles@hashpling.org> writes:
>
>> I suspect that the LF endings in the file is due to the fact that in
>> builtin-merge-file.c, the file is opened (fopen) in binary mode
>> ("wb"), but xdl_merge terminates all lines with a raw '\n'.
>>
>> The obvious fix would be to change fopen in builtin-file-merge.c to
>> use "w" instead, but this doesn't work in a number of scenarios. In
>> particular, it is wrong for repositories on windows with core.autocrlf
>> set to false, and would not fix non-windows repositories with
>> core.autocrlf set to true.
>>
>> Currently, I've no idea as to what the solution should be.
>
> "git file-merge" is designed to be a replacement for stock RCS merge, and
> unfortunately it does not call convert_to_working_tree(), nor has any way
> to know for which path it should take the attributes to apply to affect
> what convert_to_working_tree() should do even if it were to call it.
>
> I think we would need a new option to the command that says "pretend this
> is about merging this path, and use the gitattributes specified for it
> when writing out the result."

Perhaps something along this line to teach

    $ git merge-file --attribute-path=frotz.c file1 orig_file file2

to merge what happened since orig_file to file2 into file1, and deposit
the result after converting it appropriately for path "frotz.c" obeying
core.autocrlf and gitattribute rules.

I see rerere.c::merge() has the exact same issue, but its breakage is half
hidden by its use of fopen(path, "w").  It should explicitly use
convert_to_working_tree() like this patch does, and write the results out
in binary mode.

 builtin-merge-file.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git i/builtin-merge-file.c w/builtin-merge-file.c
index 96edb97..61d1092 100644
--- i/builtin-merge-file.c
+++ w/builtin-merge-file.c
@@ -5,7 +5,7 @@
 #include "parse-options.h"
 
 static const char *const merge_file_usage[] = {
-	"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2",
+	"git merge-file [options] [-L name1 [-L orig [-L name2]]] [--attribute-path path] file1 orig_file file2",
 	NULL
 };
 
@@ -30,10 +30,13 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
 	int merge_style = 0, quiet = 0;
 	int nongit;
+	char *attribute_path = NULL;
 
 	struct option options[] = {
 		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
 		OPT_SET_INT(0, "diff3", &merge_style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT_STRING('a', "attribute-path", &attribute_path, "path",
+			   "apply work-tree conversion for the path"),
 		OPT__QUIET(&quiet),
 		OPT_CALLBACK('L', NULL, names, "name",
 			     "set labels for file1/orig_file/file2", &label_cb),
@@ -73,6 +76,19 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < 3; i++)
 		free(mmfs[i].ptr);
 
+	if (ret >= 0 && attribute_path) {
+		struct strbuf buf = STRBUF_INIT;
+		ret = convert_to_working_tree(attribute_path,
+					      result.ptr, result.size,
+					      &buf);
+		free(result.ptr);
+		if (!ret) {
+			size_t len;
+			result.ptr = strbuf_detach(&buf, &len);
+			result.size = len;
+		}
+	}
+
 	if (ret >= 0) {
 		const char *filename = argv[0];
 		FILE *f = to_stdout ? stdout : fopen(filename, "wb");

  reply	other threads:[~2009-01-26 22:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-21 16:55 git diff, git mergetool and CRLF conversion Hannu Koivisto
2009-01-21 17:23 ` Charles Bailey
2009-01-21 21:03   ` Theodore Tso
2009-01-21 22:57     ` [PATCH] mergetool: respect autocrlf by using checkout-index Charles Bailey
2009-01-23 17:20       ` Junio C Hamano
2009-01-23 18:18         ` Charles Bailey
2009-01-26 16:15         ` Hannu Koivisto
2009-01-26 16:31           ` Charles Bailey
2009-01-26 21:28             ` Junio C Hamano
2009-01-26 22:08               ` Junio C Hamano [this message]
2009-01-26 23:09                 ` Junio C Hamano
2009-01-27 13:58                   ` Hannu Koivisto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vskn53em1.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=azure@iki.fi \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.