-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update rebar.config to allow compilation on Windows #4
Conversation
incorporating esl/fast_pbkdf2#4 for now
incorporating esl/fast_pbkdf2#4 for now
incorporating esl/fast_pbkdf2#4 for now
incorporating esl/fast_pbkdf2#4 for now
incorporating esl/fast_pbkdf2#4 for now
incorporating esl/fast_pbkdf2#4 for now
Hi @big-r81! Thanks a lot for this PR. Unfortunately, it fails to build in linux, I'm pasting you below the rebar3.crashdump I generated locally (with some lines removed for privacy), hopefully it can help you debug the issue. Let me know if you need any assistance :)
|
@NelsonVides Hi, unfortunately I can't reproduce this locally on an Ubuntu 22.04 VM. I tested it with Erlang v24.2.1. Maybe it's something with the GI CI? |
I could reproduce at home on an ubuntu 22.04 native server, with Erlang 26.1, 25.3, and 24.3, so it seems pretty reproducible to me unfortunately, though that might make it easier to fix. All three versions generate the same crashdump as pasted above 🥲 |
Also tried on my intel-based mac and the same error happens. I don't have a silicon mac nor a windows to try it out though. Do you think the crashdump could help you with a fix? Seems like the port_compiler didn't like something. |
@NelsonVides Hi, I know why it was working on my side. We use rebar2 for compiling and not rebar3. With rebar2 all is working, with rebar3, I get your errors too. Do you have any idea how to port this to rebar3? |
Oooooh, rebar2, I see! Well, that's quite an old one, I'm lost there 😅 |
@NelsonVides Hey, I updated the PR. It should work under Windows/*nix and Rebar 2/3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! Only one thing is code coverage for the C code, see
https://app.codecov.io/github/esl/fast_pbkdf2/tree/patch-1
in comparison to master in
https://app.codecov.io/github/esl/fast_pbkdf2
the c_src
directory is not listed.
That's why I had duplicated the port compiler directories with a global config and an overriding one in the test
profile, the test one had an extra CFLAGS and LDFLAGS one, --coverage
:)
0b616fc
to
607a206
Compare
5cf0501
to
d447560
Compare
Add compiler flags for Windows (MSVC).
@NelsonVides Hi, can you look if the coverage is increased? I updated thr PR again... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! Now it all works! 💪🏽
Will cleanup github actions scripts and prepare a new release in hex. Thanks for the PR, very nice 😄
Add compiler flags for Windows (MSVC).
Fixes #3.