Hi Jim, and sorry for the delay, * Jim Meyering wrote on Tue, Jul 22, 2008 at 09:42:30PM CEST: > Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > > > >> 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 wouldn't call it admitting defeat. > It's more like admitting that not everyone will go to > the required lengths to make their code robust enough > to deal with such situations. Granted. But please don't disallow white space in the build dir for a normal "./configure && make all install". That would hurt some users. > >> 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 > > I don't see how that would change anything. The cd is just to > get out of the directory we're about to remove. It worked. > The chmod is what failed, operating on the prefix up to, > but not including the space. Hmm. I think I fail to see what exactly the shell-under-quoting was then. Could you be bothered to explain? Thanks. > > 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? > > I'd be more amenable to a patch that makes a macro like > posix-shell.m4 choose a shell with the features I use. > Since no one has reported trouble yet, perhaps it's not a problem > in practice. Hmm. Maybe you're right. That would be very cool to have 'local' available. > > 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 ... > > Yes. That was deliberate. > With those rules, I have never bothered to accommodate white space in TMPDIR. > I expect that anyone clueful enough to run those optional rules also > knows not to choose a TMPDIR value that is likely to cause trouble. That is fine with me. I just thought I'd mention it. Cheers, Ralf _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils