Build with the unbundled libsass

[saper]

If LIBSASS_EXT is set to auto in the proces
environment, use pkg-config to locate
system libsass to be linked.

This way there is no need to checkout
a src/libsass submodule at all and
the library from the package management
system can be used.

Otherwise fallback to the bundled libsass.

By default, use the bundled library.

PR: #139
PR: #389
PR: #744

[am11]

Use single quotes in join('').

[am11]

Indentation check.

[am11]

Thanks for the contribution. 👌

I think, we can use LIBSASS_PKG_RELEASE instead of PKGCONFIG, which is a very general name.

Also, in JS code, you can just check the presence of process.env.LIBSASS_PKG_RELEASE and emit YES and NO. This way user will just need to set (export) or unset the environment variable.

@mgreter, do we already have RPM packages available for RH distros? Any thoughts about this PR?

[saper]

I also wonder whether to have another switch to link external static libsass.a library instead of the shared one.

Don't have Windows running right now, can you check the build? I am not sure about this

      'product_prefix': 'lib',

in libsass.gyp

[am11]

We can probably have SASS_C_LIB_EXT (sass-c-lib-external, does it sound reasonable?) expects string value and SASS_C_LIB_SHARED just need to be set (value will be ignore, its presence would matter).

Windows CI (AppVeyor) is also configured. The jobs are running at https://ci.appveyor.com/project/sass/node-sass/build/719 (their free package is slower than a tortoise, so we check back after an hour or so). Also, it will show that it fails some tests (due to the timeout issue), but the only thing that matters is it builds and start running tests.

[am11]

AppVeyor is fine. (ignore the io.js job as well -- the last one -- that is due to nodejs/node#751).
Welcome to Windows hell! 🔥

[am11]

Regarding 'product_prefix', I think we can just skip it? It seems like node-gyp's code has its instance defined as emtpty string (link). Not sure if that is relevant though..

[saper]

You are right, let's drop the prefix as it generates liblibsass.a on Windows. We can live with sass.a for a bundled library on UNIX-likes.

What about this:

Variable LIBSASS_EXT

• auto - detect library and include path using pkg-config
• yes - use
  LIBSASS_LDFLAGS and LIBSASS_CFLAGS provided by the builder/packager
• no - (default) use bundled sass.a

Variable LIBSASS_STATIC (only used if LIBSASS_EXT!=no)

• yes - link statically libsass.a
• no - (default?) link to a shared library libsass.so

For http://github.com/sass/node-sass-binaries binary building I will be using LIBSASS_EXT=auto and LIBSASS_STATIC=yes to make sure no dependencies will be needed for the dropped binary.

[am11]

This is a sound idea. I think we can try it for beta and see the reaction from masses. :)

[saper]

I have given up on LIBSASS_STATIC, so there will be no such option (because it would require some deeper gyp magic to do it properly and portably). I can achieve my goal by linking libsass.a directly.

[saper]

Re "": too much python recently, sorry.

I decided to lowercase gyp variables because the convention there is that uppercase are only pre-defined variables (like OS). And by (another) convention we use uppercase environment variables.

[am11]

Thanks!

[am11]

LGTM!

BTW, I am going forward with v3.0.0-alpha.1 with few fixes including these build enhancements. Do you have anything else in mind, you need to add? You can send PR for FreeBSD binaries anytime on node-sass-binaries repo (and update the coverage matrix in the README there).

//cc @xzyfer

[xzyfer]

👍
On 18 Mar 2015 09:14, "Adeel Mujahid" notifications@github.com wrote:

LGTM!

BTW, I am going forward with v3.0.0-alpha.1 with few fixes including
these build enhancements. Do you have anything else in mind, you need to
add? You can send PR for FreeBSD binaries anytime on node-sass-binaries
https://github.com/sass/node-sass-binaries/ repo (and update the
coverage matrix in the README there).

//cc @xzyfer https://github.com/xzyfer

—
Reply to this email directly or view it on GitHub
#763 (comment).

[saper]

Thanks! This is great.

I have run into tiny issue that prevents me from doing

$ cd /some/fresh/directory
$ npm install /existing/deployment/node-sass

without npm refetching or recompiling stuff again (not sure if we have an open PR about it).

In short: I think we should check the binary first if it is already there and then proceed with download/rebuild. What do you think? Is it worth opening another PR?

[am11]

@saper, I think we don't have this feature at all:

Installing node-sass from local directory or network path while avoiding re-download of the binary.

If you are thinking about a use-case, like we can install Ruby's gem locally (from a .gem file, with no internet connection), then I have to admit, I just tried it for the first time after reading your comment. But I am not sure if there is a way to determine the source from where the npm had downloaded the package from within our install script. Our install script runs from the destination after all the package files are placed at destination.

[saper]

I was thinking on using the same test as we do now to check the downloaded binary (as a quick test).

But actually I think we should sign binaries and verify that.

[am11]

🎉
Interesting idea of signing the binaries. But I think in our mini world of Sass, we can live with simple testing on install (for now).

There are number of features we need to implement for LibSass v3.2.0. Once we get stable again, we can continue enhancing the build aspect of node-sass.

[pgib]

@saper Is there a trick to getting LIBSASS_EXT=auto working on FreeBSD (10.1amd64)? I've installed the textproc/libsass port, but node-sass keeps wanting to rebuild using the bundled version.

[saper]

Currently we have an old libsass in the ports. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198489 proposes an upgrade to 3.1.0. Besides, you need to call something like

env CC=clang CXX=clang++ LINK=clang++ GYP_DEFINES=clang=1 SKIP_SASS_BINARY_DOWNLOAD_FOR_CI=yes  LIBSASS_EXT=auto npm install

In this case it builds and not just fetches the binary. I have so half-working node-sass port that is doing this.

[am11]

@saper, based on the FreeBSD bug, please see this conversation (till end): sass/libsass#382 (comment).

Which means that long standing "needs old CentOS support" issue is fixed without us needing to apply patches to libsass code every time we need to compile it for linux with GCC4.4.7 (because with docker, CentOS 5.11 is getting GCC 4.7.x, problem solved !)

Perhaps Docker is the answer for FreeBSD ports as well, if the binaries built against old FreeBSD are forward compatible, then find a oldest-possible version of FreeBSD in docker contribs with has GCC 4.7.x support.

[saper]

I think even gcc 4.6 worked for me. It would have helped if libsass compiled with gcc 4.2 :) but really instead of Docker we just could have invoked pkg add gcc-4.6 or even pkg add node-sass (but this kind of implies installing globally, just like npm install -g).

On a FreeBSD system many compiler versions can live in parallel (system gcc42 or clang, gcc46, 47, 49, 5...), so for that particular problem we don't need docker or jails.

[am11]

we don't need docker or jails

This is awesome! We also have plans to release v3 binaries for Solaris. Hopefully they have the similar toolchians available .

BTW, is nullptr supported in 4.6? I had to apply this 4.4.7 patch: https://github.com/am11/libsass/commits/gcc-4.4.7-patch to compile v2/v3-alpha binaries, I also tested v4.6 with CentOS (v6.3 I guess, which has default tool-chain packaged with gcc46). But hey, anyway as far it is compiling with whatever version without us altering the libsass code, it is all good! (-8

[am11]

Unfortunately, this is causing issues with npm install: #780 (comment).

npm install node-sass@3.0.0-beta.2

It builds only libsass, because npm publish ignores both binding.gyp as well as libsass.gyp, if you have more than one gyp defined in package.json (like we have now).

Apparently, npm does not allow to have more than one gyp file.

Although it is not explicitly clear in docs, but it does mention "a .gyp file": https://docs.npmjs.com/misc/scripts.

Also, I can't find a reference, if someone has tried this kind of thing before: separating C++ libs within node.js module and having multiple gyp files in node.js project. Nevertheless, it would be an interesting issue to open with https://github.com/npm/npm repo.

If this is only concerning the .gyp file, can't we just bundle both of them into one?

Otherwise we would need to revert this back..

[saper]

The problem is that we are not using a built-in npm facitlity to build bindings via gyp files directly - because we are using pangyp and not the default node-gyp. Maybe this can be tweaked in package.json too. We are also probably incorrectly using install hook for building which results with problems like npm install <localpath> trying to rebuild the whole thing again. We might need to switch to using the prepublish hook instead. I am not very well versed in the npm lifecycle so I need to dig more to understand this. http://github.com/mapbox/node-pre-gyp seems to be using install lifecycle script too and they might be running into similar issues.

But I think that d89b901 solves the original problem. Those two .gyp files are chained together if necessary.

I have built a pretty nice autoconf structure to reduce a number of C++11 features required to build http://github.com/sass/libsass, so I really need this feature to allow using libsass that was configured using the ./configure script before building. This currently cannot be ported easily to node-gyp so we will need to rely on a relatively complete C++11 compiler to build a bundled libsass; but soon autoconf will allow us to compile libsass on pretty ancient GCC editions, too - but this has to be a standalone build (i.e. not a part of building node-sass).

[am11]

because we are using pangyp and not the default node-gyp

Which is because io.js is not supported by node-gyp yet.

Maybe this can be tweaked in package.json too

I am all for such a tweak which we can handle at our part, rather shifting the problem to the consumer domain.

But I think that d89b901 solves the original problem.

Actually it did not solved any problem, but presented another flavour of same problem.

I have built a pretty nice autoconf structure to reduce ...

That is all good stuff. We are fixing that issue differently using newer compiler with older glibc. But all that is besides the point. If npm publish is not equipped for handing multiple gyp files, any such cool fancy feature would have to wait unless this limitation is lifted.

To recap:
The npm publish fails to package more than one .gyp files. Between binding.gyp and libsass.gyp, it took the later one, renamed it to binding.gyp and used it on install (don't ask me why). Even if npm folks fix this issue today, which is highly unlikely, it will not be landed for all the node-sass consumers using older npm. We need to find reference to at least one package, where they are using multiple gyp files in the project Or we can open this issue with npm/npm and hope someone prescribe a quasi-official workaround.

For right now, I am reverting this back for 3.0 beta and hoping it to come back as soon we have publishable solution ready.

[saper]

Can you show me how to reproduce this? Nothing in the npm, node-gyp or pangyp code touches binding.gyp like this...