search

infinitered / cdq

Core Data Query for RubyMotion
MIT License
172 stars 35 forks source link

remove coordinator.lock and coordinator.unlock #141

Closed cmckni3 closed 4 years ago

cmckni3 commented 5 years ago

use performBlockAndWait to create iCloud store

andrewhavens commented 5 years ago

Ah, I see the lock/unlock methods have been deprecated. https://developer.apple.com/documentation/coredata/nspersistentstorecoordinator/1468775-lock?language=objc

I wonder if @wndxlori has any thoughts on this

wndxlori commented 5 years ago

Let me fix the build, then we can get this merged in.

wndxlori commented 5 years ago

For anyone else reading, here’s more details on the what and why of this https://stackoverflow.com/questions/34334264/lock-is-deprecated-first-deprecated-in-ios-8-0-use-performblockandwait-in

andrewhavens commented 5 years ago

Sorry @wndxlori I didn't see your comment. I just finished fixing the build. So the part that is still failing should be fixed by this PR.

cmckni3 commented 5 years ago

Exactly, I forgot to mention that in the commit message

Merged master into my branch. Can clean up the history if necessary

cmckni3 commented 5 years ago

hmmm is performBlockAndWait deprecated? I can't find anything about such but @amirrajan just informed me of it

andrewhavens commented 5 years ago

@cmckni3 No, it's not deprecated. It's right here: https://developer.apple.com/documentation/coredata/nspersistentstorecoordinator/1468862-performblockandwait?language=objc

cmckni3 commented 5 years ago

correct, I found it in the Objective-C documentation. Swift docs only mention perform

wndxlori commented 5 years ago

Sorry I spaced. Pneumonia will do that. The build fails for this PR, and I downloaded your repo clone, and tried to build this branch and it barfed for me too. Can you fix that, so we can get this merged and released @cmckni3?

cmckni3 commented 5 years ago

@wndxlori are you referring to the spec errors?

This error happens on the specs on master for me:

  - performs a limited fetchCoreData: error: -addPersistentStoreWithType:SQLite configuration:(null) URL:file:///Users/cmcknight/Library/Developer/CoreSimulator/Devices/5C1C25DC-6C75-4490-B549-49745455CC8E/data/Containers/Data/Application/A2998C22-83A8-40C1-95C4-637F09C81A55/Documents/CDQ.sqlite options:{
    NSInferMappingModelAutomaticallyOption = 1;
    NSMigratePersistentStoresAutomaticallyOption = 1;
} ... returned error NSCocoaErrorDomain(512) with userInfo dictionary {
    reason = "Failed to create file; code = 24";
}
wndxlori commented 5 years ago

That’s odd. I get no errors on master. And the Travis build for master is green too.

I’m building on Mojave, and using Ruby 2.6.3 to run the tests.

On Oct 17, 2019, at 4:05 PM, Chris McKnight notifications@github.com wrote:

@wndxlori https://github.com/wndxlori are you referring to the spec errors?

This error happens on the specs on master for me:

cmckni3 commented 5 years ago

OSX: Mojave RM: 6.4 XCode: 11.1 Ruby: 2.5.6

I get this error sporadically on my branch:

2019-10-21 17:50:05.878 CDQ[31370:12472113] *** Terminating app due to uncaught exception 
'TypeError', reason: 'motion/cdq/store.rb:97:in `block in create_icloud_store': exception object expected (TypeError)

EDIT I see the same error on Travis CI

Let me run on master a few times

cmckni3 commented 5 years ago

Yeah there's something going on with specs on my branch. The app included doesn't use icloud storage so that doesn't help test.

Timestamp Update Case
  - should not set created_atCoreData: 

Bacon::Error: #<NSManagedObjectContext:0x600003b6a3e0>.==(nil) failed
    spec/cdq/context_spec.rb:75:in `<main>': CDQ Context Manager - pops temporarily if passed a block
    motion/cdq/context.rb:50:in `block in pop'
    motion/cdq/context.rb:303:in `save_stack'
    motion/cdq/context.rb:48:in `pop'
    spec/cdq/context_spec.rb:74:in `<main>'
wndxlori commented 5 years ago

@cmckni3 Have you had any time to work on fixing the build for this PR?

cmckni3 commented 5 years ago

Haven't had time to look at it.

Interesting but a bit unrelated: NSPersistentStoreUbiquitousContentNameKey and NSPersistentStoreUbiquitousContentURLKey have been deprecated for awhile.

cmckni3 commented 5 years ago

🤷‍♂️ (╯°□°）╯︵ ┻━┻

wndxlori commented 5 years ago

So I pulled your branch, and this failure is weird. I can duplicate the error that Travis is showing... sometimes. And here's the thing that really bothers me... in the test that fails, it is failing in a method that shouldn't even have been called. Basically, current calls this:

    def create_store
      if invalid?
        raise "No model found.  Can't create a persistent store coordinator without it."
      else
        if @icloud
          create_icloud_store
        else
          create_local_store
        end
      end
    end

And for this test, invalid is SUPPOSED to be TRUE. Instead, it's running down into create_icloud_store and dying (as it should, because there's no model).

cmckni3 commented 5 years ago

Yeah it’s very flaky. I did some investigation yesterday and master sometimes fails too as seen in #149

wndxlori commented 5 years ago

Seriously tempting to just ditch the iCloud stuff completely at this point. https://stackoverflow.com/questions/47137659/alternate-to-nspersistentstoreubiquitouscontentnamekey-key-in-core-data-in-ios-1

cmckni3 commented 5 years ago

I ran into the exception crashing because the wrong path and some database I/O errors yesterday.

Thinking it’s a race condition due to the async queue.

cmckni3 commented 5 years ago

yeah ditching the iCloud piece is an option. It was deprecated in iOS 10 iirc from my digging yesterday.

I probably should have tried those keys out in a blank XCode project with CoreData.

cmckni3 commented 4 years ago

Want to close in favor of #150?

wndxlori commented 4 years ago

Probably, yes. However, I think we have a deeper race condition that needs fixing there, too. In #150, the build works fine for iOS, and fails ~25% of the time for macOS. Grrr.

cmckni3 commented 4 years ago

I noticed that. How annoying