basho / erlang_js

A linked-in driver for Erlang to Mozilla's Spidermonkey Javascript runtime.
Apache License 2.0
238 stars 88 forks source link

Only build `libjs.a` #67

Open Licenser opened 6 years ago

Licenser commented 6 years ago

The original makefile build libjs.a along with the js CLI tool which isn't needed. This change should fix the issue at #1669

russelldb commented 6 years ago

Does not work for me. On OS X 10.9.5 basho r16b02 10 I get

Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file js/src/config/SunOS5.1x_i86pc.mk.rej
Skipping patch
patching file js/src/config.mk
patching file js/src/jstypes.h
cat: ../../dist/Darwin_OPT.OBJ/nspr/Version: No such file or directory
make[3]: *** No rule to make target `libjs.a'.  Stop.
make[2]: *** [/Users/russell/dev/e/NHS/erlang_js/c_src/system/lib/libjs.a] Error 2
make[1]: *** [c_src] Error 2
ERROR: Command [compile] failed!

Full output here:

https://gist.github.com/russelldb/712ecbeb83700c8f40e49c6d38f74367

russelldb commented 6 years ago

I also tried just cherry-picking this commit against a branch from the 1.3.0 tag (As that is what we need for riak-2.2.5) and the result is the same as above.

bryanhuntesl commented 6 years ago

I can confirm the same behavior here. Neat fix for Freebsd, but a distraction. I suggest we just kill erlang_js on 2.2.6 and move on. The build system is so convoluted, we could be here forever with no meaningful benefit.

bryanhuntesl commented 6 years ago

If anyone's interested, I've got a safe (but extremely ugly) fix (basically an if/else on the same OS check code contained in the rest of the make files :

vagrant@freebsd:~/riak/deps/erlang_js/c_src % git diff Makefile | cat 
diff --git a/c_src/Makefile b/c_src/Makefile
index 169408a..fe9141b 100644
--- a/c_src/Makefile
+++ b/c_src/Makefile
@@ -20,6 +20,8 @@ INC_DIR    := $(SYSTEM_DIR)/include
 JS_DIR     := $(CURDIR)/js
 NSPR_DIR   := $(CURDIR)/nsprpub

+OS_ARCH    := $(subst /,_,$(shell uname -s | sed /\ /s//_/))
+
 # NSPR_SIXTYFOUR is defined in erlang_js/rebar.config

 js: $(LIB_DIR)/libjs.a
@@ -29,11 +31,24 @@ $(LIB_DIR)/libjs.a: $(LIB_DIR)/libnspr4.a
        @for I in patches/js-*.patch; do \
                ($(PATCH) -p1 < $${I} || echo "Skipping patch"); \
        done
+
+ifeq ($(OS_ARCH),FreeBSD)
+
+       @$(MAKE) -C $(JS_DIR)/src BUILD_OPT=1 JS_DIST=$(SYSTEM_DIR) \
+               JS_THREADSAFE=1 \
+               XCFLAGS="-DHAVE_VA_COPY -DVA_COPY=va_copy $(CFLAGS)" \
+               XLDFLAGS="$(LDFLAGS)" \
+               -f Makefile.ref libjs.a
+else
+
        @$(MAKE) -C $(JS_DIR)/src BUILD_OPT=1 JS_DIST=$(SYSTEM_DIR) \
                JS_THREADSAFE=1 \
                XCFLAGS="-DHAVE_VA_COPY -DVA_COPY=va_copy $(CFLAGS)" \
                XLDFLAGS="$(LDFLAGS)" \
                -f Makefile.ref
+
+endif
+
        @mkdir $(INC_DIR)/js
        @cp $(JS_DIR)/src/*.h $(INC_DIR)/js
        @cp $(JS_DIR)/src/*.tbl $(INC_DIR)/js

But seriously, lets just kill erlang_js.

bryanhuntesl commented 6 years ago

That last comment wont apply (tabs to space). Patch file attached. erlang_js_build_on_freebsd.patch.txt

Licenser commented 6 years ago

Sorry I kicked this off, it makes no sense of the make target working on one os but not another. I highly doubht it's worth the trouble when erlang_js gets killed anyway

Bob-The-Marauder commented 6 years ago

Although I hate having to do OS specific patches (reminds me of having to do dirty IE 6 fixes to otherwise well coded websites), sometimes we have no option in the short term. If we are killing erlang_js in the future, then I see no problem with having this as one of the final commits to make it easier in the future for people trying to build what will by then be an older version of Riak.

bryanhuntesl commented 6 years ago

@Licenser Riak should be running everywhere. Doesn't FreeBSD provide a mechanism for repeatably applying patches during the package build? What is best practice? I would like us to provide Riak for FreeBSD. If Riak is acceptable to that community, then surely everyone? Maybe we could get some contributions to improve the gnarly build system.