Hi, thanks for the comments. On Fri, 15 Mar 2019 13:48:25 -0500 Eric Blake <ebl...@redhat.com> wrote:
> On 3/15/19 1:40 PM, Peter Maydell wrote: > > > If you do this after the point where we make the source path absolute, you > > can skip the realpath (which avoids the problem that 'realpath' doesn't > > exist > > on OSX by default). It will also then be after the handling of the > > --source-path option argument. > > > > Do we also need to check for spaces in the path of the build directory > > (which is always the current working directory of the script) ? > > I wasn't thinking about VPATH builds, but yes, in general, we should > ensure that both srcdir and builddir are sane names, while still > allowing symlinks to work around otherwise problematic canonical names. > [...] On Fri, 15 Mar 2019 13:46:58 -0500 Eric Blake <ebl...@redhat.com> wrote: > > Why realpath? If my sources live in "/home/me/bad dir" but I have a > symlink "/home/me/good", this prevents me from building even though I > won't trip the problem. > The rationale behind the current patch was that the check should be done as soon as possible to avoid unneeded work, and since $source_path is a relative path early on in the script I thought about realpath. TBH I used realpath unconditionally because I saw it in the Makefile but I overlooked the fact that it is an internal function in make. I will move the check after the expansion of $source_path. After Eric's comment I also tried building from a sane symlink, and while configure is fine with it "make" still does not like it for in-tree builds: apparently CURDIR (used to set BUILD_PATH in the Makefile) resolves symlinks and brings back the bad path. I do get your point tho, do I did some testing to see the current status and base the changes on that. Assuming this setup: ├── no_spaces │ ├── build │ ├── qemu │ └── qemulink -> ../with spaces/qemu └── with spaces ├── build ├── qemu └── qemulink -> ../no_spaces/qemu This are the results with the current master branch: in-tree build: no_spaces/qemu $ ./configure # OK no_spaces/qemu $ make # OK no_spaces/qemulink $ ./configure # OK no_spaces/qemulink $ make # FAILS because of CURDIR with spaces/qemu $ ./configure # FAILS because of source_path with spaces/qemu $ make # FAILS because of SRC_PATH with spaces/qemulink $ ./configure # FAILS because of source_path with spaces/qemulink $ make # FAILS because of SRC_PATH out-of-tree builds: no_spaces/build $ ../qemu/configure # OK no_spaces/build $ make # OK no_spaces/build $ ../qemulink/configure # OK no_spaces/build $ make # OK no_spaces/build $ ../../with\ spaces/qemu/configure # FAILS no_spaces/build $ make # FAILS no Makefile no_spaces/build $ ../../with\ spaces/qemulink/configure # FAILS no_spaces/build $ make # FAILS ^ with spaces/build $ ../qemu/configure # FAILS with spaces/build $ make # FAILS no Makefile with spaces/build $ ../qemulink/configure # FAILS with spaces/build $ make # FAILS no Makefile with spaces/build $ ../../no_spaces/qemu/configure # OK with spaces/build $ make # FAILS (CURDIR) with spaces/build $ ../../no_spaces/qemulink/configure # OK with spaces/build $ make # FAILS (CURDIR) So checking both source_path and PWD is a probably a good thing. I'd add the check in the Makefile too, to be on the safe side and cover the case of: no_spaces/qemulink $ make Yeah, it is a slow Saturday. :) Ciao, Antonio -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?