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.

Sign up for GitHub

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

Jump to bottom

Add build-essential for debian/ubuntu installation #556

Closed
wants to merge 1 commit into from
Closed

Add build-essential for debian/ubuntu installation #556

wants to merge 1 commit into from

Conversation

germsvel
Copy link
Contributor

I am not a debian/ubuntu user, but when running the bin/setup script in a remote box, the installation failed once because make was not available and another time because c++ was not available (when compiling rox).

To fix this, I know we can add the build-essential package which includes make, gcc, and g++. I don't believe the same package is available for fedora/centos, so I did not include that package for those two.

@germsvel
Add build-essential for debian/ubuntu installation
3b5e0cd 
I am not a debian/ubuntu user, but when running the `bin/setup` script
in a remote box, the installation failed once because `make` was not
available and another time because `c++` was not available (when
compiling `rox`).

To fix this, I know we can add the `build-essential` package which
includes `make`, `gcc`, and `g++`. I don't believe the same package is
available for fedora/centos, so I did not include that package for them.
@ayrat555
Copy link
Member

While you're at it can you please remove libsecp256k1 instructions from this script. It was fixed

@hayesgm
Copy link
Contributor

hayesgm commented Oct 31, 2018

Note: pull #543 changes the dep from rox to an erlang rocksdb library. This may change the requirements here.

@hayesgm
Copy link
Contributor

hayesgm commented Nov 3, 2018

Can we rebase this on master and verify it works as expected with the changes to rocksdb from rox?

@germsvel
Copy link
Contributor Author

germsvel commented Nov 5, 2018

@hayesgm I'm not sure I'll be able to test this properly again. I was only updating this as I was setting up a new remote box. I'm not really a debian/ubuntu user, so I don't have a machine on which I can test this again. Do we want to just close this or does it seem like worth merging as is?

MattMSumner
MattMSumner approved these changes Nov 6, 2018
View reviewed changes
Copy link
Contributor

@MattMSumner MattMSumner left a comment

Choose a reason for hiding this comment

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

Assuming this works again on a new machine this LGTM.

@ayrat555
Copy link
Member

ayrat555 commented Nov 7, 2018

@hayesgm @germsvel what do you think about running this script in the build CircleCI workflow?

@germsvel
Copy link
Contributor Author

germsvel commented Nov 7, 2018

@ayrat555 so long as it doesn't make the build process longer, I'd be okay. So I think in practice that would mean so long as it is faster than Constantinople common tests, then we could do that.

@germsvel germsvel mentioned this pull request Nov 7, 2018
Open
@germsvel
Copy link
Contributor Author

germsvel commented Nov 7, 2018

I'm closing this in favor of #587. I think we should have a way of knowing whether the changes work or not.

@germsvel germsvel closed this Nov 7, 2018
@germsvel germsvel deleted the gv-update-bin-setup branch November 7, 2018 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MattMSumner MattMSumner MattMSumner approved these changes

Assignees
No one assigned
Labels
priority: high tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@germsvel @ayrat555 @hayesgm @MattMSumner