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.

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

Address some MS Windows test failures #1683

Closed
wants to merge 135 commits into from
Closed

Conversation

ninsbl
Copy link
Member

@ninsbl ninsbl commented Jun 25, 2021

Should be backported to the release branch.

@ninsbl ninsbl added bug Something isn't working windows Microsoft Windows specific labels Jun 25, 2021
@ninsbl
Copy link
Member Author

ninsbl commented Jun 25, 2021

The build still fails. It probably needs to be adjusted from here: #136

@hellik
Copy link
Member

hellik commented Jun 25, 2021

Have a look here #1561

@hellik
Copy link
Member

hellik commented Jun 25, 2021

We need to adapt our building scripts to updated dependencies in OSGeo4W v2

@ninsbl
Copy link
Member Author

ninsbl commented Jun 27, 2021

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?

@jef-n
Copy link
Contributor

jef-n commented Jun 28, 2021

@hellik
Copy link
Member

hellik commented Jun 28, 2021

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

@ninsbl
Copy link
Member Author

ninsbl commented Jul 1, 2021

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

@jef-n
Copy link
Contributor

jef-n commented Jul 1, 2021

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.

@nilason
Copy link
Contributor

nilason commented Jul 1, 2021

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.

@nilason
Copy link
Contributor

nilason commented Jul 1, 2021

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 #1423 (comment) for better location for this.

@nilason nilason mentioned this pull request Jul 6, 2021
@veroandreo
Copy link
Contributor

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?

@ninsbl
Copy link
Member Author

ninsbl commented Aug 27, 2021

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...

@wenzeslaus
Copy link
Member

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 ninsbl removed this from the 8.2.0 milestone Sep 15, 2021
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
ninsbl added a commit that referenced this pull request Jan 11, 2022
With this PR merged, CI uses packages from OSGeo4W Version2 (https://github.com/jef-n/OSGeo4W/) to build GRASS GIS using MSYS. Now also shellscript tests are executed. The PR replaces #1873 and parts of #1683. Compilation for tests is now also configured with OpenMP and LAPACK support (amongst others).
neteler pushed a commit that referenced this pull request Jan 11, 2022
With this PR merged, CI uses packages from OSGeo4W Version2 (https://github.com/jef-n/OSGeo4W/) to build GRASS GIS using MSYS. Now also shellscript tests are executed. The PR replaces #1873 and parts of #1683. Compilation for tests is now also configured with OpenMP and LAPACK support (amongst others).
@neteler
Copy link
Member

neteler commented Jan 12, 2022

@ninsbl is this PR still relevant given that #2074 has been merged?

@ninsbl
Copy link
Member Author

ninsbl commented Jan 12, 2022

@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...

@neteler neteler modified the milestones: 8.0.0, 8.0.1 Jan 12, 2022
@ninsbl ninsbl modified the milestones: 8.0.1, 8.0.2 Feb 20, 2022
@ninsbl
Copy link
Member Author

ninsbl commented Feb 20, 2022

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.

@wenzeslaus wenzeslaus modified the milestones: 8.0.2, 8.4.0 Mar 16, 2022
@ninsbl
Copy link
Member Author

ninsbl commented Apr 3, 2022

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....

@ninsbl ninsbl changed the title OSGeo4W workflow: switch to OSGeo4W v2 Address some MS Windows test failures Apr 3, 2022
@neteler
Copy link
Member

neteler commented May 1, 2022

@ninsbl Could you please git rebase this PR?

@wenzeslaus
Copy link
Member

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.

ninsbl added a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
With this PR merged, CI uses packages from OSGeo4W Version2 (https://github.com/jef-n/OSGeo4W/) to build GRASS GIS using MSYS. Now also shellscript tests are executed. The PR replaces OSGeo#1873 and parts of OSGeo#1683. Compilation for tests is now also configured with OpenMP and LAPACK support (amongst others).
@ninsbl
Copy link
Member Author

ninsbl commented Oct 28, 2022

Outdated and replaced by: #2616 now. Closing this one.

@ninsbl ninsbl closed this Oct 28, 2022
@neteler neteler added invalid This doesn't seem to be a bug and removed backport_needed labels Nov 12, 2022
ninsbl added a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
With this PR merged, CI uses packages from OSGeo4W Version2 (https://github.com/jef-n/OSGeo4W/) to build GRASS GIS using MSYS. Now also shellscript tests are executed. The PR replaces OSGeo#1873 and parts of OSGeo#1683. Compilation for tests is now also configured with OpenMP and LAPACK support (amongst others).
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
With this PR merged, CI uses packages from OSGeo4W Version2 (https://github.com/jef-n/OSGeo4W/) to build GRASS GIS using MSYS. Now also shellscript tests are executed. The PR replaces OSGeo#1873 and parts of OSGeo#1683. Compilation for tests is now also configured with OpenMP and LAPACK support (amongst others).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem to be a bug windows Microsoft Windows specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants