Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 builtin-hash-object.c  |    8 ++++----
 t/t1007-hash-object.sh |    4 ----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/builtin-hash-object.c b/builtin-hash-object.c
index 6a5f5b5..080af1a 100644
--- a/builtin-hash-object.c
+++ b/builtin-hash-object.c
@@ -33,6 +33,8 @@ static void hash_object(const char *path, const char *type, int write_object,
 	hash_fd(fd, type, write_object, vpath);
 }
 
+static int no_filters;
+
 static void hash_stdin_paths(const char *type, int write_objects)
 {
 	struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
@@ -44,7 +46,8 @@ static void hash_stdin_paths(const char *type, int write_objects)
 				die("line is badly quoted");
 			strbuf_swap(&buf, &nbuf);
 		}
-		hash_object(buf.buf, type, write_objects, buf.buf);
+		hash_object(buf.buf, type, write_objects,
+		    no_filters ? NULL : buf.buf);
 	}
 	strbuf_release(&buf);
 	strbuf_release(&nbuf);
@@ -60,7 +63,6 @@ static const char *type;
 static int write_object;
 static int hashstdin;
 static int stdin_paths;
-static int no_filters;
 static const char *vpath;
 
 static const struct option hash_object_options[] = {
@@ -100,8 +102,6 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 			errstr = "Can't specify files with --stdin-paths";
 		else if (vpath)
 			errstr = "Can't use --stdin-paths with --path";
-		else if (no_filters)
-			errstr = "Can't use --stdin-paths with --no-filters";
 	}
 	else {
 		if (hashstdin > 1)
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index fd98e44..1509fe3 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -65,10 +65,6 @@ test_expect_success "Can't use --path with --stdin-paths" '
 	echo example | test_must_fail git hash-object --stdin-paths --path=foo
 '
 
-test_expect_success "Can't use --stdin-paths with --no-filters" '
-	echo example | test_must_fail git hash-object --stdin-paths --no-filters
-'
-
 ...
[ message continues ]

If I enable core.autocrlf and perform a "git svn rebase" that fetches
a change containing CRLFs, the git-svn meta-data gets corrupted.

Commit d3c9634e worked around this by setting core.autocrlf to "false"
in the per-repo config when initing the clone. However if the config
variable was changed, the breakage would still occur. This made it
painful to work with git-svn on repos with mostly checked in LFs on
Windows.

This patch tries to fix the same problem while allowing core.autocrlf
to be enabled, by disabling filters when when hashing.

git-svn is currently the only call-site for hash_and_insert_object
(apart from the test-suite), so changing it should be safe.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

With this patch applied, I guess we can also revert d3c9634e. I didn't
do this in this series, because I'm lazy and selfish and thus only
changed the code I needed to get what I wanted to work ;)

I've been running git-svn with these patches with core.autocrlf enabled
since December, and never seen the breakage that I saw before d3c9634e.

 perl/Git.pm |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index e8df55d..2378da4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -842,7 +842,7 @@ sub _open_hash_and_insert_object_if_needed {
 
 	($self->{hash_object_pid}, $self->{hash_object_in},
 	 $self->{hash_object_out}, $self->{hash_object_ctx}) =
-		command_bidi_pipe(qw(hash-object -w --stdin-paths));
+		command_bidi_pipe(qw(hash-object -w --stdin-paths --no-filters));
 }
 
 sub _close_hash_and_insert_object {
-- 
1.7.0.rc2

--

[ message continues ]

Hi Erik,

How does reverting d3c9634e affect dcommit?  I've never dealt with (or
even looked at) autocrlf, so I'll put my trust in you and Dscho with
anything related to it.

-- 
Eric Wong
--

[ message continues ]

I don't think it affects svn dcommit in any way, except from the
implicit svn rebase that svn dcommit performs. d3c9634e sets
core.autocrlf to "false" on init, but re-enabling it hasn't shown any
problems in my end. I'm using git-svn with these patches and
core.autocrlf enabled every day at my day-job.

