You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Martin Furter <mf...@rola.ch> on 2009/11/23 02:46:14 UTC

[PATCH] find -mindepth/-maxdepth is not portable

[[[
The options -mindepth and -maxdepth of find are not portable.
Since there aren't any other python files in subversion/tests/ except 
those at depth 2 and 3 just drop those options.

* Makefile.in
   (fast-clean): Remove -mindepth and -maxdepth from find command.
]]]

[[[
Index: Makefile.in
===================================================================
--- Makefile.in (revision 883010)
+++ Makefile.in (working copy)
@@ -369,8 +369,7 @@
         rm -f $(CLEAN_FILES)
         find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
                 $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
-       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
-               -name "*.pyc" -exec rm {} ';'
+       find $(abs_srcdir)/subversion/tests/ -name "*.pyc" -exec rm {} ';'

  # clean all but bulky test output, returning to before './configure' was run.
  SVN_CONFIG_SCRIPT_FILES = @SVN_CONFIG_SCRIPT_FILES@
]]]

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Nov 24, 2009 at 03:30:20PM +0100, Stefan Sperling wrote:
> Avoid the GNU-specific "mindepth" and "maxdepth" options to "find".
> 
> * Makefile.in: CLEAN_FILES can be a long list, so use an "echo | xargs"
>    construct which is less likely to cause a "too many arguments" error
>    since the maximum length of the argument list is limited by the shell
>    rather than the kernel.
>    Don't use find to locate .pyc files created by running the test suite,
>    gen_make.py now puts them into CLEAN_FILES. Clean .pyc files in
>    tests/cmdline/svntest explicitly since they are not in the scripts
>    list known to gen_make.py.
> 
> * build/generator/gen_make.py: Add .pyc files potentially created by
>    running the test suite to the CLEAN_FILES list.
> ]]]

Committed in r884264.

Stefan

