xforty / drush-deploy

Drupal deploy tool based on Capistrano and Drush
https://github.com/xforty/drush-deploy/wiki
Other
35 stars 4 forks source link

Remove dependency on php_serialize #26

Open patcon opened 11 years ago

patcon commented 11 years ago

A totally minor thing, but you could probably remove the dep on that lib if you used drush eval 'print json_encode([...]);' instead of using php_serialize() on the PHP array objects :)

I see you also use it in a drush php-script call in databases.rb, but it should be swappable too. Maybe I'm missing something though.

➜  drupal-skeletor git:(master) ✗ irb
irb(main):001:0> require 'json'
=> true
irb(main):002:0> require 'pp'
=> true
irb(main):003:0> pp JSON.parse %x[drush eval 'print json_encode(_drush_sitealias_all_list());']
{"@none"=>{"root"=>"", "uri"=>""},
 "@test.vagrant"=>
  {"uri"=>"default",
   "root"=>"/var/www/skeletor/current",
   "remote-user"=>"vagrant",
   "remote-host"=>"33.33.33.10",
   "remote-port"=>"2222"},
 "@test"=>{"site-list"=>["@test.vagrant"]}}
=> {"@none"=>{"root"=>"", "uri"=>""}, "@test.vagrant"=>{"uri"=>"default", "root"=>"/var/www/skeletor/current", "remote-user"=>"vagrant", "remote-host"=>"33.33.33.10", "remote-port"=>"2222"}, "@test"=>{"site-list"=>["@test.vagrant"]}}
irb(main):004:0> json_aliases = JSON.parse %x[drush eval 'print json_encode(_drush_sitealias_all_list());']
=> {"@none"=>{"root"=>"", "uri"=>""}, "@test.vagrant"=>{"uri"=>"default", "root"=>"/var/www/skeletor/current", "remote-user"=>"vagrant", "remote-host"=>"33.33.33.10", "remote-port"=>"2222"}, "@test"=>{"site-list"=>["@test.vagrant"]}}
irb(main):005:0> json_aliases['@test.vagrant']['remote-user']
=> "vagrant"
irb(main):006:0> 
sylus commented 11 years ago

Hey @patcon I am interested in this as getting an error in windows using PHP Serialize but not in my mac

'do_unserialize' : undefined_method '[]' for nil:NilClass (NoMethodError)

How would the following look leveraging json_encode?

def load_configuration
      @aliases = PHP.unserialize(`#{@drush} eval 'print serialize(_drush_sitealias_all_list());'`).inject({}) do |h,(k,v)|
        if k != '@none'
          h[k.sub(/^@/,'')] = v
        end
        h
      end
      @normalized_aliases = {}
    end
patcon commented 11 years ago

Heyo!

    def load_configuration
      stdout = %x[ #{@drush} eval 'print json_encode(_drush_sitealias_all_list());' ]
      @aliases = JSON.parse(stdout).inject({}) do |h,(k,v)|
        if k != '@none'
          h[k.sub(/^@/,'')] = v
        end
        h
      end
      @normalized_aliases = {}
    end
patcon commented 11 years ago

Basically just swap:

:)

patcon commented 11 years ago

Untested, but that should... work?

medlefsen commented 11 years ago

I'm not in a position to test right now but I created a branch based on the one from @patcon with one small change that I think is needed.

https://github.com/xforty/drush-deploy/tree/feature/remove-serialize-lib

patcon commented 11 years ago

Ah cool! Just wanted to point out that the function name might be misleading, as it's not really unserializing, right? Or am I missing something? It's decoding a json string into a hash :)

sylus commented 11 years ago

Tested out the feature branch and works great! :)

sylus commented 11 years ago

Wait my bad I wasn't running off of the feature branch since wasn't using bundle exec. Now running with it I get the following error:

configuration.rb:34:in 'load_configuration': uninitialized constant Drush Deploy::Configuration::JSON (NameError)

I added to drush_deploy.rb:

require 'json'

and now the error I receive is:

common.rb:155:in 'initialize' : A JSON text must at least contain two octets.

I think the second error is actually just windows + msysgit again. As when I added require 'json' the script worked on my mac.

patcon commented 11 years ago

Might be odd that it's empty regardless, but output should probably be initialized somewhere with output = {} http://stackoverflow.com/questions/8390256/a-json-text-must-at-least-contain-two-octets

Edit: I don't think common.rb:155 is the useful place in the stacktrace :) Anyhow, we haven't actually tested any of this stuff, so you'll probably need to go through and debug some other logic around assumptions of what's in a string -- My guess is that it should be possible to get it working fine on both systems