Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Error message on libstdc missing.#972

Merged
johnhaley81 merged 3 commits intonodegit:masternodegit/nodegit:masterfrom
martinheidegger:feature/install-error-messagemartinheidegger/nodegit:feature/install-error-messageCopy head branch name to clipboard
Mar 28, 2016
Merged

Error message on libstdc missing.#972
johnhaley81 merged 3 commits intonodegit:masternodegit/nodegit:masterfrom
martinheidegger:feature/install-error-messagemartinheidegger/nodegit:feature/install-error-messageCopy head branch name to clipboard

Conversation

@martinheidegger
Copy link
Contributor

nodegit is usually used as library for other projects. Nodegit is also, unfortunately, depends on a version of libstdc++ that is not installed on many linux systems. Any person that uses nodegit as a library would need to explain this problem in their issues. In order to reduce the work of libraries/tools that depend on nodegit this PR adds an error message on postinstall if the dependency is missing and adds steps on how to mitigate that issue.

Note: This has been a follow-up on #969.

@martinheidegger martinheidegger force-pushed the feature/install-error-message branch from 1dc7faa to 1c7ad24 Compare March 28, 2016 15:59
@tbranyen
Copy link
Member

Love it. Means I'll have less repetitive issues to close, and indirect users will have a better idea on how to solve any issues with installing.

@johnhaley81
Copy link
Collaborator

Testing on windows right now :)

@johnhaley81
Copy link
Collaborator

Does windows not run the post-install script?

@johnhaley81
Copy link
Collaborator

Ahh I didn't see the second commit I'll confirm locally then gtg IMO.

postinstall.sh Outdated
#!/usr/bin/env bash

if [ -n "$(node lib/nodegit.js 2>&1 | grep libstdc++)" ]; then
echo "[ERROR] Seems like you the latest libstdc++ is missing on your system!"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little grammatical error. Should read `"[ERROR] Seems like the latest libstdc++ is missing on your system!"

@johnhaley81
Copy link
Collaborator

Confirmed that it works locally. I did make 2 minor comments though.

@martinheidegger
Copy link
Contributor Author

@johnhaley81 Pushed the adjustments

@johnhaley81
Copy link
Collaborator

Thanks @martinheidegger!

@johnhaley81 johnhaley81 merged commit 38e4467 into nodegit:master Mar 28, 2016
@martinheidegger martinheidegger deleted the feature/install-error-message branch March 29, 2016 00:02
@anaisbetts
Copy link

This is breaking builds in Windows, you can't invoke Bash scripts in npm packages. Can you rewrite this as a JS file? Should be super easy and you can bail out if process.platform !== 'linux'

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.