[PATCH] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[PATCH] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

If a previous version of git was installed on a system without a
proper Error.pm, git will install its own.  But the next time
git is compiled on that system, that Error.pm will prevent git from
installing its own copy the second time.  This causes a broken
git install on such systems.

This patch fixes this bug by tagging git's Error.pm with an
INSTALLED_BY flag, and checking for it during the compile.

Signed-off-by: Chris Frey <cdfrey@foursquare.net>
---

	I use 'stow' to handle multiple versions of git installations.
	So when I uninstall a version of git, all those files are
	truly gone.  Including Error.pm.

	I think it is wise to mark our own copy of Error.pm in some way
	anyhow, just so people can tell the difference between
	versions on their systems.

	The drawback to this patch is that once git installs its own
	copy, it will always install its own copy, unless the user
	uninstalls the old git first.  Usually this is the desired
	behaviour, but my perl-fu isn't strong enough to make this
	check even smarter.  Ideally, if a newer version is on the
	system already, git shouldn't have to install its own.

	And on the last hand, this whole automated, secondary Error.pm
	installation method is rather suspect when it comes to creating
	binary packages.  Hopefully distro maintainers never build git
	packages on systems with a git-Error.pm, while also blindly trusting
	git's make install.

	I'd love to see this in the official git tree soon, so I don't
	run into these issues myself. :-)  Tips on how to make this
	smarter are welcome.

	- Chris


 perl/Makefile.PL      |   14 ++++++++++++--
 perl/private-Error.pm |    1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 320253e..26f7a8c 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -11,9 +11,19 @@ MAKE_FRAG
 my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');
 
 # We come with our own bundled Error.pm. It's not in the set of default
-# Perl modules so install it if it's not available on the system yet.
+# Perl modules.  So, unless it was a copy we installed, install it
+# if it's not available on the system yet.
 eval { require Error };
