{"thread":{"id":"44370","subject":"[PATCH] git-svn: do not reuse caches memoized for a different architecture","startedAt":"2016-10-25T15:30:35Z","lastAt":"2016-10-27T20:51:22Z","messageCount":4,"participants":["Johannes Schindelin","Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"304846","messageId":"653aa0cd566a2486bbc38cfd82ddfcfdfe48271c.1477398004.git.johannes.schindelin@gmx.de","threadId":"44370","inReplyTo":null,"subject":"[PATCH] git-svn: do not reuse caches memoized for a different architecture","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-25T15:30:11Z","receivedAt":"2016-10-25T15:30:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Gavin Lambert \n\nReusing cached data speeds up git-svn by quite a fair bit. However, if\nthe YAML module is unavailable, the caches are written to disk in an\narchitecture-dependent manner. That leads to problems when upgrading,\nsay, from 32-bit to 64-bit Git for Windows.\n\nLet's just try to read those caches back if we detect the absence of the\nYAML module and the presence of the file, and delete the file if it\ncould not be read back correctly.\n\nNote that the only way to catch the error when the memoized cache could\nnot be read back is to put the call inside an `eval { ... }` block\nbecause it would die otherwise; the `eval` block should also return `1`\nin case of success explicitly since the function reading back the cached\ndata does not return an appropriate value to test for success.\n\nThis fixes https://github.com/git-for-windows/git/issues/233.\n\nSigned-off-by: Gavin Lambert \nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/svn-multi-arch-v1\nFetch-It-Via: git fetch https://github.com/dscho/git svn-multi-arch-v1\n\nWe carried this for some time in Git for Windows.\n\n perl/Git/SVN.pm | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 018beb8..025c894 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1658,6 +1658,11 @@ sub tie_for_persistent_memoization {\n \tif ($memo_backend > 0) {\n \t\ttie %$hash => 'Git::SVN::Memoize::YAML', \"$path.yaml\";\n \t} else {\n+\t\t# first verify that any existing file can actually be loaded\n+\t\t# (it may have been saved by an incompatible version)\n+\t\tif (-e \"$path.db\") {\n+\t\t\tunlink \"$path.db\" unless eval { retrieve(\"$path.db\"); 1 };\n+\t\t}\n \t\ttie %$hash => 'Memoize::Storable', \"$path.db\", 'nstore';\n \t}\n }\n\nbase-commit: 659889482ac63411daea38b2c3d127842ea04e4d\n-- \n2.10.1.583.g721a9e0\n"},{"id":"304867","messageId":"20161025212357.GA8683@starla","threadId":"44370","inReplyTo":"653aa0cd566a2486bbc38cfd82ddfcfdfe48271c.1477398004.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] git-svn: do not reuse caches memoized for a different architecture","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-10-25T21:23:57Z","receivedAt":"2016-10-25T21:24:03Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Johannes Schindelin wrote:\n> +++ b/perl/Git/SVN.pm\n> @@ -1658,6 +1658,11 @@ sub tie_for_persistent_memoization {\n> \tif ($memo_backend > 0) {\n> \t\ttie %$hash => 'Git::SVN::Memoize::YAML', \"$path.yaml\";\n> \t} else {\n> +\t\t# first verify that any existing file can actually be loaded\n> +\t\t# (it may have been saved by an incompatible version)\n> +\t\tif (-e \"$path.db\") {\n> +\t\t\tunlink \"$path.db\" unless eval { retrieve(\"$path.db\"); 1 };\n> +\t\t}\n\nThat retrieve() call is unlikely to work without \"use Storable\"\nto import it into the current package.\n\nI also favor setting \"$path.db\" once to detect typos and avoid\ngoing over 80 columns. Additionally, having error-checking for\nunlink might be useful.\n\nSo perhaps squashing this on top:\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 025c894..b3c1460 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1660,10 +1660,15 @@ sub tie_for_persistent_memoization {\n \t} else {\n \t\t# first verify that any existing file can actually be loaded\n \t\t# (it may have been saved by an incompatible version)\n-\t\tif (-e \"$path.db\") {\n-\t\t\tunlink \"$path.db\" unless eval { retrieve(\"$path.db\"); 1 };\n+\t\tmy $db = \"$path.db\";\n+\t\tif (-e $db) {\n+\t\t\tuse Storable qw(retrieve);\n+\n+\t\t\tif (!eval { retrieve($db); 1 }) {\n+\t\t\t\tunlink $db or die \"unlink $db failed: $!\";\n+\t\t\t}\n \t\t}\n-\t\ttie %$hash => 'Memoize::Storable', \"$path.db\", 'nstore';\n+\t\ttie %$hash => 'Memoize::Storable', $db, 'nstore';\n \t}\n }\n \n\nThoughts? Thanks.\n"},{"id":"304982","messageId":"xmqqmvhp60gp.fsf@gitster.mtv.corp.google.com","threadId":"44370","inReplyTo":"20161025212357.GA8683@starla","subject":"Re: [PATCH] git-svn: do not reuse caches memoized for a different architecture","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T19:44:38Z","receivedAt":"2016-10-27T19:45:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Johannes Schindelin wrote:\n>> +++ b/perl/Git/SVN.pm\n>> @@ -1658,6 +1658,11 @@ sub tie_for_persistent_memoization {\n>> \tif ($memo_backend > 0) {\n>> \t\ttie %$hash => 'Git::SVN::Memoize::YAML', \"$path.yaml\";\n>> \t} else {\n>> +\t\t# first verify that any existing file can actually be loaded\n>> +\t\t# (it may have been saved by an incompatible version)\n>> +\t\tif (-e \"$path.db\") {\n>> +\t\t\tunlink \"$path.db\" unless eval { retrieve(\"$path.db\"); 1 };\n>> +\t\t}\n>\n> That retrieve() call is unlikely to work without \"use Storable\"\n> to import it into the current package.\n>\n> I also favor setting \"$path.db\" once to detect typos and avoid\n> going over 80 columns. Additionally, having error-checking for\n> unlink might be useful.\n>\n> So perhaps squashing this on top:\n>\n> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n> index 025c894..b3c1460 100644\n> --- a/perl/Git/SVN.pm\n> +++ b/perl/Git/SVN.pm\n> @@ -1660,10 +1660,15 @@ sub tie_for_persistent_memoization {\n> \t} else {\n> \t\t# first verify that any existing file can actually be loaded\n> \t\t# (it may have been saved by an incompatible version)\n> -\t\tif (-e \"$path.db\") {\n> -\t\t\tunlink \"$path.db\" unless eval { retrieve(\"$path.db\"); 1 };\n> +\t\tmy $db = \"$path.db\";\n> +\t\tif (-e $db) {\n> +\t\t\tuse Storable qw(retrieve);\n> +\n> +\t\t\tif (!eval { retrieve($db); 1 }) {\n> +\t\t\t\tunlink $db or die \"unlink $db failed: $!\";\n> +\t\t\t}\n> \t\t}\n> -\t\ttie %$hash => 'Memoize::Storable', \"$path.db\", 'nstore';\n> +\t\ttie %$hash => 'Memoize::Storable', $db, 'nstore';\n> \t}\n> }\n> \n>\n> Thoughts? Thanks.\n\nJust peeking from the sideline, but the your squash looks like an\nimprovement to me.\n\nHopefully the final version after your interaction with Dscho can\ncome to me via another \"pull this now\"?\n\nThanks.\n"},{"id":"304989","messageId":"20161027205115.GA5706@starla","threadId":"44370","inReplyTo":"xmqqmvhp60gp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-svn: do not reuse caches memoized for a different architecture","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-10-27T20:51:15Z","receivedAt":"2016-10-27T20:51:22Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Just peeking from the sideline, but the your squash looks like an\n> improvement to me.\n\nThanks.\n\n> Hopefully the final version after your interaction with Dscho can\n> come to me via another \"pull this now\"?\n\nNot sure if I'll be online the next few days,\nbut I've preeptively pushed the patch + squash to my repo:\n\nThe following changes since commit 2cc2e70264e0fcba04f9ef791d144bbc8b501206:\n\n Eleventh batch for 2.11 (2016-10-26 13:28:47 -0700)\n\nare available in the git repository at:\n\n git://bogomips.org/git-svn.git svn-cache\n\nfor you to fetch changes up to a2c761ce5b7a5fd8b505b036f3509a9e6617dee8:\n\n git-svn: do not reuse caches memoized for a different architecture (2016-10-27 20:17:36 +0000)\n\n----------------------------------------------------------------\nGavin Lambert (1):\n git-svn: do not reuse caches memoized for a different architecture\n\n perl/Git/SVN.pm | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n"}]}