Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Manage gems + Ruby at current stable version(s) #266

Merged
merged 1 commit into from May 21, 2014

Conversation

Projects
None yet
5 participants
Contributor

jm3 commented Nov 13, 2013

The current stable version of Ruby is 2.0. Bitcoin.org's version, 1.9.1, has been end-of-life'd and is not supported.

Most popular ruby projects have also adopted Bundler to simplify managing their gem dependencies. Bundler lets developers have more than one version of a gem (or of Ruby itself) on their machines without conflict.

This pull request updates bitcoin.org to Ruby 2.0.0 and encapsulates all Ruby dependencies in a Gemfile so that one command (bundle install) now downloads and install all required gems. Other than making it easier for developers to contribute, there are no changes. Any curmudgeonly developers who do not wish to run modern Ruby can simply ignore the Gemfile and everything will work the same (though you really should update :).

README has also been updated to reflect the change.

Contributor

saivann commented Nov 13, 2013

Thanks for submitting patches.

While I didn't review / test everything, ruby2.0 isn't available in most distro. ruby2.0 will only be supported in the next ubuntu release. So it needs to be ruby1.9.1-dev for now (which is actually 1.9.3).

Bundler seems cool, however it needs to work with previous ruby versions. Also, when tested, it downloaded a lot more gems than required and took more time than what we have now (sudo gem install ffi-icu). Perhaps the Gemfile / Gemfile.lock should be reviewed to include appropriate dependencies? All in all, if the goal of this pull request is to help new contributors, I think it shouldn't increase the time required to install the dependencies.

It would also be good to test changes in the README on an old and recent clean install (ex. ubuntu 12.04 and 13.10) to make sure contributors won't hit problems. For example, "sudo" is missing in "gem install bundler" .

Contributor

jm3 commented Nov 13, 2013

Cool!

One thing I made sure to write in my PR note above is that any developers who do not want to run modern Ruby can simply ignore the Gemfile and everything will work as it does today (though you really should update :)

I'll take your specific questions one by one:

While I didn't review / test everything, ruby2.0 isn't available...

Ruby 2.0 works well on Ubuntu 12.04, and on previous versions too. I personally run Ruby 2.0 on both 11.10 and 12.04 with no issues; it just uses less memory.

> cat /etc/lsb-release|grep RELEASE
DISTRIB_RELEASE=11.10
> ruby --version
ruby 2.0.0p247 (2013-06-27 revision 41674) [x86_64-linux]

While it's possible to install ruby using a system package manager, this is strongly discouraged by the Ruby team because package maintainers rarely incorporate the latest bug + security fixes into their packages. That's why the most popular tool for installing Ruby is not apt but RVM. Other options are rbenv or building from source for those who really want to be secure. Ruby is ANSI C so it compiles almost anywhere POSIX-y. (The situation is not unique to Ubuntu, either; the same holds true on both Windows and OS X as well, hence the popularity of rvm+.)

Bundler seems cool, however it needs to work with previous ruby versions.

Luckily, Bundler works great with previous versions, even all the way back to Ruby 1.8.7 and earlier. When Bundler was released, Ruby 2.0 did not even exist :)

Also, when tested, it downloaded a lot more gems than required

It might have seemed that way, but what you saw was the same required gems being downloaded once, the first time, and then isolated from the system gems so that upgrades do not break the project. The only gems in the Gemfile are the ones listed in the README -- jekyll, json, less, therubyracer, and ffi-icu -- plus their dependencies. One of the major benefits of Bundler is that it calculates the gem dependency graph for you, saving the list of required gems in Gemfile.lock and isolating your project's gems from other gems installed on your system. This means that, by design, other installed gems cannot cause conflicts: they are effectively sandboxed out.

...and took more time than what we have now (sudo gem install ffi-icu).

Installing the gems takes the same amount of time, and the first time you install, the gems and their dependencies will be downloaded. That happens once and never again unless the gem dependencies change. (If you run bundle install again, though, nothing new will be installed, since all the required gems are already available in the bundle.) I was a skeptic too, at first, but I've fully come around and now have hewed to the community standard. To learn more the rationale behind Bundler, the developers have written a nice explanation on bundler.io.

It would also be good to test changes in the README on an old and recent clean install (ex. ubuntu 12.04 and 13.10) to make sure contributors won't hit problems.

"sudo" is missing in "gem install bundler" .

Not missing - no longer necessary! 👍 From the bundler docs:

