I reordered the quoted email a bit in order to make it easier to understand my
response.
On 3/3/14, 9:34 AM, Philip Martin wrote:
> The use of -mindepth 1 -maxdepth 1 is also something I don't understand,
> what advantage is there over simply using a shell expansion? Like this:
>
> rm -rf some/path/*
>
> The shell expansion doesn't appear to have a problem with spaces when I
> test it.
I started this usage with r1421636. The purpose is to avoid length limitations
on the argument list not spaces creating problems. We could change it to just:
rm -rf subversion/tests/cmdline/svn-test-work/*
We have other uses where we're using shell expansions like this that are far
more likely to run into argument list issues already.
I wasn't aware that -mindepth and -maxdepth were going to cause problems with
Solaris or I would have just let wildcard expansion do it.
> Are we using -print0 because problems with spaces in paths have been
> observed or simply as some sort of future-proofing? Don't we control
> the paths thus allowing us to avoid spaces?
It's an attempt to avoid problems when people put their build tree on paths
that have spaces in them. However, reviewing this it's clear we have other
problems. Trying it, our build doesn't even work in other ways (most notably
cd without quoted arguments).
In some of these cases we're using relative paths within the tree and so then
the only potential issue would be people creating files with spaces in their
names (which we don't do).
So here are the find cases in our Makefile.in (these aren't exact literal
copies of the Makefile.in, I've inserted the target above the code snippet to
make it easier to find/understand in this email).
1)
fast-clean:
find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
$(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline/svntest \
-name "*.pyc" -exec rm {} ';'
This one uses absolute paths but doesn't quote the paths, breaks some of the
absolute paths end up having spaces in their names. We need to quote the list
of search directories.
2)
clean-javahl:
if [ -d $(javahl_test_rootdir) ]; then \
find $(javahl_test_rootdir) -mindepth 1 -maxdepth 1 \
-print0 | xargs -0 rm -rf --; \
fi
Missing quoting on the search path, breaks when the build dir has a space in
the absolute path. Probably should become:
rm -rf -- "$(javahl_test_rootdir)/*"
3)
gcov-clean:
find . -name "*.gcda" -o -name "*.gcno" -print0 | xargs -0 rm -f --
This is relative paths so the only concern would be paths someone makes by hand
with spaces. So probably can switch to just -print and -exec rm -f -- {} \;
4)
check-clean:
if [ -d subversion/tests/cmdline/svn-test-work ]; then \
find subversion/tests/cmdline/svn-test-work -mindepth 1 -maxdepth 1 \
-print0 | xargs -0 rm -rf --; \
fi
Again relative paths, so no issue with the absolute paths. To avoid be more
portable can just become:
rm -rf subversion/tests/cmdline/svn-test-work/*
5)
clean-swig-py:
find $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) -name "*.pyc" -exec rm {} ';'
Unquoted absolute paths.
Received on 2014-03-03 19:43:44 CET