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

GDAL: deprecate 2.X #30668

Merged
merged 1 commit into from
May 17, 2022
Merged

GDAL: deprecate 2.X #30668

merged 1 commit into from
May 17, 2022

Conversation

adamjstewart
Copy link
Member

@adamjstewart adamjstewart commented May 13, 2022

This PR deprecates all GDAL 2.X versions. GDAL 3.X was released over 3 years ago, and most packages have been ported to support GDAL 3.X by now.

Motivation

GDAL's build system keeps changing. GDAL 2.X only has configure options to help find dependencies, so our variants are named after those dependencies. GDAL 3.X added configure options to enable/disable specific raster and vector drivers. In my opinion, this is much more intuitive, and I would like to change all of our variants to match the driver names, not the dependency names. GDAL 3.5 (released today) adds a CMake build system, and 3.6 will remove the current Autotools build system, adding more complexity to our build recipe. This PR is an attempt to reduce the number of versions of GDAL we (i.e. I) have to support and keep the recipe maintainable.

Future

I'm hoping to get this PR into Spack 0.18 (coming next week), then immediately remove GDAL 2.X and rename all of our variants on develop. This is a pretty disruptive change. I wish there were a better way to support this. Maybe @alalazo has some ideas? @tgamblin this pertains to our discussion about version deprecation and removal.

Pinging some folks who have contributed to our GDAL recipe in the past: @Sinan81 @snehring @haralmha @rouault @amaji @payerle @michaelkuhn @iarspider @neilflood

Will anyone (or any packages) be negatively impacted by removing these versions? What about removing GDAL 3.0:3.4? Here's what I found from our recipes:

  • gplates: requires GDAL 1 (gplates: add v2.3.0 #30676)
  • grass+gdal: requires GDAL 3.2 and older
  • ncl+gdal: requires GDAL 2 and older
  • py-rasterio: requires GDAL 3.3 and older

We'll likely have to keep 3.0:3.4 around for now, as I'm heavily dependent on py-rasterio. If anyone wants to continue to support GDAL 2, I might ask you to become an official "maintainer" of our build recipe, because I'm no longer able to test GDAL 2.

@adamjstewart adamjstewart added the deprecation A feature got deprecated in Spack label May 13, 2022
@adamjstewart
Copy link
Member Author

@tgamblin assures me that @alalazo's new multi-build system support will make this much easier, so this deprecation may not be strictly necessary, but it's something I wanted to at least point out as a complicated use case where we might want 3 separate classes:

  • gdal@3.5: - cmake build system, driver variants
  • gdal@3.0:3.4' - autotools build system, driver variants
  • gdal@:2' - autotools build system, dependency variants

We could also use driver variants for GDAL 2.X and hope for the best since there's no way to explicitly enable/disable them...

@snehring
Copy link
Contributor

snehring commented May 13, 2022

A quick survey of our environment shows we've got a few versions of gdal 2.x installed but I suspect that's just left over from legacy installs at this point.

As far as dependents go, our users use r-rgdal a fair amount, which seems fine with 3.x per the recipe and documentation.

I'm fine with deprecating them and dealing with any (likely none in our env) fallout.

@Sinan81
Copy link
Contributor

Sinan81 commented May 15, 2022

🤔

@Sinan81
Copy link
Contributor

Sinan81 commented May 15, 2022

I am generally in favor of providing backwards compatibility to the extent that it's practical or with the understanding that users won't get support from maintainers for non-trivial issues. Hence, it would be great to provide support for multiple builds systems. Or could one use a provider set-up so that let's say gdal-legacy package would provide older versions with autotools while gdal package would do so for newer (with cmake).

@Sinan81
Copy link
Contributor

Sinan81 commented May 15, 2022

I recently encountered a similar problem whiling building gtk, and I basically renamed older package as gtk-legacy so that I could build older versions with autotool build as opposed to meson.

@Sinan81
Copy link
Contributor

Sinan81 commented May 15, 2022

Of course the change from dependency variants to driver variants complicates the situation.

@Sinan81
Copy link
Contributor

Sinan81 commented May 15, 2022

Also, could we use a version dependent dependency to handle this situation. For example:

def Mypackage(...):
    depends('gdal-legacy +dependency_opt', when('@...'))
    depends('gdal +driver_opt', when('@...')

@adamjstewart
Copy link
Member Author

Yes, multi-build system support works fine (https://spack.readthedocs.io/en/latest/build_systems/multiplepackage.html), and @alalazo is working on making it even better (#30411). But you can't have separate classes for these at the moment, so having separate variants for different versions is cumbersome and requires a lot of nested with when(). So this PR deprecates older versions so I can focus on the build system differences only.

@iarspider
Copy link
Contributor

@adamjstewart I'm fine with dropping 2.x - CMS doesn't even use gdal. Pinging @haralmha (SFT) and @vvolkl (Key4HEP)

@haralmha
Copy link
Contributor

I am also fine with dropping 2.x. We use newer versions in the LCG stack

Copy link
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

@adamjstewart Feel free to go ahead merging when you think it is ok to do so based on people responses

@adamjstewart
Copy link
Member Author

Will merge just to ensure that this makes it into 0.18, but if anyone has any problems with this after I merge, let me know and I can always revert. Hopefully #30411 will make this kind of thing easier in the future. Also see #30676.

@adamjstewart adamjstewart merged commit dfd0702 into spack:develop May 17, 2022
@adamjstewart adamjstewart deleted the packages/gdal branch May 17, 2022 15:45
@Sinan81
Copy link
Contributor

Sinan81 commented May 17, 2022

I guess I am fine with dropping 2.x too.

@adamjstewart adamjstewart mentioned this pull request May 29, 2022
18 tasks
bhatiaharsh pushed a commit to bhatiaharsh/spack that referenced this pull request Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation A feature got deprecated in Spack new-version update-package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants