Upgrade RubyZip Gem to 1.2 #6938

jerryk55 · 2016-02-25T14:46:10Z

The rubyzip gem has been locked at 0.9.5 for 4 years due to a namespace conflict
caused by a change in 0.9.7. The api changed in 1.0 (3 years ago) and there are other gems that
rely on the new API, including winrm-fs, which is needed for winrm-elevated, being added
via another PR.

In addition gem ZipZip allows older RubyZip api code to continue to function alongside code using
the newer api so no code changes are necessary in our log handling code for this upgrade.

Spec tests against the new gem which cause the issue back with 0.9.7 pass.
A different spec test for an empty zip file was changed because the gem error message
changed between 0.9.5 and 1.2.0.

@Fryguy @jrafanie @chessbyte @roliveri please review and merge when appropriate. Thank you.

Fryguy · 2016-02-25T15:59:38Z

gems/pending/Gemfile

@@ -39,7 +39,8 @@ gem "ovirt",                   "~>0.7.2",           :require => false
 gem "pg",                      "~>0.18.2",          :require => false
 gem "psych",                   "~>2.0.12"
 gem "rest-client",             "=2.0.0.rc1",        :require => false
-gem "rubyzip",                 "=0.9.5",            :require => false  # TODO: Review 0.9.7 breaking log collection in FB14646
+gem "rubyzip",                 "~>1.2",             :require => false
+gem "zip-zip"


Should this be locked down to a specific version series? Also, should we lock down the rubyzip to the third digit?

I can put in 1.2.0 for ruby zip and 0.3.0 for zip-zip....

jerryk55 · 2016-02-25T16:14:07Z

@Fryguy Gemfile changes made as recommended. Please merge when ready. Thanks.

jrafanie · 2016-02-25T21:45:17Z

@jerryk55 I'm still seeing the original problem locally with your PR... trying to fix it.

irb(main):001:0> VMDB::Util.zip_logs("joe.zip", ["log/*.log"])
NoMethodError: undefined method `to_binary_dos_time' for 2016-02-19 11:44:52 -0500:Time
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:257:in `pack_local_entry'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:271:in `write_local_entry'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:161:in `block in update_local_headers'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:in `call'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:in `block in each'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:in `each'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:in `each'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:159:in `update_local_headers'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:70:in `close'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:in `ensure in open'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:in `open'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:311:in `block in commit'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:407:in `on_success_replace'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:310:in `commit'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:334:in `close'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:in `ensure in open'
    from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:in `open'
    from /Users/joerafaniello/Code/manageiq/lib/vmdb/util.rb:111:in `zip_logs'

jerryk55 · 2016-02-25T21:52:09Z

Really! I looked at the test and it looked like it made the same call....
Let me go back and look at it again.

Its definitely this line in util.rb:

zip.file.utime(time, entry)

Commenting out:

irb(main):001:0> VMDB::Util.zip_logs("joe.zip", ["log/*.log"])
=> "/mnt/hgfs/jerryk/redhat/manageiq/data/user/system/joe.zip"
irb(main):002:0>

Looking further....

On Thu, Feb 25, 2016 at 4:45 PM, Joe Rafaniello notifications@github.com
wrote:

@jerryk55 https://github.com/jerryk55 I'm still seeing the original
problem locally with your PR... trying to fix it.

irb(main):001:0> VMDB::Util.zip_logs("joe.zip", ["log/*.log"])
NoMethodError: undefined method to_binary_dos_time' for 2016-02-19 11:44:52 -0500:Time from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:257:inpack_local_entry'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:271:in write_local_entry' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:161:inblock in update_local_headers'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:in call' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:inblock in each'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:in each' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:ineach'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:159:in update_local_headers' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:70:inclose'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:in ensure in open' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:inopen'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:311:in block in commit' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:407:inon_success_replace'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:310:in commit' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:334:inclose'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:in ensure in open' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:inopen'
from /Users/joerafaniello/Code/manageiq/lib/vmdb/util.rb:111:in `zip_logs'

—
Reply to this email directly or view it on GitHub
#6938 (comment).

Jerry Keselman
Red Hat Virtualization R&Djerryk@redhat.com | p. 212 530 7873

jerryk55 · 2016-02-25T22:08:45Z

Yeah you're right (obviously). I guess the test doesn't test. Who woulda
thunk, four years later. Looking further.

On Thu, Feb 25, 2016 at 4:45 PM, Joe Rafaniello notifications@github.com
wrote:

@jerryk55 https://github.com/jerryk55 I'm still seeing the original
problem locally with your PR... trying to fix it.

irb(main):001:0> VMDB::Util.zip_logs("joe.zip", ["log/*.log"])
NoMethodError: undefined method to_binary_dos_time' for 2016-02-19 11:44:52 -0500:Time from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:257:inpack_local_entry'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry.rb:271:in write_local_entry' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:161:inblock in update_local_headers'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:in call' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:38:inblock in each'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:in each' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/entry_set.rb:37:ineach'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:159:in update_local_headers' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:70:inclose'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:in ensure in open' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/output_stream.rb:55:inopen'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:311:in block in commit' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:407:inon_success_replace'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:310:in commit' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:334:inclose'
from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:in ensure in open' from /Users/joerafaniello/.gem/ruby/2.2.4/gems/rubyzip-1.2.0/lib/zip/file.rb:104:inopen'
from /Users/joerafaniello/Code/manageiq/lib/vmdb/util.rb:111:in `zip_logs'

—
Reply to this email directly or view it on GitHub
#6938 (comment).

Jerry Keselman
Red Hat Virtualization R&Djerryk@redhat.com | p. 212 530 7873

jerryk55 · 2016-02-29T18:05:22Z

VMDB::Util:zip_logs updated to work correctly with RubyZip 1.2.
@jrafanie and I have worked to get this right.

@jrafanie, @Fryguy @chessbyte @roliveri please review and merge.

This is a prerequisite for #6945 so it would be great to get this in! Thanks.

The rubyzip gem has been locked at 0.9.5 for 4 years due to a namespace conflict caused by a change in 0.9.7. The api changed in 1.0 and there are other gems that rely on the new API, including winrm-fs, which is needed for winrm-elevated. Adding gem ZipZip allows older api code to continue to function alongside code using the newer api. Spec tests against the new gem which failed with 0.9.7 pass.

The error message for an empty zip file has been changed between rubyzip 0.9.5 and 1.2.0. Fix the test to the correct message.

The interface to change the utime on an entry in a zipfile has changed between RubyZip 0.9.5 and 1.2. Instead of calling zip.add with a filename and zip.file.utime with a File.mtime, convert the mtime to a Zip::DOSTime, and create a new Zip Entry, setting the time on the entry initializer. Then call zip.add with the Entry rather than the name.

Modify tests for add_zip_entry to coincide with changes required when updating from RubyZip 0.9.5 to 1.2.0.

jerryk55 · 2016-02-29T21:12:50Z

Spec tests for VMDB::Util needed tweaking to allow for the changes in RubyZip as well. Tested and pushed. Review as before when possible. Thanks @jrafanie

miq-bot · 2016-02-29T21:24:19Z

Checked commits jerryk55/manageiq@9820dd3~...93766ab with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
7 files checked, 1 offense detected

lib/vmdb/util.rb

🔴 Warn - Line 129, Col 5 - Lint/IneffectiveAccessModifier - private (on line 126) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.

miq-bot · 2016-02-29T21:46:37Z

<github_pr_commenter_batch />Some comments on commits jerryk55/manageiq@9820dd3~...93766ab

Gemfile changes detected. /cc @JPrause @simaishi

jrafanie · 2016-02-29T22:24:40Z

Looks good! Thanks for getting to the bottom of this @jerryk55.

Upgrade RubyZip Gem to 1.2

miq-bot added the dependencies label Feb 25, 2016

Fryguy reviewed Feb 25, 2016
View reviewed changes

jerryk55 force-pushed the rubyzip_1.1 branch from 7156e7f to 4f413f4 Compare February 25, 2016 16:13

jerryk55 mentioned this pull request Feb 25, 2016

Implement SSA of network-mounted HyperV virtual disks #6945

Merged

jerryk55 force-pushed the rubyzip_1.1 branch from 4f413f4 to 1ea3238 Compare February 25, 2016 21:27

jerryk55 force-pushed the rubyzip_1.1 branch from 1ea3238 to ac76634 Compare February 29, 2016 18:00

jerryk55 added 4 commits February 29, 2016 16:10

Fix Test for Empty Zip File

8090f60

The error message for an empty zip file has been changed between rubyzip 0.9.5 and 1.2.0. Fix the test to the correct message.

Adjust VMDB::Util spec tests for add_zip_entry

93766ab

Modify tests for add_zip_entry to coincide with changes required when updating from RubyZip 0.9.5 to 1.2.0.

jerryk55 force-pushed the rubyzip_1.1 branch from ac76634 to 93766ab Compare February 29, 2016 21:11

jrafanie added a commit that referenced this pull request Feb 29, 2016

Merge pull request #6938 from jerryk55/rubyzip_1.1

bb62d68

Upgrade RubyZip Gem to 1.2

jrafanie merged commit bb62d68 into ManageIQ:master Feb 29, 2016

jrafanie added this to the Sprint 37 Ending Mar 7, 2016 milestone Feb 29, 2016

jrafanie added the core label Feb 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade RubyZip Gem to 1.2 #6938

Upgrade RubyZip Gem to 1.2 #6938

jerryk55 commented Feb 25, 2016

Fryguy Feb 25, 2016

jerryk55 Feb 25, 2016

jerryk55 commented Feb 25, 2016

jrafanie commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 29, 2016

jerryk55 commented Feb 29, 2016

miq-bot commented Feb 29, 2016

miq-bot commented Feb 29, 2016

jrafanie commented Feb 29, 2016

Upgrade RubyZip Gem to 1.2 #6938

Upgrade RubyZip Gem to 1.2 #6938

Conversation

jerryk55 commented Feb 25, 2016

Fryguy Feb 25, 2016

Choose a reason for hiding this comment

jerryk55 Feb 25, 2016

Choose a reason for hiding this comment

jerryk55 commented Feb 25, 2016

jrafanie commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 29, 2016

jerryk55 commented Feb 29, 2016

miq-bot commented Feb 29, 2016

miq-bot commented Feb 29, 2016

jrafanie commented Feb 29, 2016