[GRASS-dev] g.mremove bug?

Hello,

to validate a modelling module I generated output raster (ca. 10000 - but I
did not want to look at each of them ,-) )

but when I try to remove them using g.mremove I get:

g.mremove rast=out_ST* -f
Collecting map names for current mapset <nlm>...
Forcing ...
/usr/local/grass-6.3.svn/scripts/g.mremove: line
218: /usr/local/grass-6.3.svn/bin/g.remove: Lists of arguments is too long
/usr/local/grass-6.3.svn/scripts/g.mremove: line
218: /usr/local/grass-6.3.svn/bin/g.remove: success

translated from german output.

Is it a bug of g.mremove or somewhere else? Any idea how to remove all
intermediate maps?

Martin

--
**********************************************************************

University of Wuerzburg
Institute of Geography
Department of Remote Sensing
Am Hubland
97074 Wuerzburg, Germany
@
German Aerospace Center (DLR)
German Remote Sensing Data Center (DFD)
Environment and Security (US)

Phone: +49-(0)931-888-4797
Fax: +49-(0)931-888-4961
Email: martin.wegmann@uni-wuerzburg.de

**********************************************************************

On 21/02/08 12:05, Martin Wegmann wrote:

Hello,

to validate a modelling module I generated output raster (ca. 10000 - but I did not want to look at each of them ,-) )

but when I try to remove them using g.mremove I get:

g.mremove rast=out_ST* -f
Collecting map names for current mapset <nlm>...
Forcing ...
/usr/local/grass-6.3.svn/scripts/g.mremove: line 218: /usr/local/grass-6.3.svn/bin/g.remove: Lists of arguments is too long
/usr/local/grass-6.3.svn/scripts/g.mremove: line 218: /usr/local/grass-6.3.svn/bin/g.remove: success

The shell has a maximum line length (and maximum argument number ?) which you probably exceed with this list.

You'd probably be better off doing something like this:

for map in `g.mlist rast pattern="out_ST*"`
   do
     g.remove $map
   done

Moritz

On Donnerstag, 21. Februar 2008 13:03:11 Moritz Lennert wrote:

On 21/02/08 12:05, Martin Wegmann wrote:
> Hello,
>
> to validate a modelling module I generated output raster (ca. 10000 - but
> I did not want to look at each of them ,-) )
>
> but when I try to remove them using g.mremove I get:
>
> g.mremove rast=out_ST* -f
> Collecting map names for current mapset <nlm>...
> Forcing ...
> /usr/local/grass-6.3.svn/scripts/g.mremove: line
> 218: /usr/local/grass-6.3.svn/bin/g.remove: Lists of arguments is too
> long /usr/local/grass-6.3.svn/scripts/g.mremove: line
> 218: /usr/local/grass-6.3.svn/bin/g.remove: success

The shell has a maximum line length (and maximum argument number ?)
which you probably exceed with this list.

You'd probably be better off doing something like this:

for map in `g.mlist rast pattern="out_ST*"`
   do
     g.remove $map
   done

thanks, that solved the problem - perhaps we can add it to the g.remove man
page as an example and add a hint in g.mremove.
But removing more than 1000 raster is probably not a very common task.

Martin

--
**********************************************************************

University of Wuerzburg
Institute of Geography
Department of Remote Sensing
Am Hubland
97074 Wuerzburg, Germany
@
German Aerospace Center (DLR)
German Remote Sensing Data Center (DFD)
Environment and Security (US)

Phone: +49-(0)931-888-4797
Fax: +49-(0)931-888-4961
Email: martin.wegmann@uni-wuerzburg.de

**********************************************************************

Moritz Lennert <mlennert@club.worldonline.be> writes:

>> to validate a modelling module I generated output raster (ca. 10000
>> - but I did not want to look at each of them ,-) )

>> but when I try to remove them using g.mremove I get:

[...]

  This is a bug in `g.mremove'.

  It may be reasonable to rewrite `g.mremove' using `xargs',
  though unfortunately `xargs' isn't required to support `-0' per
  POSIX [1].

> The shell has a maximum line length (and maximum argument number ?)
> which you probably exceed with this list.

  These are usually OS limits, not just that of the shell.

> You'd probably be better off doing something like this:

> for map in `g.mlist rast pattern="out_ST*"` ; do g.remove $map ; done

  Or (see also [1-4]):

$ g.mlist rast pattern="out_ST*" | sed -e s/^/rast=/ | xargs g.remove

  This variant runs `g.remove' in ``batches'', supplying each time
  just enough arguments to reasonably fill the limited command
  line space.

  The use of `xargs' is safe as long as the arguments don't
  contain any of the following characters:

  * newlines (ASCII LF, 10) or blanks (ASCII HT, space, etc.), as
    they're used to separate arguments;

  * backslashes, single- and double-quote characters, as they're
    used to quote newlines, blanks and themselves.

[1] http://www.opengroup.org/onlinepubs/009695399/utilities/xargs.html
[2] http://www.opengroup.org/onlinepubs/009695399/utilities/sed.html
[3] http://www.gnu.org/software/sed/manual/
[4] http://www.gnu.org/software/findutils/manual/html_node/find_html/Invoking-xargs.html

Ivan Shmakov wrote:

$ g.mlist rast pattern="out_ST*" | sed -e s/^/rast=/ | xargs g.remove

  This variant runs `g.remove' in ``batches'', supplying each time
  just enough arguments to reasonably fill the limited command
  line space.

  The use of `xargs' is safe as long as the arguments don't
  contain any of the following characters:

  * newlines (ASCII LF, 10) or blanks (ASCII HT, space, etc.), as
    they're used to separate arguments;

  * backslashes, single- and double-quote characters, as they're
    used to quote newlines, blanks and themselves.

That isn't a problem; GRASS map names aren't allowed to contain any of
those characters.

The more fundamental problem with using xargs is that the GRASS parser
requires multiple "answers" for any given option to be separated by
commas, not spaces.

That effectively requires g.mremove to collate names itself.

Invoking g.remove once per map is simpler, but it's also inefficient.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements <glynn@gclements.plus.com> writes:

>> $ g.mlist rast pattern="out_ST*" | sed -e s/^/rast=/ | xargs g.remove

>> This variant runs `g.remove' in ``batches'', supplying each time
>> just enough arguments to reasonably fill the limited command line
>> space.

>> The use of `xargs' is safe as long as the arguments don't contain
>> any of the following characters:

[...]

> That isn't a problem; GRASS map names aren't allowed to contain any
> of those characters.

  ACK, thanks.

> The more fundamental problem with using xargs is that the GRASS
> parser requires multiple "answers" for any given option to be
> separated by commas, not spaces.

  Does it? IIUC, the following commands are completely equivalent
  as long as the GRASS parser is concerned:

$ g.remove rast=foo,bar

$ g.remove rast=foo rast=bar

> That effectively requires g.mremove to collate names itself.

> Invoking g.remove once per map is simpler, but it's also inefficient.

  Huh? Even simpler than the following?

may_be_foo () {
    if [ -z "$2" ]; then
        ## .
        return
    fi
    g.mlist $regex type="$1" mapset="$MAPSET" pattern="$2" \
        | sed -e s,^,"$1"=,
}

{
    may_be_foo rast "$r"
    may_be_foo rast3d "$r3"
    may_be_foo vect "$v"
    may_be_foo icon "$i"
    may_be_foo labels "$l"
    may_be_foo region "$rg"
    may_be_foo group "$g"
    may_be_foo 3dview "$d"
} | xargs g.remove

  But actually, since POSIX `xargs' lacks `-r', it's necessary to
  check for the empty list case, e. g.:

{
    ...
} > "$tmp"

if grep -q . -- "$tmp"; then
    xargs g.remove < "$tmp"
