Skip to content
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

Build problem on Debian #60

Closed
shimaore opened this issue Sep 2, 2014 · 13 comments
Closed

Build problem on Debian #60

shimaore opened this issue Sep 2, 2014 · 13 comments

Comments

@shimaore
Copy link

shimaore commented Sep 2, 2014

When trying to build node-stringprep (master) on recent Debian (testing) Linux amd64, the linker step doesn't work properly and tries to link to V8 (which I assume shouldn't happen since those symbols will be provided by Node.js at the time the module is loaded?).
The output of icu-config --ldflags is:

-fPIE -pie -Wl,-z,relro -Wl,-z,now  -ldl -lm   -L/usr/lib/x86_64-linux-gnu -licui18n -licuuc -licudata  -ldl -lm   

which led me to Debian #759792.
If I manually replace the call to icu-config in binding.gyp with:

'libraries': [ '-Wl,-z,relro -Wl,-z,now  -ldl -lm   -L/usr/lib/x86_64-linux-gnu -licui18n -licuuc -licudata  -ldl -lm' ],

the build completes, but npm test then complains about

> grunt test

Running "mochacli:all" (mochacli) task
Cannot load StringPrep-0.5.2 bindings (using fallback). You may need to `npm install node-stringprep`
Cannot load StringPrep-0.5.2 bindings (using fallback). You may need to `npm install node-stringprep`

and I just wanted to make sure this was OK before I update the Debian ticket, since I've never build node-stringprep before and I don't know whether the Cannot load ... messages are expected or not. Thanks :)

@lloydwatkin
Copy link
Contributor

Presumably you have libicu-dev installed?
Have you read: nodejs/node-gyp#363

node-stringprep builds against version 49 at present and not other versions.

If its complaining about string prep bindings not being built it will fallback to javascript fallbacks.

Let me knock up a quick branch to test in travis with your updates and update you.

@lloydwatkin
Copy link
Contributor

I've added your updates, we didn't have a bug before and we don't have one now with your update so I'm happy to merge if you would like me too https://travis-ci.org/node-xmpp/node-stringprep/builds/34188882

@shimaore
Copy link
Author

shimaore commented Sep 6, 2014

Hi @lloydwatkin
I didn't know about nodejs/node-gyp#363 but it doesn't appear to be the issue.
libicu-dev is version 52.1, but node-stringprep builds just fine against it if I make the change I mentioned (which is basically removing the first two parameters provided by icu-config as suggested in the Debian ticket).

Hardcoding these will probably break a different platform somewhere, so I'll update the Debian ticket to have icu-config fixed since this appears to be the culprit.

@dodo
Copy link
Contributor

dodo commented Sep 24, 2014

any update?
this bug is hitting me now on sid.

@lloydwatkin
Copy link
Contributor

@dodo does the branch https://github.com/node-xmpp/node-stringprep/tree/issues/60 fix things for you? If so I'm happy to merge and roll out.

@dodo
Copy link
Contributor

dodo commented Sep 25, 2014

ah yes, hadn't seen this branch before. and yes its working :)
lets roll this out and revert it when debian is fixed lol.

@lloydwatkin
Copy link
Contributor

Will merge and deploy first thing tomorrow.
On 25 Sep 2014 19:14, "▟ ▖▟ ▖" notifications@github.com wrote:

ah yes, hadn't seen this branch before. and yes its working :)
lets roll this out and revert it when debian is fixed lol.


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

@lloydwatkin
Copy link
Contributor

Have released as 0.6.0

@lloydwatkin
Copy link
Contributor

This is now causing issues on other OSs :(

@lloydwatkin
Copy link
Contributor

The debian bug looks like its reported as fixed. Does this mean I can revert the change included?

@shimaore
Copy link
Author

shimaore commented Oct 9, 2014

@lloydwatkin Just tested with icu 52.1-6 and node-stringprep installs like a charm.

EDIT: ...which means nothing since I'm using the version with the patch. Re-testing now.
EDIT2: Works OK with icu-config --ldflags instead of the static string. The change should be reverted.

@lloydwatkin
Copy link
Contributor

@shimaore great, will revert the change and publish as a patch update. Thanks!

@lloydwatkin
Copy link
Contributor

Done 0.6.2 is released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants