[GRASS-dev] [GRASS GIS] #1051: wxgui: SEARCH_PATH corruption

#1051: wxgui: SEARCH_PATH corruption
----------------------+-----------------------------------------------------
Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Keywords: | Platform: All
      Cpu: All |
----------------------+-----------------------------------------------------
1. In wxgui wizard, create a location with 2 mapsets inside (mapset1,
mapset2).

2. Enter mapset2.

3. Settings > GRASS working environment > Mapset access: mark mapset1.

The mapset search path in mapset2 is wrong:

{{{
$ g.gisenv get=MAPSET
mapset2

g.mapsets -p
mapset1 PERMANENT mapset2
}}}

If maps of the same names exist in mapset1 and mapset2, GRASS will pick
the one from mapset1 although the current mapset is mapset2. This might
lead to various troubles.

The bug is propably in gui/wxpython/gui_modules/preferences.py.

g.mapsets.tcl (g.mapsets -s) doesn't have this bug.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords:
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by msieczka):

  * priority: major => critical

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords:
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Comment (by glynn):

Replying to [ticket:1051 msieczka]:
> The mapset search path in mapset2 is wrong:
>
{{{
$ g.gisenv get=MAPSET
mapset2

g.mapsets -p
mapset1 PERMANENT mapset2
}}}
>
> If maps of the same names exist in mapset1 and mapset2, GRASS will pick
the one from mapset1 although the current mapset is mapset2. This might
lead to various troubles.

This isn't necessarily a problem. GRASS itself doesn't care whether the
current mapset is first in the search path, or even if it's in the search
path at all.

OTOH, it's probably a good idea for the GUI interface to place additional
mapsets after the current mapset, at least by default.

However: I'd like to take this opportunity to remind programmers that an
unqualified input map name does '''NOT''' necessarily refer to the map of
that name in the '''current''' mapset, but to the first map with that name
found in the mapset search path (which may '''or may not''' have the
current mapset first).

If you intend to refer specifically to a map in the current mapset, you
'''need''' to specify the mapset.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords:
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Comment (by msieczka):

Replying to [comment:2 glynn]:
> Replying to [ticket:1051 msieczka]:

>> If maps of the same names exist in mapset1 and mapset2, GRASS will pick
the one from mapset1
>> although the current mapset is mapset2. This might lead to various
troubles.

> This isn't necessarily a problem. GRASS itself doesn't care whether the
current mapset is
> first in the search path, or even if it's in the search path at all.

According to my tests, the SEARCH_PATH file, if present, overrides the
default "current mapset 1st, PERMANENT 2nd, other mapsets next" behaviour,
up to even excluding any mapset from the search path. Mapsets' order in
the SEARCH_PATH file determines the search order too. Do the steps 1-3 in
#comment:description. Now:

4. Create a vector map: v.random out=point n=1.

5. Start a new session in mapset1.

6. Create a vector map of the same name as in mapset2, i.e. do "v.random
out=point n=1".

6. Back in mapset2 troubles begin:

a) v.info picks the map from mapset1 although we are in mapset2:

{{{
$ v.info -h point
WARNING: 'vector/point' was found in more mapsets (also found in
<mapset2>)
WARNING: Using <point@mapset1>
WARNING: 'vector/point' was found in more mapsets (also found in
<mapset2>)
WARNING: Using <point@mapset1>
COMMAND: v.random output="point" n=1 zmin=0.0 zmax=0.0
GISDBASE: /home/grassdata
LOCATION: test_location MAPSET: mapset1 USER: pok DATE: Sun May 16
11:15:28 2010
}}}

b) same does g.region:

{{{
$ g.region vect=point
WARNING: 'vector/point' was found in more mapsets (also found in
<mapset2>)
WARNING: Using <point@mapset1>
}}}

c) same does v.to.rast:

{{{
$ v.to.rast use=cat in=point out=point_rast
WARNING: 'vector/point' was found in more mapsets (also found in
<mapset2>)
WARNING: Using <point@mapset1>
Loading data...
Reading features...
  100%
Writing raster map...
  100%
Converted areas: 0 of 0
Converted points/lines: 1 of 1
v.to.rast complete
}}}

Any module I tried prefers the "point" vector map from the first mapset in
the SEARCH_PATH (mapset1) rather the one in the current mapset (mapset2).
I understand what Glynn says in his furher comments in #comment:2, fully
second that, but getting back to the original subject: wxGUI's `Settings >
GRASS working environment > Mapset access' needs a fix.

I have attached a patch against SVN develbranch6, r42262. Please anybody
let me know if it's acceptable. In general, it's purpose is to make
'Mapset access' GUI work same as 'g.mapsets addmapset=/removemapset=',
which:

1. Doesn't allow the user to remove the current mapset from the
SEARCH_PATH.

2. The current mapset is always first in the SEARCH_PATH.

3. Treats PERMANENT as any other mapset AFAICT. PERMANENT is the second in
the search path only if the SEARCH_PATH file is not present.

4. Doesn't sort mapset names in the SEARCH_PATH file. Thanks to this, the
search order depends on g.mapsets add/remove commands sequence, thus can
be controlled from GRASS itself.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords:
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Comment (by msieczka):

Replying to [comment:3 msieczka]:
> I understand what Glynn says in his furher comments in #comment:2,

Reading this again I can see I didn't fully understand Glynn's remarks and
to big extent I just repeated after him in my recent comment, but anyway
the proposed patches are valid IMHO.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords:
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Comment (by glynn):

