erlangpack / bcrypt

Erlang wrapper for OpenBSD's Blowfish password hashing code
https://hex.pm/packages/bcrypt
Other
16 stars 19 forks source link

SmartOS / Solaris #12

Open mworrell opened 4 years ago

mworrell commented 4 years ago

@Licenser and @siepkes we have released 1.1.0 - but I now realize that our new Makefile (based on erlang-mk) does not include any special options for Solaris or SmartOS.


UNAME_SYS := $(shell uname -s)
ifeq ($(UNAME_SYS), Darwin)
    CC ?= cc
    CFLAGS ?= -O3 -std=c99 -arch x86_64 -Wall -Wmissing-prototypes
    LDFLAGS ?= -arch x86_64 -flat_namespace -undefined suppress
    DRV_LDFLAGS = -flat_namespace -undefined suppress $(ERL_LDFLAGS)
else ifeq ($(UNAME_SYS), FreeBSD)
    CC ?= cc
    CFLAGS ?= -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes
    DRV_LDFLAGS = $(ERL_LDFLAGS)
else ifeq ($(UNAME_SYS), Linux)
    CC ?= gcc
    CFLAGS ?= -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes
    DRV_LDFLAGS = $(ERL_LDFLAGS)
endif

Could you have a look?

siepkes commented 4 years ago

Definitely willing to help out here!

However since the implications of this are a bit bigger then just fixing the bcrypt package build which I did previously I would like to ping @jperkin. He is probably way more impacted then me on anything that is decided here.

It might also be an idea to make a shift between Illumos and Solaris while we are at it since those are slowly drifting apart.

jperkin commented 4 years ago

@siepkes if this is intended for pkgsrc then we would likely completely ignore this kind of section, it's way too OS-specific, so I wouldn't worry too much about it.

mworrell commented 4 years ago

I am also a bit surprised by the specificity of this section from the erlang makefiles.

Would be good to at least have it build and work on Solaris/SmartOS and/or illumos.

As I don't have access to these OS's I would be grateful if you could add the correct options and make it work on one or more of those OS's.

I also suspect that the "Linux" section could be the more generic fallback.

When we have fixed this, then I will include it into the 1.2.0

siepkes commented 4 years ago

uname -s will report SunOS on all Solarish OS'es (Solaris, Illumos, etc.). For now it's probably easiest to put all the Solarish OSes under one umbrella.

@mworrell In which repo is the Makefile you reference so I can add the flags, test it and make a PR for it?

mworrell commented 4 years ago

hi @siepkes, the Makefile is here:

https://github.com/erlangpack/bcrypt/blob/master/c_src/Makefile

This is a new Makefile (derived from erlang-mk) as we are now using rebar3. As you can see it is missing SunOS completely.

And also Windows... we might want to change the Linux entry into a catch-all.

mworrell commented 4 years ago

I changed the FreeBSD entry into a catch all. This might not be enough for Windows, but will be ok for most *nix OS machines.