unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#15647: 24.3.50; python.el does not clean up temp file
@ 2013-10-18 18:51 Jorgen Schaefer
  2013-10-19 17:04 ` Andreas Röhler
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jorgen Schaefer @ 2013-10-18 18:51 UTC (permalink / raw)
  To: 15647

python.el will use temporary files to communicate with the inferior
python process. These temporary files are never cleaned up.

To reproduce, simply run M-x run-python and check the temp dir. It has a
"py?????" file in it. Kill the buffer and repeat, such files will
accumulate.

The following simple patch will delete the file after loading it.


--- lisp/progmodes/python.el	2013-10-07 18:51:26 +0000
+++ lisp/progmodes/python.el	2013-10-18 18:47:02 +0000
@@ -2048,6 +2048,8 @@
                (file-name (or (buffer-file-name) temp-file-name)))
           (with-temp-file temp-file-name
             (insert string)
+            (insert (format "\n\nimport os ; os.remove('''%s''')\n"
+                            temp-file-name))
             (delete-trailing-whitespace))
           (python-shell-send-file file-name process temp-file-name))
       (comint-send-string process string)





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-18 18:51 bug#15647: 24.3.50; python.el does not clean up temp file Jorgen Schaefer
@ 2013-10-19 17:04 ` Andreas Röhler
  2013-10-19 21:51   ` Stefan Monnier
  2013-10-20  2:01 ` Glenn Morris
  2013-11-23  3:13 ` Glenn Morris
  2 siblings, 1 reply; 15+ messages in thread
From: Andreas Röhler @ 2013-10-19 17:04 UTC (permalink / raw)
  To: 15647

Am 18.10.2013 20:51, schrieb Jorgen Schaefer:
> python.el will use temporary files to communicate with the inferior
> python process. These temporary files are never cleaned up.
>
> To reproduce, simply run M-x run-python and check the temp dir. It has a
> "py?????" file in it. Kill the buffer and repeat, such files will
> accumulate.
>
> The following simple patch will delete the file after loading it.
>
>
> --- lisp/progmodes/python.el	2013-10-07 18:51:26 +0000
> +++ lisp/progmodes/python.el	2013-10-18 18:47:02 +0000
> @@ -2048,6 +2048,8 @@
>                  (file-name (or (buffer-file-name) temp-file-name)))
>             (with-temp-file temp-file-name
>               (insert string)
> +            (insert (format "\n\nimport os ; os.remove('''%s''')\n"
> +                            temp-file-name))
>               (delete-trailing-whitespace))
>             (python-shell-send-file file-name process temp-file-name))
>         (comint-send-string process string)
>
>
>
>

Beside of other nasty side-effects to expect, when the code to send is extended that way:
Why relying on Python process in order to delete a file?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-19 17:04 ` Andreas Röhler
@ 2013-10-19 21:51   ` Stefan Monnier
  2013-10-20  7:51     ` Andreas Röhler
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2013-10-19 21:51 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 15647

> Beside of other nasty side-effects to expect, when the code to send is
> extended that way: Why relying on Python process in order to delete
> a file?

IIUC the purpose is to make sure it's erased and to make sure it's
erased *after* the use.  Whether it does that, I don't know.
But doing it in Elisp would otherwise require detecting the next prompt
to figure out when the temp file can be erased.


        Stefan





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-18 18:51 bug#15647: 24.3.50; python.el does not clean up temp file Jorgen Schaefer
  2013-10-19 17:04 ` Andreas Röhler
@ 2013-10-20  2:01 ` Glenn Morris
  2013-11-23  3:13 ` Glenn Morris
  2 siblings, 0 replies; 15+ messages in thread
From: Glenn Morris @ 2013-10-20  2:01 UTC (permalink / raw)
  To: Jorgen Schaefer; +Cc: 15647


Thanks. It bugs me that every `make check' in Emacs trunk leaves several
python temp files behind, so I hope something like this gets applied.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-19 21:51   ` Stefan Monnier
@ 2013-10-20  7:51     ` Andreas Röhler
  2013-10-20 15:33       ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Andreas Röhler @ 2013-10-20  7:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 15647

Am 19.10.2013 23:51, schrieb Stefan Monnier:
>> Beside of other nasty side-effects to expect, when the code to send is
>> extended that way: Why relying on Python process in order to delete
>> a file?
>
> IIUC the purpose is to make sure it's erased and to make sure it's
> erased *after* the use.  Whether it does that, I don't know.
> But doing it in Elisp would otherwise require detecting the next prompt

Don't think so. Once the file is sent to process, it's sent.

A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.
OTOH python-mode will do an error-checking anyway, will point to it, so deleting might made depend from this.

For several reasons in favor of an Emacs Lisp solution as far as possible.
Executing code should not change the state of Python more than the code demands.
If a module isn't there, Emacs should it not provide it slightly - unless there is no other way to make things work.

If Emacs changes the Python state without the user is alerted, error tracking of Python itself might puzzle people.

Well, in the precise case it's probably rather a sanitary issue, a matter of code quality.

> to figure out when the temp file can be erased.
>
>
>          Stefan
>






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20  7:51     ` Andreas Röhler
@ 2013-10-20 15:33       ` Eli Zaretskii
  2013-10-20 16:25         ` Andreas Röhler
                           ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Eli Zaretskii @ 2013-10-20 15:33 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 15647

> Date: Sun, 20 Oct 2013 09:51:32 +0200
> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
> Cc: 15647@debbugs.gnu.org
> 
> > IIUC the purpose is to make sure it's erased and to make sure it's
> > erased *after* the use.  Whether it does that, I don't know.
> > But doing it in Elisp would otherwise require detecting the next prompt
> 
> Don't think so. Once the file is sent to process, it's sent.

That's Unix-speak: deleting a file that is potentially in use.
Outside of Posix, deleting a file immediately after submitting it to a
process will at best fail (because the process is still using it), and
at worst cause trouble (because the process didn't yet have enough
time to even open the file).

In fact, in this case, even on Unix this proposal will cause trouble,
because the command sent to Python might take time to execute on the
Python side, and we might already have deleted the file when Python
tries to open it.

I think in this case the better place to delete the file is in
python-shell-send-file, as part of the command sent to Python, because
that's where we know that the file was used up and closed by the
Python interpreter.

> A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
> Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.

No, it's an argument to add independent logging facilities to
python.el, IMO.  IOW, if python.el wants to have debugging features,
it should have them without relying on the Python interpreter and
without interfering with the "normal" workflow (whereby the file is
deleted after being used).  Relying on a temporary file to remain in
the filesystem for prolonged periods of time is not a good idea
anyway.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 15:33       ` Eli Zaretskii
@ 2013-10-20 16:25         ` Andreas Röhler
  2013-10-20 18:27         ` Glenn Morris
  2013-11-23 19:39         ` Glenn Morris
  2 siblings, 0 replies; 15+ messages in thread
From: Andreas Röhler @ 2013-10-20 16:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 15647

Am 20.10.2013 17:33, schrieb Eli Zaretskii:
>> Date: Sun, 20 Oct 2013 09:51:32 +0200
>> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
>> Cc: 15647@debbugs.gnu.org
>>
>>> IIUC the purpose is to make sure it's erased and to make sure it's
>>> erased *after* the use.  Whether it does that, I don't know.
>>> But doing it in Elisp would otherwise require detecting the next prompt
>>
>> Don't think so. Once the file is sent to process, it's sent.
>
> That's Unix-speak: deleting a file that is potentially in use.
> Outside of Posix, deleting a file immediately after submitting it to a
> process will at best fail (because the process is still using it), and
> at worst cause trouble (because the process didn't yet have enough
> time to even open the file).
>
> In fact, in this case, even on Unix this proposal will cause trouble,
> because the command sent to Python might take time to execute on the
> Python side, and we might already have deleted the file when Python
> tries to open it.
>
> I think in this case the better place to delete the file is in
> python-shell-send-file, as part of the command sent to Python, because
> that's where we know that the file was used up and closed by the
> Python interpreter.
>

Agreed.

>> A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
>> Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.
>
> No, it's an argument to add independent logging facilities to
> python.el, IMO.  IOW, if python.el wants to have debugging features,
> it should have them without relying on the Python interpreter and
> without interfering with the "normal" workflow (whereby the file is
> deleted after being used).  Relying on a temporary file to remain in
> the filesystem for prolonged periods of time is not a good idea
> anyway.
>

Agreed also. However don't think it's a big issue if the one or other file remains. Just it should not happen all the time.
BTW python-mode.el does this behind an unwind-protect, so errors should not go into the way.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 15:33       ` Eli Zaretskii
  2013-10-20 16:25         ` Andreas Röhler
@ 2013-10-20 18:27         ` Glenn Morris
  2013-10-20 18:31           ` Glenn Morris
  2013-10-20 19:12           ` Eli Zaretskii
  2013-11-23 19:39         ` Glenn Morris
  2 siblings, 2 replies; 15+ messages in thread
From: Glenn Morris @ 2013-10-20 18:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 15647

Eli Zaretskii wrote:

> In fact, in this case, even on Unix this proposal will cause trouble,
> because the command sent to Python might take time to execute on the
> Python side, and we might already have deleted the file when Python
> tries to open it.

Whut?
It's python itself that deletes the file, as the last thing in the file.
So it's already done whatever it was supposed to actually do.
It's like a self-deleting shell script.
FWIW, the patch looks fine to me (I say this was zero testing of it).





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 18:27         ` Glenn Morris
@ 2013-10-20 18:31           ` Glenn Morris
  2013-10-20 19:12           ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Glenn Morris @ 2013-10-20 18:31 UTC (permalink / raw)
  To: 15647

Glenn Morris wrote:

> FWIW, the patch looks fine to me (I say this was zero testing of it).
                                              s/was/with





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 18:27         ` Glenn Morris
  2013-10-20 18:31           ` Glenn Morris
@ 2013-10-20 19:12           ` Eli Zaretskii
  2013-10-21  0:53             ` Glenn Morris
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2013-10-20 19:12 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 15647

> From: Glenn Morris <rgm@gnu.org>
> Cc: Andreas Röhler <andreas.roehler@easy-emacs.de>,
>   15647@debbugs.gnu.org
> Date: Sun, 20 Oct 2013 14:27:46 -0400
> 
> Eli Zaretskii wrote:
> 
> > In fact, in this case, even on Unix this proposal will cause trouble,
> > because the command sent to Python might take time to execute on the
> > Python side, and we might already have deleted the file when Python
> > tries to open it.
> 
> Whut?
> It's python itself that deletes the file, as the last thing in the file.

I was talking about Andreas's proposal, not about the patch.  Sorry
for being unclear.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 19:12           ` Eli Zaretskii
@ 2013-10-21  0:53             ` Glenn Morris
  0 siblings, 0 replies; 15+ messages in thread
From: Glenn Morris @ 2013-10-21  0:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 15647

Eli Zaretskii wrote:

> I was talking about Andreas's proposal, not about the patch.  Sorry
> for being unclear.

Probably my bad for not reading properly.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-18 18:51 bug#15647: 24.3.50; python.el does not clean up temp file Jorgen Schaefer
  2013-10-19 17:04 ` Andreas Röhler
  2013-10-20  2:01 ` Glenn Morris
@ 2013-11-23  3:13 ` Glenn Morris
  2013-11-23  4:21   ` Glenn Morris
  2 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2013-11-23  3:13 UTC (permalink / raw)
  To: 15647-done

Version: 24.4

Thanks; applied.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-11-23  3:13 ` Glenn Morris
@ 2013-11-23  4:21   ` Glenn Morris
  2013-11-23 13:34     ` Andreas Röhler
  0 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2013-11-23  4:21 UTC (permalink / raw)
  To: 15647; +Cc: forcer


Actually on second thought, un-applied and reopened.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-11-23  4:21   ` Glenn Morris
@ 2013-11-23 13:34     ` Andreas Röhler
  0 siblings, 0 replies; 15+ messages in thread
From: Andreas Röhler @ 2013-11-23 13:34 UTC (permalink / raw)
  To: 15647

Am 23.11.2013 05:21, schrieb Glenn Morris:
>
> Actually on second thought, un-applied and reopened.
>
>
>
>

Seems wise. Nonetheless, may you tell your reflections?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#15647: 24.3.50; python.el does not clean up temp file
  2013-10-20 15:33       ` Eli Zaretskii
  2013-10-20 16:25         ` Andreas Röhler
  2013-10-20 18:27         ` Glenn Morris
@ 2013-11-23 19:39         ` Glenn Morris
  2 siblings, 0 replies; 15+ messages in thread
From: Glenn Morris @ 2013-11-23 19:39 UTC (permalink / raw)
  To: 15647-done

Version: 24.4

Eli Zaretskii wrote:

> I think in this case the better place to delete the file is in
> python-shell-send-file, as part of the command sent to Python, because
> that's where we know that the file was used up and closed by the
> Python interpreter.

Agreed; done.





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-11-23 19:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-18 18:51 bug#15647: 24.3.50; python.el does not clean up temp file Jorgen Schaefer
2013-10-19 17:04 ` Andreas Röhler
2013-10-19 21:51   ` Stefan Monnier
2013-10-20  7:51     ` Andreas Röhler
2013-10-20 15:33       ` Eli Zaretskii
2013-10-20 16:25         ` Andreas Röhler
2013-10-20 18:27         ` Glenn Morris
2013-10-20 18:31           ` Glenn Morris
2013-10-20 19:12           ` Eli Zaretskii
2013-10-21  0:53             ` Glenn Morris
2013-11-23 19:39         ` Glenn Morris
2013-10-20  2:01 ` Glenn Morris
2013-11-23  3:13 ` Glenn Morris
2013-11-23  4:21   ` Glenn Morris
2013-11-23 13:34     ` Andreas Röhler

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).