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

osgeo4w: build fixes and use proj 7 and gdal 3 #136

Closed
wants to merge 2 commits into from

Conversation

jef-n
Copy link
Contributor

@jef-n jef-n commented Sep 22, 2019

No description provided.

@metzm
Copy link
Contributor

metzm commented Sep 25, 2019

proj 6 and gdal 3 are not yet supported, waiting for PR #118 to be merged and backported

@neteler
Copy link
Member

neteler commented Oct 20, 2019

#137 has been merged and forward-ported to master in a1d75c7

@jef-n Could you please rebase this PR accordingly?

@hellik
Copy link
Member

hellik commented Oct 25, 2019

@neteler citing here @metzm

proj 6 and gdal 3 are not yet supported, waiting for PR #118 to be merged and backported

is GRASS now proj 6 and gdal 3 ready?

#137 is now merged; is this PR now only about proj 6 and gdal 3?

Copy link
Member

@hellik hellik left a comment

Choose a reason for hiding this comment

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

when GRASS is proj 6 and gdal 3 ready, then this PR seems to be ok

@hellik
Copy link
Member

hellik commented Oct 29, 2019

see https://lists.osgeo.org/pipermail/grass-dev/2019-October/093434.html

should we now merge also this PR? or do we have to adapt OSGeo4W env first?

@jef-n
Copy link
Contributor Author

jef-n commented Oct 29, 2019

should we now merge also this PR? or do we have to adapt OSGeo4W env first?

Depends. As it is it build with the nightlies of GDAL and PROJ - so it should work now #118 is merged. It probably should be changed to use the regular GDAL & PROJ packages again when those are updated.

@neteler
Copy link
Member

neteler commented Oct 29, 2019

@jef-n ok, what do you recommend? RC1 today without this change or wait for this PR being merged and then RC1?

@jef-n
Copy link
Contributor Author

jef-n commented Oct 29, 2019

@jef-n ok, what do you recommend? RC1 today without this change or wait for this PR being merged and then RC1?

This change was meant to build the daily builds with the nightlies of GDAL and PROJ. So no - no need to merge into the release branch.

@hellik
Copy link
Member

hellik commented Oct 29, 2019

At the moment, proj4x and gdal2x are still the default in OSGeo4W, right?

Any idea when the switch to proj6 and gdal3 as default will happen?

@jef-n so your suggestion is to keep upcoming winGRASS 7.8.1 with proj4/gdal2?

Why not with winGRASS 7.8.1 RC1 switch to proj6/gdal3?

@jef-n
Copy link
Contributor Author

jef-n commented Oct 29, 2019

At the moment, proj4x and gdal2x are still the default in OSGeo4W,
Any idea when the switch to proj6 and gdal3 as default will happen?
@jef-n so your suggestion is to keep upcoming winGRASS 7.8.1 with proj4/gdal2?
Why not with winGRASS 7.8.1 RC1 switch to proj6/gdal3?

Huh? Sure it is still GDAL2 - but that's because we couldn't update because that would have broken GRASS. Now we just need to coordinate. GDAL & PROJ needs to be updated in OSGeo4W and GRASS needs to be rebuilt on top of the two and QGIS on top of all three. I could put new GDAL & PROJ builds as test versions into OSGeo4W (ie. test: version in setup.hint), you install those into your build environment (Exp in setup), produce GRASS on top of those, also upload them as test: versions and I use these to rebuild the QGIS packages. Then we remove the test: and promote them all.

But that doesn't factor in that GRASS is "only" a RC. And GDAL and PROJ also both currently have RCs out - so it might make sense to wait for those. As I wouldn't like to build the QGIS release packages against RCs or something that is going to need an update very soon.

@neteler
Copy link
Member

neteler commented Oct 29, 2019

But that doesn't factor in that GRASS is "only" a RC.

So far it is not. This is what I want to know: RC1 tonight or not yet?

And GDAL and PROJ also both currently have RCs out - so it might make sense to wait for those.

You suggest to start the GRASS GIS RC cylce only after that? I am not sure why this would be helpful to delay the GRASS GIS release but maybe I am overlooking something.
Again: I am talking about the source code release, not the winGRASS/OSGeo4W package here.

@metzm
Copy link
Contributor

metzm commented Oct 29, 2019 via email

@hellik
Copy link
Member

hellik commented Oct 29, 2019

lets try to get out of the confusion ;-)

(1) OSGeo4W environment has to prepared that winGRASS will switch to proj6 and gdal3

(2) GRASS source ( i.e. winGRASS build scripts) have to be adapted to an adapted (1)

OSGeo4W environment will be changed to be ready for (1) when new proj6 and gdal3 RC will be then official releases incorporated in OSGeo4W. It's too much maintaining burden to offer an interim solution in OSGeo4W.

Regarding (2) winGRASS 7.8.1 RC1 won't be available with proj6 and gdal3 support at the moment as OSGeo4W isn't yet ready, see (1), thus nothing to test the new capabilties in the windows side of the world, no chance to catch bugs etc.

