Problem with migrations path inside a JAR file #588

Closed
bernd opened this Issue Nov 24, 2012 · 5 comments

Projects

None yet

2 participants

@bernd

I'm getting the following error running migrations with Sequel::Migrator.run inside a JAR.

Sequel::Migrator::Error: No target version available, probably because no migration files found or filenames don't follow the migration filename convention
        initialize at jar:file:/tmp/trafcol-0.0.1.jar!/gems/sequel-3.41.0/lib/sequel/extensions/migration.rb:501
               run at jar:file:/tmp/trafcol-0.0.1.jar!/gems/sequel-3.41.0/lib/sequel/extensions/migration.rb:381
  create_database! at jar:file:/tmp/trafcol-0.0.1.jar!/trafcol/lib/trafcol/db_connection.rb:23
        initialize at jar:file:/tmp/trafcol-0.0.1.jar!/trafcol/lib/trafcol/db_connection.rb:12
        initialize at jar:file:/tmp/trafcol-0.0.1.jar!/trafcol/lib/trafcol/persister.rb:12
          __send__ at org/jruby/RubyBasicObject.java:1667
          __send__ at org/jruby/RubyBasicObject.java:1673
              send at org/jruby/RubyKernel.java:2081
          dispatch at jar:file:/tmp/trafcol-0.0.1.jar!/gems/celluloid-0.12.3/lib/celluloid/calls.rb:57
    handle_message at jar:file:/tmp/trafcol-0.0.1.jar!/gems/celluloid-0.12.3/lib/celluloid/actor.rb:323
        initialize at jar:file:/tmp/trafcol-0.0.1.jar!/gems/celluloid-0.12.3/lib/celluloid/tasks/task_fiber.rb:22

The directory in Sequel::IntegerMigrator#get_migration_files looks like this: jar:file:/home/work/git/trafcol/trafcol-0.0.1.jar!/trafcol/config/migrations. It looks like Dir.new(directory) cannot handle the JAR path. After applying the following diff it starts to work.

diff --git lib/sequel/extensions/migration.rb lib/sequel/extensions/migration.rb
index 7820bf8..107db6e 100644
--- lib/sequel/extensions/migration.rb
+++ lib/sequel/extensions/migration.rb
@@ -536,7 +536,7 @@ module Sequel
     # Returns any found migration files in the supplied directory.
     def get_migration_files
       files = []
-      Dir.new(directory).each do |file|
+      Dir["#{directory}/*.rb"].map {|f| File.basename(f) }.each do |file|
         next unless MIGRATION_FILE_PATTERN.match(file)
         version = migration_version_from_file(file)
         if version >= 20000101

Not sure if that'd be a proper fix. :)

Environment

  • jruby 1.7.0 (1.9.3p203) 2012-10-22 ff1ebbe on Java HotSpot(TM) 64-Bit Server VM 1.7.0_09-b05 [linux-amd64]
  • Sequel 3.41.0
@jeremyevans
Owner

Dir.new not accepting a jar path accepted by Dir.[] seems like a bug in JRuby. The problem with your fix is that if there are special characters in the directory path (e.g. *), Dir.[] will treat them specially, which is not desired.

Can you find out from the JRuby team if this is a bug or expected behavior? If it is expected behavior on JRuby, can you find out from them how to iterate over a directory inside a jar file?

@bernd

Yep, will do. Thanks!

@bernd

There are JRuby tickets for similar problems. It seems to be a new behaviour since JRuby 1.7.0.

So this might be a workaround util it's fixed in JRuby 1.7.x.

diff --git lib/sequel/extensions/migration.rb lib/sequel/extensions/migration.rb
index 7820bf8..9bf3fc4 100644
--- lib/sequel/extensions/migration.rb
+++ lib/sequel/extensions/migration.rb
@@ -536,7 +536,7 @@ module Sequel
     # Returns any found migration files in the supplied directory.
     def get_migration_files
       files = []
-      Dir.new(directory).each do |file|
+      Dir.new(directory.gsub(/^jar:/, '')).entries.each do |file|
         next unless MIGRATION_FILE_PATTERN.match(file)
         version = migration_version_from_file(file)
         if version >= 20000101
@jeremyevans
Owner

I'm against adding hacks like this to work around bugs in ruby implementations. Your hack would break code that uses jar: outside of JRuby (e.g. jar:head is a valid directory name in most unix operating systems, and your code would change the directory name to head).

In this case it looks like JRuby will fix the underlying issue in a point release, so you can either use 1.6.8 until they have fixed the 1.7 series, or you can fix this in the app by applying the substitution to the directory name before passing it to the migration code. I would recommend using \A instead of ^ and sub instead of gsub, since you only want to change the beginning of the string.

@bernd

Oh, stupid me! Passing the cleaned directory is of course much simpler. Thanks for the clue stick! :)

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