I'd say that reverting d3c9634e would be the Right Thing To Do(tm),
because it makes git svn clone work just as bad as git clone, when
cloning a repo with CRLFs in it ;)

-- 
Erik "kusma" Faye-Lund
--

[ message continues ]

Hi,


To elicit a warm and fuzzy feeling about your patch, you will have to 
analyze the code paaths of dcommit, and how crlf affects them. Then you 
will have to describe why dcommit does not have a problem with crlf with 
your patches anymore.

Remember, the idea of a commit message is to optimize the overall time 
balance, i.e. avoid the many to perform what the one can do for them. And 
since you have to do that analysis for yourself anyway, it makes sense to 
write up the result in the commit message.

Thanks,
Dscho

--

[ message continues ]

On Sun, Feb 14, 2010 at 12:59 AM, Johannes Schindelin

I'm sorry, but I'm confused. What missed from my commit message?

The question of dcommit was a question that Eric asked, and I'm not
really sure why he did. I tried to explain why in my reply. d3c9634e
never was about dcommit the way I understand it, but about clone:
http://code.google.com/p/msysgit/issues/detail?id=232

If there's something that isn't sufficiently explained in the commit
message, I'd like to know so I can improve it for the next round...

-- 
Erik "kusma" Faye-Lund
--

[ message continues ]

Hi,


I had the impression that you sent a mail asking to revert the commit that 
hardcoded autocrlf to false for git svn. For that commit, you would have 

Well, technically, you are right, it is only about clone.

But.

If you set autocrlf to false in every git svn clone, then of course, 
dcommit is very much affected by the setting. Along with all other git svn 
operations.

And since your patches aimed at undoing that patch, i.e. no longer setting 
autocrlf to false upon git svn clone, you have to show that git svn in 
general can handle autocrlf = true (or = input) just fine.

And by "to show" I do not mean just test it. That is not good enough, 
because your workflow is more than just likely to miss out on ways other 
people use git svn. You have the source code, and you can look all git 
calls and analyze them for potential autocrlf problems.

Ciao,
Dscho

--

[ message continues ]

On Sun, Feb 14, 2010 at 1:46 AM, Johannes Schindelin

No. I sent a patch series that fixes this issue properly instead of
just disabling core.autocrlf. I added a comment that mentioned that


My patch aimed at allowing me to use core.autocrlf. I don't care what
the default for core.autocrlf in a git-svn clone is, I'm just sick of
accidentally checking in CRLFs into my company's SVN repo because

What makes you think that I haven't properly analyzed the issue? I'm
wondering especially about this, considering that you yourself
described your fix as "so obvious it did not even need testing"...
What's the required amount of analysis here?

I'm sorry if I misunderstand you here, but I'm still not understanding
what's lacking...

I guess "This patch tries to fix the same problem while allowing
core.autocrlf to be enabled, by disabling filters when when hashing"
could be rewritten into something like this:

"This patch tries to fix the same problem while allowing core.autocrlf
to be enabled, by making sure that filters aren't applied while
importing files into the git-repo"

Perhaps I should add a paragraph explaining why the issue triggered in
the first place (before d3c9634e)?

-- 
Erik "kusma" Faye-Lund
--

[ message continues ]

Hi,


I have no idea how reverting said commit affects dcommit, and I do not 
have the time to check, sorry!

Ciao,
Dscho

--

[ message continues ]

Hi Erik,

I don't remember why I made --no-filters incompatible with --stdin-paths
when I added it. So your patch makes perfect sense to me. However, I
think you should correct synopsis in Documentation/git-hash-object.txt,
so it will be clear that these two options can be used together now.
Also, it would be nice if you add a test case to make sure that it works
as expected and will not broken in the future..


Thanks,
Dmitry
--

[ message continues ]

Thanks. I've fixed the synopsis and added a test for the next round.

-- 
Erik "kusma" Faye-Lund
--

[ message continues ]