{"thread":{"id":"42494","subject":"[PATCH 2/4] Git.pm: Add $path and $enable_filters arguments to hash_and_insert_object.","startedAt":"2016-05-31T15:07:45Z","lastAt":"2016-06-02T21:13:46Z","messageCount":9,"participants":["Matteo Bertini","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"287904","messageId":"20160531150749.24840-1-naufraghi@develer.com","threadId":"42494","inReplyTo":null,"subject":"[PATCH/RFC 0/4] Add option to enable filters in git-svn","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:07:45Z","receivedAt":"2016-05-31T15:07:45Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"===========\nDescription\n===========\n\nThis is a RFC for a patch that allows to enable filters in the repositories\ncreated/managed by git-svn.\n\n\n==================\nExample scenario\n==================\n\nThe usage scenario I’m facing is the migration of a large SVN repository to a\ngit repository, with big files being transparently handled through git-lfs\n\nTo recap, these migrations are usually handled through a transitioning repo\ncreated by git-svn, which is then pushed to a remote git server; this server\nacts as a read-only mirror for a while, until the main SVN server is shut down.\nFor large repos, the process can take several weeks to adjust the migration\nscripts, branch mapping, authorship, etc., and it’s also common to update the\nmirror incrementally.\n\n\nWithout this patch\n==================\n\nWithout this patch, the migration needs to be setup with a pipeline with:\n\n* incremental git svn fetch,\n* bfg-repo-cleaner with —convert-to-git-lfs option (which puts invalid\n  .gitattributes files, see https://github.com/rtyley/bfg-repo-cleaner/issues/143)\n* git-filter-branch (to put in place the correct .gitattributes files)\n\nA minimal change in the pipeline results in unrelated branches after\nthe rebases.\n\nEach deviation from the svn sources must be committed back or saved\nas patch and reapplied after the rebase.\n\n\nWith this patch\n===============\n\nEverything is much simpler: filters can be triggered during the fetch, so that\ngit-lfs already runs while the repository is fetched.\nAt the end of the process, the local git repository is already in its final\nform, and no further git-filter-branch/rebases is required.\n\nThe transitioning git repo can diverge partially from the svn source\nthanks to git svn rebase.\n\n\nDetails\n=======\n\nIn the current implementation git-svn uses the hash-object command\nwith the --no-filters option. There is no gain in removing that option\nbecause the git-svn uses temporary file names.\n\nIn this patch hash-object accepts a new format for the streaming input,\nfiles and paths, tab separated. git-svn can provide the real path\nto hash-object and thanks to that we can enable the filters.\n\n\nStatus\n======\n\nThe patch is in RFC, no new test where written (yet), just runned for\nregressions (https://travis-ci.org/naufraghi/git/builds/134170350).\n\nI'd like to gain some feedback before spending more time on the\nfeature, mainly:\n\n * feature comments,\n * coding comments (neither Perl or C are my main languages),\n * user interface (option names, config placements) comments.\n\nBest,\nMatteo\n\n\nMatteo Bertini (4):\n  hash-object.c: Allow distinct file/path in stdin mode too.\n  Git.pm: Add $path and $enable_filters arguments to\n    hash_and_insert_object.\n  SVN/Fetcher.pm: Add svn-remote.<id>.enable-filters to enable the\n    filters.\n  git-svn.perl: Add git svn init --enable-filters option.\n\n builtin/hash-object.c   | 29 +++++++++++++++++++++++++++--\n git-svn.perl            |  4 ++++\n perl/Git.pm             | 19 +++++++++++++------\n perl/Git/SVN/Fetcher.pm | 16 ++++++++++++----\n 4 files changed, 56 insertions(+), 12 deletions(-)\n\n-- \n2.9.0.rc0.39.gb9f310b.dirty\n"},{"id":"287902","messageId":"20160531150749.24840-2-naufraghi@develer.com","threadId":"42494","inReplyTo":"20160531150749.24840-1-naufraghi@develer.com","subject":"[PATCH 1/4] hash-object.c: Allow distinct file/path in stdin mode too.","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:07:46Z","receivedAt":"2016-05-31T15:07:46Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"From: Matteo Bertini <matteo@naufraghi.net>\n\nThe hash-object command has the --path option to use a name for the filters\nthat is different from the file containing the data.\n\nThis patch exposes the same functionality for the --stdin-paths, using \\t\nas separator.\n\nSigned-off-by: Matteo Bertini <naufraghi@develer.com>\n---\n builtin/hash-object.c | 29 +++++++++++++++++++++++++++--\n 1 file changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex f7d3567..f74c6bd 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -58,20 +58,45 @@ static void hash_object(const char *path, const char *type, const char *vpath,\n static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,\n \t\t\t     int literally)\n {\n+\tint has_path = 0;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf unquoted = STRBUF_INIT;\n+\tstruct strbuf **pair;\n \n \twhile (strbuf_getline(&buf, stdin) != EOF) {\n+\t\tpair = strbuf_split_max(&buf, '\\t', 2);\n+\t\tif (pair[0]) {\n+\t\t\tif (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '\\t') {\n+\t\t\t\tstrbuf_setlen(pair[0], pair[0]->len - 1);\n+\t\t\t}\n+\t\t\tstrbuf_swap(&buf, pair[0]);\n+\t\t}\n+\t\tif (pair[1]) {\n+\t\t\tstrbuf_swap(&path, pair[1]);\n+\t\t\thas_path = 1;\n+\t\t}\n+\t\tstrbuf_list_free(pair);\n+\n \t\tif (buf.buf[0] == '\"') {\n \t\t\tstrbuf_reset(&unquoted);\n \t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n \t\t\t\tdie(\"line is badly quoted\");\n \t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t}\n-\t\thash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags,\n-\t\t\t    literally);\n+\n+\t\tif (has_path && path.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, path.buf, NULL))\n+\t\t\t\tdie(\"line is badly quoted\");\n+\t\t\tstrbuf_swap(&path, &unquoted);\n+\t\t}\n+\n+\t\thash_object(buf.buf, type, no_filters ? NULL : (has_path ? path.buf : buf.buf),\n+\t\t\t\t  flags, literally);\n \t}\n \tstrbuf_release(&buf);\n+\tstrbuf_release(&path);\n \tstrbuf_release(&unquoted);\n }\n \n-- \n2.9.0.rc0.39.gb9f310b.dirty\n"},{"id":"287900","messageId":"20160531150749.24840-3-naufraghi@develer.com","threadId":"42494","inReplyTo":"20160531150749.24840-1-naufraghi@develer.com","subject":"[PATCH 2/4] Git.pm: Add $path and $enable_filters arguments to hash_and_insert_object.","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:07:47Z","receivedAt":"2016-05-31T15:07:47Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"From: Matteo Bertini <matteo@naufraghi.net>\n\nThe option $enable_filters skips the --no-filters option,\nthe $path argument provide a path to be used alike the --path argument\nto hash-object in the non streaming invocation.\n\nSigned-off-by: Matteo Bertini <naufraghi@develer.com>\n---\n perl/Git.pm | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex ce7e4e8..6c2e3fc 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -963,11 +963,13 @@ The function returns the SHA1 hash.\n \n # TODO: Support for passing FILEHANDLE instead of FILENAME\n sub hash_and_insert_object {\n-\tmy ($self, $filename) = @_;\n-\n-\tcarp \"Bad filename \\\"$filename\\\"\" if $filename =~ /[\\r\\n]/;\n+\tmy ($self, $filename, $path, $enable_filters) = @_;\n+\tcarp \"Bad filename \\\"$filename\\\"\" if $filename =~ /[\\r\\n\\t]/;\n+\tif (defined($path)) {\n+\t\t$filename = join(\"\\t\", $filename, $path);\n+\t}\n \n-\t$self->_open_hash_and_insert_object_if_needed();\n+\t$self->_open_hash_and_insert_object_if_needed($enable_filters);\n \tmy ($in, $out) = ($self->{hash_object_in}, $self->{hash_object_out});\n \n \tunless (print $out $filename, \"\\n\") {\n@@ -985,13 +987,18 @@ sub hash_and_insert_object {\n }\n \n sub _open_hash_and_insert_object_if_needed {\n-\tmy ($self) = @_;\n+\tmy ($self, $enable_filters) = @_;\n \n \treturn if defined($self->{hash_object_pid});\n \n+\tmy @command = qw(hash-object -w --stdin-paths);\n+\tif (!$enable_filters) {\n+\t\tpush(@command, \"--no-filters\");\n+\t}\n+\n \t($self->{hash_object_pid}, $self->{hash_object_in},\n \t $self->{hash_object_out}, $self->{hash_object_ctx}) =\n-\t\t$self->command_bidi_pipe(qw(hash-object -w --stdin-paths --no-filters));\n+\t\t$self->command_bidi_pipe(@command);\n }\n \n sub _close_hash_and_insert_object {\n-- \n2.9.0.rc0.39.gb9f310b.dirty\n"},{"id":"287903","messageId":"20160531150749.24840-4-naufraghi@develer.com","threadId":"42494","inReplyTo":"20160531150749.24840-1-naufraghi@develer.com","subject":"[PATCH 3/4] SVN/Fetcher.pm: Add svn-remote.<id>.enable-filters to enable the filters.","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:07:48Z","receivedAt":"2016-05-31T15:07:48Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"From: Matteo Bertini <matteo@naufraghi.net>\n\nGiven the fact that git-svn uses temporary files to build the index,\nprovide the real $path to hash_and_insert_object if the filters are\nenabled.\n\nSigned-off-by: Matteo Bertini <naufraghi@develer.com>\n---\n perl/Git/SVN/Fetcher.pm | 16 ++++++++++++----\n 1 file changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm\nindex d8c21ad..3557abe 100644\n--- a/perl/Git/SVN/Fetcher.pm\n+++ b/perl/Git/SVN/Fetcher.pm\n@@ -1,7 +1,7 @@\n package Git::SVN::Fetcher;\n use vars qw/@ISA $_ignore_regex $_include_regex $_preserve_empty_dirs\n             $_placeholder_filename @deleted_gpath %added_placeholder\n-            $repo_id/;\n+            $repo_id $_enable_filters/;\n use strict;\n use warnings;\n use SVN::Delta;\n@@ -46,6 +46,10 @@ sub new {\n \t\t$_placeholder_filename = $v;\n \t}\n \n+\t$k = \"svn-remote.$repo_id.enable-filters\";\n+\t$v = eval { command_oneline('config', '--get', '--bool', $k) };\n+\t$_enable_filters = 1\n+\t\tif ($v && $v eq 'true');\n \t# Load the list of placeholder files added during previous invocations.\n \t$k = \"svn-remote.$repo_id.added-placeholder\";\n \t$v = eval { command_oneline('config', '--get-all', $k) };\n@@ -415,9 +419,13 @@ sub close_file {\n \t\t\t\tGit::temp_release($tmp_fh, 1);\n \t\t\t}\n \t\t}\n-\n-\t\t$hash = $::_repository->hash_and_insert_object(\n-\t\t\t\tGit::temp_path($fh));\n+\t\tif ($_enable_filters) {\n+\t\t\t$hash = $::_repository->hash_and_insert_object(\n+\t\t\t\t\tGit::temp_path($fh), $path, $_enable_filters);\n+\t\t} else {\n+\t\t\t$hash = $::_repository->hash_and_insert_object(\n+\t\t\t\t\tGit::temp_path($fh));\n+\t\t}\n \t\t$hash =~ /^[a-f\\d]{40}$/ or die \"not a sha1: $hash\\n\";\n \n \t\tGit::temp_release($fb->{base}, 1);\n-- \n2.9.0.rc0.39.gb9f310b.dirty\n"},{"id":"287901","messageId":"20160531150749.24840-5-naufraghi@develer.com","threadId":"42494","inReplyTo":"20160531150749.24840-1-naufraghi@develer.com","subject":"[PATCH 4/4] git-svn.perl: Add git svn init --enable-filters option.","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:07:49Z","receivedAt":"2016-05-31T15:07:49Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"From: Matteo Bertini <matteo@naufraghi.net>\n\nThe enabled option activates the propagation of the real paths towards\nhash-object and the removal of the --no-filters option.\n\nSigned-off-by: Matteo Bertini <naufraghi@develer.com>\n---\n git-svn.perl | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 05eced0..4d4d6c4 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -153,6 +153,7 @@ my %init_opts = ( 'template=s' => \\$_template, 'shared:s' => \\$_shared,\n \t\t  'use-svnsync-props' => sub { $icv{useSvnsyncProps} = 1 },\n \t\t  'rewrite-root=s' => sub { $icv{rewriteRoot} = $_[1] },\n \t\t  'rewrite-uuid=s' => sub { $icv{rewriteUUID} = $_[1] },\n+\t\t  'enable-filters' => \\$Git::SVN::Fetcher::_enable_filters,\n                   %remote_opts );\n my %cmt_opts = ( 'edit|e' => \\$_edit,\n \t\t'rmdir' => \\$Git::SVN::Editor::_rmdir,\n@@ -495,6 +496,9 @@ sub do_git_init_db {\n \t\tcommand_noisy('config', \"$pfx.preserve-empty-dirs\", 'true');\n \t\tcommand_noisy('config', \"$pfx.placeholder-filename\", $$fname);\n \t}\n+\n+\tcommand_noisy('config', \"$pfx.enable-filters\", 'true')\n+\t\tif defined $Git::SVN::Fetcher::_enable_filters\n }\n \n sub init_subdir {\n-- \n2.9.0.rc0.39.gb9f310b.dirty\n"},{"id":"287907","messageId":"33130d8be1ad6edaa75a75c43901a2fa@develer.com","threadId":"42494","inReplyTo":"20160531150749.24840-1-naufraghi@develer.com","subject":"Re: [PATCH/RFC 0/4] Add option to enable filters in git-svn","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T15:34:25Z","receivedAt":"2016-05-31T15:34:25Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"Sorry to all, but I missed a Checksum mismatch error, I'll have a look \nand submit an update.\n\nCheers,\nMatteo\n"},{"id":"287924","messageId":"20160531181241.GA28818@dcvr.yhbt.net","threadId":"42494","inReplyTo":"33130d8be1ad6edaa75a75c43901a2fa@develer.com","subject":"Re: [PATCH/RFC 0/4] Add option to enable filters in git-svn","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-31T18:12:41Z","receivedAt":"2016-05-31T18:12:41Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Matteo Bertini <naufraghi@develer.com> wrote:\n> Sorry to all, but I missed a Checksum mismatch error, I'll have a\n> look and submit an update.\n\nSure, when you reroll can you also ensure lines are wrapped at\n80 cols or less? (at least for the git-svn code, but probably\nfor the rest of it, too).\n\nAs for the option name, the \"enable\" prefix doesn't seem\nright.  We already have some \"use\" prefixes (use-svm-props),\nso maybe \"--use-filters\" is better.\n\nI haven't looked into filters at all, yet, but you can probably\nuse the existing rot13 filter in t0021 for writing tests.\n\nThanks.\n"},{"id":"287935","messageId":"cb49a08e3ec44f7d37603e5b1fe20a22@develer.com","threadId":"42494","inReplyTo":"20160531181241.GA28818@dcvr.yhbt.net","subject":"Re: [PATCH/RFC 0/4] Add option to enable filters in git-svn","fromName":"Matteo Bertini","fromEmail":"naufraghi@develer.com","sentAt":"2016-05-31T20:34:46Z","receivedAt":"2016-05-31T20:34:46Z","isPatch":true,"sender":{"key":"naufraghi@develer.com","avatar":"https://gravatar.com/avatar/9f8be1cbd2f2671587fc084c8aabbc1a229d65d82f44c5f61b9ceeb8de4e1860?d=mp&s=160"},"body":"Il 2016-05-31 20:12 Eric Wong ha scritto:\n> Matteo Bertini <naufraghi@develer.com> wrote:\n>> Sorry to all, but I missed a Checksum mismatch error, I'll have a\n>> look and submit an update.\n\nI've had a look, the problem is a missing smudge filter.\nUnluckily the small svn revision range i was using was never updating\na filtered file.\n\nThe code actually uses `cat-file --batch` to get the blobs,\nbut the stored blob is not what SVN::TxDelta::apply need.\nWhat I need is the smudged blob, but cat-file does not supports it.\n\nI can modify cat-file with a new option to call \n`convert_to_working_tree`,\nand have the filters applied, for example --use-filters, that expects an\nextra field, like $sha\\t$path, in the stdin stream.\n\nI don't like a lot putting an high level feature in a low level command,\nbut --textconv seems very similar to this.\n\nOpinions or other suggestions?\n\n> Sure, when you reroll can you also ensure lines are wrapped at\n> 80 cols or less? (at least for the git-svn code, but probably\n> for the rest of it, too).\n\nSure\n\n> As for the option name, the \"enable\" prefix doesn't seem\n> right.  We already have some \"use\" prefixes (use-svm-props),\n> so maybe \"--use-filters\" is better.\n\nOk\n\n> I haven't looked into filters at all, yet, but you can probably\n> use the existing rot13 filter in t0021 for writing tests.\n\nThanks for the pointer, I'll have a look.\n\n-- \nMatteo Bertini - naufraghi@develer.com\nDeveler S.r.l. - http://www.develer.com/\n.hardware .software .innovation\nTel.: +39 055 3986627 - ext.: 211\n"},{"id":"288156","messageId":"20160602211346.GA24915@dcvr.yhbt.net","threadId":"42494","inReplyTo":"cb49a08e3ec44f7d37603e5b1fe20a22@develer.com","subject":"Re: [PATCH/RFC 0/4] Add option to enable filters in git-svn","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-02T21:13:46Z","receivedAt":"2016-06-02T21:13:46Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Matteo Bertini <naufraghi@develer.com> wrote:\n> Il 2016-05-31 20:12 Eric Wong ha scritto:\n> >Matteo Bertini <naufraghi@develer.com> wrote:\n> >>Sorry to all, but I missed a Checksum mismatch error, I'll have a\n> >>look and submit an update.\n> \n> I've had a look, the problem is a missing smudge filter.\n> Unluckily the small svn revision range i was using was never updating\n> a filtered file.\n> \n> The code actually uses `cat-file --batch` to get the blobs,\n> but the stored blob is not what SVN::TxDelta::apply need.\n> What I need is the smudged blob, but cat-file does not supports it.\n> \n> I can modify cat-file with a new option to call\n> `convert_to_working_tree`,\n> and have the filters applied, for example --use-filters, that expects an\n> extra field, like $sha\\t$path, in the stdin stream.\n> \n> I don't like a lot putting an high level feature in a low level command,\n> but --textconv seems very similar to this.\n> \n> Opinions or other suggestions?\n\nIn the absense of other opinions or suggestions, I suggest you\ntry it and see what others think :)\n\nI'm not that experienced at the C code of this project,\nbut I can try to follow along as best as I can.\n"}]}