grass
grass copied to clipboard
OSGeo
Address some MS Windows test failures
The build still fails. It probably needs to be adjusted from here: https://github.com/OSGeo/grass/pull/136
We need to adapt our building scripts to updated dependencies in OSGeo4W v2
@hellik build for the tests configures now. But compilation of esp. display modules throws errors. FFTW seems to be missing from OSGeo4W v2. Did I overlook something, @jef-n, if not could it be added?
Should test-builds and build for packaging (e.g. with regards to configure options and underlying libraries) be unified?
https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/package.sh builds with fftw from mingw.
Should test-builds and build for packaging (e.g. with regards to configure options and underlying libraries) be unified?
yes, as both are based upon the OSGeo4W environment. @landam
https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/package.sh builds with fftw from mingw.
Thanks @jef-n very much appreciated!
I see you have some patches in https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/patch
Those should probably be incorporated upstream? The font issue in the cairo driver for example breaks the workflow-build also here... @nilason, would you mind having a look at lib/cairodriver/text.c?
Other changes are in:
- lib/gis/gisinit.c
- mswindows/env.bat
- mswindows/osgeo4w/env.bat.tmpl
- mswindows/osgeo4w/package.sh
- mswindows/osgeo4w/postinstall.bat
- gui/wxpython/wxgui.py
With the changes included upstream, would the best approach be to separate packaging from build, so the build part can be directly used in GH workflows? So we do not have to maintain .github\workflows\build_osgeo4w.sh in addition to mswindows/osgeo4w/package.sh
Those should probably be incorporated upstream?
Yes. I just didn't want to start a fight about gisinit.c again - and maybe env.bat. I was already afraid that https://lists.osgeo.org/pipermail/osgeo4w-dev/2021-May/004251.html would already start it. ;)
With the changes included upstream, would the best approach be to separate packaging from build, so the build part can be directly used in GH workflows? So we do not have to maintain .github\workflows\build_osgeo4w.sh in addition to mswindows/osgeo4w/package.sh
I originally wanted to have workflows in the OSGeo4W repo - and also have updates to reverse dependencies trigger builds. But at least qgis (let alone qt - I didn't think about trying) needs too much disk space to build - and moreover takes ages to eventually run out of space. So I postponed that for "later". That's also why I didn't migrate all nightlies yet.
The font issue in the cairo driver for example breaks the workflow-build also here... @nilason, would you mind having a look at lib/cairodriver/text.c?
Attempt to address this at #1697.
Other changes are in:
- lib/gis/gisinit.c
- mswindows/env.bat
- mswindows/osgeo4w/env.bat.tmpl
- mswindows/osgeo4w/package.sh
- mswindows/osgeo4w/postinstall.bat
- gui/wxpython/wxgui.py
Re: wxpython/wxgui.py fix, this is reported with #1423, please see https://github.com/OSGeo/grass/issues/1423#issuecomment-803233845 for better location for this.
Now several of the patches above have been integrated (not all yet) and GRASS builds and starts tests with OSGeo4W v2. However, for some reason sh is not found when running tests. OSGeo4W/bin and msys/bin seem to be on PATH... Anyone with an idea how to get sh executable in OSGeo4W/MSYS, @jef-n maybe?
Or can I safely replace sh with bash?
osgeo4w v2 doesn't have sh. msys was a dependency of grass6.
Ok. Thanks so much for the swift reply @jef-n !
GRASS has some tests that are shell scripts. Is there any way to make sh available in the workflow? The workflow uses msys for the build anyway?
Do I miss a link or setting an environment variable correctly?
This: https://github.com/OSGeo/grass/pull/1683/checks?check_run_id=3152466539#step:8:49 is what is on PATH right now...
osgeo4w v2 doesn't have sh. msys was a dependency of grass6.
Ok. Thanks so much for the swift reply @jef-n ! GRASS has some tests that are shell scripts. Is there any way to make
shavailable in the workflow? The workflow uses msys for the build anyway? Do I miss a link or setting an environment variable correctly? This: https://github.com/OSGeo/grass/pull/1683/checks?check_run_id=3152466539#step:8:49 is what is on PATH right now...
Not sure how the test environment looks like, but C:\OSGeo4W\apps\msys\bin does not exists, I guess it should be c:\msys64\usr\bin (judging from .github/workflows/test_thorough.bat)
Not sure how to handle the Travis and CodeQL issues, but apart from some clean up this PR should be ready for review. It would be great if at least one with CI and one with MS Windows knowledge could check the PR.
Successrate of tests is up to 85% and the remaining 15% are mostly due to platform issues in other areas of the code (e.g. ctypes). But I guess #1788 will increase the success rate even more.
If that is prefered, I can separate out fixes in modules (like t.rast.export) made because of CI issues....
Not sure how to handle the Travis and CodeQL issues,
The Travis issue is everywhere. Probably general update is needed there.
The CodeQL issue appeared and disappeared again. I don't know why we are seeing it here again. Likely easy to address because it may be a false positive (which is why I though it disappeared).
If that is prefered, I can separate out fixes in modules (like t.rast.export) made because of CI issues....
That would be great. It is 28 files now. Ideally not only modules, but also the grass.gunittest changes should be in separate PRs. I see some prints/echos here and there in the code, but it will be easier to spot these when the changes are shorter (although the "checked viewed file" feature helps).
If that is prefered, I can separate out fixes in modules (like t.rast.export) made because of CI issues....
That would be great. It is 28 files now. Ideally not only modules, but also the grass.gunittest changes should be in separate PRs. I see some prints/echos here and there in the code, but it will be easier to spot these when the changes are shorter (although the "checked viewed file" feature helps).
OK, will split the PR then...
Since the OSGeo4W Trac wiki download link points to v2, does it makes sense to have 8.0 there? Otherwise it won't get to the OSGeo4W users, no? It would be good to clarify this since it has milestone 8.2. Are we including this in 8.0? Do we want? Do we have to?
Since the OSGeo4W Trac wiki download link points to v2, does it makes sense to have 8.0 there? Otherwise it won't get to the OSGeo4W users, no? It would be good to clarify this since it has milestone 8.2. Are we including this in 8.0? Do we want? Do we have to?
I'd like grass 8.0 to be in OSGeo4W please. Is that possible?
I'd like grass 8.0 to be in OSGeo4W please. Is that possible?
I am confident that @jef-n will get GRASS GIS 8 into OSGeo4W when the time comes. If there is anything I can contribute I am willing to do that, as mentioned by e-mail earlier.
This PR is however more for the MS Windows CI, which I hope can use the same build-setup as OSGeo4W in the future and that test become similarly comprehensive and successful as on Linux (some tests are currently platformdependent)...
I am about to separate out the different components in this PR so we can go ahead...
Thanks, @ninsbl. I'm still don't understand 100%.
This PR is however more for the MS Windows CI...
...more, but not only? I see some changes in mswindows dir.
...will get GRASS GIS 8 into OSGeo4W when the time comes.
That sounds great. I just want to be clear that nothing from here needs to go to 8.0, i.e., we create the branch for 8.0 and we don't backport anything from here and that will be fine (that's what milestone 8.2 suggests).
@ninsbl is this PR still relevant given that #2074 has been merged?
There are some changes here (mostly the last 4 commits) that have not been merged / addressed. These fix MS Windows specific CI issues and I would like to move them to a separate PR. Kept this ope just to not forget about it...
Bumping up milestone as 8.0.1 is due in two days, while this has not been part of RC1 and there has not been activity for some time.
The 8.0 releasebranch uses actually already V2: https://github.com/OSGeo/grass/blob/releasebranch_8_0/.github/workflows/osgeo4w.yml
What remains from this PR is to address some specific test failures on MS Windows...
Renaming this PR and bumping up milestone....
Are there any changes here which are still relevant? If yes, new separate PRs for each group of changes would be most appropriate at this point.