"You should never use sudo bundle install [...] you should run bundle install as the current user, and bundler will ask for your password if it is needed to put the gems into their final location."

Again, as per above: you can add an additional sudo, but it's not necessary, and can cause problems for other users later.

Let me know if you'd like me to test on other/older platforms than Ubuntu 11.10, 12.04, and OS X 10.9, which is where I tested — I understand that you want to have full confidence in this and I'm happy to help with that.

Contributor

saivann commented Nov 15, 2013

@tcatm @schildbach : Any opinion on this?

@jm3: Well it seems that sudo is required in "gem install builder" if you install ruby from apt-get. Perhaps current setup is less portable, but using rvm to install ruby requires much more steps I'd prefer not to push on new contributors unless they really have problems with the current setup.

Regarding dependencies, I still think some are not needed. therubyracer and aquarium for instance are not required anymore unless I'm mistaken.

Contributor

jm3 commented Nov 15, 2013

Only included them because the docs on here say they're required. :) The other Jekyll projects I work on don't use either of them.

Contributor

mikehearn commented Mar 27, 2014

If old Ruby's are not being supported anymore we should probably update to one that is, regardless of what distro's ship. Linux distros are notorious for shipping ancient software that's full of bugs ...

Contributor

buren commented May 7, 2014

@jm3 +1 for this PR..
Personally I feel that using bundler almost always results in an easier setup, especially if the number of dependencies increases.

Contributor

jm3 commented May 8, 2014

@buren agreed, thank you 👍

Contributor

saivann commented May 18, 2014

@jm3 Could you replace your commit with a new one tested with a fresh distro install and without "aquarium" and "therubyracer" dependencies?

"bundle install" fails on Ubuntu 14.04 with current Gemfile.lock, and it fails on all Ubuntu versions when trying to re-generate the file from scratch. So it may be that we should completely stop using apt and switch to using rvm. If you can provide these instructions in the README as well, we could test this and (finally) close this pull request.

Contributor

jm3 commented May 18, 2014

@saivann no problem, I've already rebased the patch against current HEAD and it bundles fine on Ubuntu 14.04 with Ruby 2 (and OS X, where I develop).

Following the instructions in the README, therubyracer gem seems to be required: the specific error is: [WARNING] Please install gem 'therubyracer' to use Less.

Please post your instructions to run the app without the gem and I'll happily remove it :)

Contributor

saivann commented May 18, 2014

@jm3 If therubyracer is a dependency for less, than we have to keep it, thanks!

@harding harding commented on an outdated diff May 18, 2014

README.md
- sudo apt-get install jekyll node-less ruby1.9.1-dev libicu-dev
- sudo gem install ffi-icu
+ sudo apt-get update
+ sudo apt-get install jekyll node-less ruby2.0
+ gem install bundler; bundle install
@harding

harding May 18, 2014

Contributor

I'm a complete bundler noob, but if I understand correctly, this last bundle install command needs to be run from the local git repository directory. We should probably specify that, as the rest of the commands are directory-independent.

Contributor

jm3 commented May 18, 2014

@harding that's exactly right, bundler encapsulates the project's gems to keep them safe from conflicting with any other project's gems or installed system gems. In my experience, the default for project READMEs is that all commands are run from within the project unless otherwise specified; this is what I've seen in every other Github project I've experienced.

I'm happy to add, "# cd into project directory", but I'm curious, was there a previous step in the instructions that led you outside the project?

Contributor

jm3 commented May 18, 2014

@saivann added the conversion to RVM as well; eliminates the need for deb packages jekyll, node-less, ruby1.9.1-dev, libicu-dev, and ffi-icu.

@saivann saivann commented on an outdated diff May 18, 2014

