Change false to empty string to prevent error on line 41 of mailman gem ...

[griffithac]

Get Gem. Big help. I did however have a problem and I was hoping you could merge in this fix. The use of false in this case is causing an error for me with the mailman gem. The error occurred line 41 of application.rb file in the instance method "run" of Mailman::Application class.

I believe that if you use an empty string you will still prevent mailman from loading the Rails environment and at the same time prevent this this error.

Here is the console output for the error I was getting before the fix:

$ env RAILS_ENV=development bundle exec rake mailman --trace

/Users/andrewg/.rvm/gems/ruby-1.9.3-p194/gems/mailman-rails-0.0.3/lib/mailman-rails/tasks/mailman.rake * Invoke mailman (first_time) * Invoke environment (first_time) * Execute environment * Execute mailman rake aborted! can't convert false into String /Users/andrewg/.rvm/gems/ruby-1.9.3-p194/gems/mailman-0.5.3/lib/mailman/application.rb:41:in join' /Users/andrewg/.rvm/gems/ruby-1.9.3-p194/gems/mailman-0.5.3/lib/mailman/application.rb:41:inrun' ...........

Thanks.

Andrew

[johncant]

Hi @griffithac, and thanks for contributing! I find it very annoying when I use someone elses newly written gem and it doesn't work, so I really appreciate the contribution.

I made a bit of a mistake in thinking that Mailman wanted config.rails_root to be set to false to not load Rails. I made this commit so that Mailman would accept false. In fact, the documentation says that it should be set to nil. That pull request isn't yet in rubygems, so mailman-rails will break with mailman-0.5.3 :-(

Technically, according to the documentation, config.rails_root should be set to nil. Looking at <a href=https://github.com/titanous/mailman/blob/master/lib/mailman/application.rb#L47">https://github.com/titanous/mailman/blob/master/lib/mailman/application.rb#L47, it looks as though an empty string will load config/environment.rb in the current dir and load the Rails env. Have you tested this?

Solutions (1) We set config.rails_root to nil in mailman-rails (2) We persuade titanous to release mailman-0.5.4

[griffithac]

Yes I have tested it. In my case anyway, Mailman will not load/require my enviroment.rb file because of the File.exists? check at line 49 https://github.com/titanous/mailman/blob/master/lib/mailman/application.rb#L49. Mailman needs a path that will actually locate the file (thus why we would need to supply the value in the first place). The path generated at https://github.com/titanous/mailman/blob/master/lib/mailman/application.rb#L48 by File.join(Mailman.config.rails_root, 'config', 'environment.rb') is '/config/enviroment.rb' because I am setting the Mailman.config.rail_root = "" (notice no leading period which would create a relative path). In my case anyway there is no directory at "/" root called "config" and therefore no file called "enviroment.rb" within it, so the File.exist?(rails_env) check returns false and therefore "require rails_env" at Line 51 is never called.

I decided to test setting config.rails_root to nil. This did produce what you suspected an empty string would. In this case File.join(Mailman.config.rails_root, 'config', 'environment.rb') returned "./config/enviroment.rb" which is the relative path to my environment.rb which in my quick testing Mailman tried to require.

Although my solution seems to work, in retrospect it is something of a hack. I think the better solution would be your option #2.

[johncant]

Setting nil as config.rails_root also fails in mailman master.

This stuff is relevant: https://github.com/titanous/mailman/issues/37 https://github.com/titanous/mailman/blob/master/lib/mailman/configuration.rb#L47

I'd imagine it would be better if someone got that issue fixed before @titanous released 0.5.4. When it's fixed, we might want to change mailman-rails to set config.rails_root to nil, since it's the documented behaviour.

In the meantime, the latest master branch of mailman should work with mailman-rails. I'll close the pull request if you don't mind.

[griffithac]

I don't mind. I will keep an eye out for the new release of the mailman gem. Hopefully titianious will make the change. In the meantime I am already in production with my hack.

Thanks,

Andrew -----Original Message----- From: John Cant notifications@github.com Date: Mon, 17 Dec 2012 04:27:53 To: johncant/mailman-railsmailman-rails@noreply.github.com Reply-To: johncant/mailman-rails reply@reply.github.com Cc: Andrew Griffithgriffithac@gmail.com Subject: Re: [mailman-rails] Change false to empty string to prevent error on line 41 of mailman gem ... (#1)

Setting nil as config.rails_root also fails in mailman master.

This stuff is relevant: https://github.com/titanous/mailman/issues/37 https://github.com/titanous/mailman/blob/master/lib/mailman/configuration.rb#L47

I'd imagine it would be better if someone got that issue fixed before @titanous released 0.5.4. When it's fixed, we might want to change mailman-rails to set config.rails_root to nil, since it's the documented behaviour.

In the meantime, the latest master branch of mailman should work with mailman-rails. I'll close the pull request if you don't mind.

---

Reply to this email directly or view it on GitHub: https://github.com/johncant/mailman-rails/pull/1#issuecomment-11439822

[johncant]

Awesome!