[GRASS-dev] Re: [bug #4450] (grass) g.remove vect: empty dirs in the $MAPSET/.tmp/$HOSTNAME left

Maciej_Sieczka3 · June 17, 2006, 5:12pm

On Sat, 17 Jun 2006 15:04:52 +0200 (CEST)
Markus Neteler via RT <grass-bugs@intevation.de> wrote:

On Sat, Jun 17, 2006 at 01:32:24PM +0200, Maciek Sieczka via RT wrote:
> I have built current Grass 6.1 CVS on Ubuntu Breezy with
> http://mpa.itc.it/markus/tmp/clean_temp2.c apllied.

> I'd prefer each such empty dir be removed right after 'g.remove
> vect'is done, but maybe this is not so critical.

This is an issue of the program creating the vector map in
question. Which one did you use? A script, r.* or v.*?

It is 'g.remove vect' that leaves empty temp dirs in the .tmp. As my
bug report reads:

For each vector file it removes, g.remove leaves 1 empty, redundant
directory in the $MAPSET/.tmp/$HOSTNAME.

To reproduce:

1. Create a vector file
2. Empty your $MAPSET/.tmp/$HOSTNAME entirely.
3. g.remove that vector.
4. See an orphaned empty dir left over.

Maciek

------------------------------------------------------------------------
CIEP?E KRAJE - CIEP?E MORZA. Szukasz atrakcyjnego wypoczynku w przyst?pnej cenie, zapoznaj si? z nasz? ofert?.
ZAPRASZAMY

www.skarpatravel.pl

Markus_Neteler1 · June 17, 2006, 5:27pm

On Sat, Jun 17, 2006 at 07:12:10PM +0200, Maciek Sieczka wrote:

On Sat, 17 Jun 2006 15:04:52 +0200 (CEST)
Markus Neteler via RT <grass-bugs@intevation.de> wrote:

> On Sat, Jun 17, 2006 at 01:32:24PM +0200, Maciek Sieczka via RT wrote:
> > I have built current Grass 6.1 CVS on Ubuntu Breezy with
> > http://mpa.itc.it/markus/tmp/clean_temp2.c apllied.

> > I'd prefer each such empty dir be removed right after 'g.remove
> > vect'is done, but maybe this is not so critical.

> This is an issue of the program creating the vector map in
> question. Which one did you use? A script, r.* or v.*?

It is 'g.remove vect' that leaves empty temp dirs in the .tmp.

In a sense yes: 'g.remove vect' doesn't remove the empty temp dirs
in the LOCATION/MAPSET/.tmp/

But that's not even the idea of g.remove.

As my bug report reads:

>For each vector file it removes, g.remove leaves 1 empty, redundant
>directory in the $MAPSET/.tmp/$HOSTNAME.

To reproduce:

1. Create a vector file

My question was: which command did you use to create a vector file?
That command should not leave files in .tmp/. There we have to
search for the problem IMHO.

2. Empty your $MAPSET/.tmp/$HOSTNAME entirely.
3. g.remove that vector.

No - g.remove removes a vector map and doesn't care for tmp files.

Markus

4. See an orphaned empty dir left over.

Maciek

Maciej_Sieczka3 · June 17, 2006, 5:53pm

On Sat, 17 Jun 2006 19:27:55 +0200
Markus Neteler <neteler@itc.it> wrote:

On Sat, Jun 17, 2006 at 07:12:10PM +0200, Maciek Sieczka wrote:
> On Sat, 17 Jun 2006 15:04:52 +0200 (CEST)
> Markus Neteler via RT <grass-bugs@intevation.de> wrote:
>
> > On Sat, Jun 17, 2006 at 01:32:24PM +0200, Maciek Sieczka via RT
> > wrote:
> > > I have built current Grass 6.1 CVS on Ubuntu Breezy with
> > > http://mpa.itc.it/markus/tmp/clean_temp2.c apllied.
>
> > > I'd prefer each such empty dir be removed right after 'g.remove
> > > vect'is done, but maybe this is not so critical.
>
> > This is an issue of the program creating the vector map in
> > question. Which one did you use? A script, r.* or v.*?
>
> It is 'g.remove vect' that leaves empty temp dirs in the .tmp.

In a sense yes: 'g.remove vect' doesn't remove the empty temp dirs
in the LOCATION/MAPSET/.tmp/

But that's not even the idea of g.remove.

> As my bug report reads:
>
> >For each vector file it removes, g.remove leaves 1 empty, redundant
> >directory in the $MAPSET/.tmp/$HOSTNAME.
>
> To reproduce:
>
> 1. Create a vector file

My question was: which command did you use to create a vector file?
That command should not leave files in .tmp/. There we have to
search for the problem IMHO.

And I'm telling you that command that created the temp dir, is 'g.remove
vect'. And that it is 'g.remove vect' that is supposed to remove a
temporary dir, which it created.

Or did I missunderstood?

> 2. Empty your $MAPSET/.tmp/$HOSTNAME entirely.
> 3. g.remove that vector.

No - g.remove removes a vector map and doesn't care for tmp files.

But it should remove temp dirs which it created. Or not?

> 4. See an orphaned empty dir left over.

Maciek

------------------------------------------------------------------------
CIEP?E KRAJE - CIEP?E MORZA. Szukasz atrakcyjnego wypoczynku w przyst?pnej cenie, zapoznaj si? z nasz? ofert?.
ZAPRASZAMY

www.skarpatravel.pl

Markus_Neteler1 · June 17, 2006, 7:00pm

On Sat, Jun 17, 2006 at 07:53:29PM +0200, Maciek Sieczka wrote:

On Sat, 17 Jun 2006 19:27:55 +0200
Markus Neteler <neteler@itc.it> wrote:

> On Sat, Jun 17, 2006 at 07:12:10PM +0200, Maciek Sieczka wrote:
> > On Sat, 17 Jun 2006 15:04:52 +0200 (CEST)
> > Markus Neteler via RT <grass-bugs@intevation.de> wrote:
> >
> > > On Sat, Jun 17, 2006 at 01:32:24PM +0200, Maciek Sieczka via RT
> > > wrote:
> > > > I have built current Grass 6.1 CVS on Ubuntu Breezy with
> > > > http://mpa.itc.it/markus/tmp/clean_temp2.c apllied.
> >
> > > > I'd prefer each such empty dir be removed right after 'g.remove
> > > > vect'is done, but maybe this is not so critical.
> >
> > > This is an issue of the program creating the vector map in
> > > question. Which one did you use? A script, r.* or v.*?
> >
> > It is 'g.remove vect' that leaves empty temp dirs in the .tmp.
>
> In a sense yes: 'g.remove vect' doesn't remove the empty temp dirs
> in the LOCATION/MAPSET/.tmp/
>
> But that's not even the idea of g.remove.
>
> > As my bug report reads:
> >
> > >For each vector file it removes, g.remove leaves 1 empty, redundant
> > >directory in the $MAPSET/.tmp/$HOSTNAME.
> >
> > To reproduce:
> >
> > 1. Create a vector file
>
> My question was: which command did you use to create a vector file?
> That command should not leave files in .tmp/. There we have to
> search for the problem IMHO.

And I'm telling you that command that created the temp dir, is 'g.remove
vect'. And that it is 'g.remove vect' that is supposed to remove a
temporary dir, which it created.

Or did I missunderstood?

Mhh, I didn't know that g.remove creates temporal directories.
Not sure where that should happen:

grep -i mkdir */*.c
lib/do_copy.c:#define mkdir(name, mode) mkdir(name)
lib/do_copy.c: if(mkdir(dst, mode & 0777))
lib/do_copy.c: if(remove(dst) || mkdir(dst, mode & 0777))

> > 2. Empty your $MAPSET/.tmp/$HOSTNAME entirely.
> > 3. g.remove that vector.

> No - g.remove removes a vector map and doesn't care for tmp files.

But it should remove temp dirs which it created. Or not?

Each command who creates temp files/dirs should remove them.

Markus

Markus_Neteler1 · June 17, 2006, 7:18pm

On Sat, Jun 17, 2006 at 09:00:19PM +0200, Markus Neteler wrote:

On Sat, Jun 17, 2006 at 07:53:29PM +0200, Maciek Sieczka wrote:

...

> > > It is 'g.remove vect' that leaves empty temp dirs in the .tmp.

Now I understand and have the bug reproduced.

In fact g.remove vect=map somewhere creates a
directory in .tmp/ when removing an existing vector map.
It happens in G_find_vector2() which is called in
line 24 of
general/manage/lib/do_remove.c

but I am getting list in the G_find_vector2() and subsequent stuff.

Markus

Glynn_Clements1 · June 18, 2006, 5:33am

Markus Neteler wrote:

On Sat, Jun 17, 2006 at 09:00:19PM +0200, Markus Neteler wrote:
> On Sat, Jun 17, 2006 at 07:53:29PM +0200, Maciek Sieczka wrote:
...
> > > > It is 'g.remove vect' that leaves empty temp dirs in the .tmp.

Now I understand and have the bug reproduced.

In fact g.remove vect=map somewhere creates a
directory in .tmp/ when removing an existing vector map.
It happens in G_find_vector2() which is called in
line 24 of
general/manage/lib/do_remove.c

That's incorrect; it happens in Vect_delete() at line 26 of that file.

It appears that it moves the vector directory to the .tmp directory
before trying to delete it, but doesn't delete it correctly. From
Vect_delete(), in vector/Vlib/map.c:

tmp = G_tempfile();

G_debug (3, "rename '%s' to '%s'", buf, tmp );
ret = rename ( buf, tmp );

    if ( ret == -1 ) {
  G_warning ( "Cannot rename directory '%s' to '%s'", buf, tmp );
  return -1;
    }

G_debug (3, "unlink directory '%s'", tmp );
ret = unlink ( tmp );

unlink() doesn't work on directories, you have to use rmdir() instead.

Or, better still, remove(), which calls either unlink() or rmdir()
depending upon whether the target is a directory; remove() is ANSI,
whereas unlink() and rmdir() are POSIX.

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

Markus_Neteler1 · June 18, 2006, 8:32am

On Sun, Jun 18, 2006 at 06:33:50AM +0100, Glynn Clements wrote:

Markus Neteler wrote:

> On Sat, Jun 17, 2006 at 09:00:19PM +0200, Markus Neteler wrote:
> > On Sat, Jun 17, 2006 at 07:53:29PM +0200, Maciek Sieczka wrote:
> ...
> > > > > It is 'g.remove vect' that leaves empty temp dirs in the .tmp.
>
> Now I understand and have the bug reproduced.
>
> In fact g.remove vect=map somewhere creates a
> directory in .tmp/ when removing an existing vector map.
> It happens in G_find_vector2() which is called in
> line 24 of
> general/manage/lib/do_remove.c

That's incorrect; it happens in Vect_delete() at line 26 of that file.

It appears that it moves the vector directory to the .tmp directory
before trying to delete it, but doesn't delete it correctly. From
Vect_delete(), in vector/Vlib/map.c:

    tmp = G_tempfile();

    G_debug (3, "rename '%s' to '%s'", buf, tmp );
    ret = rename ( buf, tmp );

    if ( ret == -1 ) {
  G_warning ( "Cannot rename directory '%s' to '%s'", buf, tmp );
  return -1;
    }

    G_debug (3, "unlink directory '%s'", tmp );
    ret = unlink ( tmp );

unlink() doesn't work on directories, you have to use rmdir() instead.

Or, better still, remove(), which calls either unlink() or rmdir()
depending upon whether the target is a directory; remove() is ANSI,
whereas unlink() and rmdir() are POSIX.

Thanks for finding it. I have changed unlink() to remove() which
solves the problem.

Markus

H_B · June 19, 2006, 12:52am

Glynn:

remove() is ANSI, whereas unlink() and rmdir() are POSIX.

What's our target anyway - ANSI C or POSIX compliance?

fwiw, the debian help page for remove() says:

#include <stdio.h>
CONFORMING TO
ANSI C, SVID, AT&T, POSIX, X/OPEN, BSD 4.3

while rmdir():

#include <unistd.h>
CONFORMING TO
SVr4, SVID, POSIX, BSD 4.3

Hamish

Glynn_Clements1 · June 19, 2006, 5:52am

Hamish wrote:

> remove() is ANSI, whereas unlink() and rmdir() are POSIX.

What's our target anyway - ANSI C or POSIX compliance?

ANSI where possible (and practical), POSIX where necessary.

Note that ANSI C's I/O functionality is inadequate for non-trivial
programs, e.g. it doesn't say anything about directories (a system
doesn't even have to /have/ directories to fully implemement ANSI C).

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