fi

  May I suggest the following patch?

scripts/g.mremove/g.mremove: Fixed: don't hit the OS command line limits
by using `xargs'.
(nntp://news.gmane.org/gmane.comp.gis.grass.devel/25028)

--- a/scripts/g.mremove/g.mremove
+++ b/scripts/g.mremove/g.mremove
@@ -119,100 +119,40 @@ if [ $force -eq 1 ] ; then
    g.message "Forcing ..."
fi

-r=$GIS_OPT_RAST
-r3=$GIS_OPT_RAST3D
-v=$GIS_OPT_VECT
-i=$GIS_OPT_ICON
-l=$GIS_OPT_LABEL
-rg=$GIS_OPT_REGION
-g=$GIS_OPT_GROUP
-d=$GIS_OPT_DVIEW
-
-if [ -n "$r" ] ; then
- rast=`g.mlist $regex type=rast sep=, mapset=$MAPSET pattern="$r"`
-fi
-
-if [ -n "$r3" ] ; then
- rast3d=`g.mlist $regex type=rast3d sep=, mapset=$MAPSET pattern="$r3"`
-fi
-
-if [ -n "$v" ] ; then
- vect=`g.mlist $regex type=vect sep=, mapset=$MAPSET pattern="$v"`
-fi
-
-if [ -n "$i" ] ; then
- icon=`g.mlist $regex type=icon sep=, mapset=$MAPSET pattern="$i"`
-fi
-
-if [ -n "$l" ] ; then
- labels=`g.mlist $regex type=labels sep=, mapset=$MAPSET pattern="$l"`
-fi
-
-if [ -n "$rg" ] ; then
- region=`g.mlist $regex type=region sep=, mapset=$MAPSET pattern="$rg"`
-fi
-
-if [ -n "$g" ] ; then
- group=`g.mlist $regex type=group sep=, mapset=$MAPSET pattern="$g"`
-fi
-
-if [ -n "$d" ] ; then
- dview=`g.mlist $regex type=3dview sep=, mapset=$MAPSET pattern="$d"`
-fi
-
-found=
-if [ "$rast" ] ; then
- found=1
- rast="rast=$rast"
-fi
-
-if [ "$rast3d" ] ; then
- found=1
- rast3d="rast3d=$rast3d"
-fi
-
-if [ "$vect" ] ; then
- found=1
- vect="vect=$vect"
-fi
-
-if [ "$icon" ] ; then
- found=1
- icon="icon=$icon"
-fi
-
-if [ "$labels" ] ; then
- found=1
- labels="labels=$labels"
-fi
-
-if [ "$region" ] ; then
- found=1
- region="region=$region"
-fi
-
-if [ "$group" ] ; then
- found=1
- group="group=$group"
-fi
-
-if [ "$dview" ] ; then
- found=1
- dview="3dview=$dview"
-fi
-
-if [ ! "$found" ] ; then
+tmp="$(g.tempfile pid=$$)"
+
+may_be () {
+ if [ -z "$2" ]; then
+ ## .
+ return
+ fi
+ g.mlist $regex type="$1" mapset="$MAPSET" pattern="$2" \
+ | sed -e s,^,"$1"=,
+}
+
+{
+ may_be rast "$GIS_OPT_RAST"
+ may_be rast3d "$GIS_OPT_RAST3D"
+ may_be vect "$GIS_OPT_VECT"
+ may_be icon "$GIS_OPT_ICON"
+ may_be labels "$GIS_OPT_LABEL"
+ may_be region "$GIS_OPT_REGION"
+ may_be group "$GIS_OPT_GROUP"
+ may_be 3dview "$GIS_OPT_DVIEW"
+} > "$tmp"
+
+if ! grep -q . -- "$tmp" ; then
   g.message -e "No data found."
   exit 1
fi

if [ $force -eq 0 ] ; then
     g.message "The following files would be deleted:"
- echo "g.remove $rast $rast3d $vect $icon $labels $region $group $dview"
+ ## FIXME: should probably use `g.message'
+ cat -- "$tmp"
     g.message message=""
     g.message "You must use the force flag to actually remove them. Exiting."
     exit 0
