Use custom allocator on Mac OS X #112

Merged
merged 3 commits into from Jan 27, 2017

Projects

None yet

4 participants

@pnkfelix
Contributor

Use custom allocator on Mac OS X to uphold the rule that all calls to malloc/realloc/free go through their unexec_-prefixed variants on OS X.

The changes are hopefully properly conditionalized so that they do not affect non Mac OS X targets. It would be good for someone with a Linux box to verify this claim.


We will probably need to go through a similar process for targets that do not have native support for unexec; you can get a feel for the set of targets this means by doing % ls src/unex*.c:

src/unexaix.c		src/unexcw.c		src/unexhp9k800.c	src/unexsol.c
src/unexcoff.c		src/unexelf.c		src/unexmacosx.c	        src/unexw32.c

(Of course we do not actually care about all of those targets...)


Also, one more thing: I am not sure whether this is a bug in my PR, or a pre-existing problem with the remacs build system, but making changes to rust_src/alloc_unexecmacosx/src/lib.rs does not on its own suffice to cause make to properly rebuild the project. I had to manually go in and do cargo clean in rust_src before each of my builds before I observed changes I had made. So, that is something to keep an eye out for. (Or maybe there's some trivial fix to the Makefile; I didn't really investigate.)


Fix #38

@pnkfelix
Contributor

Oh I just realized: Since this PR is using #![feature(allocator)] in the crate it adds, that means we have to use the nightly channel for rustc to build remacs on OS X.

I don't think there's a good alternative in the short term (and I don't know if building on the stable/beta channels is actually a goal for the project), but I figured I should at least point out this drawback.

@jeandudey
Collaborator
jeandudey commented Jan 24, 2017 edited

It builds correctly on my machine (on linux with Rust nightly).

am not sure whether this is a bug in my PR, or a pre-existing problem with the remacs build system, but making changes to rust_src/alloc_unexecmacosx/src/lib.rs does not on its own suffice to cause make to properly rebuild the project.

That's a known problem, maybe an issue should be opened.

@pnkfelix
Contributor

Hmm I don't understand why this PR is causing Travis to fail in the manner it is reporting ...

Running 7 tests (2017-01-24 22:35:11+0000)
Library: `inotify'
   passed  1/7  file-notify-test00-availability
   passed  2/7  file-notify-test01-add-watch
   passed  3/7  file-notify-test02-events
Reverting buffer `file-notify-test22460g3k'.
Reverting buffer `file-notify-test22460g3k'.
   passed  4/7  file-notify-test03-autorevert
   passed  5/7  file-notify-test04-file-validity
   passed  6/7  file-notify-test05-dir-validity
Test file-notify-test07-backup backtrace:
  (progn (ert-fail (car (cdr (progn "Access slot \"condition\" of `(er
  (if (progn nil (and (vectorp result) (>= (length result) 6) (memq (a
  (let ((result (car --dolist-tail--))) (if (progn nil (and (vectorp r
  (while --dolist-tail-- (let ((result (car --dolist-tail--))) (if (pr
  (let ((--dolist-tail-- file-notify--test-results)) (while --dolist-t
  (let* ((events (if (consp (car (quote (... ...)))) (quote ((changed)
  (progn (setq file-notify--test-tmpfile (file-notify--test-make-temp-
  (unwind-protect (progn (setq file-notify--test-tmpfile (file-notify-
  (closure (t) nil (let ((fn-317 (function file-notify--test-local-ena
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test file-notify-test07-backup "Check th
  ert-run-or-rerun-test([cl-struct-ert--stats (not (tag :expensive-tes
  ert-run-tests((not (tag :expensive-test)) #[385 "\306�\307\"\203G\2
  ert-run-tests-batch((not (tag :expensive-test)))
  ert-run-tests-batch-and-exit((not (tag :expensive-test)))
  eval((ert-run-tests-batch-and-exit (quote (not (tag :expensive-test)
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/filenotify-tests.el"
  command-line()
  normal-top-level()
Test file-notify-test07-backup condition:
    (ert-test-failed
     ((should
       (equal
	(car file-notify--test-event)
	file-notify--test-desc))
      :form
      (equal
       (2)
       (2 . "file-notify-test22460g-Y"))
      :value nil :explanation
      (one-list-proper-one-improper
       (2)
       (2 . "file-notify-test22460g-Y"))))
   FAILED  7/7  file-notify-test07-backup

Ran 7 tests, 6 results as expected, 1 unexpected (2017-01-24 22:35:21+0000)

1 unexpected results:
   FAILED  file-notify-test07-backup

ERROR: lisp/filenotify-tests.log

Do any of these tests fail sporadically on their own? In the rust project itself we sometimes re-run failing travis runs, but usually the failing tests in those cases are known to have unpredictable failures.

@DavidDeSimone
Contributor

@pnkfelix I've seen that specific test fail sporadically on my currently open PR, #106.

@Wilfred
Owner
Wilfred commented Jan 25, 2017

Awesome, thanks for this.

Regarding nightly, ideally we'd build on stable where possible, but I agree I can't see an alternative here. I think we should pin to a specific version of nightly on Travis so we can guarantee that new contributors can get a working build.

@Wilfred

This looks good to me.

If you rebase on master, the test failure should hopefully disappear as we're no longer running the test suite in parallel.

We can then merge this! 👯

@pnkfelix
Contributor

@Wilfred okay, rebased!

@pnkfelix
Contributor

That's a known problem, maybe an issue should be opened.

Hmm, actually, maybe I need to just do something similar to what #108 did, but add in rust_src/alloc_unexecmacosx ?

@pnkfelix
Contributor

(in any case I don't think that needs to hold up landing this PR!)

@Wilfred Wilfred merged commit 0d60ba0 into Wilfred:master Jan 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Wilfred
Owner
Wilfred commented Jan 27, 2017

Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment