Installation failure caused by CDPATH environment variable

This week I've installed Git on two different machines and one of them mysteriously failed making the "install" target in the "templates/Makefile". Specifically, the problem was occuring here:

         (cd blt && $(TAR) cf - .) | \
         (cd '$(DESTDIR_SQ)$(template_dir_SQ)' && $(TAR) xf -)

And the error message was:

tar: This does not look like a tar archive tar: Skipping to next header tar: Error exit delayed from previous errors

Upon investigation, I discovered that the cause was that on the failing machine, the CDPATH environment variable was set and included "." (the current directory). This meant that upon executing:

         (cd blt && $(TAR) cf - .)

The data from tar was getting prepended with garbage because "cd blt" was echoing the path to the directory.

The workaround is to "unset CDPATH" or change the value of CDPATH so that it doesn't include the "." (although the latter is not very feasible because without "." at the front of CDPATH changing into a directory relative to the current directory can be quite painful).

What do you think about altering the templates/Makefile to make it more robust against this kind of environment?

Here's a possible patch:

 From 057630bdcfeee63b90468d1a69153171b15780c0 Mon Sep 17 00:00:00 2001
From: Wincent Colaiuta <win@wincent.com>
Date: Wed, 11 Jul 2007 18:43:59 +0200

[PATCH] Proof Makefile against possible problems with CDPATH environment

If the CDPATH environment variable is set and contains a period Bash may echo the current directory name while performing a cd, and when this extra output is piped to tar as part of template installation it can cause tar to abort with an error. This patch avoids this problem by invoking cd as a separate step prior to invoking tar.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
  templates/Makefile |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

Hi,

On Wed, 11 Jul 2007, Wincent Colaiuta wrote:

You exported CDPATH. You're guaranteed to run into problems with that. I doubt that your patch catches all problems in git, and even if we tried to avoid breakage, you can only do so much about that.

It is _wrong_ to export CDPATH.

Hth, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

But it is not our job to educate people about that. So I'd just add something like

[ "X" = "X$CDPATH" ] || unset CDPATH # ignore braindamaged exports

to the top of possibly affected scripts and be done.

After the first few dozen bug reports (which are just the tip of an iceberg of people who say "git is crap and fails in strange ways") you just recognize that you should just make that somebody else's problem.

I've done my fair amount of such educational tasks myself with packages of mine. It just gets you annoyed more and more each time, it is tedious, it gets people get a bad opinion of both your software _and_ your person (try being friendly after the 20th actually technically completely unnecessary such request, while basically explaining that you won't fix it because it is a Darwinian trap for stupid people and you want evolution to run its course).

A one-liner that makes this somebody else's problem (if at all) is worth its weight in gold.

Trust me on that.

My personal lowlight in that area are dozens of university and company document classes derived from some old AMSLaTeX class in prehistoric times that redefined \endfigure in a way incompatible with quite a few packages. Among them preview.sty written by me.

After about 5 bug reports about 5 different classes, explaining what was wrong and why it broke quite a few packages, not just preview-latex, and how people should fix it and how they should complain to the people giving them the package... preview.sty just patches the problematic definitions if it has a reasonable guess that they are in place, and blows out a stern and verbose warning explaining where to complain and why.

Of course, nobody ever does.

Don't educate people. Just don't trigger their problems. Of course, there are millions of ways of shooting oneself in the foot, but in this case the same foot has been hit several times already.

David Kastrup <dak@gnu.org> writes:

Yup. We do exactly that in git-clone, git-sh-setup and t/test-lib to avoid getting bugged by this stupidity.

El 12/7/2007, a las 10:34, Junio C Hamano escribió:

El 12/7/2007, a las 9:51, David Kastrup escribió:

Whatever decision is taken in the end, I think we should avoid terms like "stupidity" and "braindamaged" to avoid causing possible offense. Exporting CDPATH is a simple mistake that can be made inadvertently or unwittingly, but very easily (Googling for "export CDPATH" yields 18,000+ results, many of them purporting to be Bash "tutorials").

Cheers, Wincent

Hi,

On Thu, 12 Jul 2007, Wincent Colaiuta wrote:

Sorry, my words were indeed to strong. I apologise. My only excuse is that this crops up ever so often, and it does get slightly annoying. Of course, you did not read all those mails, and so my words were unmerited.

For an example, see

http://article.gmane.org/gmane.comp.version-control.git/13736/match=cdpath

Ciao, Dscho

Hi,

[please Cc' me if you already quote me and answer directly to my message]

On Thu, 12 Jul 2007, David Kastrup wrote:

We have that already, and we're not done.

That is right. And a one-liner that pretends to be a solve-all, but is not, is worth its weight in lead.

I bet that in ten years we find yet another instance where exporting CDPATH, which you should not have done in the first place, breaks Git.

Ciao, Dscho "who chants wrong, wrong, wrong"

Wincent Colaiuta <win@wincent.com> writes:

Actually I suspect replacing the "cd blt" in the original with "cd ./blt", without changing anything else, would squelch the CDPATH problem.

We could write it as "$(TAR) Ccf blt - ." if we can rely on the 'C' option, but I suspect it is a GNU extension. Does anybody have POSIX.1 handy?

Hello,

Junio C Hamano wrote:

I don't have POSIX.1 handy, but on Solaris 10, you need to say:

	tar cf - -C blt .

(That is, Solaris' tar has the 'C' option, which is quite a good indication, that it's included in POSIX :-)

Best regards Uwe

Uwe Kleine-König <ukleinek@informatik.uni-freiburg.de> writes:

Oh, I cannot resist ;-)

Solaris has unlink(2) capable of removing a directory, but I do not think it is included in POSIX.

El 17/7/2007, a las 20:37, Junio C Hamano escribió:

I'd say the answer is "no", "C" isn't in the POSIX tar specification.

The latest, Version 3, says "the Shell and Utilities volume of IEEE Std 1003.1-2001 no longer contains the tar utility" (<http:// www.opengroup.org/onlinepubs/009695399/basedefs/tar.h.html>).

And Version 2, which does contain tar, does not list "C" as an option (<http://www.opengroup.org/onlinepubs/7990989775/xcu/tar.html>).

In fact, the example they give for moving directory hierarchies is exactly this:

   (cd fromdir; tar cf - . ) | (cd todir; tar xf -)

Cheers, Wincent

Hello Junio,

Junio C Hamano wrote:

I don't have access to the POSIX documents, but it would surprise me if they forbid unlink(2) being able to unlink directories. :-)

You need some effort to support tar -C and to catch directories in unlink. So Sun put the effort in tar, because it may be needed to be conformant to POSIX or some other standard.

Best regards Uwe