[GRASS-dev] [GRASS GIS] #843: v.digit broken on new WinGrass release

#843: v.digit broken on new WinGrass release
------------------------------+---------------------------------------------
Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.0
Component: Vector | Version: unspecified
Keywords: wingrass,v.digit | Platform: MSWindows XP
      Cpu: x86-64 |
------------------------------+---------------------------------------------
Vector digitizing windows open properly, both in the tcltk and the new
wxpython menu item.

However after a feature is digitized, as I right-click to finish the
feature, the window closes and the following error message shows up in the
"Output" window or "Command output" tab depending on the GUI

{{{
v.digit map=test2@user1
ERROR: F_open is not supported on Windows
Building topology for vector map <test2>...
Registering primitives...
2 primitives registered
18 vertices registered
Building areas...
0 areas built
0 isles built
Attaching islands...
Attaching centroids...
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/843&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hellik):

Replying to [ticket:843 cnielsen]:
>
> {{{
> v.digit map=test2@user1
> ERROR: F_open is not supported on Windows
> Building topology for vector map <test2>...
> Registering primitives...
> 2 primitives registered
> 18 vertices registered
> Building areas...
> 0 areas built
> 0 isles built
> Attaching islands...
> Attaching centroids...
> }}}
>
>

confirmed at WinVista32 with the error message:
ERROR: F_open is not supported on Windows
[...]

Helmut

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/843#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hamish):

hmmm, worked for me in my test with "v.digit -n" which did not open an
existing map.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hamish):

looking at the F_open code in question (lib/form/open.c), there seem to be
#ifdefs in there around the UNIX socket code already, so why would it be
disabled?

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by cmbarton):

Could it be the fully qualified map name (map@mapset)?

Michael

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Changes (by hamish):

  * priority: normal => critical

Comment:

I can reproduce this now from both the wx and tcltk GUIs.

  digitize [new] -> settings -> table tab -> create table -> ok
(interestingly the dbf.exe empty dosbox locked up when I tried to run it
from gis.m)

then draw a boundary with "No category" set, move vertex to snap it shut,
then try to add a centroid with cats set to "Next not used".
Boom, G_fatal_error( "F_open_*() doesn't do ms windows").

In my earlier test I didn't try to do anything with an attribute table.

I expect that the tcl version of d.what.vect will have the same problem as
it also uses the form library.

Replying to [comment:4 cmbarton]:
> Could it be the fully qualified map name (map@mapset)?

it's this in lib/form/open.c:

{{{
/* Open new form
  *
  * returns: 0 success
  */
#ifdef __MINGW32__
int F_open(char *title, char *html)
{
     G_fatal_error("F_open is not supported on Windows");
     return 1;
}
#else
int F_open(char *title, char *html)
{
     /* parent */
     int c;

     /* common */
     static int pid;

#ifndef HAVE_SOCKET
     static int p1[2], p2[2];
#endif /*HAVE_SOCKET */
     int length;

     /* child */

     G_debug(2, "F_open(): title = %s", title);

     if (first) {
#ifdef HAVE_SOCKET
...
}}}

the question is, is that fatal error really necessary if all the UNIX
socket code is protected?

bumping up the priority level of this bug as currently there is no vector
digitizing functionality available on MS Win, which is not a good
situation.

Workarounds: use the qgis digitizer or cygwin build.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by cmbarton):

This is too bad, especially since we just finally got this easily
available to Windows users.

Michael

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:5 hamish]:

> it's this in lib/form/open.c:

Huh? That matches the error message, but v.digit shouldn't be using
lib/form. It has its own embedded copy of the form library.

Oh. develbranch_6 has LIBES=$(FORMLIB) in vector/v.digit/Makefile,
6.4.0-RC5 doesn't; see r38954. That explains why v.digit works for me
(6.4.0-RC5, native Windows).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hamish):

Replying to [comment:7 glynn]:
> Huh? That matches the error message, but v.digit shouldn't be
> using lib/form. It has its own embedded copy of the form library.
>
> Oh. develbranch_6 has LIBES=$(FORMLIB) in vector/v.digit/Makefile,
> 6.4.0-RC5 doesn't; see r38954. That explains why v.digit works for
> me (6.4.0-RC5, native Windows).