-if ($@ || $Error::VERSION < 0.15009) {
+if ($@ || $Error::VERSION < 0.15009 || $Error::INSTALLED_BY eq 'git') {
+	if ($Error::INSTALLED_BY eq 'git') {
+		print "**************************************************\n";
+		print "WARNING: detected an Error.pm from a previous git\n";
+		print "         install, so assuming that you wish to\n";
+		print "         continue using git's version.  If this is\n";
+		print "         not the case, uninstall your old version\n";
+		print "         of git before compiling the new.\n";
+		print "**************************************************\n";
+	}
 	$pm{'private-Error.pm'} = '$(INST_LIBDIR)/Error.pm';
 }
 
diff --git a/perl/private-Error.pm b/perl/private-Error.pm
index 11e9cd9..a399983 100644
--- a/perl/private-Error.pm
+++ b/perl/private-Error.pm
@@ -16,6 +16,7 @@ use vars qw($VERSION);
 use 5.004;
 
 $VERSION = "0.15009";
+$Error::INSTALLED_BY = "git";
 
 use overload (
 	'""'	   =>	'stringify',
-- 
1.5.4.4

Re: [PATCH] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

On Fri, May 16, 2008 at 09:16:14PM -0400, Chris Frey wrote:
> 	The drawback to this patch is that once git installs its own
> 	copy, it will always install its own copy, unless the user
> 	uninstalls the old git first.  Usually this is the desired
> 	behaviour, but my perl-fu isn't strong enough to make this
> 	check even smarter.  Ideally, if a newer version is on the
> 	system already, git shouldn't have to install its own.

The other way to deal with this is to put the check in ./configure,
and don't let the user build unless the dependencies are solved.

How important is that private-Error.pm?

- Chris

[PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

If a previous version of git was installed on a system without a
proper Error.pm, git will install its own.  But the next time
git is compiled on that system, that Error.pm will prevent git from
installing its own copy the second time.  This causes a broken
git install on such systems.

This patch fixes this bug by tagging git's Error.pm with an
INSTALLED_BY flag, and checking for it during the compile.

Signed-off-by: Chris Frey <cdfrey@foursquare.net>
---

	Resending patch from last week, as I saw no comments.
	Please apply.  Thanks!


	Thoughts on the patch:

	I use 'stow' to handle multiple versions of git installations.
	So when I uninstall a version of git, all those files are
	truly gone.  Including Error.pm.  But if a new version was
	compiled while the old was still there, the new stow
	install will be missing Error.pm.

	This bug was hit in April by "carbonated beverage":
		http://marc.info/?l=git&m=120805594920430&w=2

	I think it is wise to mark our own copy of Error.pm in some way,
	just so people can tell the difference between versions on
	their systems.

	The drawback to this patch is that once git installs its own
	copy, it will always install its own copy, unless the user
	uninstalls the old git first.  Usually this is the desired
	behaviour, but my perl-fu isn't strong enough to make this
	check even smarter.  Ideally, if a newer version is on the
	system already, git shouldn't have to install its own.

	- Chris

 perl/Makefile.PL      |   14 ++++++++++++--
 perl/private-Error.pm |    1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 320253e..26f7a8c 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -11,9 +11,19 @@ MAKE_FRAG
 my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');
 
 # We come with our own bundled Error.pm. It's not in the set of default
-# Perl modules so install it if it's not available on the system yet.
+# Perl modules.  So, unless it was a copy we installed, install it
+# if it's not available on the system yet.
 eval { require Error };
-if ($@ || $Error::VERSION < 0.15009) {
+if ($@ || $Error::VERSION < 0.15009 || $Error::INSTALLED_BY eq 'git') {
+	if ($Error::INSTALLED_BY eq 'git') {
+		print "**************************************************\n";
+		print "WARNING: detected an Error.pm from a previous git\n";
+		print "         install, so assuming that you wish to\n";
+		print "         continue using git's version.  If this is\n";
+		print "         not the case, uninstall your old version\n";
+		print "         of git before compiling the new.\n";
+		print "**************************************************\n";
+	}
 	$pm{'private-Error.pm'} = '$(INST_LIBDIR)/Error.pm';
 }
 
diff --git a/perl/private-Error.pm b/perl/private-Error.pm
index 11e9cd9..a399983 100644
--- a/perl/private-Error.pm
+++ b/perl/private-Error.pm
@@ -16,6 +16,7 @@ use vars qw($VERSION);
 use 5.004;
 
 $VERSION = "0.15009";
+$Error::INSTALLED_BY = "git";
 
 use overload (
 	'""'	   =>	'stringify',
-- 
1.5.4.4

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Junio C Hamano]

Chris Frey <cdfrey@foursquare.net> writes:

> If a previous version of git was installed on a system without a
> proper Error.pm, git will install its own.  But the next time
> git is compiled on that system, that Error.pm will prevent git from
> installing its own copy the second time.  This causes a broken
> git install on such systems.
>
> This patch fixes this bug by tagging git's Error.pm with an
> INSTALLED_BY flag, and checking for it during the compile.

I think this is a wrong direction to go.

We do not currently deal with broken installations, and "stow" is just one
easy way to install and keep a stale version.  The right solution would be
to check if "Error.pm" we find on the system (be it installed by previous
incarnation of git or some other packages) works as expected, and refrain
from using it if it doesn't.

When the system has a slightly older version of Error.pm, it does not
really matter if that old one case from our own Error.pm (because back
then the system did not have Error.pm at all), or the user installed a
slightly older version of Error.pm from elsewhere.

IOW, I won't be interested in a solution that adds INSTALLED_BY.  Even if
it is ours, as long as it is fresh enough, there is no reason to replace
it with a new copy.  Even if it is _not_ ours, if it is stale and does not
work as we expect, we might have to install our own on our path.

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

On Wed, May 21, 2008 at 03:51:40PM -0700, Junio C Hamano wrote:
> I think this is a wrong direction to go.
> 
> We do not currently deal with broken installations, and "stow" is just one
> easy way to install and keep a stale version.  The right solution would be
> to check if "Error.pm" we find on the system (be it installed by previous
> incarnation of git or some other packages) works as expected, and refrain
> from using it if it doesn't.

Thank you for your response.

The problem as I see it, is that the decision about whether the existing
Error.pm "works as expected" is done at compile time.  And git will break
itself by repeated installs.

Is it really valid to expect users to uninstall their current version of
git before they can compile a new one correctly?  For systems without
a working Error.pm, this is currently how git behaves.

- Chris

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Johannes Schindelin]

Hi,

On Wed, 21 May 2008, Chris Frey wrote:

> The problem as I see it, is that the decision about whether the existing 
> Error.pm "works as expected" is done at compile time.  And git will 
> break itself by repeated installs.

I do not see how it is _Git_ that breaks itself by repeated installs.  It 
detects that there is an Error.pm.  Fine.  You _remove_ it while 
installing Git.  Not fine.  Not Git's error.

Ciao,
Dscho

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

On Thu, May 22, 2008 at 12:46:12PM +0100, Johannes Schindelin wrote:
> I do not see how it is _Git_ that breaks itself by repeated installs.  It 
> detects that there is an Error.pm.  Fine.  You _remove_ it while 
> installing Git.  Not fine.  Not Git's error.

I guess I take the view that anything _installed_ by git is a part of git.
If git installs Error.pm, it can't expect the user to keep it around
if he uninstalls that version of git.

The user can do the following, and not have any warning that he'll be
bitten, and won't know what he did wrong.  In fact, when I first saw this
error, I thought it was a bug that was fixed in a point release.

	(slightly abbreviated for clarity, version numbers arbitrary)

	tar xjf git-1.5.4.3.tar.bz2
	cd git...  && ./configure --prefix=/usr/local/stow/git-1.5.4.3
	make && make install
	stow git-1.5.4.3

	tar xjf git-1.5.4.4.tar.bz2
	cd git... && ./configure --prefix=/usr/local/stow/git-1.5.4.4
	make	# git is built assuming git-1.5.4.3 will always exist
	make install

	stow -D git-1.5.4.3
	stow git-1.5.4.4

The user now has a broken git, and has no idea why.  I don't consider the
above to be unusual or user error.

- Chris

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Junio C Hamano]

Chris Frey <cdfrey@foursquare.net> writes:

> On Thu, May 22, 2008 at 12:46:12PM +0100, Johannes Schindelin wrote:
>> I do not see how it is _Git_ that breaks itself by repeated installs.  It 
>> detects that there is an Error.pm.  Fine.  You _remove_ it while 
>> installing Git.  Not fine.  Not Git's error.
>
> I guess I take the view that anything _installed_ by git is a part of git.
> If git installs Error.pm, it can't expect the user to keep it around
> if he uninstalls that version of git.

True, as we do not give "uninstall" target.  That's what distros are for.

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Chris Frey]

On Thu, May 22, 2008 at 10:26:14AM -0700, Junio C Hamano wrote:
> > I guess I take the view that anything _installed_ by git is a part of git.
> > If git installs Error.pm, it can't expect the user to keep it around
> > if he uninstalls that version of git.
> 
> True, as we do not give "uninstall" target.  That's what distros are for.

I don't see how you can say "true" to what I said and keep git doing the
exact opposite. :-)