Replying to [comment:3 msieczka]:

> According to my tests, the SEARCH_PATH file, if present, overrides the
default "current mapset 1st, PERMANENT 2nd, other mapsets next" behaviour,
up to even excluding any mapset from the search path. Mapsets' order in
the SEARCH_PATH file determines the search order too.

Yep. If SEARCH_PATH doesn't exist, the default is the current mapset then
PERMANENT. If SEARCH_PATH exists, it entirely specifies the search path;
no other mapsets are searched, including the current and PERMANENT
mapsets.

g.mapsets ensures that the current mapset appears somewhere in the list
(if it isn't, it will add it at the beginning), but libgis doesn't care.

> I have attached a patch against SVN develbranch6, r42262. Please anybody
let me know if it's acceptable. In general, it's purpose is to make
'Mapset access' GUI work same as 'g.mapsets addmapset=/removemapset=',
which:
>
> 1. Doesn't allow the user to remove the current mapset from the
SEARCH_PATH.
>
> 2. The current mapset is always first in the SEARCH_PATH.

g.mapsets allows other mapsets to be placed ahead of the current mapset.
If the current mapset is entirely missing, it will add it to the head of
the search path, but it won't move it if it's already present.

> 3. Treats PERMANENT as any other mapset AFAICT. PERMANENT is the second
in the search path only if the SEARCH_PATH file is not present.

This is implemented in libgis; specifically by G_get_list_of_mapsets() in
lib/gis/mapset_nme.c .

The search path shouldn't really be relevant for the GUI, which should
always be using qualified map names.

However, to follow up on my previous comment:
> If you intend to refer specifically to a map in the current mapset, you
need to specify the mapset.
I've probably forgotten this enough times. In fact, I'd guess that almost
(?) every script which creates temporary maps subsequently refers to them
using an unqualified name.

In retrospect, I'd conclude that having anything ahead of the current
mapset in the search path is asking for trouble. So I'm inclined suggest
that G_get_list_of_mapsets() should be modified to forcibly place the
current mapset at the head of the search path, even if SEARCH_PATH says
otherwise.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by hamish):

  * keywords: => mapsets

Comment:

Replying to [comment:5 glynn]:
> The search path shouldn't really be relevant for the GUI, which
> should always be using qualified map names.

as a complete aside, for a while I've had the wish to clean that up a bit
in the wx GUI as I find it rather cluttering and hard to read.

perhaps:
`roads @ PERMANENT`
or
`roads (PERMANENT)`

both in the map layer-list and in the list of maps selector. or maybe just
omit it in the map-selector pull-down list as it is already given by the
higher level mapset name in the mapset tree (blue text).

... and pack the `(opacity: 100%) [:]` stuff on the right edge of the row.

thoughts?

Hamish

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Comment (by msieczka):

Fixed in develbranch6 r42307 and trunk r42308. I hope to backport it to
releasebranch_6_4 if it's OK.

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

#1051: wxgui: SEARCH_PATH corruption
----------------------+-----------------------------------------------------
Reporter: msieczka | Owner: grass-dev@…
     Type: defect | Status: new
Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Keywords: mapsets | Platform: All
      Cpu: All |
----------------------+-----------------------------------------------------

Comment(by neteler):

Please backport if possible.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: fixed | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by aghisla):

  * status: new => closed
  * resolution: => fixed

Comment:

Backported to releasebranch_6_4, r42681.

+1 for Hamish's suggestion.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: fixed | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------

Comment(by msieczka):

Replying to [comment:9 aghisla]:
> Backported to releasebranch_6_4, r42681.

Thanks, Anne!

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by hamish):

  * status: closed => reopened
  * resolution: fixed =>

Comment:

some comments re making PERMANENT mapset always available to follow.
reopening bug, assigning to me (todo once I clear some time)

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: hamish
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by hamish):

* cc: grass-dev@… (added)
  * owner: grass-dev@… => hamish
  * status: reopened => new

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: hamish
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.1
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by hamish):

  * priority: critical => minor
  * milestone: 6.4.0 => 6.4.1

Comment:

ok, I've unconfused myself on this and added some new help notes in
6.5svn+trunk and moved away my off-topic wish to #1104.

The one last thing to do is to figure out how to grey-out the top
(current) mapset on the list with `Enable(item, False)`. It's already
forcibly ticked but this would be more obvious if greyed as well. Any
tips?

suggest to backport mapset notes on mapset access rules for 6.4.1.

Hamish

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: hamish
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.1
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------

Comment(by hamish):

Replying to [comment:13 hamish]:
> The one last thing to do is to figure out how to grey-out the
> top (current) mapset on the list with `Enable(item, False)`.
> It's already forcibly ticked but this would be more obvious if
> greyed as well.

Any ideas on how to do that?

> suggest to backport mapset notes on mapset access rules for 6.4.1.

done by martinl in r43911.

Hamish

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: hamish
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.1
Component: wxGUI | Version: svn-releasebranch64
Resolution: | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------

Comment(by neteler):

Closing since backport has been done. Reopen if needed.

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

#1051: wxgui: SEARCH_PATH corruption
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: hamish
      Type: defect | Status: closed
  Priority: minor | Milestone: 6.4.1
Component: wxGUI | Version: svn-releasebranch64
Resolution: fixed | Keywords: mapsets
  Platform: All | Cpu: All
-----------------------+----------------------------------------------------
Changes (by neteler):

  * status: new => closed
  * resolution: => fixed

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