threads / patch / 22896

patch, 3 partshash-object: support --stdin-paths with --no-filters

Subject: [PATCH 1/3] hash-object: support --stdin-paths with --no-filters

## tl;dr

9 messages between Mar 3, 2010 and Mar 5, 2010. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Erik Faye-Lund· Mar 3, 2010, 20:10 UTC · lore
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 Documentation/git-hash-object.txt |    2 +-
 builtin-hash-object.c             |    8 ++++----
 t/t1007-hash-object.sh            |   18 ++++++++++++++----
 3 files changed, 19 insertions(+), 9 deletions(-)
Show changes to 3 files +19 −9

Documentation/git-hash-object.txt, builtin-hash-object.c, t/t1007-hash-object.sh

diff --git a/Documentation/git-hash-object.txt b/Documentation/git-hash-object.txt
index 479fce4..6904739 100644
--- a/Documentation/git-hash-object.txt
+++ b/Documentation/git-hash-object.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git hash-object' [-t <type>] [-w] [--path=<file>|--no-filters] [--stdin] [--] <file>...
-'git hash-object' [-t <type>] [-w] --stdin-paths < <list-of-paths>
+'git hash-object' [-t <type>] [-w] --stdin-paths [--no-filters] < <list-of-paths>
 
 DESCRIPTION
 -----------
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..dd32432 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
-'
-
 test_expect_success "Can't use --path with --no-filters" '
 	test_must_fail git hash-object --no-filters --path=foo
 '
@@ -141,6 +137,20 @@ test_expect_success 'check that --no-filters option works' '
 	git config --unset core.autocrlf
 '
 
+test_expect_success 'check that --no-filters option works with --stdin-paths' '
+	echo fooQ | tr Q "\\015" >file0 &&
+	cp file0 file1 &&
+	echo "file0 -crlf" >.gitattributes &&
+	echo "file1 crlf" >>.gitattributes &&
+	git config core.autocrlf true &&
+	file0_sha=$(git hash-object file0) &&
+	file1_sha=$(git hash-object file1) &&
+	test "$file0_sha" != "$file1_sha" &&
+	nofilters_file1=$(echo "file1" | git hash-object --stdin-paths --no-filters) &&
+	test "$file0_sha" = "$nofilters_file1" &&
+	git config --unset core.autocrlf
+'
+
 pop_repo
 
 for args in "-w --stdin" "--stdin -w"; do
-- 
1.7.0.1.141.gc5984a
Erik Faye-Lund· Mar 3, 2010, 20:10 UTC · re: Erik Faye-Lund · lore

[PATCH 2/3] git-svn: support fetch with autocrlf on

Before commit d3c9634e, performing a "git svn rebase" that fetched a change containing CRLFs corrupted the git-svn meta-data. This was worked around in d3c9634e by setting core.autocrlf to "false" in the per-repo config when initing the clone. However, if the config variable was later changed, the corruption would still occur.

This patch tries to fix it 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>
---
 perl/Git.pm |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to perl/Git.pm +1 −1
diff --git a/perl/Git.pm b/perl/Git.pm
index 970fe43..1926dc9 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.1.141.gc5984a
Erik Faye-Lund· Mar 3, 2010, 20:10 UTC · re: Erik Faye-Lund · lore

[PATCH 3/3] Revert "git-svn: always initialize with core.autocrlf=false"

git-svn rebase used to have issues with CRLF conversion. Since these issues have been fixed, we can safely revert the work-around that disables CRLF conversion.

This reverts commit d3c9634eacdcaa71cbd69a160e6f4e80ddb7ab63.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

git-svn dcommit uses diff-tree (which does not apply CRLF conversion) to generate SVN commits, so there never were any CRLF conversion issues when commiting in the first place.

 git-svn.perl |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Show changes to git-svn.perl +0 −1
diff --git a/git-svn.perl b/git-svn.perl
index b7c03b6..b8a3fc2 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -369,7 +369,6 @@ sub do_git_init_db {
 		command_noisy(@init_db);
 		$_repository = Git->repository(Repository => ".git");
 	}
-	command_noisy('config', 'core.autocrlf', 'false');
 	my $set;
 	my $pfx = "svn-remote.$Git::SVN::default_repo_id";
 	foreach my $i (keys %icv) {
-- 
1.7.0.1.141.gc5984a
Erik Faye-Lund· Mar 3, 2010, 20:16 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 3/3] Revert "git-svn: always initialize with core.autocrlf=false"

On Wed, Mar 3, 2010 at 9:10 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
> git-svn dcommit uses diff-tree (which does not apply CRLF conversion) to
> generate SVN commits, so there never were any CRLF conversion issues when
> commiting in the first place.
>

Uhm, this comment is slightly wrong. diff-tree is used to find out if there's a change in a file at all. cat_blob(), which in turn uses "git cat-file", is used to get the file contents.

Sorry for the noise.
-- 
Erik "kusma" Faye-Lund
Eric Wong· Mar 4, 2010, 09:53 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 3/3] Revert "git-svn: always initialize with core.autocrlf=false"