fi

-
-exec g.remove $rast $rast3d $vect $icon $labels $region $group $dview
+xargs g.remove < "$tmp"

Ivan Shmakov wrote:

> The more fundamental problem with using xargs is that the GRASS
> parser requires multiple "answers" for any given option to be
> separated by commas, not spaces.

  Does it? IIUC, the following commands are completely equivalent
  as long as the GRASS parser is concerned:

$ g.remove rast=foo,bar

$ g.remove rast=foo rast=bar

Right; I was thinking specifically about the former case. It's so long
since I've seen the latter form used that I had forgotten about it.

The former case may be slightly more efficient (you'll get more maps
into a maxium-length command line), but it's probably not worth the
additional effort.

> That effectively requires g.mremove to collate names itself.

> Invoking g.remove once per map is simpler, but it's also inefficient.

  Huh? Even simpler than the following?

Yes.

Generating shell commands using sed then executing them is anything
but simple; particularly when the sed command is itself generated
using shell features (variable substitution).

"while read map ; do g.remove ... ; done" may be more verbose, but
it's much easier to understand.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements <glynn@gclements.plus.com> writes:

>>> The more fundamental problem with using xargs is that the GRASS
>>> parser requires multiple "answers" for any given option to be
>>> separated by commas, not spaces.

>> Does it? IIUC, the following commands are completely equivalent as
>> long as the GRASS parser is concerned:

>> $ g.remove rast=foo,bar

>> $ g.remove rast=foo rast=bar

> Right; I was thinking specifically about the former case. It's so
> long since I've seen the latter form used that I had forgotten about
> it.

  That's strange -- I use the latter form quite often, mostly as:

$ g.mlist ... | sed -e s/^/OPTION=/ | xargs GRASS-COMMAND ...

  or, e. g.:

$ r.patch output=foo $(g.mlist ... | sed -e s/^/input=/)

[...]

>>> That effectively requires g.mremove to collate names itself.

>>> Invoking g.remove once per map is simpler, but it's also
>>> inefficient.

>> Huh? Even simpler than the following?

> Yes.

> Generating shell commands using sed then executing them is anything
> but simple;

  But the idea is overly simple: since `g.remove' requires every
  map name to be prefixed by its type (e. g., `rast='), we're
  going exactly this way -- changing every line to begin with
  `TYPE='.

> particularly when the sed command is itself generated using shell
> features (variable substitution).

> "while read map ; do g.remove ... ; done" may be more verbose, but
> it's much easier to understand.

  That'll be:

foo () {
    while read map ; do g.remove "$1=$map" ; done
}

  if there's a desire to have just one function for every map
  type. I don't think it's much more complex than:

foo () {
    while read map ; do echo "$1=$map" ; done | xargs g.remove
}

  or, with a more appropriate tool to do the substitution:

foo () {
    sed s/^/"$1"/ | xargs g.remove
}

  For my students, I've explained it roughly as follows:

  * there're some commands that read a file with a list of objects
    (files) to perform the operation on (e. g., $ wget -i);
    standard input is a particular case of a file ($ wget -i -);

  * there're much more commands that obtain the list from the
    command line (e. g., $ ls, $ chmod, etc.);

  * there're commands which can do both ($ wget URL, or $ echo URL
    | wget -i -);

  * for these commands that cannot read the list from a file,
    there's the `xargs' command, which ``transforms'' the contents
    of the given file into the command line arguments for a
    command; then, there're OS limits on the command line length.

  Anyway, could you please comment on my patch [1]? I've tested
  it a bit, but the `g.mremove' command wouldn't probably be the
  one I'll use myself much.

[1] nntp://news.gmane.org/gmane.comp.gis.grass.devel/25056