magicalpanda / MagicalRecord

Super Awesome Easy Fetching for Core Data!
Other
10.79k stars 1.79k forks source link

Import directives are broken in Carthage artefact #1047

Open dodikk opened 9 years ago

dodikk commented 9 years ago

Steps :

  1. Follow the install instruction for Carthage
  2. Look at the built framework

    Actual result

Headers layout is flat

screen shot 2015-07-17 at 12 34 57 pm

Expected result

Headers layout matches the repo layout

screen shot 2015-07-17 at 12 35 37 pm

dodikk commented 9 years ago

I've proposed to add the feature to Carthage but it's getting rejected https://github.com/Carthage/Carthage/issues/623

dodikk commented 9 years ago

The #import <MagicalRecord/MagicalRecord.h> fails when I attempt to use the code as a static library

In file included from /Users/dodikk/Desktop/MagicalRecordTest/MagicalRecordTest/ViewController.m:11:
MagicalRecord/MagicalRecord/MagicalRecord.h:17:9: fatal error: 'MagicalRecord/MagicalRecordInternal.h' file not found
#import <MagicalRecord/MagicalRecordInternal.h>
        ^
1 error generated.
dodikk commented 9 years ago

https://github.com/dodikk/MagicalRecord-filesystem-bug example project

dodikk commented 9 years ago

@tonyarnold , what do you think is the right way organising the repo?

tonyarnold commented 9 years ago

Hi @dodikk I haven't had time to look at your example project yet — I'll try to have a look before the end of the week. The static library should be exporting it's headers into a subdirectory called "MagicalRecord" — not sure if that's happening or not (I don't use static libraries).

Leave it with me.

dodikk commented 9 years ago

Thanks for replying.

My point is I'd really like to use a umbrella header when building MagicalRecord from source

Unfortunately, I can't do that due to limitations of Apple's framework target template and the workaround used in the MagicalRecord library.

fanxiangyang commented 9 years ago

i also use the cartage

  1. Build Phases->magicalRecord.Framework request为optional 2.appdelegate.m [MagicalRecord setupCoreDataStackWithStoreNamed:@"MyData.sqlite"]; 3.NSManagedObjectContext *localContext = [NSManagedObjectContext MR_defaultContext];
// Create a new Person in the current thread context
Preson *person                          = [Preson MR_createEntityInContext:localContext];
person.name                        = firstname;
person.age                              = age;

4.error :MR_defaultContext or MR_createEntityInContext an so on unrecognized selector

tonyarnold commented 9 years ago

OK guys, could I get you to verify that the fix in #1051 fixes the problems you're seeing? Thanks!

dodikk commented 9 years ago

@tonyarnold so, now you force the users enabling a recursive "User header search path" lookup. Moreover, the #import "" directive is not framework friendly.

So, I can suggest the following solutions : 1) Storing all public headers in a directory named MagicalRecord User's code : #import <MagicalRecord/MRSomePublicHeader.h>

2) Using a custom script to post-process a build and restore the framework's structure. User's code : #import <MagicalRecord/Path/To/MRSomePublicHeader.h>

3) Just ignore the "headers issue" as the current carthage build, kind of, works. \ static library users and contributors are on their own

dodikk commented 9 years ago

could I get you to verify that the fix in #1051 fixes the problems you're seeing?

@tonyarnold , the pull request does not fix the issue for me. See the line comments and the one above.

dodikk commented 9 years ago

@fanxiangyang I believe, it is a different issue. Have you tried enabling the -all_load linker flag? Such unrecognized selector errors sometimes happen while using third-party categories.

tonyarnold commented 9 years ago

Using a custom script to post-process a build and restore the framework's structure

Your own framework above is the first time I've seen a framework with internal folder structure around the headers — it's always flat, with everything in a single directory. I'm not sure what you're doing is invalid, I just don't see what difference it makes to the end result when a framework is in use.

All of the other solutions you've suggested are already in place in the current public release.

static library users and contributors are on their own

I dislike abandoning building by dragging the source into your own project, but that should be able to be resolved by setting the appropriate header search locations. Contributors are fine — everything builds as expected within the project.

fanxiangyang commented 9 years ago

@dodikk I use _all_load not solve my question ,last I use pod replace Carthage ,it is OK

dodikk commented 9 years ago

Your own framework above is the first time I've seen a framework with internal folder structure around the headers

Actually, the MagicalRecord repo has an internal folder structure. It has "Core" and "Categories". For some reason you make things differently when building a framework.

dodikk commented 9 years ago

I just don't see what difference it makes to the end result when a framework is in use.

Suppose, I'm using MagicalRecord in my app and I have discovered a bug. So I fix it and build MR from source now. Hence, I have to replace the MR imports all over the place in my code base. I have to do so because now the directories structure is not flat (It has "Core" and "Categories").

dodikk commented 9 years ago

I dislike abandoning building by dragging the source into your own project, but that should be able to be resolved by setting the appropriate header search locations.

Actually, I was referring to the sub-project and target dependencies approach https://www.cocoanetics.com/2011/12/sub-projects-in-xcode/

Moreover, the framework and the "drag&drop" use different header settings : header search path VS user header search path

tonyarnold commented 9 years ago

For some reason you make things differently when building a framework

I think you might have some misunderstandings about how the headers within frameworks work. Xcode flattens the directory structure when copying headers into the framework target. This is the default behaviour of the Xcode IDE, not something I'm doing specially for this project. All frameworks compile this way, unless you do some pretty serious mucking about with custom build phases.

Regardless, this doesn't impede how headers are imported once they are inside a framework: #import statements don't care about paths to files — if you're compiling in Xcode, you can refer to a header from 3 directories deeper or higher up than the current file. So long as that other header (and it's corresponding implementation file) are part of the Xcode project, they'll be found without specifying any kind of directory prefix.

Suppose, I'm using MagicalRecord in my app and I have discovered a bug. So I fix it and build MR from source now.

I build MagicalRecord from source almost every day, including during the development of the last 3 major point releases. I've not had a single issue, nor have I had to change the imports within the MR project, nor within my own projects. You don't need to add the source directly to your project to work on it — I've never worked that way in the years I've been working on MagicalRecord.

Actually, I was referring to the sub-project and target dependencies approach

This is exactly how I integrate MagicalRecord into my projects — the framework or static library is compiled by the MagicalRecord.xcodeproj that's a sub project of my app's xcodeproj, and copied into my app's "Frameworks" subdirectory. There's no fiddling with header search paths, because frameworks expose their headers automatically when built this way (the build directories are in the framework search paths by default).

In terms of compiling from source without using the provided Xcode project — well, I'm pretty happy to let that go. If people want to continue doing so, they can adjust their header search paths to search recursively from within Xcode, but I'd recommend they add MagicalRecord as a subproject and add the appropriate framework target to their link/copy frameworks build phases. This ensures they get the build settings, warnings and optimisation flags that we've set on this project.

I've just tested integrating this way via Carthage (specifying --no-build) now in a couple of Swift and Objective-C projects and it's all working just fine.

I don't want to dismiss your concerns, but I don't believe there's an issue here (happy to be proven wrong). The sample project that you provided earlier — can it be updated to demonstrate the broken state you are concerned about?

dodikk commented 9 years ago

import statements don't care about paths to files

Only if you select the "recursive" checkbox option recursive

USER_HEADER_SEARCH_PATHS = ../../lib-third-party/MagicalRecord/**
dodikk commented 9 years ago

Unfortunately, this breaks the encapsulation when you build from source as some private headers might be referenced.

dodikk commented 9 years ago

You can include headers inside frameworks, but not quoted headers, which are not in scope of the framework's public headers.

http://blog.cocoapods.org/Pod-Authors-Guide-to-CocoaPods-Frameworks/ This is related to the pull request discussion. Unable to write there as you've restricted the access.