Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Phillip Lord]

I've just found commit 73d213 has been added, which restores the
$EMACS behaviour in comint, shell and compile. In other words $EMACS
gets set to "t".

Unfortunately, this affects both cask and ert-runner which I had altered
to take account of and depend on the new behaviour for Emacs-25. In this
case, cask and ert-runner use $EMACS to mean "the location of the Emacs
to run". This will require explicit setting now.

In an truly entertaining piece of irony, my fixes to cask and ert-runner
were merged on 23 Mar, the same day as commit 73d213, which breaks the
fixes.

Not sure how to fix this, but the commentary prior to commit beaab89
says "we are going to rid of this binding, probably in Emacs 23". It
seems a shame to be three major versions wrong.

Phil

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Paul Eggert]

On 04/05/2016 04:05 AM, Phillip Lord wrote:
> Not sure how to fix this, but the commentary prior to commit beaab89
> says "we are going to rid of this binding, probably in Emacs 23". It
> seems a shame to be three major versions wrong.
Yes, this change has been frustrating to implement (see Bug#20484), 
because we need to wait until the new Bash is released and more widely 
available, as current Bash is incompatible with the change.

It sounds like you have a workaround in mind for cask and ert-runner. Is 
there some problem in installing the workaround? Presumably they need to 
work in older Emacs anyway, so they already have a way to deal with this 
issue.

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 04/05/2016 04:05 AM, Phillip Lord wrote:
>> Not sure how to fix this, but the commentary prior to commit beaab89
>> says "we are going to rid of this binding, probably in Emacs 23". It
>> seems a shame to be three major versions wrong.
> Yes, this change has been frustrating to implement (see Bug#20484), because we
> need to wait until the new Bash is released and more widely available, as
> current Bash is incompatible with the change.

I understand the problem, and did read the bug report.


> It sounds like you have a workaround in mind for cask and ert-runner. Is there
> some problem in installing the workaround? Presumably they need to work in
> older Emacs anyway, so they already have a way to deal with this issue.

Well, it doesn't work now. Cask used to just ditch $EMACS. Now it checks
the value of INSIDE_EMACS and does one thing in Emacs-24 (ditches
$EMACS) and another in Emacs-25 (uses $EMACS). In other words, I don't
have a workaround, I have limited the scope of the existing workaround
to Emacs-24. All of this semantics will now have to be backed out, since
it is all wrong.

The only saving grace is that I added $CASK_EMACS at the same time,
which avoids the name clash entirely. But this also needs to be
percolated to downstream tools like ert-runner.

Is there not a different solution to the bash problem? Passing
--noediting as an option would work, I think, and it should work with
both the old and new $EMACS handling of bash.

Phil

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Paul Eggert]

On 04/05/2016 09:38 AM, Phillip Lord wrote:
> I don't
> have a workaround, I have limited the scope of the existing workaround
> to Emacs-24.

Would something like the following be an adequate workaround? The idea 
is to have code that works with both Emacs 25 and Emacs 24 (and Emacs 23 
etc., for that matter):

     def get_emacs_from_env():
         emacs = ENVB.get(b'CASK_EMACS')
         if emacs:
             return emacs
         emacs = ENVB.get(b'EMACS')
         if emacs != 't':
             return emacs
         return None

> Is there not a different solution to the bash problem? Passing
> --noediting as an option would work, I think, and it should work with
> both the old and new $EMACS handling of bash.
>

Part of the worry here is that programs other than Bash will be 
affected, and that we need to give people some time to adjust to the 
changed behavior of Emacs, by announcing the planned change in Emacs 25 
and then actually making the change in a later Emacs release.

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 04/05/2016 09:38 AM, Phillip Lord wrote:
>> I don't
>> have a workaround, I have limited the scope of the existing workaround
>> to Emacs-24.
>
> Would something like the following be an adequate workaround? The idea is to
> have code that works with both Emacs 25 and Emacs 24 (and Emacs 23 etc., for
> that matter):
>
>     def get_emacs_from_env():
>         emacs = ENVB.get(b'CASK_EMACS')
>         if emacs:
>             return emacs
>         emacs = ENVB.get(b'EMACS')
>         if emacs != 't':
>             return emacs
>         return None

Maybe. Given that I spent several weeks working on the last pull
request, it may take a while for me to get up the energy to try.

The main problem is that use of cask tends to weave in and out of
different Emacs and also Makefiles. So, I use this:

EMACS ?= emacs
CASK ?= cask

-include makefile-local

all: install test

install:
	EMACS=$(EMACS) cask install

Which (did) work fine on Emacs-25 but now fails because EMACS the
invocation ends up as the entirely useless EMACS=t cask install

So, the problem isn't one just for cask, but also make. As other people
have said, the only safe solution is to not use the EMACS variable. This
was suggested for cask, and I have added that also. But, ultimately,
EMACS is the obvious name and CASK_EMACS is not.

Still, all of this has been said before.

>> Is there not a different solution to the bash problem? Passing
>> --noediting as an option would work, I think, and it should work with
>> both the old and new $EMACS handling of bash.
>>
>
> Part of the worry here is that programs other than Bash will be affected, and
> that we need to give people some time to adjust to the changed behavior of
> Emacs, by announcing the planned change in Emacs 25 and then actually making
> the change in a later Emacs release.

Emacs doesn't really have a formal deprecation policy, I think. So, even
if the change is announced in Emacs-25, there will still be breakage in
Emacs-26. As Stefan said "if we wait, nothing is ever going to move".

http://lists.gnu.org/archive/html/emacs-devel/2015-03/msg01034.html

I'm also not convinced that a bug report should be enough to revert a
change during pre-test. Reverting a breaking change that has been applied
deliberately is, effectively a breaking change all of its own.

Anyway, I guess bug #20202 needs re-opening.

Phil

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Paul Eggert]

