> Thanks. However, > > > + (let ((tramp-minimum-emacs-version "30.1")) > > + (should-error (tramp-check-version)))) > > this does not work as expected. What is this check good for? I > understand your intention, but do we need to test this case?
That test has already justified its existence, for it has revealed a flaw in the implementation! TRAMP_EMACS_REQUIRED_VERSION was getting substituted in two separate places, so tramp-check-version wasn't checking against tramp-minimum-emacs-version, but rather a hardcoded value. I've attached a new version of the patch. Hopefully this one is right.
From 1b47fd85fef2590a7da252d0a2f5d9dcc3ace81c Mon Sep 17 00:00:00 2001 From: Nick Drozd <[email protected]> Date: Sat, 11 Jul 2020 10:02:27 -0500 Subject: [PATCH] Clean up Tramp version check * aclocal.m4 (TRAMP_EMACS_VERSION_CHECK): Rewrite * trampver.el.in (tramp-minimum-emacs-version): New variable (tramp-check-version): New function (tramp-tests.el): New test --- aclocal.m4 | 17 +++++++++++++---- lisp/trampver.el.in | 4 +--- test/tramp-tests.el | 8 ++++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/aclocal.m4 b/aclocal.m4 index d4a82cf9..4940fbef 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -75,10 +75,19 @@ AC_DEFUN(AC_EMACS_INFO, [ AC_SUBST(TRAMP_EMACS_REQUIRED_VERSION) dnl Starting with Emacs 26.1, we could use `string-version-lessp'. TRAMP_EMACS_VERSION_CHECK="\ - (if (not (string-lessp emacs-version \"${TRAMP_EMACS_REQUIRED_VERSION}\")) - \"ok\" - (format \"${PACKAGE_STRING} is not fit for %s\" - (replace-regexp-in-string \"\\n\" \"\" (emacs-version))))" +(progn ; <-- don't touch this progn + (defconst tramp-minimum-emacs-version \"${TRAMP_EMACS_REQUIRED_VERSION}\" + \"The minimum Emacs version required by Tramp.\") + + (defun tramp-check-version () + \"Error if `emacs-version' is less than `tramp-minimum-emacs-version'.\" + (progn ; <-- don't touch this progn + (when (string-lessp emacs-version tramp-minimum-emacs-version) + (error \"${PACKAGE_VERSION} is not fit for %s\" + (replace-regexp-in-string \"\n\" \"\" (emacs-version)))) + \"ok\")) + + (tramp-check-version))" AC_SUBST(TRAMP_EMACS_VERSION_CHECK) AC_MSG_CHECKING([for $EMACS version]) diff --git a/lisp/trampver.el.in b/lisp/trampver.el.in index f487b332..04dd4006 100644 --- a/lisp/trampver.el.in +++ b/lisp/trampver.el.in @@ -69,9 +69,7 @@ (emacs-repository-get-version dir)))) "The repository revision of the Tramp sources.") -;; Check for Emacs version. -(let ((x @TRAMP_EMACS_VERSION_CHECK@)) - (unless (string-equal "ok" x) (error "%s" x))) +@TRAMP_EMACS_VERSION_CHECK@ ;; Tramp versions integrated into Emacs. If a user option declares a ;; `:package-version' which doesn't belong to an integrated Tramp diff --git a/test/tramp-tests.el b/test/tramp-tests.el index 51722210..f0f8d079 100644 --- a/test/tramp-tests.el +++ b/test/tramp-tests.el @@ -6670,6 +6670,14 @@ Since it unloads Tramp, it shall be the last test to run." (ignore-errors (all-completions "tramp" (symbol-value x))) (ert-fail (format "Hook `%s' still contains Tramp function" x)))))) +(ert-deftest tramp-test47-version () + "Check that the version check checks out." + (should (string-equal "ok" (tramp-check-version))) + (let ((emacs-version "20.3")) + (should-error (tramp-check-version))) + (let ((tramp-minimum-emacs-version "40.1")) + (should-error (tramp-check-version)))) + (defun tramp-test-all (&optional interactive) "Run all tests for \\[tramp]. If INTERACTIVE is non-nil, the tests are run interactively." -- 2.17.1