or more precisely it happened at r39141, when that Makefile change was
backported to the 6.4 branch. so the latest 6.4.0svn40049 native wingrass
build I'm testing uses $(FORMLIB), while the 6.4.0rc5 release did not use
that.

I am not familiar enough with this part of the code to provide much
comment beyond that, only that I am glad that this is not as deep as I
feared it could be.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by marisn):

Replying to [comment:7 glynn]:
> Huh? That matches the error message, but v.digit shouldn't be using
lib/form. It has its '''own embedded copy''' of the form library.
>
> Oh. develbranch_6 has LIBES=$(FORMLIB) in vector/v.digit/Makefile,
6.4.0-RC5 doesn't; see r38954. That explains why v.digit works for me
(6.4.0-RC5, native Windows).
>
"Own embedded copy" is a good reason for trouble. Before r38954 form lib
was out of sync. If lib/form was broken before r38954 then something else
was broken too (nviz? d.what?). Unfortunately I have no idea about
sockets/pipes on windows. According to lib/form history, Glynn should have
an idea how sockets/pipes/etc. works on Windows.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:9 marisn]:

> > Huh? That matches the error message, but v.digit shouldn't be using
lib/form. It has its '''own embedded copy''' of the form library.

> "Own embedded copy" is a good reason for trouble. Before r38954 form lib
was out of sync. If lib/form was broken before r38954 then something else
was broken too (nviz? d.what?).

By "embedded", I mean "in-process". The form library in lib/form consists
of a custom "wish" which has a command for executing SQL, and a library
which spawns this as a slave process. The version in v.digit doesn't use a
separate child process; the form code has been merged into the main
v.digit program (which is also a custom wish).

The separate form library is still used by nviz and d.what.vect.

> Unfortunately I have no idea about sockets/pipes on windows. According
to lib/form history, Glynn should have an idea how sockets/pipes/etc.
works on Windows.

I don't have much of an idea either. I do know that Windows doesn't
understand Unix-domain sockets (so no socketpair()). It does have a
_pipe() function which is roughly equivalent to the Unix version, but the
lack of fork() means that creating a slave process is nothing like on
Unix.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by marisn):

As lib/forms F_open is used only by v.digit and d.what.vect, it's not
worth to fix forms lib. Reverted to old v.digit F_open in r40128. Please
test on Windows and backport to releasebranch if it helps.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:11 marisn]:
> As lib/forms F_open is used only by v.digit and d.what.vect,

It's used by d.what.vect and NVIZ.

v.digit doesn't use lib/form (at least, it's not '''supposed''' to use
lib/form, as it has its own version of the code).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by marisn):

Replying to [comment:12 glynn]:
> Replying to [comment:11 marisn]:
> > As lib/forms F_open is used only by v.digit and d.what.vect,
>
> It's used by d.what.vect and NVIZ.
>
d.what.vect AFAIK doesn't work on Windows due to X dependency. Not a
issue.
NVIZ does '''NOT''' use F_open(). lib/forms are used by more than v.digit
and d.what.vect modules, but only those two call F_open(). Others use
plain F_generate, which works just fine.
> v.digit doesn't use lib/form (at least, it's not '''supposed''' to use
lib/form, as it has its own version of the code).
Only part of the form lib that '''could''' be v.digit own, is F_open() and
also only because it's not worth effort to make lib/form F_open() Windows-
ready. F_generate() is generic enough to NOT keep many copies around.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:13 marisn]:

> > > As lib/forms F_open is used only by v.digit and d.what.vect,
> >
> > It's used by d.what.vect and NVIZ.
> >
> d.what.vect AFAIK doesn't work on Windows due to X dependency. Not a
issue.
> NVIZ does '''NOT''' use F_open(). lib/forms are used by more than
v.digit and d.what.vect modules, but only those two call F_open(). Others
use plain F_generate, which works just fine.