On 04/05/2016 03:08 PM, Phillip Lord wrote:
> The main problem is that use of cask tends to weave in and out of
> different Emacs and also Makefiles. So, I use this:
>
> EMACS ?= emacs
> CASK ?= cask
>
> -include makefile-local
>
> all: install test
>
> install:
> 	EMACS=$(EMACS) cask install
>
> Which (did) work fine on Emacs-25

But that doesn't work on Emacs 24, right? And if you do something like 
the following:

     EMACS ?= emacs
     CASK ?= cask

     ifeq ($(EMACS),t)
       EMACS = emacs
     endif

     install:
         EMACS=$(EMACS) cask install

This should work with both Emacs 24 and emacs-25 current.

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 04/05/2016 03:08 PM, Phillip Lord wrote:
>> The main problem is that use of cask tends to weave in and out of
>> different Emacs and also Makefiles. So, I use this:
>>
>> EMACS ?= emacs
>> CASK ?= cask
>>
>> -include makefile-local
>>
>> all: install test
>>
>> install:
>> 	EMACS=$(EMACS) cask install
>>
>> Which (did) work fine on Emacs-25
>
> But that doesn't work on Emacs 24, right?

It doesn't have to work on Emacs-24. I don't use Emacs-24 to develop in,
only Emacs-25, at least partly because Emacs-25 doesn't (didn't) set
EMACS=t. 


> And if you do something like the
> following:
>
>     EMACS ?= emacs
>     CASK ?= cask
>
>     ifeq ($(EMACS),t)
>       EMACS = emacs
>     endif
>
>     install:
>         EMACS=$(EMACS) cask install
>
> This should work with both Emacs 24 and emacs-25 current.

At this juncture, I think the sensible thing is to give up on using
EMACS as both an environment variable and a makefile variable. Putting
hacks into my make file to distinguish between running inside and
outside of Emacs is not a solution. The same make file should work.

To repeat, my belief is that a behaviour that was installed
deliberately, and which has been on master/emacs-25 for a year should
not have been removed in pre-test. I think Stefan's patch needs
restoring. Are you prepared to do this?

Failing that, I will back out my changes to cask.

Phil

Re: Considered Harmful 73d213: "Comint, term, and compile new set Emacs"

[Paul Eggert]

On 04/06/2016 10:53 AM, Phillip Lord wrote:
> To repeat, my belief is that a behaviour that was installed
> deliberately, and which has been on master/emacs-25 for a year should
> not have been removed in pre-test. I think Stefan's patch needs
> restoring. Are you prepared to do this?

I think we should be more conservative and stick to the behavior that is 
in the latest stable Emacs release. Stefan wrote in May 2015 that the 
plan was to take this more-conservative approach; see 
<http://bugs.gnu.org/20484#40>. I'm sorry that you didn't know this and 
that it messed you up, but that's part of life when lived on the 
bleeding edge.

Re: Considered Harmful 73d213: 'Comint, term, and compile new set Emacs'

[Phillip Lord]