In all cases, if GRASS 7.8.1 RC1 will be tagged now, winGRASS build scripts have to be adapted for (1) anyway..

Will be then there a RC2 or RC3 or RC4 or RC5 if needed?

it's all about keeping the maintaining and release burden now!

anything unclear? ;-)

@neteler
Copy link
Member

neteler commented Oct 29, 2019

In all cases, if GRASS 7.8.1 RC1 will be tagged now, winGRASS build scripts have to be adapted for (1) anyway..

Here we are!

https://github.com/OSGeo/grass/releases/tag/7.8.1RC1

Will be then there a RC2 or RC3 or RC4 or RC5 if needed?

Sure, as needed.

@metzm
Copy link
Contributor

metzm commented Oct 30, 2019

lets try to get out of the confusion ;-)

(1) OSGeo4W environment has to prepared that winGRASS will switch to proj6 and gdal3

[...]

In all cases, if GRASS 7.8.1 RC1 will be tagged now, winGRASS build scripts have to be adapted for (1) anyway..

I thought it would be easier to first include GRASS 7.8.1 (because it works also with GDAL 1/2 + PROJ 4/5), and later on switch to GDAL 3 + PROJ 6. If you switch to GDAL 3 + PROJ 6 first, you break the GRASS build until GRASS is updated to 7.8.1. But then I am not a OSGeo4W maintainer.

@neteler
Copy link
Member

neteler commented Nov 9, 2019

FYI: GRASS GIS 7.8.1 with support for Python 3, PROJ 6 and GDAL 3 has been released:

https://github.com/OSGeo/grass/releases/tag/7.8.1

@kikislater
Copy link
Contributor

kikislater commented Nov 9, 2019

FYI: GRASS GIS 7.8.1 with support for Python 3, PROJ 6 and GDAL 3 has been released:

https://github.com/OSGeo/grass/releases/tag/7.8.1

Why 7.8.1 is considered as stable ? v.import doesn't work for example : https://trac.osgeo.org/grass/ticket/3869
Latest stable for me is 7.6.1
It was a problem here and since it exists, v.import is widely used OpenDroneMap/WebODM#735

@metzm
Copy link
Contributor

metzm commented Nov 9, 2019

Why 7.8.1 is considered as stable ? v.import doesn't work for example : https://trac.osgeo.org/grass/ticket/3869

According to ticket 3869, v.import does work, it just does not have all the options of v.in.ogr.
Originally, v.import was meant to be a simple vector import tool with only a few options to keep the user interface simple. If you want more control over the import process, use v.in.ogr.

@kikislater
Copy link
Contributor

Yes but it was written :

v.import might not work as expected with proj 6+ and gdal 3+ in future releases of GRASS GIS

So if you have previous code working and update to 7.8.1, it breaks the workflow. So it partially works ... Could not be considered as stable or put message on release log like : modules break !

@metzm
Copy link
Contributor

metzm commented Nov 9, 2019

Yes but it was written :

v.import might not work as expected with proj 6+ and gdal 3+ in future releases of GRASS GIS

So if you have previous code working and update to 7.8.1, it breaks the workflow. So it partially works ... Could not be considered as stable or put message on release log like : modules break !

It's the other way around: if you upgrade to GDAL 3 + PROJ 6, v.import of GRASS 7.6 is guaranteed to not work correctly. You need GRASS 7.8.1 if you want to use GRASS with GDAL 3 + PROJ 6.

Rephrasing your words: if you have previous code working and update to gdal 3 + proj 6, it breaks the workflow unless you update GRASS to 7.8.1.

@kikislater
Copy link
Contributor

Makes sense but what happen to 7.8 documentation ?

If the projection of the input does not match the projection of the location, the input is reprojected into the current location
https://grass.osgeo.org/grass78/manuals/v.import.html

@metzm
Copy link
Contributor

metzm commented Nov 9, 2019

Makes sense but what happen to 7.8 documentation ?

If the projection of the input does not match the projection of the location, the input is reprojected into the current location
https://grass.osgeo.org/grass78/manuals/v.import.html

This is still valid. If you want to use PROJ 6, you need GRASS 7.8.1. As before, for more control over the import and reprojection process, use v.in.ogr and v.proj instead of v.import. Regarding PROJ 6, it is worth to use projinfo to select an appropriate reprojection method.

@kikislater
Copy link
Contributor

Ok thank you. As I know these limitations now it's ok for me but others could be in trouble regarding documentation and reprojection from this module

@neteler
Copy link
Member

neteler commented Nov 29, 2019

@hellik @landam what about this PR? Is it relevant for the upcoming 7.8.2?

@jef-n
Copy link
Contributor Author

jef-n commented Nov 29, 2019

@hellik @landam what about this PR? Is it relevant for the upcoming 7.8.2?

no.

@neteler
Copy link
Member

neteler commented Apr 26, 2020

Re: PROJ 7 support, see also #395

@jef-n jef-n closed this Jun 28, 2021
@neteler neteler added this to the 7.8.1 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants