Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#1041 closed defect (fixed)

BIND10 build fails if a directory in the path has spaces

Reported by: cas Owned by: jelte
Priority: medium Milestone: Sprint-20110830
Component: configuration Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Medium
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 3.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Version: bind10-devel-20110519
OS: Darwin 10.7.0 Darwin Kernel Version 10.7.0: Sat Jan 29 15:17:16 PST 2011; root:xnu-1504.9.37~1/RELEASE_I386 i386

when compiling BIND 10 from a directory with spaces, the build fails:

pwd -> /Developer/source/te st/bind10-devel-20110519

/bin/sh ../../../libtool --tag=CXX   --mode=link g++ -Wall -Wextra -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -Werror -fPIC -g -O2  -R/opt/local/lib       -o libutil.la -rpath /usr/local/lib filename.lo strutil.lo time_utilities.lo sha1.lo base_n.lo qid_gen.lo ../../../src/lib/exceptions/libexceptions.la 
libtool: link: g++ -dynamiclib -Wl,-undefined -Wl,dynamic_lookup -o .libs/libutil.0.dylib  .libs/filename.o .libs/strutil.o .libs/time_utilities.o .libs/sha1.o .libs/base_n.o .libs/qid_gen.o   ../../../src/lib/exceptions/.libs/libexceptions.dylib    -install_name  /usr/local/lib/libutil.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-single_module
libtool: link: dsymutil .libs/libutil.0.dylib || :
libtool: link: (cd ".libs" && rm -f "libutil.dylib" && ln -s "libutil.0.dylib" "libutil.dylib")
libtool: link: ar cru .libs/libutil.a  filename.o strutil.o time_utilities.o sha1.o base_n.o qid_gen.o
libtool: link: ranlib .libs/libutil.a
/opt/local/bin/gsed: can't read st/bind10-devel-20110519/src/lib/exceptions/libexceptions.la: No such file or directory
libtool: link: `st/bind10-devel-20110519/src/lib/exceptions/libexceptions.la' is not a valid libtool archive
make[5]: *** [libutil.la] Error 1
make[4]: *** [all-recursive] Error 1
make[3]: *** [all-recursive] Error 1
make[2]: *** [all-recursive] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

Subtickets

Change History (19)

comment:1 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:2 follow-up: Changed 8 years ago by jinmei

Do we really want to solve this?

Did it happen in someone's normal environment, or is it just as a
result of testing pathological cases? If it's the latter, I think
the priority is quite low (in fact this doesn't seem to specific to
BIND 10, and I suspect many other products would fail to compile
in this situation).

comment:3 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 3

comment:4 in reply to: ↑ 2 ; follow-up: Changed 8 years ago by cas

Replying to jinmei:

Do we really want to solve this?

Did it happen in someone's normal environment, or is it just as a
result of testing pathological cases?

it was happen to me in a normal environment (I unpacked the source of BIND 10 from a tgz file using the MacOS X finder, which creates a directory using the same name as the tgz file. The tgz file had a space).

While hving a space in the path is uncommon in Unix environments, it is quite common on Windows. So it might become a problem for users trying to compile on windows (and they might not notice what the problem is from the error message given).

A quick fix might be a check script in the make file that will give a meaningful error message early in the build process like 'BIND 10 cannot build from a path that contains spaces. exit'

If it's the latter, I think
the priority is quite low (in fact this doesn't seem to specific to
BIND 10, and I suspect many other products would fail to compile
in this situation).

I've tested BIND 9 and unbound, and both also fail. it is a problem of proper quoting for the shell, but I agree that this is an upstream problem with autotools.

-- Carsten

comment:5 Changed 8 years ago by stephen

  • Milestone changed from Next-Sprint-Proposed to Sprint-20110816

comment:6 in reply to: ↑ 4 Changed 8 years ago by zhanglikun

Replying to cas:

While hving a space in the path is uncommon in Unix environments, it is quite common on Windows. So it might become a problem for users trying to compile on windows (and they might not notice what the problem is from the error message given).

A quick fix might be a check script in the make file that will give a meaningful error message early in the build process like 'BIND 10 cannot build from a path that contains spaces. exit'

I not sure which compiler we will use when we port the the code to windows, if building with visual stdio, it runs well with the path name including spaces.

But I don't oppose to make our bind10 more smarter and friendly by adding the sexy warning "10 cannot build from a path that contains spaces. exit".

comment:7 Changed 8 years ago by jelte

something like

if [ test `echo $PWD | grep -c '\s'` != "0"  ]; then
    AC_MSG_ERROR([BIND 10 cannot be built in a directory that contains whitespace, because of libtool limitations])
fi

near the top of configure.ac?

comment:8 Changed 8 years ago by jreed

We should report this upstream.

comment:9 Changed 8 years ago by jelte

I think they know, and I do not think it would be fixed anytime soon, there's discussions about it going back to 2001 on the libtool mailing list...

comment:10 Changed 8 years ago by zhanglikun

If we can't do any help to libtool, automake, etc, just to make our bind10 more friendly and humanized. I agree with jelte's sex prompt.

comment:11 Changed 8 years ago by jelte

  • Owner set to UnAssigned
  • Status changed from new to reviewing

OK, even if this will be fixed in future version of libtool, right now we should error on it; I added a slightly modified version of the above code to configure.ac (\s does not work on all systems, so I made it a ' ', i'll just assume newlines and tabs are uncommon enough ;), and i added a hint about what to do to the error)

comment:12 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:13 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

Hello

I tested it and it works. However, I'd like to point out two things:

  • The test is needlessly complicated. Actually, there's no need to use test at all, this will work as well:
    	if echo "$PWD" | grep -q \  ; then
    		...
    	fi
    
    Because -q makes grep silent, only sets the return value if it found anything or not.
  • I'd have a proposal for more generic test. The problem isn't only spaces, it's any kind of strange characters that make sh act differently. So why not test if the path acts differently than it should?
    	if test "$PWD" != $PWD ; then
    		...
    	fi
    
    It has the problem it also prints a syntax error, though.

What do you think about it?

Thank you

comment:14 follow-up: Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

I didn't use -q because it is not supported on all systems (even though it's part of posix, our default solaris grep doesn't support it, for instance. posix my %ss).

I'm sure we can come up with a cleaner one than my proposal, but I must say I am not a fan of the second either, it makes misuse of being badly quoted, which I understand is the exact goal, but relying on such an error failing nicely as it were sounds brittle IMO

another option would be to use sed, but then we'd also need to specifically enumerate what characters not to accept.

comment:15 in reply to: ↑ 14 Changed 8 years ago by vorner

Hello

Replying to jelte:

I didn't use -q because it is not supported on all systems (even though it's part of posix, our default solaris grep doesn't support it, for instance. posix my %ss).

Ah, crap.

We could use test -z "echo | grep" instead to avoid using any parameters, but it's not important.

I'm sure we can come up with a cleaner one than my proposal, but I must say I am not a fan of the second either, it makes misuse of being badly quoted, which I understand is the exact goal, but relying on such an error failing nicely as it were sounds brittle IMO

OK, I see. We could try to sandbox the shell failing somehow (eg. running it in subshell and redirecting its outputs, etc), but that sounds an overkill anyway for a thing that will fail anyway. Whoever has newlinest and `rm -rf /` in the path deserves the punishment.

So, let's merge your version.

comment:16 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

comment:17 Changed 8 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

Ok. Well if anyone wants to spend more time on it they are welcome to do so, at least it'll fail nicely for the most common problem now. Merged, closing ticket.

comment:18 Changed 8 years ago by jreed

The [ or test don't work that way. Use something like:

if `echo $foo | grep " " >/dev/null 2>&1` ; then
  echo contains space;
fi

(As for if it is $PWD or the $PREFIX or both may need to check.)

comment:19 Changed 8 years ago by jreed

I didn't realize this ticket was already closed. It works for me. It is confusing to have the [ test ... ] (test repeated) in configure.ac, but it is fine since the generated configure does not have that unexpected operator.

Note: See TracTickets for help on using tickets.