Anyway, there are other ways to fix this than INSTALLED_BY and I'm not
married to that idea, but if this isn't seen as a problem in the first
place, there's little use.

Would you accept a patch to the INSTALL file explaining this dependency?

Thanks,
- Chris

Re: [PATCH resend] perl/Makefile.PL: teach makefiles about possible old Error.pm files

[Sverre Rabbelier]

On Thu, May 22, 2008 at 7:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> True, as we do not give "uninstall" target.  That's what distros are for.

Why is this though? I think there are a few distro's out there (call
them the hardcore/diehard ones) that don't use packages, instead they
rely on keeping the install dir around so that you can 'make
uninstall' later.

-- 
Cheers,

Sverre Rabbelier

[PATCH] INSTALL: explain Error.pm dependency

[Chris Frey]

Signed-off-by: Chris Frey <cdfrey@foursquare.net>
---
 INSTALL |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/INSTALL b/INSTALL
index d9b425f..c5cafc4 100644
--- a/INSTALL
+++ b/INSTALL
@@ -86,6 +86,13 @@ Issues of note:
 	- "cpio" is used by git-clone when doing a local (possibly
 	  hardlinked) clone.
 
+	- "Error.pm" version >= 0.15009 is required by perl scripts.
+
+	  If you don't have a fresh enough Error.pm, git will install its
+	  own.  If you already have git installed, it will not install
+	  another Error.pm.  If this causes problems for you, uninstall
+	  git before compiling, or just install your distro's Error.pm.
+
  - Some platform specific issues are dealt with Makefile rules,
    but depending on your specific installation, you may not
    have all the libraries/tools needed, or you may have
-- 
1.5.4.4