README.md
@@ -41,19 +41,24 @@ You simply need to push additionnal commits on the appropriate branch of your Gi
**Easy preview**: Simple text changes can be previewed live on bitcoin.org. You only need to click anywhere on the page and hold your mouse button for one second. You'll then be able to edit the page just like a document. Changes will be lost as soon as the page is refreshed.
-**Real preview**: Install [dependencies](#requirements), run jekyll (or "jekyll build" on older setups), and copy the output files from _site/ to the root of your web server. If you have no web server, run jekyll --server (or "jekyll serve" on older setups). This server requires you to add a trailing ".html" by hand in your browser address bar.
+**Real preview**: Install [dependencies](#requirements), run `bundle exec jekyll build` and copy the output files from _site/ to the root of your web server. If you have no web server, run `bundle exec jekyll serve`.
@saivann

saivann May 18, 2014

Contributor

@jm3 Is there a reason why you removed the ".html browser workaround"? Did a recent jekyll version fix this?

@saivann saivann and 1 other commented on an outdated diff May 18, 2014

README.md
- sudo apt-get install rubygems ruby1.9.1-dev build-essential libicu-dev
- sudo gem install jekyll json less therubyracer ffi-icu
+ gem install bundler
+ bundle install
+
+on older Ubuntu and Debian distributions:
@saivann

saivann May 18, 2014

Contributor

@jm3 Can we have instructions that apply both to current and older distribution? Or otherwise provide them with rvm instructions as well?

@jm3

jm3 May 18, 2014

Contributor

RVM doesn't really care what distribution of ubuntu you're running. From the docs: "RVM supports most UNIX like systems, the basic requirements are bash, curl and overall GNU version of tools - but RVM tries to autodetect it and install anything what is needed."

What would you expect to be different on older distributions, and how much "older" are you considering older?

@saivann

saivann May 19, 2014

Contributor

That's what I mean. If we provide RVM instructions, why providing APT instructions for older distributions? IMO we could drop this part and only use your RVM intructions.

@saivann saivann and 1 other commented on an outdated diff May 18, 2014

README.md
- sudo apt-get install jekyll node-less ruby1.9.1-dev libicu-dev
- sudo gem install ffi-icu
+ \curl -sSL https://get.rvm.io | bash -s stable
@saivann

saivann May 18, 2014

Contributor

Maybe we should be adding source ~/.rvm/scripts/rvm under this line? The script mentions this step after installing rvm, but I wouldn't be surprised many user just don't notice.

@jm3

jm3 May 18, 2014

Contributor

Right, the rvm install instructions and RVM itself both remind the user to do so, and I think unless you specifically pass an option to rvm, it will append the rvm source line to your dotfiles, if I'm not mistaken.

Do you think it would be acceptable to simply point the user to the docs for RVM like I did in the README, and say "install RVM", rather than duplicating the instructions here? (we could also include the link to rbenv; developers are free to install ruby 2.0 however they like.)

Contributor

harding commented May 18, 2014

@jm3 re: changing directory

I think the confusion arises from differing context. If I cloned the repository and then read the README.md, it'd be a reasonable assumption that I was in the top-level directory of the repository.

But with GitHub displaying the README.md as the project's main info page, and with the file including instructions for people who haven't cloned yet, I think it's reasonably likely that users might try to install the "Requirements" from outside the repository---or even try to install them before creating the cloned repository.

Contributor

saivann commented May 18, 2014

Re: changing directory, FWIW, I would also expect some people to install dependencies from outside the repository unless explicitely specified.

@harding harding and 1 other commented on an outdated diff May 18, 2014

Gemfile
@@ -0,0 +1,14 @@
+source 'http://rubygems.org'
@harding

harding May 18, 2014

Contributor

Is it possible to change this to https to prevent MITM attacks during install/upgrade?

Also, is there a way of using GPG or something to verify that the software being automatically downloaded is the same code tested by the person who last updated the Gemfile or Gemfile.lock? I'm not keen on automatically installing gobs of third-party software which, if any one part is compromised, could compromise my development environment.

@jm3

jm3 May 19, 2014

Contributor

@harding : Definitely! If you like, you can run bundle install --trust-policy HighSecurity on your local system, which will restrict gem to only installing gems that are cryptographically signed. Unfortunately for you, not all gem developers sign their gems, so you'll have to appeal to the developers of RedCloth, ffi-icu, etc. to begin signing their gems with widely available public keys.

Lack of https was an oversight. Fixed!

Contributor

jm3 commented May 19, 2014

@harding i think the perceived risk of confusion about CWD stems from the fact that the README as currently written has two sets of instructions: one for getting the code and one for getting the other dependencies. Currently the instructions for getting the code are mixed up with some tips like "here's how to use git" and so on; I'd prefer to clarify things by moving that meta stuff to the end of the readme (usually under a heading like, "How to Contribute") and moving the git clone git@github.com/bitcoin/bitcoin.org; cd bitcoin.org line into the right place in the dependency install instructions. Then there's no confusion about which directory you're in; the cd command will be part of one set of instructions to get everything set up.

In my experience, a great README install section has a block that can be literally pasted into the terminal to get the project set up; if I move those git clone + cd lines to sit within the other setup instructions, then that will be the case here as well! :)

I'll update the PR; please let me know if you have a different idea.

Contributor

harding commented May 19, 2014

@jm3 the solution you propose for the possible CWD confusion sounds excellent to me. Thanks!

Contributor

jm3 commented May 19, 2014

@harding awesome, done! Since it's hard to read markdown in diff format, you can preview the README changes here, if you like: https://github.com/jm3/bitcoin.org#setup

@harding harding commented on an outdated diff May 19, 2014

README.md
-You can report any problem or help to improve bitcoin.org by opening an issue or a [pull request](#working-with-github) on [GitHub](https://github.com/bitcoin/bitcoin.org). You can also help [translating bitcoin.org](#translation) on [Transifex](https://www.transifex.com/projects/p/bitcoinorg/).
+You can report problems or help improve bitcoin.org by opening an issue or a [pull request](#working-with-github) on [GitHub](https://github.com/bitcoin/bitcoin.org). You can also help by [translating](#translation) bitcoin.org's text on [Transifex](https://www.transifex.com/projects/p/bitcoinorg/).
@harding

harding May 19, 2014

Contributor

the working-with-github link is broken.

@harding harding and 1 other commented on an outdated diff May 19, 2014

README.md
-In order to use GitHub, you need to [sign up](http://github.com/signup) and [set up git](https://help.github.com/articles/set-up-git). You will also need to click the **Fork** button on the bitcoin.org [GitHub page](https://github.com/bitcoin/bitcoin.org) and clone your GitHub repository into a local directory using the following command lines:
+ # install ruby 2 using rvm
+ \curl -sSL https://get.rvm.io | bash -s stable
@harding

harding May 19, 2014

Contributor

Why is the c in curl escaped?

@jm3

jm3 May 19, 2014

Contributor

from http://rvm.io/rvm/install:
"Point to be noted is, there is a backslash before curl. This prevents misbehaving if you have aliased it with configuration in your ~/.curlrc file."

Contributor

saivann commented May 19, 2014

@jm3 You actually did more change than you described, and I think this creates various problems:

  • I think we shouldn't change titles (Working with GitHub, How to participate, Previewing, Requirements) since this breaks any external link pointing to these texts.
  • Although mixing Requirements with Git is good, we could probably avoid mixing setup with preview since the later will be used regularly.
  • We shouldn't remove text describing the problem and workaround with "jekyll serve", and how to use "jekyll build" with your own server, otherwise people will run into problems.
  • I also think that the "Working with GitHub" text shouldn't be moved at the end of the document, since it's the first thing people need to learn, I think it would make sense to keep it right after "Requirements" (Setup).

@saivann saivann and 1 other commented on an outdated diff May 19, 2014

README.md
-GitHub allows you to make changes to a project using git, and later submit them in a "pull request" so they can be reviewed and discussed. Many online how-tos exist so you can learn git, [here's a good one](https://www.atlassian.com/git/tutorial/git-basics).
+If `ruby -v` reports 2.0.0 or higher on your system, you're all set.
@saivann

saivann May 19, 2014

Contributor

Shouldn't we tell people to install gems with bundle regardless if they already have the right ruby version installed? If I understand this correctly, perhaps we should drop the "If you have ruby 2.0.0, you're all set"?

@saivann

saivann May 19, 2014

Contributor

@jm3 Exactly, so to my understanding, "you're not all set" if you have ruby v2.0.0, because you still need to install the gems with builder.

@jm3

jm3 May 19, 2014

Contributor

k, i'll change it again.

Contributor

saivann commented May 19, 2014

@jm3 As an alternative suggestion, if you really think "Setup" should remain merged with "Preview", I suggest you move "Previewing text changes" under "Translation" right before "Importing translations". This easter egg is generally used by translators.

Contributor

jm3 commented May 19, 2014

@saivann ah, great compromise, I like it. Done.

Contributor

jm3 commented May 19, 2014

@saivann Having used GitHub daily, for years, I think it's pretty safe to say that the prevailing practice is to put "How to Contribute" stuff at the end of the README. I can't recall seeing a lot of popular projects that put this at the front. By putting it at the end, you let developers get right to the content, not wade through a bunch of boilerplate, which helps get them started more quickly and makes them feel like the project is easy to contribute to.

If you really feel strongly about it, I think the best solution is to split the README into a few files and link them from the main README: translation stuff can be moved to its own README file (presumably in translations/ and the "getting started with github" stuff into a similar Contributing.md file. That way you're not mixing instructions for non-developers, like translators, with the technical instructions for getting the code running.

Contributor

saivann commented May 19, 2014

@jm3 If you fix other things I've asked in this comment, the first title will be "How to participate" and the short text under it links to "Working with GitHub", so keeping this text at the end of the README would be OK with me in this case.

Although my experience with this project tells me that this text is one of the most important part of the README. Remember this is a website (not a software) so many non-developer contributors want to use GitHub in order to make simple pull requests (submit events, fix typos, update translations, submit new texts). Current README has been written for these requirements and has proven useful thus far.

Contributor

harding commented May 19, 2014

Re: Working With GitHub, I concur with @saivann. When I started contributing, I just wanted to contribute text, and I was quite happy that it was easy to do that without installing Jekyll and its dependencies. In general, I think the README (and almost any document) should proceed from the most generally-useful information to the most specialized information, and that means that using GitHub GUI comes before git command line, which itself comes before compiling stuff with jekyll.

Contributor

jm3 commented May 19, 2014

Gentlemen, I don't at all agree, but I'm not here to argue. Reverted README back to the previous state; I kept the spelling + capitalization fixes.

Contributor

harding commented May 19, 2014

@jm3 Thanks! @saivann and I will be doing some testing and README editing, so please don't rebase/amend f77c2dc.

Contributor

jm3 commented May 19, 2014

@harding awesome! let me know if i can help with additional testing.

Contributor

harding commented May 19, 2014

Status update: I started following the instructions on an version of Debian (the current stable) but decided to give up for the day when it wanted root permissions during the ruby2 install. Tomorrow I'll make a chroot and try it in there.

I committed and pushed my minor changes to the README during the process over here: harding/bitcoin.org@ce37a54#diff-04c6e90faac2675aa89e2176d2eec7d8

Contributor

jm3 commented May 20, 2014

@harding Cool, I left some comments.

Contributor

harding commented May 20, 2014

@jm3, @saivann

Followed the README instructions using my usual build chroot (Debian Unstable) and it worked fine, although rvm insall ruby-2.0.0 still prompted me for my root password in order to install the following packages using apt-get: libyaml-dev, libsqlite3-dev, sqlite3, libffi-dev

(I wish it told me what packages it wanted so I could install them myself, but I don't see an option to do that in the code (and that's certainly not @jm3's fault!).)

This is the chroot I currently use the Debian jekyll package to build with, and diff says the _site built by bundle exec jekyll build is identical to the _site built by /usr/bin/jekyll build, so this looks good to me.

I tested on both current master and the devel-docs branch.

I did update the config file to prevent Gemfile and Gemfile.lock from being included in _site.

@jm3 I suggest you wait for @saivann's comments and then merge my branch into yours and squash all the commits down into your commit. If @saivann has any more changes he wants to make, he'll probably have a branch for you to sqash merge too. Thanks for all of your work!

Contributor

jm3 commented May 20, 2014

@harding sounds great, I'll stand by

Contributor

buren commented May 20, 2014

Let me known you need more testing

Contributor

saivann commented May 21, 2014

Just tested last commit on @harding branch on Ubuntu 10.04 12.04 and 14.04 and this works fine!

@jm3 can you update you squashed commit?

@buren More testing or reviews is always welcome! Although my bet is that you won't find any big issues at this point.

Manage gems + Ruby @ current stable versions w/RVM
* update to modern, supported Ruby 2.0 version
* update gems to current versions using Bundler
* make jekyll configs confirm to current version
* switch deb package dependencies to cross-platform rvm
* ignore bundled dependencies
* some README mods as per @harding + @saivann
Contributor

jm3 commented May 21, 2014

@saivann @harding BLAMMO! 💥

Contributor

saivann commented May 21, 2014

Commit 944e7c3 LGTM!

Contributor

harding commented May 21, 2014

944e7c3 LGTM. Thanks!

saivann added a commit that referenced this pull request May 21, 2014

Merge pull request #266 from jm3/master
Manage gems + Ruby at current stable version(s)

@saivann saivann merged commit 8f4f9d4 into bitcoin-dot-org:master May 21, 2014

Contributor

saivann commented May 21, 2014

@jm3 @buren @harding Thanks! I think this is a nice improvement. If you see anything else that needs to be fixed, feel free to open a new pull request.

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