Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Circular dependencies on High Sierra #810

Closed
amirrajan opened this issue May 12, 2018 · 11 comments
Closed

Circular dependencies on High Sierra #810

amirrajan opened this issue May 12, 2018 · 11 comments
Milestone
3.0.0

Comments

@amirrajan
Copy link
Collaborator

amirrajan commented May 12, 2018 •
edited
Loading

On High Sierra, I'm getting the following runtime error:

(main)> 2018-05-11 20:01:59.342 The Ensign[11573:6208462] /usr/local/var/rbenv/versions/2.3
1/lib/ruby/gems/2.3.0/bundler/gems/ProMotion-0fcbf81019a2/lib/Pro: uninitialized constant P
oMotion::StatusBarModule (NameError)
        from /usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/ProMotion
0fcbf81019a2/lib/Pro
        from /usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/ProMotion
0fcbf81019a2/lib/Pro
2018-05-11 20:01:59.346 The Ensign[11573:6208462] *** Terminating app due to uncaught excep
ion 'NameError', reason: '/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/g
ms/ProMotion-0fcbf81019a2/lib/Pro: uninitialized constant ProMotion::StatusBarModule (NameE
ror)
        from /usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/ProMotion
0fcbf81019a2/lib/Pro
        from /usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/ProMotion
0fcbf81019a2/lib/Pro

Here is the dependency graph for ProMotion.

{"lib/ProMotion/version.rb"=>["lib/ProMotion/pro_motion.rb",
                              "motion_print-1.2.0/lib/../motion/motion_print/version.rb"],
 "lib/ProMotion/cocoatouch/table_view_cell.rb"=>["lib/ProMotion/table/cell/table_view_cell_module.rb"],
 "lib/ProMotion/table/cell/table_view_cell_module.rb"=>["lib/ProMotion/styling/styling.rb",
                                                        "./app/extensions/ui_color.rb",
                                                        "motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb"],
 "lib/ProMotion/cocoatouch/collection_view_cell.rb"=>["lib/ProMotion/collection/cell/collection_view_cell_module.rb"],
 "lib/ProMotion/collection/collection_screen.rb"=>["lib/ProMotion/cocoatouch/collection_view_controller.rb",
                                                   "lib/ProMotion/screen/screen_module.rb",
                                                   "lib/ProMotion/collection/collection_builder.rb",
                                                   "lib/ProMotion/collection/collection.rb",
                                                   "lib/ProMotion/collection/cell/collection_view_cell_module.rb"],
 "lib/ProMotion/collection/cell/collection_view_cell_module.rb"=>["lib/ProMotion/styling/styling.rb"],
 "lib/ProMotion/delegate/delegate.rb"=>["lib/ProMotion/delegate/delegate_parent.rb",
                                        "lib/ProMotion/delegate/delegate_module.rb"],
 "lib/ProMotion/delegate/delegate_parent.rb"=>["lib/ProMotion/delegate/delegate_module.rb"],
 "lib/ProMotion/delegate/delegate_module.rb"=>["lib/ProMotion/support/support.rb",
                                               "lib/ProMotion/tabs/tabs.rb",
                                               "lib/ProMotion/ipad/split_screen.rb"],
 "lib/ProMotion/screen/screen.rb"=>["lib/ProMotion/screen/screen_module.rb",
                                    "lib/ProMotion/cocoatouch/view_controller.rb"],
 "lib/ProMotion/screen/screen_navigation.rb"=>["lib/ProMotion/support/support.rb"],
 "lib/ProMotion/screen/screen_module.rb"=>["lib/ProMotion/styling/styling.rb",
                                           "lib/ProMotion/tabs/tabs.rb",
                                           "lib/ProMotion/screen/nav_bar_module.rb",
                                           "lib/ProMotion/screen/screen_navigation.rb",
                                           "lib/ProMotion/ipad/split_screen.rb",
                                           "lib/ProMotion/screen/status_bar_module.rb",
                                           "lib/ProMotion/support/support.rb",
                                           "motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb"],
 "lib/ProMotion/table/data/table_data.rb"=>["lib/ProMotion/table/data/table_data_builder.rb",
                                            "lib/ProMotion/table/table.rb",
                                            "lib/ProMotion/table/table_utils.rb"],
 "lib/ProMotion/collection/data/collection_data.rb"=>["lib/ProMotion/collection/data/collection_data_builder.rb",
                                                      "lib/ProMotion/collection/collection.rb",
                                                      "lib/ProMotion/table/table_utils.rb"],
 "lib/ProMotion/table/table.rb"=>["lib/ProMotion/table/table_class_methods.rb",
                                  "lib/ProMotion/table/table_builder.rb",
                                  "lib/ProMotion/table/table_utils.rb",
                                  "lib/ProMotion/table/extensions/searchable.rb",
                                  "lib/ProMotion/table/extensions/refreshable.rb",
                                  "lib/ProMotion/table/extensions/indexable.rb",
                                  "lib/ProMotion/table/extensions/longpressable.rb",
                                  "./app/extensions/ui_view.rb",
                                  "lib/ProMotion/styling/styling.rb",
                                  "motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb"],
 "lib/ProMotion/collection/collection.rb"=>["lib/ProMotion/collection/collection_class_methods.rb",
                                            "lib/ProMotion/collection/collection_builder.rb",
                                            "lib/ProMotion/table/table_utils.rb",
                                            "lib/ProMotion/cocoatouch/collection_view_cell.rb",
                                            "lib/ProMotion/styling/styling.rb"],
 "lib/ProMotion/web/web_screen.rb"=>["lib/ProMotion/web/web_screen_module.rb",
                                     "lib/ProMotion/cocoatouch/view_controller.rb",
                                     "lib/ProMotion/screen/screen_module.rb"],
 "lib/ProMotion/styling/styling.rb"=>["./app/extensions/ui_color.rb"],
 "lib/ProMotion/screen/nav_bar_module.rb"=>["./app/extensions/ui_view.rb",
                                            "lib/ProMotion/cocoatouch/navigation_controller.rb",
                                            "/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb",
                                            "/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/bubble-wrap-1.9.6/motion/ui/ui_bar_button_item.rb"],
 "lib/ProMotion/screen/status_bar_module.rb"=>["./app/extensions/ui_view.rb",
                                               "lib/ProMotion/tabs/tabs.rb"],
 "lib/ProMotion/stubs/dummy_view.rb"=>["./app/extensions/ui_view.rb"],
 "lib/ProMotion/collection/data/collection_data_builder.rb"=>["lib/ProMotion/cocoatouch/collection_view_cell.rb"],
 "lib/ProMotion/pro_motion.rb"=>["lib/ProMotion/web/web_screen_module.rb"],
 "lib/ProMotion/cocoatouch/ns_string.rb"=>["lib/ProMotion/cocoatouch/ns_url.rb"],
 "lib/ProMotion/web/web_screen_module.rb"=>["lib/ProMotion/cocoatouch/ns_url.rb",
                                            "/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/bubble-wrap-1.9.6/motion/core/ns_url_request.rb"],
 "lib/ProMotion/ipad/split_screen.rb"=>["lib/ProMotion/cocoatouch/split_view_controller.rb"],
 "lib/ProMotion/tabs/tabs.rb"=>["lib/ProMotion/cocoatouch/tab_bar_controller.rb",
                                "/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb"],
 "lib/ProMotion/table/data/table_data_builder.rb"=>["lib/ProMotion/cocoatouch/table_view_cell.rb"],
 "lib/ProMotion/table/grouped_table_screen.rb"=>["lib/ProMotion/cocoatouch/table_view_controller.rb",
                                                 "lib/ProMotion/screen/screen_module.rb",
                                                 "lib/ProMotion/table/table_utils.rb",
                                                 "lib/ProMotion/table/grouped_table.rb",
                                                 "lib/ProMotion/table/table_builder.rb"],
 "lib/ProMotion/table/table_screen.rb"=>["lib/ProMotion/cocoatouch/table_view_controller.rb",
                                         "lib/ProMotion/screen/screen_module.rb",
                                         "lib/ProMotion/table/table_utils.rb",
                                         "lib/ProMotion/table/table_builder.rb"],
 "lib/ProMotion/collection/collection_builder.rb"=>["lib/ProMotion/collection/cell/collection_view_cell_module.rb"],
 "lib/ProMotion/repl/repl.rb"=>["lib/ProMotion/repl/live_reloader.rb"],
 "lib/ProMotion/table/table_builder.rb"=>["lib/ProMotion/table/cell/table_view_cell_module.rb"],
 "lib/ProMotion/cocoatouch/tab_bar_controller.rb"=>["/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/bubble-wrap-1.9.6/motion/core/ns_user_defaults.rb",
                                                    "/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/motion_print-1.2.0/lib/../motion/motion_print/core_ext/string.rb"],
 "lib/ProMotion/table/extensions/refreshable.rb"=>["/usr/local/var/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/bubble-wrap-1.9.6/motion/core/time.rb"]}

You can add more logging statements within /Library/RubyMotion/lib/motion/project/dependency.rb.

The graph above was generated using the following patch:

Update the Dependency constructor to include a @has_circular_dependencies IVar

    def initialize(paths, dependencies)
      @file_paths = paths.flatten.sort
      @dependencies = dependencies
      @has_circular_dependencies = false
    end

Set @has_circular_dependencies to true within Dependency#cyclic?:

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

And then at the bottom of Dependency#run, print out the dependency graph:

    def run
      consts_defined  = {}
      consts_referred = {}
      @file_paths.each do |path|
     [....]
      if @has_circular_dependencies
        App.warn("===========================================================================================================================================================")
        App.warn("Full dependency graph #{dependency}")
        App.warn("===========================================================================================================================================================")
      end
      dependency
    end

To reason through dependency issues refer to this comment: rubymotion-community/motion-support#38 (comment)

Keep in mind that when trouble shooting this, be sure to run rake clean:all so that all files will be recompiled (performing rake will only recompile touched files which can change the dependency order).
@amirrajan
Copy link
Collaborator Author

This build order has worked in the wild and may give some insight on how things should be bult:

"/lib/ProMotion/tabs/tabs.rb"
"/lib/ProMotion/stubs/dummy_image_view.rb"
"/lib/ProMotion/stubs/dummy_view.rb"
"/lib/ProMotion/cocoatouch/collection_view_controller.rb"
"/lib/ProMotion/styling/styling.rb"
"/lib/ProMotion/screen/nav_bar_module.rb"
"/lib/ProMotion/support/support.rb"
"/lib/ProMotion/screen/screen_navigation.rb"
"/lib/ProMotion/ipad/split_screen.rb"
"/lib/ProMotion/screen/status_bar_module.rb"
"/lib/ProMotion/screen/screen_module.rb"
"/lib/ProMotion/collection/collection_builder.rb"
"/lib/ProMotion/collection/collection_class_methods.rb"
"/lib/ProMotion/table/table_utils.rb"
"/lib/ProMotion/collection/collection.rb"
"/lib/ProMotion/collection/cell/collection_view_cell_module.rb"
"/lib/ProMotion/collection/collection_screen.rb"
"/lib/ProMotion/collection/data/collection_data_builder.rb"
"/lib/ProMotion/collection/data/collection_data.rb"
"/lib/ProMotion/logger/logger.rb"
"/lib/ProMotion/cocoatouch/navigation_controller.rb"
"/lib/ProMotion/cocoatouch/ns_url.rb"
"/lib/ProMotion/cocoatouch/ns_string.rb"
"/lib/ProMotion/cocoatouch/view_controller.rb"
"/lib/ProMotion/cocoatouch/collection_view_cell.rb"
"/lib/ProMotion/table/cell/table_view_cell_module.rb"
"/lib/ProMotion/cocoatouch/table_view_cell.rb"
"/lib/ProMotion/cocoatouch/tab_bar_controller.rb"
"/lib/ProMotion/cocoatouch/table_view_controller.rb"
"/lib/ProMotion/cocoatouch/split_view_controller.rb"
"/lib/ProMotion/web/web_screen_module.rb"
"/lib/ProMotion/web/web_screen.rb"
"/lib/ProMotion/repl/live_reloader.rb"
"/lib/ProMotion/repl/repl.rb"
"/lib/ProMotion/delegate/delegate_module.rb"
"/lib/ProMotion/delegate/delegate_parent.rb"
"/lib/ProMotion/delegate/delegate.rb"
"/lib/ProMotion/pro_motion.rb"
"/lib/ProMotion/table/table_class_methods.rb"
"/lib/ProMotion/table/table_builder.rb"
"/lib/ProMotion/table/extensions/searchable.rb"
"/lib/ProMotion/table/extensions/refreshable.rb"
"/lib/ProMotion/table/extensions/indexable.rb"
"/lib/ProMotion/table/extensions/longpressable.rb"
"/lib/ProMotion/table/table.rb"
"/lib/ProMotion/table/grouped_table.rb"
"/lib/ProMotion/table/table_screen.rb"
"/lib/ProMotion/table/data/table_data_builder.rb"
"/lib/ProMotion/table/data/table_data.rb"
"/lib/ProMotion/table/grouped_table_screen.rb"
"/lib/ProMotion/screen/screen.rb"
"/lib/ProMotion/version.rb"

@jamonholmgren
Copy link
Owner

This only occurs on High Sierra?

@amirrajan
Copy link
Collaborator Author

@jamonholmgren Only on High Sierra. Dir.glob doesn't behave the same way between Sierra and High Sierra. So all rubies are essentially broken when it comes to depending on Dir.glob for sort order. More details about that here: rubymotion-community/motion-support#38 (comment)

dir.c in Ruby source explicitly ordered files differently for the MacOS file system. Now that High Sierra is on a new file system type, that code path is no longer executed, which leads to a non-deterministic sort order... yay.

@jamonholmgren
Copy link
Owner

Oof, yeah, that kind of throws a kink in things for code that relies on that sort order.

@andrewhavens
Copy link
Collaborator

@amirrajan I’m back from vacation now and might be able to help. I think I recently introduced that module in the last few months, however, I’m running on High Sierra and haven’t experienced this issue.

@amirrajan
Copy link
Collaborator Author

I'll see if I can create a repo with minimum steps to force an exception.

@amirrajan
Copy link
Collaborator Author

image

Anyone see it?

@amirrajan
Copy link
Collaborator Author

Found it. It's not in the graph above. It's here: https://github.com/infinitered/ProMotion/blob/master/lib/ProMotion.rb#L20

The dependency mapping code isn't needed anymore. Explicitly setting the dependency hash short circuits RubyMotion's dependency resolution.

@jamonholmgren
Copy link
Owner

So commenting out this section fixes the problem?

@amirrajan
Copy link
Collaborator Author

Yep. You lose the ability to use app.detect_dependencies = false. But I'd strongly recommend you don't do that unless you know what you're getting into (at which point it's up to the dev to figure out the compile order in their own application). I'd recommend making this change and then doing a SemVer bump to communicate that this may introduce breaking changes if you originally went with app. detect_dependencies = false.

@amirrajan amirrajan mentioned this issue Sep 24, 2018
Closed
@andrewhavens andrewhavens added this to the 3.0.0 milestone Sep 26, 2018
@markrickert
Copy link
Contributor

I've confirmed that removing that section app.files_dependencies from lib/Promotion/rb fixes my compiling issues.

markrickert added a commit that referenced this issue Oct 24, 2018
@markrickert
Removes all app.files_dependencies
75b2c8d 
Fixes #810.

There should probably be a larger discussion as to if/when this should be included in `master` just because this is a High-Sierrra issue. Has anyone tried it on Mojave?
@markrickert markrickert mentioned this issue Oct 24, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.0
Development

No branches or pull requests
4 participants
@markrickert @andrewhavens @amirrajan @jamonholmgren