On Thu, April 7, 2016 2:05 am, Paul Eggert wrote:
> On 04/06/2016 10:53 AM, Phillip Lord wrote:
>
>> To repeat, my belief is that a behaviour that was installed
>> deliberately, and which has been on master/emacs-25 for a year should not
>> have been removed in pre-test. I think Stefan's patch needs restoring.
>> Are you prepared to do this?
>>
>
> I think we should be more conservative and stick to the behavior that is
> in the latest stable Emacs release. Stefan wrote in May 2015 that the plan
> was to take this more-conservative approach; see
> <http://bugs.gnu.org/20484#40>. I'm sorry that you didn't know this and
> that it messed you up, but that's part of life when lived on the bleeding
> edge.


I disagree. There was a 10 month gap between Stefans email and the
reversion. Making a significant breaking change in pre-test is, I think,
wrong.

Still, if there is nothing that can be done, I will submit a new bug
report to reopen 20202 which is now not fixed.

Phil

Re: Considered Harmful 73d213: 'Comint, term, and compile new set Emacs'

[Phillip Lord]

On Thu, April 7, 2016 2:05 am, Paul Eggert wrote:
> On 04/06/2016 10:53 AM, Phillip Lord wrote:
>
>> To repeat, my belief is that a behaviour that was installed
>> deliberately, and which has been on master/emacs-25 for a year should not
>> have been removed in pre-test. I think Stefan's patch needs restoring.
>> Are you prepared to do this?
>>
>
> I think we should be more conservative and stick to the behavior that is
> in the latest stable Emacs release. Stefan wrote in May 2015 that the plan
> was to take this more-conservative approach; see
> <http://bugs.gnu.org/20484#40>. I'm sorry that you didn't know this and
> that it messed you up, but that's part of life when lived on the bleeding
> edge.

As a better alternatively, would it be possible to find a solution to
20484 which does not involve EMACS=t?

This would both address 20484 and 20202, rather than robbing Peter to pay
Paul.

Phil

Re: Considered Harmful 73d213: 'Comint, term, and compile new set Emacs'

[Eli Zaretskii]

> Date: Thu, 7 Apr 2016 08:35:23 +0100
> From: "Phillip Lord" <phillip.lord@russet.org.uk>
> Cc: emacs-devel@gnu.org, eggert@penguin.cs.ucla.edu, monnier@iro.umontreal.ca,
> 	Phillip Lord <phillip.lord@russet.org.uk>
> 
> As a better alternatively, would it be possible to find a solution to
> 20484 which does not involve EMACS=t?
> 
> This would both address 20484 and 20202, rather than robbing Peter to pay
> Paul.

Bug#20484 is with a version of Bash that needs $EMACS to be set,
right?  If so, how can we solve it, if the changes to do that must be
made in Bash?

Re: Considered Harmful 73d213: 'Comint, term, and compile new set Emacs'

[Phillip Lord]

On Thu, April 7, 2016 4:07 pm, Eli Zaretskii wrote:
>> Date: Thu, 7 Apr 2016 08:35:23 +0100
>> From: "Phillip Lord" <phillip.lord@russet.org.uk>
>> Cc: emacs-devel@gnu.org, eggert@penguin.cs.ucla.edu,
>> monnier@iro.umontreal.ca, Phillip Lord <phillip.lord@russet.org.uk>
>>
>>
>> As a better alternatively, would it be possible to find a solution to
>> 20484 which does not involve EMACS=t?
>>
>>
>> This would both address 20484 and 20202, rather than robbing Peter to
>> pay Paul.
>>
>
> Bug#20484 is with a version of Bash that needs $EMACS to be set,
> right?

With all versions of bash. It's fixed in trunk, but not a release.

> If so, how can we solve it, if the changes to do that must be made
> in Bash?

There are a variety of ways, which I am happy to investigate. There is
always more than one way to skin a cat.

Phil

Re: Considered Harmful 73d213: 'Comint, term, and compile new set Emacs'

[Eli Zaretskii]

> Date: Thu, 7 Apr 2016 16:21:43 +0100
> From: "Phillip Lord" <phillip.lord@russet.org.uk>
> Cc: "Phillip Lord" <phillip.lord@russet.org.uk>,
>  eggert@cs.ucla.edu,
>  emacs-devel@gnu.org
> 
> > If so, how can we solve it, if the changes to do that must be made
> > in Bash?
> 
> There are a variety of ways, which I am happy to investigate. There is
> always more than one way to skin a cat.

We do want to be compatible with older versions of Bash.  If you can
suggest a solution that has this important property, please do.