Fix libsass build for SmartOS

[jaredmorrow]

• Use ginstall rather than install for INSTALL variable
• unset SEC because it was being used elsewhere
• Use namespacing where there were complaints

Addresses: #1308

Things I still needed to do to get npm to install node-sass

in libsass:

make
make install
make install-headers

For npm:

export npm_config_libsass_ext=auto
npm install node-sass

I don't know where this should be documented, but I thought I'd throw it in there for anyone else who might happen to hit this issue.

Fixes #1308

[jaredmorrow]

Thinking about this a bit more, the proper way to install it would be to set PREFIX to be /opt/local, and then I wouldn't have to use the CFLAGS, but then the PREFIX setting in the makefile would have to be changed to be ?= instead of = and that might start down the slippery slope of multi-platform support littering the makefile.

[xzyfer]

I believe there have previously been discussion regard the portability of -p.

#1433

[Vagabond]

I would remove this, I added it by accident when tinkering.

[xzyfer]

@QuLogic and @saper are probably best suited to make the call on this.

[jaredmorrow]

Ah, okay. Well we can fix it by specifying a proper PREFIX for SmartOS then.

[jaredmorrow]

I'll stay subscribed to this PR, so just holler and I'm willing to make any changes and re-push.

[jaredmorrow]

Fixed in latest commit, and properly set PREFIX, which was the original need for the -p. Lastly, I removed a conditional that wasn't needed when you can just use the ?= to get the same logic.

[xzyfer]

Great thanks. Give the scope of the changes I'm happy to merge this when CI is happy.

[QuLogic]

I guess the Makefile stuff depends on how much OS-specific stuff you want; as you say you can use make INSTALL=ginstall PREFIX=/opt/local.

[clord]

Ah crap, I went and basically wrote this exact same PR :) patiently waiting for a pull on this one now.

[saper]

Two questions:

1. Which "install" is now default on Solaris? Or more precisely, which part breaks when using traditional SysV "install"?

2. I think we should just rename all constants and prefix them with something like LIBSASS_ to avoid issues like the SEC one. Otherwise it'll be the endless chase...

[xzyfer]

I think we should just rename all constants and prefix them with something like LIBSASS_ to avoid issues like the SEC one. Otherwise it'll be the endless chase...

Makes sense to me. Should be its own PR though.

[jaredmorrow]

Should I remove the Makefile changes so this PR can merge? I'm fine either
way, I can easily document setting the proper variables on the command
line.

On Sunday, December 20, 2015, Michael Mifsud notifications@github.com
wrote:

I think we should just rename all constants and prefix them with something
like LIBSASS_ to avoid issues like the SEC one. Otherwise it'll be the
endless chase...

Makes sense to me. Should be its own PR though.

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

[xzyfer]

@jaredmorrow can you answer @saper's question?

Which "install" is now default on Solaris? Or more precisely, which part breaks when using traditional SysV "install"?

We want to get Makefile compatible with SmartOS. If manual intervention is required then we're not really supporting SmartOS.

[xzyfer]

I'm ok with merging this the community can start compiling LibSass from master.

I've created #1802 to track prefixing constants.