rubymotion-community / motion-support

Commonly useful extensions to the standard library for RubyMotion
MIT License
132 stars 28 forks source link

undefined method `+' for class `Date' (NameError) #38

Open naiyyar opened 6 years ago

naiyyar commented 6 years ago

Hello,

I have updated ruby motion version to v5 and using xcode9.2(ios11) and getting undefined method+' for class Date' (NameError)

/.rvm/gems/ruby-2.3.1/gems/motion-support-1.1.0/motion/core_ext/date/calculation: undefined method+' for class Date' (NameError)

While 2 days ago same was working absolutely fine with xcode8.3(ios10). and before updating rubymotion version.

Thanks

amirrajan commented 6 years ago

@naiyyar the issue is with High Sierra: the system version of ruby was upgraded to 2.3.1 from 1.8. There is a gem that ships with Ruby called ripper which is used to determine dependencies between ruby classes. The one that ships with 2.3.1 works differently than the one that ships with 1.8. I'm working on a fix for this, but there is a work around that you can use right away. Will provide more info soon.

edit: this ended up not being the issue because the Rakefile has app.detect_dependencies set to false.

amirrajan commented 6 years ago

The following update to motion-support's Rakefile is a quick fix (the files array needs to include the fully qualified location of the .rb files... I'll post a more robust version soon):

Motion::Project::App.setup do |app|
  # Use `rake config' to see complete project settings.
  app.name = 'MotionSupport'
  app.detect_dependencies = false
  app.sdk_version = '10.0'
  app.deployment_target = '10.0'

  files = [
    'motion-support/motion/_stdlib/date.rb',
    'motion-support/motion/_stdlib/array.rb',
    'motion-support/motion/_stdlib/cgi.rb',
    'motion-support/motion/_stdlib/time.rb',
    'motion-support/motion/_stdlib/enumerable.rb'
  ]

  files.each do |f|
    app.ordered_build_files.delete(f)
  end

  files.each do |f|
    app.ordered_build_files.unshift(f)
  end
end
naiyyar commented 6 years ago

Ok. Thanks @amirrajan

Now How can i fix it in my app. Should i remove the motion-support until fix for this is available?

also i'm unable to create a new app

screen shot 2018-01-20 at 12 05 23 pm
naiyyar commented 6 years ago

Now after removing motion-support Always getting below error event after removing all code from home screen

screen shot 2018-01-20 at 12 11 57 pm
naiyyar commented 6 years ago

@amirrajan i forked motion-support gem and changed rake file accordingly and still getting same issue https://github.com/naiyyar/motion-support/blob/master/Rakefile

amirrajan commented 6 years ago

@naiyyar fully qualify the paths and see if that works?

files = [
    File.expand_path('motion-support/motion/_stdlib/date.rb'),
    File.expand_path('motion-support/motion/_stdlib/array.rb'),
    File.expand_path('motion-support/motion/_stdlib/cgi.rb'),
    File.expand_path('motion-support/motion/_stdlib/time.rb'),
    File.expand_path('motion-support/motion/_stdlib/enumerable.rb')
  ]
naiyyar commented 6 years ago

Not Working. When running rake from motion-support's directory there is no error after specifying fully qualify the paths(yes before adding paths to rake there was same error)

When adding gem to my app still getting same error

screen shot 2018-01-21 at 11 14 12 am
amirrajan commented 6 years ago

It's a bug in Ruby on High Sierra it seems. Just tracked it down. Examples as screenshots.

sierra

high_sierra

Working on a patch for RubyMotion to get around this.

naiyyar commented 6 years ago

Ok i'll try this after release. For now i removed gem from the app. Thanks @amirrajan for the help.

amirrajan commented 6 years ago

I'll get the exact Rakefile setup for this repo as soon as I get it working on High Sierra.

amirrajan commented 6 years ago

Related: https://github.com/clayallsopp/motion-require/issues

jeremiahlukus commented 6 years ago

https://github.com/jeremiahlukus/motion-support/commit/52d4abf982a44abbfb98eac97dc41e950a970a45

This fixes it

colinta commented 6 years ago

I added @jeremiahlukus's patch to master, @naiyyar can you try installing the gem from source control and letting me know if that fixes things? If it does I'll push a release to rubygems.

wndxlori commented 6 years ago

@colinta that did fix my problem. It's basically a workaround (commenting out the code that crashed) for the deeper issue of of the changed behaviour on High Sierra.

My concern is that we are stripping out that duration functionality. I'm not using it, but if anyone else does, this'll break their code. But who knows if anyone is even using it... it does not appear to be covered by any unit tests.

colinta commented 6 years ago

Really good point, let's rethink this... I'll revert that commit for now.

amirrajan commented 6 years ago

The deeper issue is: https://github.com/clayallsopp/motion-require/issues/16

This is a much harder problem (code parsing with roxor). So a quick fix may be worth doing now? Up to y'all.

cc: @wndxlori , @colinta

naiyyar commented 6 years ago

@colinta yes. It's working.

Thanks

marcamillion commented 6 years ago

Any progress on this issue guys?

As an FYI, I too tried @jeremiahlukus's fork as seen here: https://github.com/jeremiahlukus/motion-support/commit/52d4abf982a44abbfb98eac97dc41e950a970a45 and it worked like a charm.

wndxlori commented 6 years ago

Amir has a fork that I'm waiting on, to test a fix @marcamillion. Hopefully we'll get it all straightened out this weekend.

colinta commented 6 years ago

Thanks, Lori!

On Feb 17, 2018, at 9:16 AM, Lori M Olson notifications@github.com wrote:

Amir has a fork that I'm waiting on, to test a fix @marcamillion. Hopefully we'll get it all straightened out this weekend.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

amirrajan commented 6 years ago

Updates:

The issue comes down to cyclical dependencies (which are easy enough to resolve... just tedious). Ultimately, I'm recommending that we remove the reliance on motion_require and set app.detect_dependencies to true within the build configuration. Motion Support is particularly susceptible to this cyclical issue because core types are being patched.

Steps to diagnose:

  1. Start with putting the following top of your Rakefile:
# -*- coding: utf-8 -*-
$:.unshift("/Library/RubyMotion/lib")

module Motion; module Project
  class Dependency
    def cyclic?(dependencies, def_path, ref_path)
      deps = dependencies[def_path]
      if deps
        if deps.include?(ref_path)
          App.warn("Possible cyclical dependency between #{def_path} and #{ref_path}'s class hierarchy. Consider revision if runtime exceptions occur around undefined symbols.")
          return true
        end
        deps.each do |file|
          return true if cyclic?(dependencies, file, ref_path)
        end
      end

      false
    end
  end
end; end
  1. Set app.detect_dependencies = true inside of the Motion::Project::App.setup do |app| block.

  2. Perform a build to see where cyclical dependencies show up (based on the warning message).

  3. Decide if a module can be created to "promote" the cyclical dependency.

  4. If needed, you can explicitly reference a class within another file to force the right compilation order. Example:

# in hash.rb
class Hash
  defined?(Enumerable)
end

# in array.rb
class Array
  defined?(Object)
  defined?(Enumerable)
end
  1. If you can't get rid of a cyclical dependency because of a module referencing a class (and vice versa), then use Object.get_const('ClassName') in one of the files to help dependency.rb figure out the compilation order. An example of this workaround is shown below:
  class Inflections
    defined?(Object) # think of this as a preprocessor directive

    StringClass = Object.const_get('String') # this ensures that a circular dependency doesn't exist between `String` and `Inflections` (Inflections should be built first). 

    # Specifies a new pluralization rule and its replacement. The rule can
    # either be a string or a regular expression. The replacement should
    # always be a string that may include references to the matched data from
    # the rule.
    def plural(rule, replacement)
      @uncountables.delete(rule) if rule.is_a?(StringClass)
      @uncountables.delete(replacement)
      @plurals.prepend([rule, replacement])
    end

    # Specifies a new singularization rule and its replacement. The rule can
    # either be a string or a regular expression. The replacement should
    # always be a string that may include references to the matched data from
    # the rule.
    def singular(rule, replacement)
      @uncountables.delete(rule) if rule.is_a?(StringClass)
      @uncountables.delete(replacement)
      @singulars.prepend([rule, replacement])
  end
end

I know this feels weird (and it's worth talking through). But at the end of the day. RubyMotion is compiled so these quasi-preprocessor-directives are needed.

I have this repo and am slowly flattening and working through the circular dependencies and removing motion_require (trying to get a minimum patch this fixes these issues): https://github.com/amirrajan/motion-support-trouble-shooting

Here is an example of a set of dependent classes that have been restructured to remove the motion_require dependency.

Ask and you shall receive contributor access so you can help me uncomment code piece by piece.

wndxlori commented 6 years ago

This is fixed in the most recent version of motion-support.