Re: [PATCH] Again, do not change the mode of all directories below $HOME.

[Ralf Wildenhues]

Hi Jim,

* Jim Meyering wrote on Tue, Jul 22, 2008 at 01:17:20PM CEST:
> Ralf Wildenhues <address@hidden> wrote:
> > this fixes a regression of "make check" when source and build tree live
> > in a directory with spaces in the name.  The regression was introduced
> > some time after I posted a fix for the issue last time.

> Look at it as an incentive not to use directories with names
> containing meta-characters ;-)

Well, *I* am doing it for fun.  Others use /c/My Files/... because yet
others thought requiring users to cope with such names would be a cool
idea.  The slightly provoking subject is to remind you that I suffered
when this happened to me for the first time.  ;-)

> But seriously, considering the potential for damage and mischief,
> perhaps it's time to add infrastructure that stops e.g., configure
> in its tracks whenever it detects a potentially troublesome source or
> build dir.

I disagree.  You can do that and admit defeat.  I worked to mostly[1]
fix Automake for this, because users kept complaining, and now refuse
to admit defeat.

> I applied your patch, then added a test to ensure there is no further
> regression, in spite of the added cost to "make distcheck".  Finally,
> I fixed a similar (and long-standing!) shell-under-quoting bug in
> test-lib.sh that was exposed by the cleanup (trap-run) code not removing
> a per-test directory for the tests that create unsearchable directories.
> With a bad build directory containing an "a b" component, instead of
> running e.g., chmod -R 700 "/.../a b/...", the pre-patch trap/cleanup
> code would run chmod -R 700 "/.../a".

Would the following not have sufficed, too?

> -trap 'st=$?; cleanup_; d='"$t_"';
> -    cd '"$test_dir_"' && chmod -R u+rwx "$d" && rm -rf "$d" && exit $st' 0
  +trap 'st=$?; cleanup_; d="$t_";
  +    cd "$test_dir_" && chmod -R u+rwx "$d" && rm -rf "$d" && exit $st' 0


BTW, I see that you use 'local' in shell functions.  This isn't required
to be supported by POSIX sh, and AFAIK won't work with some ksh variants,
for example the "Version M 1993-12-28 r" on my GNU/Linux system, but
also your test-lib.sh doesn't check for this capability.  I suggest to
just avoid local variables, since you have only a couple anyway.  Should
I write a patch to this end?

FWIW, your second patch looks like it won't quite do what you intended
if TMPDIR contains white space:

  tp := $(shell echo "$(TMPDIR)/$(PACKAGE)-$$$$")
  t_prefix = $(tp)/a
  ... using $(t_prefix) unquoted in a number of places ...

Cheers,
Ralf

[1] You currently have to do something like this:
  ./configure -C MISSING=/usr/share/automake-1.10a/missing \
                 install_sh=/usr/share/automake-1.10a/install-sh