GNU bug report logs - #42237
[PATCH] Clean up Tramp version check

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42237 in the body.
You can then email your comments to 42237 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Clean up Tramp version check
Date: Mon, 6 Jul 2020 17:19:48 -0500

[Message part 1 (text/plain, inline)]

I actually saw the "not fit for" error message in the wild. The source
looked a little strange, so I cleaned it up. This shouldn't change any
behavior.

[0001-Clean-up-Tramp-version-check.patch (text/x-patch, attachment)]

Message #8 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Tue, 07 Jul 2020 09:19:00 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

Hi Nicholas,

Thanks for working on Tramp.

> I actually saw the "not fit for" error message in the wild. The source
> looked a little strange, so I cleaned it up. This shouldn't change any
> behavior.

Well, this code snippet has its history. Initially, it was (and still
is) used in the Tramp git repository, file aclocal.m4. And that code
snippet is also propagated to trampver.el via template trampver.el.in.

If you want to improve the code, please change only these two files. The
Tramp git repository is located at
<git://git.savannah.gnu.org/tramp.git> (see also the Tramp manual).

> +(ert-deftest tramp-test46-version ()
> +  "Check that the version check checks out."
> +  (tramp-check-version)
> +  (let ((emacs-version "24.3"))
> +    (should-error (tramp-check-version)))
> +  (let ((tramp-minimum-emacs-version "30.1"))
> +    (should-error (tramp-check-version))))

This is a nice extension :-)

Message #13 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: [PATCH] Clean up Tramp version check
Date: Sun, 12 Jul 2020 15:48:40 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

Hi Nicholas,

> This turned out to be quite a piece of code indeed. I think this
> change respects all the existing requirements. As I said before, this
> shouldn't change any behavior, apart from adding the new function and
> variable. Feel free to reject.

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?

Best regards, Michael.

Message #16 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: [PATCH] Clean up Tramp version check
Date: Sun, 12 Jul 2020 14:54:57 -0500

[Message part 1 (text/plain, inline)]

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

[0001-Clean-up-Tramp-version-check.patch (text/x-patch, attachment)]

Message #19 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: [PATCH] Clean up Tramp version check
Date: Mon, 13 Jul 2020 15:06:23 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

Hi Nicholas,

> I've attached a new version of the patch. Hopefully this one is right.

Still problems:

--8<---------------cut here---------------start------------->8---
# ./configure
configure: Tramp 2.5.0-pre
checking for gmake... no
checking for make... make
checking for reasonable make version... ok
checking whether make sets $(MAKE)... yes
checking for emacs... /usr/local/bin/emacs
./configure: command substitution: line 1880: unexpected EOF while looking for matching `''
./configure: command substitution: line 1881: syntax error: unexpected end of file
checking for /usr/local/bin/emacs version... ok
...
--8<---------------cut here---------------end--------------->8---

And this:

--8<---------------cut here---------------start------------->8---
# ./configure --with-emacs=~/src/emacs-25/src/emacs
configure: Tramp 2.5.0-pre
checking for gmake... no
checking for make... make
checking for reasonable make version... ok
checking whether make sets $(MAKE)... yes
checking for ~/src/emacs-25/src/emacs... no
configure: error: no not found
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

Message #22 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: [PATCH] Clean up Tramp version check
Date: Mon, 13 Jul 2020 15:55:37 +0200

Michael Albinus <michael.albinus <at> gmx.de> writes:

Hi Nicholas,

> # ./configure --with-emacs=~/src/emacs-25/src/emacs
> configure: Tramp 2.5.0-pre
> checking for gmake... no
> checking for make... make
> checking for reasonable make version... ok
> checking whether make sets $(MAKE)... yes
> checking for ~/src/emacs-25/src/emacs... no
> configure: error: no not found

Sorry, this wasn't caused by your patch. I've fixed this error.

Best regards, Michael.

Message #25 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Fri, 14 Aug 2020 02:17:12 -0700

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

> I've attached a new version of the patch. Hopefully this one is right.

Should the patch here be installed?  Michael mentioned an issue that
then turned out to be unrelated.

Best regards,
Stefan Kangas

Message #28 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org,
 Nicholas Drozd <nicholasdrozd <at> gmail.com>
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Sat, 15 Aug 2020 15:40:33 +0200

Stefan Kangas <stefan <at> marxist.se> writes:

Hi Stefan,

> Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:
>
>> I've attached a new version of the patch. Hopefully this one is right.
>
> Should the patch here be installed?  Michael mentioned an issue that
> then turned out to be unrelated.

This patch is intended for the *Tramp* git repo. No need to do smething
for Emacs (yet).

> Best regards,
> Stefan Kangas

Best regards, Michael.

Message #31 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Sun, 18 Oct 2020 17:13:07 +0200

Hi Nicholas,

Are you still working on this?

Best regards, Michael.

Message #34 received at 42237 <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 42237 <at> debbugs.gnu.org, tramp-devel <at> gnu.org
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Mon, 19 Oct 2020 12:07:35 -0500

Hi Michael, sorry about the delay. I swear I had this working before,
but now the new test isn't passing. Because this is just a clean-up
change that doesn't actually fix anything, I don't want to put any
more effort into it. Feel free to try and salvage the patch or to
trash it.

On Sun, Oct 18, 2020 at 10:13 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> Hi Nicholas,
>
> Are you still working on this?
>
> Best regards, Michael.

Message #39 received at 42237-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: tramp-devel <at> gnu.org, 42237-done <at> debbugs.gnu.org
Subject: Re: bug#42237: [PATCH] Clean up Tramp version check
Date: Tue, 20 Oct 2020 11:31:04 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

> Hi Michael,

Hi Nicholas,

> sorry about the delay. I swear I had this working before,
> but now the new test isn't passing. Because this is just a clean-up
> change that doesn't actually fix anything, I don't want to put any
> more effort into it. Feel free to try and salvage the patch or to
> trash it.

Thanks for the feedback, I'm closing the bug. Fell free to come back if
you want to continue.

Best regards, Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.