> > > 
> > > 
> > > Index: Makefile.in
> > > ===================================================================
> > > --- Makefile.in	(revision 883642)
> > > +++ Makefile.in	(working copy)
> > > @@ -366,10 +366,9 @@ fast-clean: doc-clean
> > >               rm -rf .libs);                                          \
> > >            fi                                                         \
> > >          done
> > > -	rm -f $(CLEAN_FILES)
> > > +	echo $(CLEAN_FILES) | xargs rm -f
> > >  	find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
> > > -		$(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> > > -	find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> > > +		$(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline/svntest \
> > >  		-name "*.pyc" -exec rm {} ';'
> > >  
> > >  # clean all but bulky test output, returning to before './configure' was run.
> > > Index: build/generator/gen_make.py
> > > ===================================================================
> > > --- build/generator/gen_make.py	(revision 883642)
> > > +++ build/generator/gen_make.py	(working copy)
> > > @@ -156,6 +156,9 @@ class Generator(gen_base.GeneratorBase):
> > >           and not target.external_lib \
> > >           and target.filename[-3:] != '.la':
> > >          cfiles.append(target.filename)
> > > +    for script in self.scripts:
> > > +      if script.endswith('.py'):
> > > +        cfiles.append(script + 'c')
> > >      data.cfiles = sorted(cfiles)
> > >  
> > >      # here are all the SQL files and their generated headers. the Makefile
> > 
> 
> -- 
> printf("Eh???/n");

-- 
printf("Eh???/n");

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Nov 24, 2009 at 02:18:20PM +0000, Julian Foad wrote:
> Remember to mention the reason you are changing this at all: it was to
> avoif the GNU-specific "mindepth" and "maxdepth" options to "find",
> wasn't it?

Ooops, yes, it was, thanks.

[[[
Avoid the GNU-specific "mindepth" and "maxdepth" options to "find".

* Makefile.in: CLEAN_FILES can be a long list, so use an "echo | xargs"
   construct which is less likely to cause a "too many arguments" error
   since the maximum length of the argument list is limited by the shell
   rather than the kernel.
   Don't use find to locate .pyc files created by running the test suite,
   gen_make.py now puts them into CLEAN_FILES. Clean .pyc files in
   tests/cmdline/svntest explicitly since they are not in the scripts
   list known to gen_make.py.

* build/generator/gen_make.py: Add .pyc files potentially created by
   running the test suite to the CLEAN_FILES list.
]]]

Stefan

> > 
> > 
> > Index: Makefile.in
> > ===================================================================
> > --- Makefile.in	(revision 883642)
> > +++ Makefile.in	(working copy)
> > @@ -366,10 +366,9 @@ fast-clean: doc-clean
> >               rm -rf .libs);                                          \
> >            fi                                                         \
> >          done
> > -	rm -f $(CLEAN_FILES)
> > +	echo $(CLEAN_FILES) | xargs rm -f
> >  	find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
> > -		$(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> > -	find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> > +		$(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline/svntest \
> >  		-name "*.pyc" -exec rm {} ';'
> >  
> >  # clean all but bulky test output, returning to before './configure' was run.
> > Index: build/generator/gen_make.py
> > ===================================================================
> > --- build/generator/gen_make.py	(revision 883642)
> > +++ build/generator/gen_make.py	(working copy)
> > @@ -156,6 +156,9 @@ class Generator(gen_base.GeneratorBase):
> >           and not target.external_lib \
> >           and target.filename[-3:] != '.la':
> >          cfiles.append(target.filename)
> > +    for script in self.scripts:
> > +      if script.endswith('.py'):
> > +        cfiles.append(script + 'c')
> >      data.cfiles = sorted(cfiles)
> >  
> >      # here are all the SQL files and their generated headers. the Makefile
> 

-- 
printf("Eh???/n");

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Sperling wrote:
> On Mon, Nov 23, 2009 at 08:11:32AM -0600, Peter Samuelson wrote:
> > 
> > [Stefan Sperling]
> > > How?
> > > 'echo $(CLEAN_FILES) | xargs' has the same problem (can fail at 'echo')
> > 
> > In theory it can, but echo is almost always a shell builtin.  So the
> > kernel argument length limit does not apply, only whatever limit the
> > shell may have.
> 
> How about this diff then?
> 
> Stefan
> 
> [[[

Remember to mention the reason you are changing this at all: it was to
avoif the GNU-specific "mindepth" and "maxdepth" options to "find",
wasn't it?

> * Makefile.in: CLEAN_FILES can be a long list, so use an "echo | xargs"
>    construct which is less likely to cause a "too many arguments" error
>    since the maximum length of the argument list is limited by the shell
>    rather than the kernel.
>    Don't use find to locate .pyc files created by running the test suite,
>    gen_make.py now puts them into CLEAN_FILES. Clean .pyc files in
>    tests/cmdline/svntest explicitly since they are not in the scripts
>    list known to gen_make.py.
> 
> * build/generator/gen_make.py: Add .pyc files potentially created by
>    running the test suite to the CLEAN_FILES list.
> ]]]
> 
> 
> Index: Makefile.in
> ===================================================================
> --- Makefile.in	(revision 883642)
> +++ Makefile.in	(working copy)
> @@ -366,10 +366,9 @@ fast-clean: doc-clean
>               rm -rf .libs);                                          \
>            fi                                                         \
>          done
> -	rm -f $(CLEAN_FILES)
> +	echo $(CLEAN_FILES) | xargs rm -f
>  	find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
> -		$(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> -	find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> +		$(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline/svntest \
>  		-name "*.pyc" -exec rm {} ';'
>  
>  # clean all but bulky test output, returning to before './configure' was run.
> Index: build/generator/gen_make.py
> ===================================================================
> --- build/generator/gen_make.py	(revision 883642)
> +++ build/generator/gen_make.py	(working copy)
> @@ -156,6 +156,9 @@ class Generator(gen_base.GeneratorBase):
>           and not target.external_lib \
>           and target.filename[-3:] != '.la':
>          cfiles.append(target.filename)
> +    for script in self.scripts:
> +      if script.endswith('.py'):
> +        cfiles.append(script + 'c')
>      data.cfiles = sorted(cfiles)
>  
>      # here are all the SQL files and their generated headers. the Makefile



Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Nov 23, 2009 at 08:11:32AM -0600, Peter Samuelson wrote:
> 
> [Stefan Sperling]
> > How?
> > 'echo $(CLEAN_FILES) | xargs' has the same problem (can fail at 'echo')
> 
> In theory it can, but echo is almost always a shell builtin.  So the
> kernel argument length limit does not apply, only whatever limit the
> shell may have.

How about this diff then?

Stefan

[[[
* Makefile.in: CLEAN_FILES can be a long list, so use an "echo | xargs"
   construct which is less likely to cause a "too many arguments" error
   since the maximum length of the argument list is limited by the shell
   rather than the kernel.
   Don't use find to locate .pyc files created by running the test suite,
   gen_make.py now puts them into CLEAN_FILES. Clean .pyc files in
   tests/cmdline/svntest explicitly since they are not in the scripts
   list known to gen_make.py.

* build/generator/gen_make.py: Add .pyc files potentially created by
   running the test suite to the CLEAN_FILES list.
]]]


Index: Makefile.in
===================================================================
--- Makefile.in	(revision 883642)
+++ Makefile.in	(working copy)
@@ -366,10 +366,9 @@ fast-clean: doc-clean
              rm -rf .libs);                                          \
           fi                                                         \
         done
-	rm -f $(CLEAN_FILES)
+	echo $(CLEAN_FILES) | xargs rm -f
 	find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
-		$(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
-	find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
+		$(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline/svntest \
 		-name "*.pyc" -exec rm {} ';'
 
 # clean all but bulky test output, returning to before './configure' was run.
Index: build/generator/gen_make.py
===================================================================
--- build/generator/gen_make.py	(revision 883642)
+++ build/generator/gen_make.py	(working copy)
@@ -156,6 +156,9 @@ class Generator(gen_base.GeneratorBase):
          and not target.external_lib \
          and target.filename[-3:] != '.la':
         cfiles.append(target.filename)
+    for script in self.scripts:
+      if script.endswith('.py'):
+        cfiles.append(script + 'c')
     data.cfiles = sorted(cfiles)
 
     # here are all the SQL files and their generated headers. the Makefile

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Peter Samuelson <pe...@p12n.org>.
[Stefan Sperling]
> How?
> 'echo $(CLEAN_FILES) | xargs' has the same problem (can fail at 'echo')

In theory it can, but echo is almost always a shell builtin.  So the
kernel argument length limit does not apply, only whatever limit the
shell may have.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Nov 23, 2009 at 07:16:49AM -0600, Hyrum K. Wright wrote:
> On Nov 23, 2009, at 3:03 AM, Stefan Sperling wrote:
> > Speaking of which, rm -f $(CLEAN_FILES) could also end up being too
> > long on certain types of UNIX. So maybe put .pycs into a different list.
> > Or if we can find out the minimum length supported by systems still in
> > use today, we split up CLEAN_FILES accordingly, and run
> > rm -f $(CLEAN_FILES1); rm -rf $(CLEAN_FILES2); ... etc.
> 
> Or just use xargs(1).

How?
'echo $(CLEAN_FILES) | xargs' has the same problem (can fail at 'echo')

Stefan

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Nov 23, 2009, at 3:03 AM, Stefan Sperling wrote:

> On Mon, Nov 23, 2009 at 12:44:32AM -0600, Peter Samuelson wrote:
>> 
>> [Martin Furter]
>>> --- Makefile.in (revision 883010)
>>> +++ Makefile.in (working copy)
>>> @@ -369,8 +369,8 @@
>>>        rm -f $(CLEAN_FILES)
>>>        find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
>>>                $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
>>> -       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
>>> -               -name "*.pyc" -exec rm {} ';'
>>> +       rm -f $(abs_srcdir)/subversion/tests/*/*.pyc
>>> +       rm -f $(abs_srcdir)/subversion/tests/*/*/*.pyc
>> 
>> That may be more portable, or again, maybe not.  It could fail on some
>> Unixes due to limitations on command line length, particularly if
>> $(abs_srcdir) is itself a long path.  Probably better:
>> 
>> 	cd $(abs_srcdir)/subversion/tests; rm -f */*.pyc
>> 	cd $(abs_srcdir)/subversion/tests; rm -f */*/*.pyc
>> 
>> Or even:
>> 
>> 	cd $(abs_srcdir)/subversion/tests; echo */*.pyc | xargs rm -f
>> 	cd $(abs_srcdir)/subversion/tests; echo */*/*.pyc | xargs rm -f
> 
> Why are we trying to dynamically get the list of .pyc files in Makefile.in?
> Why not just pre-compute the .pyc list and pass it to rm -f during
> make fast-clean? Let's just put the list into build-outputs.mk.
> The TEST_PROGRAMS variable probably already contains the list of .py
> files corresponding to the .pyc files we want to remove.
> It could be very simple to make gen-make.py add the corresponding
> .pyc files to the CLEAN_FILES variable.
> 
> Speaking of which, rm -f $(CLEAN_FILES) could also end up being too
> long on certain types of UNIX. So maybe put .pycs into a different list.
> Or if we can find out the minimum length supported by systems still in
> use today, we split up CLEAN_FILES accordingly, and run
> rm -f $(CLEAN_FILES1); rm -rf $(CLEAN_FILES2); ... etc.

Or just use xargs(1).

-Hyrum

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Nov 23, 2009 at 12:44:32AM -0600, Peter Samuelson wrote:
> 
> [Martin Furter]
> > --- Makefile.in (revision 883010)
> > +++ Makefile.in (working copy)
> > @@ -369,8 +369,8 @@
> >         rm -f $(CLEAN_FILES)
> >         find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
> >                 $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> > -       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> > -               -name "*.pyc" -exec rm {} ';'
> > +       rm -f $(abs_srcdir)/subversion/tests/*/*.pyc
> > +       rm -f $(abs_srcdir)/subversion/tests/*/*/*.pyc
> 
> That may be more portable, or again, maybe not.  It could fail on some
> Unixes due to limitations on command line length, particularly if
> $(abs_srcdir) is itself a long path.  Probably better:
> 
> 	cd $(abs_srcdir)/subversion/tests; rm -f */*.pyc
> 	cd $(abs_srcdir)/subversion/tests; rm -f */*/*.pyc
> 
> Or even:
> 
> 	cd $(abs_srcdir)/subversion/tests; echo */*.pyc | xargs rm -f
> 	cd $(abs_srcdir)/subversion/tests; echo */*/*.pyc | xargs rm -f

Why are we trying to dynamically get the list of .pyc files in Makefile.in?
Why not just pre-compute the .pyc list and pass it to rm -f during
make fast-clean? Let's just put the list into build-outputs.mk.
The TEST_PROGRAMS variable probably already contains the list of .py
files corresponding to the .pyc files we want to remove.
It could be very simple to make gen-make.py add the corresponding
.pyc files to the CLEAN_FILES variable.

Speaking of which, rm -f $(CLEAN_FILES) could also end up being too
long on certain types of UNIX. So maybe put .pycs into a different list.
Or if we can find out the minimum length supported by systems still in
use today, we split up CLEAN_FILES accordingly, and run
rm -f $(CLEAN_FILES1); rm -rf $(CLEAN_FILES2); ... etc.

Stefan

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Peter Samuelson <pe...@p12n.org>.
[Martin Furter]
> --- Makefile.in (revision 883010)
> +++ Makefile.in (working copy)
> @@ -369,8 +369,8 @@
>         rm -f $(CLEAN_FILES)
>         find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
>                 $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> -       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> -               -name "*.pyc" -exec rm {} ';'
> +       rm -f $(abs_srcdir)/subversion/tests/*/*.pyc
> +       rm -f $(abs_srcdir)/subversion/tests/*/*/*.pyc

That may be more portable, or again, maybe not.  It could fail on some
Unixes due to limitations on command line length, particularly if
$(abs_srcdir) is itself a long path.  Probably better:

	cd $(abs_srcdir)/subversion/tests; rm -f */*.pyc
	cd $(abs_srcdir)/subversion/tests; rm -f */*/*.pyc

Or even:

	cd $(abs_srcdir)/subversion/tests; echo */*.pyc | xargs rm -f
	cd $(abs_srcdir)/subversion/tests; echo */*/*.pyc | xargs rm -f

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Martin Furter <mf...@rola.ch>.

On Mon, 23 Nov 2009, Arfrever Frehtes Taifersar Arahesis wrote:

> 2009-11-23 02:46:14 Martin Furter napisał(a):
>>
>> [[[
>> The options -mindepth and -maxdepth of find are not portable.
>> Since there aren't any other python files in subversion/tests/ except
>> those at depth 2 and 3 just drop those options.
>
> Searching for *.pyc files in svn-test-work directory might significantly
> decrease performance...
>
> https://svn.apache.org/viewvc?view=revision&revision=878872
>
>> * Makefile.in
>>    (fast-clean): Remove -mindepth and -maxdepth from find command.
>> ]]]

OK, what about this approach:

[[[
The options -mindepth and -maxdepth of find are not portable.
So do it without using find.

* Makefile.in
   (fast-clean): Use shell wildcards instead of find.
]]]


[[[
Index: Makefile.in
===================================================================
--- Makefile.in (revision 883010)
+++ Makefile.in (working copy)
@@ -369,8 +369,8 @@
         rm -f $(CLEAN_FILES)
         find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
                 $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
-       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
-               -name "*.pyc" -exec rm {} ';'
+       rm -f $(abs_srcdir)/subversion/tests/*/*.pyc
+       rm -f $(abs_srcdir)/subversion/tests/*/*/*.pyc

  # clean all but bulky test output, returning to before './configure' was run.
  SVN_CONFIG_SCRIPT_FILES = @SVN_CONFIG_SCRIPT_FILES@
]]]

Re: [PATCH] find -mindepth/-maxdepth is not portable

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2009-11-23 02:46:14 Martin Furter napisał(a):
> 
> [[[
> The options -mindepth and -maxdepth of find are not portable.
> Since there aren't any other python files in subversion/tests/ except 
> those at depth 2 and 3 just drop those options.

Searching for *.pyc files in svn-test-work directory might significantly
decrease performance...

https://svn.apache.org/viewvc?view=revision&revision=878872

> * Makefile.in
>    (fast-clean): Remove -mindepth and -maxdepth from find command.
> ]]]
> 
> [[[
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 883010)
> +++ Makefile.in (working copy)
> @@ -369,8 +369,7 @@
>          rm -f $(CLEAN_FILES)
>          find $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
>                  $(abs_srcdir)/build -name "*.pyc" -exec rm {} ';'
> -       find $(abs_srcdir)/subversion/tests/ -mindepth 2 -maxdepth 3 \
> -               -name "*.pyc" -exec rm {} ';'
> +       find $(abs_srcdir)/subversion/tests/ -name "*.pyc" -exec rm {} ';'
> 
>   # clean all but bulky test output, returning to before './configure' was run.
>   SVN_CONFIG_SCRIPT_FILES = @SVN_CONFIG_SCRIPT_FILES@
> ]]]

-- 
Arfrever Frehtes Taifersar Arahesis