v.digit doesn't use lib/form's F_{open,clear,close} functions; it has its
own functions with those names, which don't spawn a separate process.
Except, linking against lib/form for F_generate() seems to cause
lib/form's F_open (etc) to override the local version.

It would be possible to modify v.digit to use different names (this may
also need to be done for reset_values(), set_value() and submit() in
form.c), but I'm still concerned about the potential side-effects of
linking against lib/form. Personally, I consider cloning F_generate() to
be the lesser evil.

If d.what.vect is the only module which uses lib/form's F_open(), I'd
consider removing that code from lib/form and putting it in d.what.vect
instead. That would simplify lib/form and eliminate the Windows issues.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hamish):

Glynn:
> If d.what.vect is the only module which uses lib/form's F_open(), I'd
> consider removing that code from lib/form and putting it in d.what.vect
> instead. That would simplify lib/form and eliminate the Windows issues.

my 1/2c:
at some point we have to stop refactoring working code in 6.x. Any new
perturbation introduces the potential for new problems and this is
supposed to be the stable branch. The old lib/form/open.c + d.what.vect is
working fine on UNIX and is irrelevant elsewhere, so my vote is to just
leave well enough alone, put things back to the way they were as much as
possible, sync whatever bugfixes need syncing between the clones, and move
on.

would a more portable version of lib/form do some good in some other area,
or is this it?

thanks,
Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by marisn):

Replying to [comment:14 glynn]:
>
> It would be possible to modify v.digit to use different names (this may
also need to be done for reset_values(), set_value() and submit() in
form.c), but I'm still concerned about the potential side-effects of
linking against lib/form. Personally, I consider cloning F_generate() to
be the lesser evil.
>
> If d.what.vect is the only module which uses lib/form's F_open(), I'd
consider removing that code from lib/form and putting it in d.what.vect
instead. That would simplify lib/form and eliminate the Windows issues.
As removing something from lib/form will break backwards compatability. I
attached patch that renames v.digit F_ function copies and removes futurer
duplicate code. Worksforme, needs review.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/843#comment:16&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:16 marisn]:

> As removing something from lib/form will break backwards compatibility.

Why does that matter? I'm fairly sure that we don't want anything else
using lib/form, particularly F_open().

> I attached patch that renames v.digit F_ function copies and removes
futurer duplicate code. Worksforme, needs review.

I'm still don't see why we can't just revert the v.digit changes and sync
its copy of generate.c to the lib/form copy.

I don't particularly like cloning code, but in this case the cure is worse
than the disease.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/843#comment:17&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Changes (by hamish):

  * priority: critical => blocker

Comment:

this is a blocker as 2 of 2 vector digitizers are now broken in current
6.4 wingrass. Fortunately the solution for the tcl one seems well in hand
& forthcoming. Will backport revert then sync solution to the 6.4 branch
as soon as it is tested & confirmed in 6.5.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/843#comment:18&gt;
GRASS GIS <http://grass.osgeo.org>

#843: v.digit broken on new WinGrass release
---------------------------+------------------------------------------------
  Reporter: cnielsen | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: Vector | Version: unspecified
Resolution: | Keywords: wingrass,v.digit
  Platform: MSWindows XP | Cpu: x86-64
---------------------------+------------------------------------------------
Comment (by hellik):

Replying to [comment:18 hamish]:
> this is a blocker as 2 of 2 vector digitizers are now broken in current
6.4 wingrass. Fortunately the solution for the tcl one seems well in hand
& forthcoming. Will backport revert then sync solution to the 6.4 branch
as soon as it is tested & confirmed in 6.5.

from dev-ml:

{{{
Hamish wrote:
> > * 843 v.digit broken on new WinGrass release
> > -> contains a suggestion
>
> Maris and Glynn are working to resolve this. Seems plausible that it
> will be backported and ready for final testing by next week.

FWIW, I've changed 6.5 as I consider appropriate, i.e. removed
$(FORMLIB) and sync'd generate.c to lib/form's version.
}}}

tested on a fresh build of grass65 on WinVista32:
   => v.digit is working

Helmut

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/843#comment:19&gt;
GRASS GIS <http://grass.osgeo.org>