all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
       [not found] ` <20190321155614.E6343209B2@vcs0.savannah.gnu.org>
@ 2019-04-02 19:02   ` Daniel Colascione
  2019-04-02 19:24     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Colascione @ 2019-04-02 19:02 UTC (permalink / raw)
  To: emacs-devel, Eli Zaretskii

> branch: master
> commit 107215596c1a8edfb239a88850d822642bc0e4af
> Author: Eli Zaretskii <eliz@gnu.org>
> Commit: Eli Zaretskii <eliz@gnu.org>
>
>     Avoid duplicate entries in process-environment after re-dumping
>
>     * src/pdumper.c (Fdump_emacs_portable): Reset
>     process-environment to nil.  (Bug#34936)
> ---
>  src/pdumper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/pdumper.c b/src/pdumper.c
> index fbf17d1..f459d97 100644
> --- a/src/pdumper.c
> +++ b/src/pdumper.c
> @@ -4025,6 +4025,12 @@ types.  */)
>    Lisp_Object symbol = intern ("command-line-processed");
>    specbind (symbol, Qnil);
>
> +  /* Reset process-environment -- this is for when they re-dump a
> +     pdump-restored emacs, since set_initial_environment wants always
> +     to cons it from scratch.  */
> +  Vprocess_environment = Qnil;

Don't we want to reset process-environment to its old value in
dump_unwind_cleanup?

> +  garbage_collect ();
> +
>    CHECK_STRING (filename);
>    filename = Fexpand_file_name (filename, Qnil);
>    filename = ENCODE_FILE (filename);

Does it make sense to move this chunk before the
garbage-collect-until-we-run-all-finalizers loop above? That way, we'd run
one fewer GC.




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

* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
  2019-04-02 19:02   ` [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping Daniel Colascione
@ 2019-04-02 19:24     ` Eli Zaretskii
  2019-04-02 20:11       ` Daniel Colascione
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2019-04-02 19:24 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> Date: Tue, 2 Apr 2019 12:02:55 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> 
> > +  /* Reset process-environment -- this is for when they re-dump a
> > +     pdump-restored emacs, since set_initial_environment wants always
> > +     to cons it from scratch.  */
> > +  Vprocess_environment = Qnil;
> 
> Don't we want to reset process-environment to its old value in
> dump_unwind_cleanup?

You are thinking about re-dumping from an interactive session?  In
that case, probably yes.  But currently we only support dumping from
batch sessions, and in that case I don't see a need to restore
process-environment, am I missing something?

> > +  garbage_collect ();
> > +
> >    CHECK_STRING (filename);
> >    filename = Fexpand_file_name (filename, Qnil);
> >    filename = ENCODE_FILE (filename);
> 
> Does it make sense to move this chunk before the
> garbage-collect-until-we-run-all-finalizers loop above? That way, we'd run
> one fewer GC.

Probably.  I just wanted to do this as close to the actual dumping as
possible, but maybe it's not that important.



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

* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
  2019-04-02 19:24     ` Eli Zaretskii
@ 2019-04-02 20:11       ` Daniel Colascione
  2019-04-03  9:22         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Colascione @ 2019-04-02 20:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Colascione, emacs-devel

>> Date: Tue, 2 Apr 2019 12:02:55 -0700
>> From: "Daniel Colascione" <dancol@dancol.org>
>>
>> > +  /* Reset process-environment -- this is for when they re-dump a
>> > +     pdump-restored emacs, since set_initial_environment wants always
>> > +     to cons it from scratch.  */
>> > +  Vprocess_environment = Qnil;
>>
>> Don't we want to reset process-environment to its old value in
>> dump_unwind_cleanup?
>
> You are thinking about re-dumping from an interactive session?

Yes

> In
> that case, probably yes.  But currently we only support dumping from
> batch sessions, and in that case I don't see a need to restore
> process-environment, am I missing something?

We "support" dumping only from dedicated batch instances in that for now
we should consider only bugs in that use case release-blocking, but I
don't want to gratuitously break other use cases like dumping interactive
sessions without killing them, since these use cases are meant to work and
we'll give them the same level of support someday soon.

>
>> > +  garbage_collect ();
>> > +
>> >    CHECK_STRING (filename);
>> >    filename = Fexpand_file_name (filename, Qnil);
>> >    filename = ENCODE_FILE (filename);
>>
>> Does it make sense to move this chunk before the
>> garbage-collect-until-we-run-all-finalizers loop above? That way, we'd
>> run
>> one fewer GC.
>
> Probably.  I just wanted to do this as close to the actual dumping as
> possible, but maybe it's not that important.
>
>





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

* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
  2019-04-02 20:11       ` Daniel Colascione
@ 2019-04-03  9:22         ` Eli Zaretskii
  2019-04-03 16:58           ` Daniel Colascione
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2019-04-03  9:22 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> Date: Tue, 2 Apr 2019 13:11:53 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> Cc: "Daniel Colascione" <dancol@dancol.org>,
>  emacs-devel@gnu.org
> 
> >> Don't we want to reset process-environment to its old value in
> >> dump_unwind_cleanup?
> >
> > You are thinking about re-dumping from an interactive session?
> 
> Yes
> 
> > In
> > that case, probably yes.  But currently we only support dumping from
> > batch sessions, and in that case I don't see a need to restore
> > process-environment, am I missing something?
> 
> We "support" dumping only from dedicated batch instances in that for now
> we should consider only bugs in that use case release-blocking, but I
> don't want to gratuitously break other use cases like dumping interactive
> sessions without killing them, since these use cases are meant to work and
> we'll give them the same level of support someday soon.

OK, but is there any reason dump_unwind_cleanup doesn't restore
post-gc-hook?  Omission?



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

* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
  2019-04-03  9:22         ` Eli Zaretskii
@ 2019-04-03 16:58           ` Daniel Colascione
  2019-04-03 17:32             ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Colascione @ 2019-04-03 16:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Colascione, emacs-devel

>> Date: Tue, 2 Apr 2019 13:11:53 -0700
>> From: "Daniel Colascione" <dancol@dancol.org>
>> Cc: "Daniel Colascione" <dancol@dancol.org>,
>>  emacs-devel@gnu.org
>>
>> >> Don't we want to reset process-environment to its old value in
>> >> dump_unwind_cleanup?
>> >
>> > You are thinking about re-dumping from an interactive session?
>>
>> Yes
>>
>> > In
>> > that case, probably yes.  But currently we only support dumping from
>> > batch sessions, and in that case I don't see a need to restore
>> > process-environment, am I missing something?
>>
>> We "support" dumping only from dedicated batch instances in that for now
>> we should consider only bugs in that use case release-blocking, but I
>> don't want to gratuitously break other use cases like dumping
>> interactive
>> sessions without killing them, since these use cases are meant to work
>> and
>> we'll give them the same level of support someday soon.
>
> OK, but is there any reason dump_unwind_cleanup doesn't restore
> post-gc-hook?  Omission?

Not restoring post-gc-hook is a bug. Thanks for spotting that!





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

* Re: [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping
  2019-04-03 16:58           ` Daniel Colascione
@ 2019-04-03 17:32             ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2019-04-03 17:32 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> Date: Wed, 3 Apr 2019 09:58:35 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> Cc: "Daniel Colascione" <dancol@dancol.org>,
>  emacs-devel@gnu.org
> 
> >> We "support" dumping only from dedicated batch instances in that for now
> >> we should consider only bugs in that use case release-blocking, but I
> >> don't want to gratuitously break other use cases like dumping
> >> interactive
> >> sessions without killing them, since these use cases are meant to work
> >> and
> >> we'll give them the same level of support someday soon.
> >
> > OK, but is there any reason dump_unwind_cleanup doesn't restore
> > post-gc-hook?  Omission?
> 
> Not restoring post-gc-hook is a bug. Thanks for spotting that!

Thanks, I fixed both issues.



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

end of thread, other threads:[~2019-04-03 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190321155613.6127.22574@vcs0.savannah.gnu.org>
     [not found] ` <20190321155614.E6343209B2@vcs0.savannah.gnu.org>
2019-04-02 19:02   ` [Emacs-diffs] master 1072155: Avoid duplicate entries in process-environment after re-dumping Daniel Colascione
2019-04-02 19:24     ` Eli Zaretskii
2019-04-02 20:11       ` Daniel Colascione
2019-04-03  9:22         ` Eli Zaretskii
2019-04-03 16:58           ` Daniel Colascione
2019-04-03 17:32             ` Eli Zaretskii

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.