wellington / go-libsass

Go wrapper for libsass, the only Sass 3.5 compiler for Go
http://godoc.org/github.com/wellington/go-libsass
Apache License 2.0
206 stars 28 forks source link

"out_of_memory" redefined #57

Closed thatguystone closed 6 years ago

thatguystone commented 6 years ago

From a clean go-get:

$ go get github.com/wellington/go-libsass/
# github.com/wellington/go-libsass/libs
In file included from unity.cpp:22:0:
../libsass-build/json.cpp:49:0: warning: "out_of_memory" redefined
 #define out_of_memory() do {                    \

In file included from ../libsass-build/ast.hpp:52:0,
                 from ../libsass-build/ast.cpp:2,
                 from unity.cpp:4:
../libsass-build/util.hpp:15:0: note: this is the location of the previous definition
   #define out_of_memory() do {            \
drewwells commented 6 years ago

The amalgamation build I use is a memory beast. If you can't procure more ram, you may need to rebuild libsass and build go-libsass with a shared lib.

See the faq about using a shared lib https://github.com/wellington/go-libsass/blob/master/README.md

PR welcome to include this error on the readme!

thatguystone commented 6 years ago

This is a warning from the compiler, not the runtime. It seems that the out_of_memory macro is being redefined at libsass-build/json.cpp#L49. Since I'm not too familiar with how this library is structured, I didn't want to just remove the macro and submit a PR in case it's important for another type of build that I'm not using. Sorry if my original report was unclear.

drewwells commented 6 years ago

Oh I see. You can file a bug upstream about that. I haven't had any issues so far with this macro On Mon, Mar 26, 2018 at 6:15 PM Andrew Stone notifications@github.com wrote:

This is a warning from the compiler, not the runtime. It seems that the out_of_memory macro is being redefined at libsass-build/json.cpp#L49. Since I'm not too familiar with how this library is structured, I didn't want to just remove the macro and submit a PR in case it's important for another type of build that I'm not using. Sorry if my original report was unclear.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/wellington/go-libsass/issues/57#issuecomment-376342103, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOmE-nYaZTdfSRC1ccQYdOEt2T7yJLvks5tiXaVgaJpZM4S7-mz .

thatguystone commented 6 years ago

Upstream has merged the fix, and it looks like it will be in the next release.

drewwells commented 6 years ago

awesome you're a rock star 🎸

I'll look at the current issues when I get a chance and see if we can update this early

drewwells commented 6 years ago

So I tried pulling down 3.5 which would include this build. I'm getting some nasty errors though

In file included from cencode.c:2:
libs/../libsass-build/cencode.c:50:5: warning: declaration does not declare anything [-Wmissing-declarations]
libs/../libsass-build/cencode.c:64:5: warning: declaration does not declare anything [-Wmissing-declarations]
# github.com/wellington/go-libsass/libs
Undefined symbols for architecture x86_64:
  "Sass::traces_to_string(std::__1::vector<Sass::Backtrace, std::__1::allocator<Sass::Backtrace> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)", re
ferenced from:
      Sass::Eval::operator()(Sass::Warning*) in unity.cpp.o
      Sass::handle_error(Sass_Context*) in unity.cpp.o
  "Sass::Operators::op_numbers(Sass_OP, Sass::Number const&, Sass::Number const&, Sass_Inspect_Options, Sass::ParserState const&, bool)", referenced from:
      Sass::Eval::operator()(Sass::Binary_Expression*) in unity.cpp.o
      _sass_value_op in unity.cpp.o

I have mostly kept away from mutating the libsass source in go-libsass, but that may be necessary to get around this build error for now.

thatguystone commented 6 years ago

That's odd; I just pulled down 3.5.4, and it's compiling fine (some tests are failing) after the following changes.

diff --git a/libs/encoding.go b/libs/encoding.go
index 5f3137d..179dd9e 100644
--- a/libs/encoding.go
+++ b/libs/encoding.go
@@ -73,7 +73,7 @@ func MakeColor(c color.RGBA) UnionSassValue {

 // MakeList creates a Sass List
 func MakeList(len int) UnionSassValue {
-   return C.sass_make_list(C.size_t(len), C.SASS_COMMA)
+   return C.sass_make_list(C.size_t(len), C.SASS_COMMA, false)
 }

 // MakeMap cretes a new Sass Map
diff --git a/libs/unity.cpp b/libs/unity.cpp
index a7cb5a3..b6bdcf7 100644
--- a/libs/unity.cpp
+++ b/libs/unity.cpp
@@ -3,6 +3,7 @@

 #include "../libsass-build/ast.cpp"
 #include "../libsass-build/ast_fwd_decl.cpp"
+#include "../libsass-build/backtrace.cpp"
 #include "../libsass-build/base64vlq.cpp"
 #include "../libsass-build/bind.cpp"
 #include "../libsass-build/check_nesting.cpp"
@@ -23,6 +24,7 @@
 #include "../libsass-build/lexer.cpp"
 #include "../libsass-build/listize.cpp"
 #include "../libsass-build/node.cpp"
+#include "../libsass-build/operators.cpp"
 #include "../libsass-build/output.cpp"
 #include "../libsass-build/parser.cpp"
 #include "../libsass-build/plugins.cpp"
thatguystone commented 6 years ago

I've pushed my changes to my fork at thatguystone/go-libsass.

Some tests were broken by the removal of backtraces from error strings. Not sure if it's worth trying to add them back.

drewwells commented 6 years ago

I didn't try that tag, glad it works!

those tests are a bit fragile b/c they frequently change output upstream. Might be worth doing strings.HasPrefix() on them to verify the good parts of the errors. Submit a PR and I'll merge it