Erik Faye-Lund <kusmabite@googlemail.com> wrote:
Show 10 quoted lines
> On Wed, Mar 3, 2010 at 9:10 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
> > git-svn dcommit uses diff-tree (which does not apply CRLF conversion) to
> > generate SVN commits, so there never were any CRLF conversion issues when
> > commiting in the first place.
> 
> Uhm, this comment is slightly wrong. diff-tree is used to find out if
> there's a change in a file at all. cat_blob(), which in turn uses "git
> cat-file", is used to get the file contents.
> 
> Sorry for the noise.
Hi Erik,

Yes, diff-tree is only used to find changed paths, not the actual content changes. git svn always uses entire blobs from cat-file.

So cat-file won't do CRLF conversions at all, meaning this change is safe for previously created repos, correct? If so, consider patches 2/3 and 3/3 acked by me.

1/3 looks alright, too, but I'll wait for Dmitry as I've never hacked on (builtin-)hash-object.c

-- 
Eric Wong
Erik Faye-Lund· Mar 4, 2010, 10:24 UTC · re: Eric Wong · lore

Re: [PATCH 3/3] Revert "git-svn: always initialize with core.autocrlf=false"

On Thu, Mar 4, 2010 at 10:53 AM, Eric Wong <normalperson@yhbt.net> wrote:
Show 19 quoted lines
> Erik Faye-Lund <kusmabite@googlemail.com> wrote:
>> On Wed, Mar 3, 2010 at 9:10 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
>> > git-svn dcommit uses diff-tree (which does not apply CRLF conversion) to
>> > generate SVN commits, so there never were any CRLF conversion issues when
>> > commiting in the first place.
>>
>> Uhm, this comment is slightly wrong. diff-tree is used to find out if
>> there's a change in a file at all. cat_blob(), which in turn uses "git
>> cat-file", is used to get the file contents.
>>
>> Sorry for the noise.
>
> Hi Erik,
>
> Yes, diff-tree is only used to find changed paths, not the actual
> content changes.  git svn always uses entire blobs from cat-file.
>
> So cat-file won't do CRLF conversions at all, meaning this change is
> safe for previously created repos, correct?
Correct.

But existing git-svn clones (that was cloned after d3c9634 was introduced) will still have autocrlf-conversions disabled anyway (because d3c9634 disabled it), so people shouldn't see any difference before either re-cloning or manually enabling core.autocrlf in the repo. I don't think there's any way to automatically remove the config option that d3c9634 set up without potentially stepping on the user's toes. And I have a gut-feeling that it would be wrong thing to do anyway.

-- 
Erik "kusma" Faye-Lund
Dmitry Potapov· Mar 4, 2010, 14:35 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 1/3] hash-object: support --stdin-paths with --no-filters

On Wed, Mar 03, 2010 at 09:10:21PM +0100, Erik Faye-Lund wrote:
Show 6 quoted lines
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>  Documentation/git-hash-object.txt |    2 +-
>  builtin-hash-object.c             |    8 ++++----
>  t/t1007-hash-object.sh            |   18 ++++++++++++++----
>  3 files changed, 19 insertions(+), 9 deletions(-)
The patch looks good to me.

Thank you, Dmitry

Junio C Hamano· Mar 4, 2010, 18:11 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 1/3] hash-object: support --stdin-paths with --no-filters

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

This looks good to me. I could queue this myself and ask Eric to Ack the other two and queue them on top of it, but it probably is easier for Eric if I said

    Acked-by: Junio C Hamano <gitster@pobox.com>

now, and ask him to queue the three-patch series including this one in his tree, and then tell me to pull the finished result from his tree when he is satisfied. That way it would be easier for Eric to test and/or polish if needed.

Eric, are you Ok with that plan?
Eric Wong· Mar 5, 2010, 11:05 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] hash-object: support --stdin-paths with --no-filters

Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> Erik Faye-Lund <kusmabite@googlemail.com> writes:
> 
> > Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> 
> This looks good to me.  I could queue this myself and ask Eric to Ack the
> other two and queue them on top of it, but it probably is easier for Eric
> if I said
> 
>     Acked-by: Junio C Hamano <gitster@pobox.com>
> 
> now, and ask him to queue the three-patch series including this one in his
> tree, and then tell me to pull the finished result from his tree when he
> is satisfied.  That way it would be easier for Eric to test and/or polish
> if needed.
> 
> Eric, are you Ok with that plan?

Yes, thanks Junio. I've acked and pushed out this series to git://git.bogomips.org/git-svn along with a trivial fix by Michael.

Thanks all.
Erik Faye-Lund (3):
      hash-object: support --stdin-paths with --no-filters
      git-svn: support fetch with autocrlf on
      Revert "git-svn: always initialize with core.autocrlf=false"
Michael J Gruber (1):
      git-svn: make git svn --version work again
-- 
Eric Wong

← back to recent threads