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

Unbreak beginend(marks-test and beginend-narrowing-test #75

Merged
merged 1 commit into from
Mar 3, 2023
Merged

Conversation

aagon
Copy link
Contributor

@aagon aagon commented Feb 6, 2023

Hello !

The tests in the files test/beginend-narrowing-test.el and test/beginend-marks-test.el can fail under a specific set of conditions.

Emacs 28 has introduced native compilation of elisp code. In order to allow the advising of primitives (functions accessible from elisp implemented in C) to benefit from native compilation as well, these primitives have to be replaced by a trampoline that allows jumping to arbitrary native code (that we imagine to be the native compiled version of the elisp function the user has replaced the primitive with).

The test bed provided by the two aforementioned files advises the primitive message :

  (before-each
    (spy-on 'message)) ;; disable "Mark activated/Mark set" messages

If no trampoline is already present for the primitive at the time of evaluation of the form, a trampoline has to be compiled from it. This trampoline compilation fails with the following backtrace :

Traceback (most recent call last):
  normal-top-level()
  command-line()
  command-line-1(("-L" "." "-l" "buttercup" "-f" "buttercup-run-discover" "-...
  buttercup-run-discover()
  buttercup-run()
  buttercup--run-suites((#s(buttercup-suite "beginend in a dired buffer" nil...
  mapc(buttercup--run-suite (#s(buttercup-suite "beginend in a dired buffer"...
  buttercup--run-suite(#s(buttercup-suite "beginend" nil passed nil nil (255...
  buttercup--run-suite(#s(buttercup-suite "pushes mark" #s(buttercup-suite "...
  buttercup--run-suite(#s(buttercup-suite "when going to beginning" #s(butte...
  buttercup--run-spec(#s(buttercup-spec "if point is not at beginning" #s(bu...
  buttercup--update-with-funcall(#s(buttercup-spec "if point is not at begin...
  apply(buttercup--funcall (closure (t) nil (spy-on 'message)) nil)
  buttercup--funcall((closure (t) nil (spy-on 'message)))
  apply((closure (t) nil (spy-on 'message)) nil)
  (closure (t) nil (spy-on 'message))()
  spy-on(message)
  buttercup--spy-on-and-call-replacement(message (lambda (&rest args) nil nil))
  comp-subr-trampoline-install(message)
  comp-trampoline-compile(message)
  comp--native-compile((lambda (arg0 &rest arg1) (let ((f #'message)) (apply...
  signal(file-missing ((lambda (arg0 &rest arg1) (let ((f #'message)) (apply...
error: (file-missing (lambda (arg0 &rest arg1) (let ((f #'message)) (apply f arg0 arg1))) "Setting current directory" "No such file or directory" "/tmp/temp-fs-dGkO0V")

You probably never reach this error because you are lucky to always have the trampoline already. In specific build environments, where tests are run in a clean environment with a clean emacs (for instance the Debian build process of beginend from which the backtrace is taken), the trampoline is not there and needs to be compiled. You should be able to reproduce if you delete the trampoline (it should be named something like subr-message-<some hash>.eln) before launching the test.

We have no idea why the function comp--native-compile errors. We do know that we can simply work around the error by deactivating trampoline compilation for this primitive, thus allowing the tests to pass.

This patch does exactly that.

Let me know if you have any questions.

Best,

Aymeric Agon-Rambosson

@DamienCassou
Copy link
Owner

Thank you very much for investigating the problem, providing a solution and explaining it to me. I really appreciate the effort!

Would you mind copy/pasting your explanation into the commit message so we keep a trace?

Thank you again!

The tests in the files test/beginend-narrowing-test.el and
test/beginend-marks-test.el can fail under a specific set of conditions.

Emacs 28 has introduced native compilation of elisp code. In order to allow the
advising of primitives (functions accessible from elisp implemented in C) to
benefit from native compilation as well, these primitives have to be replaced by
a trampoline that allows jumping to arbitrary native code (that we imagine to be
the native compiled version of the elisp function the user has replaced the
primitive with).

The test bed provided by the two aforementioned files advises the primitive
`message' :

(before-each
    (spy-on 'message)) ;; disable "Mark activated/Mark set" messages

If no trampoline is already present for the primitive at the time of evaluation
of the form, a trampoline has to be compiled from it. This trampoline
compilation fails with the following backtrace :

Traceback (most recent call last):
  normal-top-level()
  command-line()
  command-line-1(("-L" "." "-l" "buttercup" "-f" "buttercup-run-discover" "-...
  buttercup-run-discover()
  buttercup-run()
  buttercup--run-suites((#s(buttercup-suite "beginend in a dired buffer" nil...
  mapc(buttercup--run-suite (#s(buttercup-suite "beginend in a dired buffer"...
  buttercup--run-suite(#s(buttercup-suite "beginend" nil passed nil nil (255...
  buttercup--run-suite(#s(buttercup-suite "pushes mark" #s(buttercup-suite "...
  buttercup--run-suite(#s(buttercup-suite "when going to beginning" #s(butte...
  buttercup--run-spec(#s(buttercup-spec "if point is not at beginning" #s(bu...
  buttercup--update-with-funcall(#s(buttercup-spec "if point is not at begin...
  apply(buttercup--funcall (closure (t) nil (spy-on 'message)) nil)
  buttercup--funcall((closure (t) nil (spy-on 'message)))
  apply((closure (t) nil (spy-on 'message)) nil)
  (closure (t) nil (spy-on 'message))()
  spy-on(message)
  buttercup--spy-on-and-call-replacement(message (lambda (&rest args) nil nil))
  comp-subr-trampoline-install(message)
  comp-trampoline-compile(message)
  comp--native-compile((lambda (arg0 &rest arg1) (let ((f #'message)) (apply...
  signal(file-missing ((lambda (arg0 &rest arg1) (let ((f #'message)) (apply...
error: (file-missing (lambda (arg0 &rest arg1) (let ((f #'message)) (apply f arg0 arg1))) "Setting current directory" "No such file or directory" "/tmp/temp-fs-dGkO0V")

You probably never reach this error because you are lucky to always have the
trampoline already. In specific build environments, where tests are run in a
clean environment with a clean emacs (for instance the Debian build process of
beginend from which the backtrace is taken), the trampoline is not there and
needs to be compiled. You should be able to reproduce if you delete the
trampoline (it should be named something like subr-message-<some hash>.eln)
before launching the test.

We have no idea why the function comp--native-compile errors. We do know that we
can simply work around the error by deactivating trampoline compilation for this
primitive, thus allowing the tests to pass.

This is what the commit does.
@aagon
Copy link
Contributor Author

aagon commented Mar 3, 2023

Sorry for the late answer !

Would you mind copy/pasting your explanation into the commit message so we keep a trace?

Done !

Copy link
Owner

@DamienCassou DamienCassou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

@DamienCassou DamienCassou merged commit e35c750 into DamienCassou:master Mar 3, 2023
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.

2 participants