backward compatibility?

jannes-m commented 1 year ago

Hey @florisvdh, I just ran into issues when running qgisprocess for an older QGIS version (version: 3.18.0-Zürich). For example, qgis_query_version() uses now the --version flag for finding out about the currently used QGIS version, however, this flag was not available for QGIS 3.18 and I am pretty sure the command line interface has also changed in various other respects. If you do not want to support all QGIS versions, one would have to say in the DESCRIPTION file which is the oldest QGIS version qgisprocess supports. Currently, it states >QGIS 3.16, however, this is no longer true.

florisvdh commented 1 year ago

Thanks for this finding and the suggestion @jannes-m!

I think it is sensible for this package to officially only support the QGIS releases that QGIS supports itself, being the latest QGIS release + the long term release (LTR). This is in the sense of 'at least support those'. To have 'guaranteed' support for older QGIS versions, one would then have to use an older version of qgisprocess. And yes, it means that the QGIS version in Description needs to be maintained, at least in order to 'drop' versions that don't work anymore (so could be less strict). E.g. we could make it:

SystemRequirements: QGIS latest or long-term release (>= 3.28). Older versions may not work; expected to work since long-term release 3.22.

Since 3.30 came out recently, 3.28 became the new LTR; before the LTR was 3.22 which didn't support the JSON-input.

The above also means that at least one more workflow is to be added to the GH Actions, i.e. to also test the package with current LTR, not only the latest QGIS release. I'll keep this in mind.

In practice, the support can go way further back than the two currently supported QGIS releases, because we are not that eager at removing code that is specific to older QGIS versions; the most obvious being the no-JSON-input method (many functions cater for both situations), which was needed for previous LTR 3.22.

It appears that also --version appeared since 3.22 (https://github.com/qgis/QGIS/commit/bb9c45d). If various qgisprocess functionality doesn't work in older versions than that, then we best drop support for older versions.

If it's only --version that hampers, then it's still easy to add support for some pre-3.22 releases.

Could you paste the output of testthat::test_local() (run from the repo root) so that we can sum up what else goes wrong with older versions like e.g. QGIS 3.18 (hence check feasibility to still support it)?

florisvdh commented 1 year ago

Well, can you actually load the package with QGIS 3.18? qgis_query_version() is run on package loading... I can fix that so that you can run the tests, does that make sense?

florisvdh commented 1 year ago

And if applicable, we could more gracefully error when a too-old QGIS version is detected at package loading.

jannes-m commented 1 year ago

Hey @florisvdh, to be able to run the tests, I have replaced the qgis_query_version() by an older version. Here is the output:

testthat::test_local("~/rpackages/misc/qgisprocess")
#> Attempting to load the cache ... Success!
#> QGIS version: 3.18.0-Zürich
#> Having access to 998 algorithms from 6 QGIS processing providers.
#> Run `qgis_configure(use_cached_data = TRUE)` to reload cache and get more details.
#> ✔ | F W S  OK | Context
#>
#> ⠏ |         0 | compat-raster
#> ⠋ |         1 | compat-raster
#> ⠙ |         2 | compat-raster
#> ⠸ |         4 | compat-raster
#> ⠼ |         5 | compat-raster
#> ⠴ |         6 | compat-raster
#> ⠦ |         7 | compat-raster
#> ⠧ |         8 | compat-raster
#> ⠇ |         9 | compat-raster
#> ⠏ |        10 | compat-raster
#> ⠋ |        11 | compat-raster
#> ⠙ |        12 | compat-raster
#> ⠸ |        14 | compat-raster
#> ⠴ |        16 | compat-raster
#> ✔ |        16 | compat-raster [9.9s]
#>
#> ⠏ |         0 | compat-sf
#> ⠋ |         1 | compat-sf
#> ⠸ |         4 | compat-sf
#> ⠋ |        11 | compat-sf
#> ⠧ |        18 | compat-sf
#> ⠙ |        22 | compat-sf
#> ✔ |        23 | compat-sf [6.8s]
#>
#> ⠏ |         0 | compat-stars
#> ⠸ |         4 | compat-stars
#> ✔ |         7 | compat-stars [0.2s]
#>
#> ⠏ |         0 | compat-terra
#> ⠦ |         7 | compat-terra
#> ✔ |         9 | compat-terra [0.1s]
#>
#> ⠏ |         0 | qgis-algorithms
#> ✔ |        14 | qgis-algorithms
#>
#> ⠏ |         0 | qgis-arguments
#> ✔ |        31 | qgis-arguments
#>
#> ⠏ |         0 | qgis-configure
#> ⠙ | 1       1 | qgis-configure
#> ⠹ | 1       2 | qgis-configure
#> ⠸ | 1   1   2 | qgis-configure
#> ⠴ | 1   2   3 | qgis-configure
#> ⠦ | 1   2   4 | qgis-configure
#> ⠋ | 1   2   8 | qgis-configure
#> ⠙ | 1   2   9 | qgis-configure
#> ⠹ | 1   2  10 | qgis-configure
#> ⠸ | 1   2  11 | qgis-configure
#> ⠼ | 1   2  12 | qgis-configure                                                  Error in value[[3L]](cond) :
#>   'wrong/path/to/qgis_process' (cached path) is not available anymore.
#> Will try to reconfigure qgisprocess and build new cache ...
#>
#> ⠴ | 2   2  12 | qgis-configure
#> ⠦ | 2   2  13 | qgis-configure
#> ⠧ | 2   2  14 | qgis-configure
#> ✖ | 2   2  15 | qgis-configure [69.8s]
#> ────────────────────────────────────────────────────────────────────────────────
#> Error ('test-qgis-configure.R:6'): qgis_version() works
#> <system_command_status_error/system_command_error/rlib_error_3_0/rlib_error/error/condition>
#> Error in `"processx::run(path, args, ...)"`: ! System command 'qgis_process' failed
#> Backtrace:
#>   1. utils::capture.output(...)
#>        at test-qgis-configure.R:6:2
#>   8. qgisprocess::qgis_version(debug = TRUE)
#>  10. qgisprocess::qgis_run(args = "--version")
#>  13. processx::run(path, args, ...)
#>  14. throw(...)
#>
#> Skip ('test-qgis-configure.R:21'): qgis_query_version() works for development versions of QGIS
#> Reason: QGIS version 3.18.0-Zürich is not a development version.
#>
#> Skip ('test-qgis-configure.R:41'): qgis_configure() returns FALSE with no QGIS
#> Reason: has_qgis() is TRUE
#>
#> Failure ('test-qgis-configure.R:177'): qgis_configure() works OK if cache conditions unmet
#> stringr::str_detect(msg, "is not available anymore") is not TRUE
#>
#> `actual`:   FALSE
#> `expected`: TRUE
#> ────────────────────────────────────────────────────────────────────────────────
#>
#> ⠏ |         0 | qgis-detect
#> ✔ |         2 | qgis-detect
#>
#> ⠏ |         0 | qgis-function
#> ⠦ |         7 | qgis-function
#> ⠧ |         8 | qgis-function
#> ✔ |         8 | qgis-function [4.4s]
#>
#> ⠏ |         0 | qgis-help
#> ⠧ |         8 | qgis-help
#> ⠼ |        15 | qgis-help
#> ⠋ |        21 | qgis-help
#> ⠋ |        31 | qgis-help
#> ⠦ |        37 | qgis-help
#> ✔ |     1  45 | qgis-help [0.6s]
#> ────────────────────────────────────────────────────────────────────────────────
#> Skip ('test-qgis-help.R:53'): qgis_arguments() and qgis_outputs() works for all algorithms
#> Reason: Test takes ~1 hr to run
#> ────────────────────────────────────────────────────────────────────────────────
#>
#> ⠏ |         0 | qgis-output
#> ⠋ |         1 | qgis-output
#> ⠙ |         2 | qgis-output
#> ⠦ |         7 | qgis-output
#> ⠇ |         9 | qgis-output
#> ✔ |        11 | qgis-output [8.9s]
#>
#> ⠏ |         0 | qgis-plugins
#> ⠇ |        19 | qgis-plugins
#> ⠸ |        24 | qgis-plugins
#> ⠇ |        29 | qgis-plugins
#> ✔ |     4  37 | qgis-plugins [6.0s]
#> ────────────────────────────────────────────────────────────────────────────────
#> Skip ('test-qgis-plugins.R:109'): qgis_enable_plugins() ignores an already enabled grassprovider plugin
#> Reason: The 'grassprovider' plugin is missing.
#>
#> Skip ('test-qgis-plugins.R:126'): qgis_disable_plugins() ignores an already disabled grassprovider plugin
#> Reason: The 'grassprovider' plugin is missing.
#>
#> Skip ('test-qgis-plugins.R:146'): qgis_*able_plugins() works for a disabled grassprovider plugin
#> Reason: The 'grassprovider' plugin is missing.
#>
#> Skip ('test-qgis-plugins.R:170'): qgis_*able_plugins() works for an enabled grassprovider plugin
#> Reason: The 'grassprovider' plugin is missing.
#> ────────────────────────────────────────────────────────────────────────────────
#>
#> ⠏ |         0 | qgis-result
#> ⠸ |         4 | qgis-result
#> ✔ |        15 | qgis-result [2.3s]
#>
#> ⠏ |         0 | qgis-run-algorithm
#> ⠋ |         1 | qgis-run-algorithm
#> ⠹ |         3 | qgis-run-algorithm
#> ⠼ |         5 | qgis-run-algorithm
#> ⠴ |         6 | qgis-run-algorithm
#> ⠧ |         8 | qgis-run-algorithm
#> ⠙ |        12 | qgis-run-algorithm
#> ✔ |        13 | qgis-run-algorithm [13.0s]
#>
#> ⠏ |         0 | qgis-run-algorithm2
#> ⠏ |         0 | qgis-tmp
#> ✔ |        13 | qgis-tmp
#>
#> ══ Results ═════════════════════════════════════════════════════════════════════
#> Duration: 122.5 s
#>
#> ── Skipped tests  ──────────────────────────────────────────────────────────────
#> • has_qgis() is TRUE (1)
#> • QGIS version 3.18.0-Zürich is not a development version. (1)
#> • Test takes ~1 hr to run (1)
#> • The 'grassprovider' plugin is missing. (4)
#>
#> [ FAIL 2 | WARN 0 | SKIP 7 | PASS 259 ]
#> Error: Test failures

^{Created on 2023-03-20 with reprex v2.0.2}

florisvdh commented 1 year ago

Hi Jannes, can you test QGIS 3.18 again with r-spatial/qgisprocess@qgis_version_retro ?

At least I hope this one will load successfully for you; it may be that some test (2nd failure e.g.) still doesn't work well (but then I probably won't be able to fix).

jannes-m commented 1 year ago

Excellent stuff, @florisvdh, thanks a lot! 260 tests successfully passed and 8 tests were skipped. As you have suggested and to reduce the amount of maintenance work for you, it might be a good idea to say that officially only the LTRs are supported. And if the user only has a very old QGIS version at his disposal with which qgisprocess no longer works, there is still the option to use a docker image running a supported QGIS version.

r-spatial / qgisprocess

backward compatibility? #138