Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid mutating invocation-name #70

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Avoid mutating invocation-name #70

merged 1 commit into from
Oct 16, 2020

Conversation

kira-bruneau
Copy link

@kira-bruneau kira-bruneau commented Oct 14, 2020

Using the pgtk build I noticed that any command that tried to launch a new Emacs process using invocation-name would fail.

For example with async-start:

async-start: Process emacs not running

I found that invocation-name was set to emacs-28-0-50, when it should have been set to emacs-28.0.50. After a lot of debugging I found that it was being modified in lisp/term/pgtk-win.el when it should have been copied like it is in lisp/term/x-win.el.

This change just wraps invocation-name with copy-sequence like in lisp/term/x-win.el to avoid mutating the global variable.

@masm11
Copy link
Owner

masm11 commented Oct 15, 2020

Thanks for the hard debugging.

I'm going to push pgtk fork to fsf's repository. To do that, we need to do paperwork for copyright assignment.
I have done it.
Have you done the paperwork?

@titaniumbones
Copy link

titaniumbones commented Oct 15, 2020 via email

@kira-bruneau
Copy link
Author

@masm11 No, I haven't done the paperwork for copyright assignment, but it looks like I can just add (tiny change) to the commit message like @titaniumbones mentioned. https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html.

I will do that now.

* lisp/term/pgtk-win.el (window-system-initialization): copy invocation-name.
@kira-bruneau
Copy link
Author

I also just updated the commit to use my full name rather than my username.

@masm11 masm11 merged commit 09a84cd into masm11:pgtk Oct 16, 2020
@masm11
Copy link
Owner

masm11 commented Oct 16, 2020

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants