search

TIGER-NET / WOIS_processing

WOIS version of the QGIS Processing plugin
2 stars 3 forks source link

GRASS6 paths #11

Closed j08lue closed 8 years ago

j08lue commented 8 years ago

By mail from @radosuav:

I've been discussing with Silvia an issue which has been discovered by Silvia and Lotte during recent QGIS workshop at DHI. It has to do with paths to GRASS 6 being set in Processing options even when only GRASS 7 is installed. Even though this doesn't impact WOIS right now (since that still includes GRASS 6) it would be good to have it fixed for the future. As far as I understand (Silvia please correct me if I'm wrong) there are actually two separate but related issues:

  1. "Msys folder" present in Processing > Options > Providers > GRASS GIS 7 commands msys is not used or installed by GRASS 7 so this field should be removed. This was done in your update_2-14-2 branch (since it includes changes from https://github.com/qgis/QGIS/commit/328000e0a0f86f20873577699aaad3ecbde25c5f) so only merging to master is required.
  2. GRASS 7 path used for GRASS 6 and msys path set even when msys directory does not exist The first part of this was fixed in https://github.com/qgis/QGIS/commit/ab5f06b9fb7083acc45f24fbf7ad3ccbd5da4495, https://github.com/qgis/QGIS/commit/c6117e0d32570d31b5a5a5d3c1a10ef1fee462f8 and https://github.com/qgis/QGIS/commit/3bfe2f14673ef3f79645c1bdce78f2d2e2cdf6b8 but I don't know why this wasn't back-ported to release-2_14 branch (at least "if subfolder.startswith('grass-6'): " should have been). So this needs to be manually included in ESA_Processing.

The second part should be fixed by replacing the last four lines of grassWinShell() in GrassUtils.py by: 

    if folder is None and GrassUtils.grassPath(): 
        folder = os.path.dirname(unicode(QgsApplication.prefixPath())) 
        folder = os.path.join(folder, 'msys') 
    return folder or ''

This change should then be submitted as PR to the QGIS repository. The PR would also be a good place to mention the backporting to the three above commits.

The fixes can only be tested on a brand new installation of QGIS (uninstalling and reinstalling QGIS doesn't work because the paths are saved in the registry and that is not cleaned on uninstallation) and I don't have access to any new computers but maybe you'll have a chance. Otherwise we can assume that it should work (at least looking at the code it looks like it should).

Please let me know if the above is not clear or if you would prefer me to handle this.

j08lue commented 8 years ago

Thanks for all the details, that made it really easy!

I did as you told and cherry-picked the changes from QGIS, added the one you suggested, and submitted a PR to QGIS, too https://github.com/qgis/QGIS/pull/3183. I did not test either, but the change is so small that it should be OK.

The way I suggest we/I work from now on is that I add changes not in ESA_processing but in a DHI-GRAS fork of QGIS (could have been TigerNET, too, but I don’t have admin rights there). I then copy the relevant bits over to ESA_processing. That way, it is much easier to PR to QGIS and merge their updates than I worked on ESA_processing directly.

Since ESA_processing is currently the same as QGIS/python/plugins/processing, I updated the whole tree now (not just the GRASS part). There were some other changes in QGIS since I last synced, so those are included now, too. I did not review them, though.