cannot buils libsass on Solaris 11

[drook]

libsass cannot be built on Solaris 11 (same story on less recent ones and probably on SmartOS/OmniOS) because Solaris has a SEC macro somewhere inside the system headers:

In file included from /usr/include/sys/select.h:28:0,
                 from /usr/include/sys/types.h:637,
                 from /usr/include/unistd.h:13,
                 from src/file.cpp:10:
src/units.hpp:38:5: error: expected identifier before numeric constant
     SEC = TIME,
     ^
src/units.hpp:38:5: error: expected ‘}’ before numeric constant
src/units.hpp:38:5: error: expected unqualified-id before numeric constant
In file included from src/ast.hpp:53:0,
                 from src/listize.hpp:7,
                 from src/functions.hpp:4,
                 from src/sass_functions.hpp:6,
                 from src/file.cpp:23:
src/units.hpp:69:1: error: expected declaration before ‘}’ token
 }
 ^
gmake: *** [src/file.o] Error 1

This can easily be fixed by changing all of the SEC entries in units.hpp/cpp to SECONDS. Please fix this (since the fix is easy and obvious), - I know Solaris is almost dead by now, and you don't officially support it, but it is still used in many places, and this bug affects the automatic building of node.js node-sass module, which otherwise would build it fine (yeah, I know node-sass has libsass bundled, so anyway the fix would work only on newer versions, but there's no other option anyway). Other than that - libsass seems to be fully working with this fix, at least in what comes when node-sass is using it's bindings.

[drook]

--- units.cpp.orig      2017-12-30 10:12:57.611866142 +0300
+++ units.cpp   2017-12-30 10:13:30.501632897 +0300
@@ -91,7 +91,7 @@
     else if (s == "rad")  return UnitType::RAD;
     else if (s == "turn") return UnitType::TURN;
     // time units
-    else if (s == "s")    return UnitType::SEC;
+    else if (s == "s")    return UnitType::SECONDS;
     else if (s == "ms")   return UnitType::MSEC;
     // frequency units
     else if (s == "Hz")   return UnitType::HERTZ;
@@ -120,7 +120,7 @@
       case UnitType::RAD:     return "rad";
       case UnitType::TURN:    return "turn";
       // time units
-      case UnitType::SEC:     return "s";
+      case UnitType::SECONDS:     return "s";
       case UnitType::MSEC:    return "ms";
       // frequency units
       case UnitType::HERTZ:   return "Hz";
--- units.hpp.orig      2017-12-30 10:13:03.372814084 +0300
+++ units.hpp   2017-12-30 10:13:23.057973304 +0300
@@ -35,7 +35,7 @@
     TURN,

     // time units
-    SEC = TIME,
+    SECONDS = TIME,
     MSEC,

     // frequency units

[saper]

Interesting, looks like we have a regression with #1308 coming back haunting us.

[drook]

Yup, in some node-sass libsass bundled versions I saw the SmartOS #define knob just to fix this SEC macro - for some reasons it doesn't fire on Solaris.

[xzyfer]

We welcome PRs to fix support issues for unsupported OSes :)

…

On 30 Dec. 2017 10:14 pm, "drook" ***@***.***> wrote: Yup, in some node-sass libsass bundled versions I saw the SmartOS #define knob just to fix this SEC macro - for some reasons it doesn't fire on Solaris. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2519 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWNdmwgJAHFtcgRHc7Dl15s39dg4Dks5tFhsfgaJpZM4RPiJh> .

[drook]

@xzyfer well, I posted my patch already - it's a working one, and libsass is working after it at least for node.js (I used it to build frontend for grafana, which isn't assembling without node-sass and especially without it's libsass binding binary). I've also tried to run the full test sequence for libsass, but seems like sassc has it's own building issues (and to my knowledge its building scripts are of poor quality), so I cannot say "I ran the full test sequence, just commit it in", but I'm looking into it.

[saper]

I wonder how this broke. The problem is that file.cpp includes <unistd.h> before we have a change to undefine __EXTENSIONS__ (this was done long ago in eae7c0a).

Moving #include "sass.hpp" to the top of that file seems to fix it...

[saper]

According to bisect, 3753948 broke this.

Also checked with SmartOS (17.3.0) and it is broken the same way.

[mgreter]

Thanks @saper for the bisect, hope the fix is according to your findings.

[alyssais]

This is still broken for me on SmartOS 2017Q4 :(

[xzyfer]

We don't have the capacity or hardware to support sunos. However we are open to pull requests that address compatibility issues.