OSGeo
win: patch from OSGeo4W applied
This PR applies https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/patch (author: @jef-n)
A dependency yet missing?
From the CI log:
...
configure: error: *** couldn't find libpng-config
Error: Process completed with exit code 1.
A dependency yet missing?
From the CI log:
... configure: error: *** couldn't find libpng-config Error: Process completed with exit code 1.
Not a dependency, but https://github.com/OSGeo/grass/blob/main/mswindows/osgeo4w/libpng-config (see #2679).
Note that build_osgeo4w.sh and osgeo4w.yml are GRASS things, that are not used in osgeo4w - they were just aligned with package.sh, which actually is.
After reintroducing mingw-w64-x86_64-libpng the CI pass successfully.
@jef-n Please could you elaborate why you are removing this dependency in the workflow (https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/patch#L9)? On the other side it's installed in package.sh (https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/package.sh#L101)
After reintroducing mingw-w64-x86_64-libpng the CI pass successfully.
@jef-n Please could you elaborate why you are removing this dependency in the workflow (https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/patch#L9)? On the other side it's installed in
package.sh(https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osgeo4w/package.sh#L101)
um, osgeo4w has libpng. not sure why it was only removed from build_osgeo4w.sh. As said I don't use that. Maybe I wanted to replace msys libpng with's OSGeo4W's png, replaced it in both and failed to re-add it in build_osgeo4w.sh, when I found that it was not working. But I don't recall… In OSGeo4W it's just a patch to an unused file ;)
Any idea why osgeo4w CI is failing on tests while compilation is successful? It seems to be unrelated to this PR...
There were a lot more failed files, like 58% of files passed, instead of 81%
Any idea why osgeo4w CI is failing on tests while compilation is successful? It seems to be unrelated to this PR...
Solved by b435503
There seems to be a bad rebase somewhere, there are some unrelated commits in the PR. (It doesn't really matter if we adjust the squashed commit message)
Was there a logic in the new order of the OSGeo4W dependencies, was it in the patch that it was in that order? It's not alphabetical neither, I don't find the logic. I'll try to compare manually which ones are changed, as word-based diff highlighting doesn't show the changes clearly.
Was there a logic in the new order of the OSGeo4W dependencies, was it in the patch that it was in that order? It's not alphabetical neither, I don't find the logic. I'll try to compare manually which ones are changed, as word-based diff highlighting doesn't show the changes clearly.
The current changes are:
cairo-devel
-fftw
freetype-devel
gdal-devel
-gdal-ecw
-gdal-mrsid
geos-devel
+libjpeg-turbo-devel
liblas-devel
libpng-devel
libpq-devel
libtiff-devel
-libxdr
netcdf-devel
-pdal-devel
-pdcurses
proj-devel
-python3-matplotlib
+python3-core
python3-numpy
python3-ply
python3-pywin32
+python3-six
python3-wxpython
-regex-devel
+sqlite3-devel
zstd-devel
Thanks @nilason! I don't have a strong opinion on the changed dependencies, apart from matplotlib that I thought it was included as a dependency in other builds, but I don't know if it is really used in our modules or just in addons...
I fixed the conflicts, and took the new deps and sorted them into the action (it would have worked without the multiline list too, but now the diff is clear)
There was one conflict in the configure flags that was recently changed to just have --with-lapack, whilst here it had an include path. Please make sure you agree with my choice
@landam Let us continue the OpenMP issue from ML here.
Extracting from the logs from CI and https://wingrass.fsv.cvut.cz reveals the following:
Win CI runner (main)
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... no
checking for GOMP_parallel_start in -lgomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp
x86_64-w64-mingw32-gcc -I/c/OSGeo4W/include -pipe -I/c/OSGeo4W/include -I/d/a/grass/grass/dist.x86_64-w64-mingw32/include -I/d/a/grass/grass/dist.x86_64-w64-mingw32/include -D_FILE_OFFSET_BITS=64 -I/c/OSGeo4W/include -I/c/OSGeo4W/include -fopenmp -DPACKAGE=\""grasslibs"\" -I/c/OSGeo4W/include -I/d/a/grass/grass/dist.x86_64-w64-mingw32/include -I/d/a/grass/grass/dist.x86_64-w64-mingw32/include -DRELDIR=\"lib/rst/interp_float\" -o OBJ.x86_64-w64-mingw32/segmen2d_parallel.o -c segmen2d_parallel.c
x86_64-w64-mingw32-gcc -shared -o /d/a/grass/grass/dist.x86_64-w64-mingw32/lib/libgrass_interpfl.8.5.dll -L/d/a/grass/grass/dist.x86_64-w64-mingw32/lib -L/d/a/grass/grass/dist.x86_64-w64-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/OSGeo4W/lib -L/c/OSGeo4W/bin OBJ.x86_64-w64-mingw32/distance.o OBJ.x86_64-w64-mingw32/func2d.o OBJ.x86_64-w64-mingw32/init2d.o OBJ.x86_64-w64-mingw32/input2d.o OBJ.x86_64-w64-mingw32/interp2d.o OBJ.x86_64-w64-mingw32/matrix.o OBJ.x86_64-w64-mingw32/minmax.o OBJ.x86_64-w64-mingw32/output2d.o OBJ.x86_64-w64-mingw32/point2d.o OBJ.x86_64-w64-mingw32/point2d_parallel.o OBJ.x86_64-w64-mingw32/resout2d.o OBJ.x86_64-w64-mingw32/ressegm2d.o OBJ.x86_64-w64-mingw32/secpar2d.o OBJ.x86_64-w64-mingw32/segmen2d.o OBJ.x86_64-w64-mingw32/segmen2d_parallel.o OBJ.x86_64-w64-mingw32/vinput2d.o OBJ.x86_64-w64-mingw32/write2d.o -lgrass_gis.8.5 -lintl -lgrass_raster.8.5 -lgrass_vector.8.5 -lgrass_gmath.8.5 -lgrass_dbmiclient.8.5 -lgrass_dbmibase.8.5 -lgrass_bitmap.8.5 -lgrass_qtree.8.5 -lgrass_interpdata.8.5 -lgomp
package.sh
checking whether to use OpenMP... "yes"
checking for location of OpenMP includes...
checking for location of OpenMP library...
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp
x86_64-w64-mingw32-gcc -I/c/osgeo4w/include -g -O2 -I/c/osgeo4w/include -I/usr/src/grass85/dist.x86_64-w64-mingw32/include -I/usr/src/grass85/dist.x86_64-w64-mingw32/include -D_FILE_OFFSET_BITS=64 -I/c/osgeo4w/include -I/c/osgeo4w/include -fopenmp -DPACKAGE=\""grasslibs"\" -I/c/osgeo4w/include -I/usr/src/grass85/dist.x86_64-w64-mingw32/include -I/usr/src/grass85/dist.x86_64-w64-mingw32/include -DRELDIR=\"lib/rst/interp_float\" -o OBJ.x86_64-w64-mingw32/segmen2d_parallel.o -c segmen2d_parallel.c
x86_64-w64-mingw32-gcc -shared -o /usr/src/grass85/dist.x86_64-w64-mingw32/lib/libgrass_interpfl.8.5.dll -L/usr/src/grass85/dist.x86_64-w64-mingw32/lib -L/usr/src/grass85/dist.x86_64-w64-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/osgeo4w/lib OBJ.x86_64-w64-mingw32/distance.o OBJ.x86_64-w64-mingw32/func2d.o OBJ.x86_64-w64-mingw32/init2d.o OBJ.x86_64-w64-mingw32/input2d.o OBJ.x86_64-w64-mingw32/interp2d.o OBJ.x86_64-w64-mingw32/matrix.o OBJ.x86_64-w64-mingw32/minmax.o OBJ.x86_64-w64-mingw32/output2d.o OBJ.x86_64-w64-mingw32/point2d.o OBJ.x86_64-w64-mingw32/point2d_parallel.o OBJ.x86_64-w64-mingw32/resout2d.o OBJ.x86_64-w64-mingw32/ressegm2d.o OBJ.x86_64-w64-mingw32/secpar2d.o OBJ.x86_64-w64-mingw32/segmen2d.o OBJ.x86_64-w64-mingw32/segmen2d_parallel.o OBJ.x86_64-w64-mingw32/vinput2d.o OBJ.x86_64-w64-mingw32/write2d.o -lgrass_gis.8.5 -lintl -lgrass_raster.8.5 -lgrass_vector.8.5 -lgrass_gmath.8.5 -lgrass_dbmiclient.8.5 -lgrass_dbmibase.8.5 -lgrass_bitmap.8.5 -lgrass_qtree.8.5 -lgrass_interpdata.8.5 -lomp
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: OBJ.x86_64-w64-mingw32/segmen2d_parallel.o: in function `IL_interp_segments_2d_parallel._omp_fn.0':
C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:108:(.text+0x1b2): undefined reference to `GOMP_loop_nonmonotonic_dynamic_start'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:144:(.text+0x5a5): undefined reference to `GOMP_loop_nonmonotonic_dynamic_next'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:144:(.text+0x5b2): undefined reference to `GOMP_loop_end'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:421:(.text+0xd53): undefined reference to `GOMP_critical_start'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:421:(.text+0xe57): undefined reference to `GOMP_critical_end'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:405:(.text+0xf83): undefined reference to `GOMP_critical_start'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:405:(.text+0xfb5): undefined reference to `GOMP_critical_end'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: OBJ.x86_64-w64-mingw32/segmen2d_parallel.o: in function `IL_interp_segments_2d_parallel':
C:/msys64/usr/src/grass85/lib/rst/interp_float/segmen2d_parallel.c:108:(.text+0x1404): undefined reference to `GOMP_parallel'
The "package" configure finds a working libomp.dll (I suppose), ~~but try to build and link with libgomp.dll~~ but compiles against libgomp.dll and links with libomp.dll.
This might be the result of ddl copying:
https://github.com/OSGeo/grass/blob/3f61cd7625f728ff0795023e97529e2ec896c5cc/mswindows/osgeo4w/package.sh#L127
The following difference is striking:
https://github.com/OSGeo/grass/blob/3f61cd7625f728ff0795023e97529e2ec896c5cc/mswindows/osgeo4w/build_osgeo4w.sh#L26
https://github.com/OSGeo/grass/blob/3f61cd7625f728ff0795023e97529e2ec896c5cc/mswindows/osgeo4w/package.sh#L157
I don't see any changes that could influence failure in 8.4.1dev (compared to the 8.4.0 release):
https://github.com/OSGeo/grass/compare/8.4.0...releasebranch_8_4
osgeo4w installs libomp.dll: https://github.com/jef-n/OSGeo4W/blob/f8ada2fe6637b1abde0a361fa8b5b2089a82e33d/src/grass/osgeo4w/package.sh#L104
which is probably why configure hooks up on it.
Perhaps
-/mingw64/bin/libgomp-1.dll
+/mingw64/bin/libomp.dll
in https://github.com/OSGeo/grass/blob/3f61cd7625f728ff0795023e97529e2ec896c5cc/mswindows/osgeo4w/package.sh#L127 could be the solution?
At least OpenMP issues need to be addressed for 8.4.1, I’ll add backport label for this PR as not to miss this.
The conflicts in the PR I might have time this weekend to address them, it's not too long, it's the sorted lines that need to be chosen with respect to the changes this PR made
I solved the conflicts by sorting the two conflicting files of the PR in same way as https://github.com/OSGeo/grass/pull/4563, then comparing the two sides of the merge making sure changes present in the PR (on GitHub) were still present even if moved. It was all ok, nothing is lost.
osgeo4w installs libomp.dll: https://github.com/jef-n/OSGeo4W/blob/f8ada2fe6637b1abde0a361fa8b5b2089a82e33d/src/grass/osgeo4w/package.sh#L104
which is probably why configure hooks up on it.
Perhaps
-/mingw64/bin/libgomp-1.dll +/mingw64/bin/libomp.dllin https://github.com/OSGeo/grass/blob/3f61cd7625f728ff0795023e97529e2ec896c5cc/mswindows/osgeo4w/package.sh#L127 could be the solution?
I've reread on a fresh mind. Isn't libomp the llvm OpenMP runtime library (https://packages.msys2.org/packages/mingw-w64-x86_64-openmp, files section at the very end of https://packages.msys2.org/packages/mingw-w64-x86_64-llvm-openmp)
And libgomp the gcc OpenMP library (https://packages.msys2.org/packages/mingw-w64-x86_64-omp, files section at the very end of https://packages.msys2.org/packages/mingw-w64-x86_64-gcc-libs)
(And libiomp the intel OpenMP one, not in msys2).
So, following the often linked advice that multiple OpenMP runtimes shouldn't be mixed (https://cpufun.substack.com/p/is-mixing-openmp-runtimes-safe), and that we are using the gcc toolchain, I think we should be removing all references to libomp.dll, and go all-in with libgomp-1.dll, and work until fixing all the issues.
I've implemented the suggested changes here: https://github.com/echoix/grass/pull/285
But since our CI doesn't use the package.sh script, I've reimplemented the patches that I've been waiting and wanting to do after this PR is merged here: https://github.com/echoix/grass/pull/286
It showed that we were missing some dlls for lapack and libblas. I didn't find (only on my phone) what branch I had solved the issue that the msys2 couldn't run the Linux print_versions.sh, where some extra paths need to be added so an msys2 shell finds inside the OSGeo4W install.
At least, in these CI runs, it uploads the compiled package that can be downloaded and installed with the OSGeo4W installer (pointing the download dir to a dir with the package from the artifact) for local testing. We could test if the two or three bugs with OpenMP linked are solved by that
OpenMP related fix (hopefully) is added with https://github.com/jef-n/OSGeo4W/commit/06d4d83912dffccd638417338f43f03f4f84c15b and https://github.com/jef-n/OSGeo4W/commit/c678c71d080e5c33e8cb64b12386be2ca4b6c54a.
OpenBLAS issue for main has been addressed with https://github.com/OSGeo/grass/commit/9984205dc8eaab7f014a5117374565a479297d9d, which doesn't need to backport.
OpenMP issues have been addressed (as mentioned above), but doesn't need changes in GRASS source (and thus no need to backport).
I removed backport label for this PR, as the build issues have likely been addressed.
@landam Could you please check if the win build server is working, as there has been no build log since end of ~~August~~ September.
In the last commit I'm keeping (for now) the --with-libpng=${SRC}/mswindows/osgeo4w/libpng-config \ line that was removed. I think it might be a merging/rebase conflict that lost it, as I would have expected that line to be changed to use ${SRC} instead of ${PWD} like the other places in the file that got updated for that. If it is expected, then we should also remove it from our parallel script.
@jef-n The daily grass-dev builds seems to have stopped after this got merged, no new builds were made since december 15, 2024. I wanted to let you know