Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

win: patch from OSGeo4W applied #4121

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

win: patch from OSGeo4W applied #4121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
cc4e522
patch from https://github.com/jef-n/OSGeo4W/blob/master/src/grass/osg…
landam Jul 31, 2024
d904476
Merge branch 'main' into osgeo4w_patch
landam Jul 31, 2024
42ef646
v.out.ogr: Add basic unit tests (#3848)
ldesousa Jul 31, 2024
3b2313f
attempt to fix osgeo4w CI
landam Jul 31, 2024
fd270e0
Revert "v.out.ogr: Add basic unit tests (#3848)"
landam Jul 31, 2024
907a884
Merge branch 'main' into osgeo4w_patch
landam Jul 31, 2024
daba1e6
re-introduce mingw-w64-x86_64-libpng
landam Aug 4, 2024
feb3a5c
Merge branch 'main' into osgeo4w_patch
landam Aug 4, 2024
56dc8ef
sync with osgeo4w patch
landam Aug 6, 2024
b435503
add numpy into deps in order to fix some tests
landam Aug 22, 2024
a78f378
Merge branch 'main' into osgeo4w_patch
landam Aug 22, 2024
3d1af45
Merge remote-tracking branch 'origin/main' into osgeo4w_patch
echoix Sep 21, 2024
5c4bd6a
CI(OSGeo4W): Sort OSGeo4W packages in a multiline list
echoix Sep 21, 2024
cb11b60
Merge branch 'main' into osgeo4w_patch
echoix Sep 23, 2024
f9f9d52
Merge branch 'main' into osgeo4w_patch
landam Oct 1, 2024
ba31883
Add back matplotlib package
echoix Oct 6, 2024
6e7532a
style: Sort packages and configure flags in mswindows build scripts
echoix Nov 10, 2024
b4a86c4
Merge branch 'main' into osgeo4w_patch
echoix Nov 10, 2024
fb0244e
Merge branch 'main' into osgeo4w_patch
echoix Nov 23, 2024
2f9c87a
packaging: Removed changes from patch that was removed upstream
echoix Nov 23, 2024
b6bd490
packaging: Apply upstream patch update
echoix Nov 23, 2024
7ffcbad
packaging: Keep --with-libpng with libpng-config from repo
echoix Nov 23, 2024
43bfc39
Merge branch 'main' into osgeo4w_patch
echoix Nov 23, 2024
2031013
Merge branch 'main' into osgeo4w_patch
echoix Nov 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions .github/workflows/osgeo4w.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
mingw-w64-x86_64-fftw mingw-w64-x86_64-openblas mingw-w64-x86_64-pkgconf
mingw-w64-x86_64-gcc mingw-w64-x86_64-ccache mingw-w64-x86_64-zlib mingw-w64-x86_64-libiconv
mingw-w64-x86_64-bzip2 mingw-w64-x86_64-gettext mingw-w64-x86_64-libsystre
mingw-w64-x86_64-libtre-git mingw-w64-x86_64-libwinpthread-git mingw-w64-x86_64-libpng
mingw-w64-x86_64-libtre-git mingw-w64-x86_64-libwinpthread-git
mingw-w64-x86_64-pcre

- name: Setup OSGeo4W environment
Expand All @@ -52,20 +52,15 @@ jobs:
package-dir: "D:/OSGeo4W_pkg"
packages: |
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-core
python3-jupyter
Expand All @@ -75,8 +70,9 @@ jobs:
python3-ply
python3-pytest
python3-pywin32
python3-six
python3-wxpython
regex-devel
sqlite3-devel
zstd-devel

- name: Set number of cores for compilation
Expand All @@ -87,8 +83,6 @@ jobs:
- name: Compile GRASS GIS
shell: msys2 {0}
run: |
export CFLAGS="${CFLAGS} -pipe"
export CXXFLAGS="${CXXFLAGS} -pipe"
.github/workflows/build_osgeo4w.sh

- name: Print installed versions
Expand Down
5 changes: 4 additions & 1 deletion mswindows/osgeo4w/build_osgeo4w.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export C_INCLUDE_PATH=".:${OSGEO4W_ROOT_MSYS}/include:${SRC}/dist.${ARCH}/includ
export PYTHONHOME=${OSGEO4W_ROOT_MSYS}/apps/Python312
export ARCH=x86_64-w64-mingw32

CFLAGS="$CFLAGS -pipe" \
CXXFLAGS="$CXXFLAGS -pipe" \
./configure \
--bindir=${OSGEO4W_ROOT_MSYS}/bin \
--enable-largefile \
Expand All @@ -34,7 +36,7 @@ export ARCH=x86_64-w64-mingw32
--with-cairo \
--with-cairo-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-cairo-ldflags="-L${SRC}/mswindows/osgeo4w/lib -lcairo" \
--with-cairo-libs=$OSGEO4W_ROOT_MSYS/lib \
--with-cairo-libs=${OSGEO4W_ROOT_MSYS}/lib \
--with-cxx \
--with-fftw \
--with-freetype \
Expand All @@ -44,6 +46,7 @@ export ARCH=x86_64-w64-mingw32
--with-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-lapack \
--with-liblas=${SRC}/mswindows/osgeo4w/liblas-config \
--with-libpng=${SRC}/mswindows/osgeo4w/libpng-config \
Copy link
Member

@echoix echoix Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #4121 (comment):

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.

--with-libs="${OSGEO4W_ROOT_MSYS}/lib ${OSGEO4W_ROOT_MSYS}/bin" \
--with-netcdf=${OSGEO4W_ROOT_MSYS}/bin/nc-config \
--with-nls \
Expand Down
2 changes: 2 additions & 0 deletions mswindows/osgeo4w/env.bat.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ REM Uncomment if you want to use Bash instead of Cmd
REM Note that msys package must be also installed
REM set GRASS_SH=%OSGEO4W_ROOT%\apps\msys\bin\sh.exe

set PYTHONPATH=%OSGEO4W_ROOT%\apps\grass\grass@POSTFIX@\etc\python;%PYTHONPATH%
set GRASS_COMPATIBILITY_TEST=0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first one, PYTHONPATH, is needed, I often had to add it to make windows tests work in CI (like when working hard to get pytest running), but the second one I don't know

Copy link
Contributor

@jef-n jef-n Nov 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fatal version check in gisinit.c that used to be patched out in osgeo4w now just causes a warning if GRASS_COMPATIBILITY_TEST is 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so you're talking about this part:

grass/lib/gis/gisinit.c

Lines 43 to 73 in 3e22b87

void G__gisinit(const char *version, const char *pgm)
{
const char *mapset;
if (initialized)
return;
G_set_program_name(pgm);
/* verify version of GRASS headers (and anything else in include) */
if (strcmp(version, GIS_H_VERSION) != 0) {
char *envstr;
char *answer = "0";
envstr = getenv("GRASS_COMPATIBILITY_TEST");
if (envstr && *envstr && strcmp(envstr, answer) == 0) {
G_warning(_("Module built against version %s but "
"trying to use version %s. "
"In case of errors you need to rebuild the module "
"against GRASS GIS version %s."),
version, GIS_H_VERSION, GRASS_VERSION_STRING);
}
else {
G_fatal_error(
_("Module built against version %s but "
"trying to use version %s. "
"You need to rebuild GRASS GIS or untangle multiple "
"installations."),
version, GIS_H_VERSION);
}
}

Shouldn't it be GRASS_COMPATIBILITY_TEST?

set GRASS_PYTHON=%OSGEO4W_ROOT%\bin\python3.exe
set GRASS_PROJSHARE=%OSGEO4W_ROOT%\share\proj

Expand Down
2 changes: 1 addition & 1 deletion mswindows/osgeo4w/libpng-config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

prefix="${OSGEO4W_ROOT_MSYS}"
version="$(sed '/^#define PNG_LIBPNG_VER_STRING/!d; s/^[^"]*"\|"//g' ${prefix}/include/libpng*/png.h)"
dll_version="$(sed '/^#define PNG_LIBPNG_VER_DLLNUM/!d; s/^[^0-9]*\|[^0-9]*$//g' ${prefix}/include/libpng*/png.h)"
dll_version="$(sed '/^#define PNG_LIBPNG_VER_SHAREDLIB/!d; s/^[^0-9]*\|[^0-9]*$//g' ${prefix}/include/libpng*/png.h)"
exec_prefix="${prefix}"
libdir="${prefix}/lib"
includedir="${prefix}/include/libpng${dll_version}"
Expand Down
74 changes: 36 additions & 38 deletions mswindows/osgeo4w/package.sh
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we document somewhere why and which DLL is needed to include in the winGRASS package? IIRC I was involved in a trial/error-way to check which of the DLLs we added finally back then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a separate issue for this, as we're mostly only applying the patches that have been used upstream. Something quite simple that we've spent 4 months on, so I would hope to not stretch it further if no easy consensus comes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a libpng update in osgeo4w broke "our" version of libpng-config and probably made someone switch from osgeo4w's libpng to mingw's mingw-w64-x86_64-libpng along with it's default libpng-config to fix the build.

Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/bash
#!/bin/bash

set -e

PWD="$(pwd)"
export ARCH=x86_64-w64-mingw32
export SRC=$PWD

if ! [ -d mswindows ]; then
echo Start from GRASS toplevel dir
Expand Down Expand Up @@ -107,31 +108,24 @@ fi
exec 3>&1 > >(tee mswindows/osgeo4w/package.log) 2>&1

DLLS="
/mingw64/bin/libbrotlicommon.dll
/mingw64/bin/libbrotlidec.dll
/mingw64/bin/libbz2-1.dll
/mingw64/bin/libcairo-2.dll
/mingw64/bin/libfftw3-3.dll
/mingw64/bin/libfontconfig-1.dll
/mingw64/bin/libfreetype-6.dll
/mingw64/bin/libgcc_s_seh-1.dll
/mingw64/bin/libgfortran-5.dll
/mingw64/bin/libglib-2.0-0.dll
/mingw64/bin/libgomp-1.dll
/mingw64/bin/libgraphite2.dll
/mingw64/bin/libharfbuzz-0.dll
/mingw64/bin/libiconv-2.dll
/mingw64/bin/libintl-8.dll
/mingw64/bin/libopenblas.dll
/mingw64/bin/libpcre-1.dll
/mingw64/bin/libpixman-1-0.dll
/mingw64/bin/libpng16-16.dll
/mingw64/bin/libquadmath-0.dll
/mingw64/bin/libreadline8.dll
/mingw64/bin/libstdc++-6.dll
/mingw64/bin/libsystre-0.dll
/mingw64/bin/libtre-5.dll
/mingw64/bin/libwinpthread-1.dll
/mingw64/bin/zlib1.dll
/mingw64/bin/libopenblas.dll
"

if ! [ -f mswindows/osgeo4w/configure-stamp ]; then
Expand All @@ -143,52 +137,54 @@ if ! [ -f mswindows/osgeo4w/configure-stamp ]; then
log remove old logs
rm -f mswindows/osgeo4w/package.log.*

mkdir -p dist.x86_64-w64-mingw32/bin
cp -uv $DLLS dist.x86_64-w64-mingw32/bin

mkdir -p mswindows/osgeo4w/lib
cp -uv $OSGEO4W_ROOT_MSYS/lib/libpq.lib mswindows/osgeo4w/lib/pq.lib
cp -uv $OSGEO4W_ROOT_MSYS/lib/sqlite3_i.lib mswindows/osgeo4w/lib/sqlite3.lib


log configure
CFLAGS="$CFLAGS -pipe" \
CXXFLAGS="$CXXFLAGS -pipe" \
./configure \
--bindir=$OSGEO4W_ROOT_MSYS/bin \
--bindir=${OSGEO4W_ROOT_MSYS}/bin \
--enable-largefile \
--enable-shared \
--host=x86_64-w64-mingw32 \
--includedir=$OSGEO4W_ROOT_MSYS/include \
--libexecdir=$OSGEO4W_ROOT_MSYS/bin \
--prefix=$OSGEO4W_ROOT_MSYS/apps/grass \
--host=${ARCH} \
--includedir=${OSGEO4W_ROOT_MSYS}/include \
--libexecdir=${OSGEO4W_ROOT_MSYS}/bin \
--prefix=${OSGEO4W_ROOT_MSYS}/apps/grass \
--with-blas \
--with-bzlib \
--with-cairo \
--with-cairo-includes=$OSGEO4W_ROOT_MSYS/include \
--with-cairo-ldflags="-L$PWD/mswindows/osgeo4w/lib -lcairo -lfontconfig" \
--with-cairo-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-cairo-ldflags="-L${SRC}/mswindows/osgeo4w/lib -lcairo" \
--with-cairo-libs=${OSGEO4W_ROOT_MSYS}/lib \
--with-cxx \
--with-fftw \
--with-freetype \
--with-freetype-includes=/mingw64/include/freetype2 \
--with-gdal=$PWD/mswindows/osgeo4w/gdal-config \
--with-geos=$PWD/mswindows/osgeo4w/geos-config \
--with-includes=$OSGEO4W_ROOT_MSYS/include \
--with-freetype-includes=${OSGEO4W_ROOT_MSYS}/include/freetype2 \
--with-gdal=${SRC}/mswindows/osgeo4w/gdal-config \
--with-geos=${SRC}/mswindows/osgeo4w/geos-config \
--with-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-lapack \
--with-liblas=$PWD/mswindows/osgeo4w/liblas-config \
--with-libs="$OSGEO4W_ROOT_MSYS/lib" \
--with-liblas=${SRC}/mswindows/osgeo4w/liblas-config \
--with-libpng=${SRC}/mswindows/osgeo4w/libpng-config \
--with-libs="${OSGEO4W_ROOT_MSYS}/lib ${OSGEO4W_ROOT_MSYS}/bin" \
--with-netcdf=${OSGEO4W_ROOT_MSYS}/bin/nc-config \
--with-nls \
--with-odbc \
--with-opengl=windows \
--with-openmp \
--with-postgres \
--with-postgres-includes=$OSGEO4W_ROOT_MSYS/include \
--with-postgres-libs=$PWD/mswindows/osgeo4w/lib \
--with-proj-includes=$OSGEO4W_ROOT_MSYS/include \
--with-proj-libs=$OSGEO4W_ROOT_MSYS/lib \
--with-proj-share=$OSGEO4W_ROOT_MSYS/share/proj \
--with-postgres-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-postgres-libs=${OSGEO4W_ROOT_MSYS}/lib \
--with-proj-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-proj-libs=${OSGEO4W_ROOT_MSYS}/lib \
--with-proj-share=${OSGEO4W_ROOT_MSYS}/share/proj \
--with-readline \
--with-regex \
--with-sqlite \
--with-sqlite-includes=$OSGEO4W_ROOT_MSYS/include \
--with-sqlite-libs=$PWD/mswindows/osgeo4w/lib \
--with-sqlite-includes=${OSGEO4W_ROOT_MSYS}/include \
--with-sqlite-libs=${OSGEO4W_ROOT_MSYS}/lib \
--with-zstd \
--without-pdal \
--without-x
Expand Down Expand Up @@ -244,9 +240,11 @@ if [ -n "$PACKAGE_PATCH" ]; then
unix2dos etc/postinstall/grass${PACKAGE_POSTFIX}.bat
unix2dos etc/preremove/grass${PACKAGE_POSTFIX}.bat

# copy dependencies (TODO: to be reduced)
cp -uv $DLLS apps/grass/grass$POSTFIX/bin
cp -uv /mingw64/etc/fonts/fonts.conf apps/grass/grass$POSTFIX/etc
# copy dependencies
cp -uv $(/usr/bin/find apps/grass/grass$POSTFIX -iname "*.dll" -o -iname "*.exe" | PATH=$PWD/apps/grass/grass$POSTFIX/lib:$PWD/bin:/mingw64/bin:/usr/bin /usr/bin/xargs /usr/bin/ldd | /usr/bin/sed -ne 's#^.* => \(/mingw64/bin/.*\) (.*)$#\1#p' | /usr/bin/sort -u) apps/grass/grass$POSTFIX/bin

Copy link
Contributor

@jef-n jef-n Nov 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is merged, DLLS (line 109/10) can be removed

# copy R batch files
cp -uv $SRC/mswindows/external/rbatch/* apps/grass/grass$POSTFIX/bin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be checked whether we include these files already in another way, e.g. in the NSIS installer-

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this exact script really used for the NSIS installer? This script is used only by OSGeo4W (yet) from the best of my knowledge. I have some work prepared even before this PR was filed to use this script for our CI, to be really closer to what OSGeo4W packages, and could upload the artifacts that could be tested locally, and use ccache (that is already scripted here, we only have to download and upload the cache in CI). To not duplicate work, I was patiently waiting for this PR to be merged before continuing, but I would really want to continue to allow shaving some time off of the longest CI job that we have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, NSIS does that here:

grass/mswindows/GRASS-Packager.bat.tmpl

Line 65 in 3e22b87

copy .\external\rbatch\* %PACKAGE_DIR%\extrabin


# creating grass package
/bin/tar -cjf $PDIR/grass$PACKAGE_POSTFIX-$VERSION-$PACKAGE_PATCH.tar.bz2 \
Expand Down
Loading