-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Automate copy-frameworks #2731
base: master
Are you sure you want to change the base?
Automate copy-frameworks #2731
Conversation
Using otool is perfectly fine. You can add integration tests, check
Looking into the |
@blender when I'm checking a Static Library (for Bolts in particular) file type, it reports Executable (surprisingly!) and not the MH_OBJECT which I was expecting. But I'll recheck. However, if I'll review |
Will this search for frameworks in |
@bwhiteley it only looks at the |
While working on changes to a framework I often drag in the framework in question as a subproject in the main project. I then modify the copy phase to take the dependency from the Would such a workflow also be possible when using this automated copy phase? |
The manually specified list will still be there @typfel, @bwhiteley . Maybe there should be a set union based on the framework name, where the manual list wins over the inferred list since I think @typfel use case would not work. |
@typfel you shouldn't worry. This PR omits need to specify standard paths such as |
@dimazen I would also consider adding a flag to enable this behavior, making it opt-in rather than default. |
Here is how I would hope this feature would work...
(This is how our Carthage wrapper works, mentioned in #2605 . When it was mentioned in #2605 it was reading the project file to get the framework list, but has since changed to use The reason for #4 is similar to what @typfel said. We have a lot of internal frameworks. Often we work on the framework individually but sometimes we work on the framework in the context of a consuming app (or another framework). To work on a framework in the context of a consuming app, simply create a temporary workspace containing the app project and the framework project. Due to #4, if the framework project is in the workspace the built framework in It would be great if the Carthage team found this behavior desirable because then we could use carthage directly in our copy frameworks build phase and remove a bunch of functionality from our wrapper tool. :) I've seen at least three separate implementation of a tool to flip frameworks into "development mode" in a carthage environment, so I know we aren't the only group interested in this functionality. Perhaps if #4 isn't part of this PR it could come later... |
@bwhiteley are you sure that if you use a workspace |
@bwhiteley I gonna play around with development projects to see how they affect @blender what do you think about automatic linking of the development dependencies as well as recursive dependencies copying (i.e. FacebookSwift will automatically copy Facebook-ObjC and Bolts)? Should we add separate option for this?
Option names are just an example (especially second one). |
If you always use a workspace (meaning you probably aren't using carthage to build the frameworks, only to resolve/fetch them) then you probably have the frameworks listed in Emedded Binaries, and you don't need a carthage copy-frameworks build phase. However, if you normally build the frameworks with carthage and open the project file, but sometimes use a workspace to temporarily have a framework in "development" mode, then the carthage copy-frameworks phase is needed to avoid needing to temporarily modify the project (such as temporarily adding the frameworks to Emedded Binaries with correct paths). |
But A dependency graph recursively built with |
It will also be useful to print the input files that were found to satisfy the graph with their full paths, as well as indicating any that were skipped because the corresponding output was already up-to-date. This could be enabled with a verbose switch, or just do it by default. |
@bwhiteley
Also, can you provide me some sample project with development dependency you're talking about please? I don't seem to understand how it works (never worked like that before). |
Using
Yeah, I can put something together. |
@bwhiteley ok, I got what you mean about |
The timestamp/up-to-date check takes care of this. The only constraint is that the carthage copy-frameworks build phase come after the |
@blender and for Alamofire it works or what was your question? |
I don't think this is related to scheme names, but maybe target names. I think the issue here is that APFS is case-insensitive, case-preserving by default, while this new framework resolver (not to be confused with the dependency resolver) in Carthage that maps otool output to frameworks found on the filesystem is case-sensitive. in the wireapp example, It looks like the app is linked against |
From the Wire-iOS project.pbxproj:
|
@dimazen last effort. Please fix the typos and then let's merge it. |
I'm a little confused. Correct me if I'm wrong: Wire's executable was probably linked against So if @typfel had a case sensitive file system, it should not have worked. Since it didn't work, I can only make out he has a case-sensitive file system. That in combination with the name mismatch on Ziphy-ziphy, would have led even the regular copy-framework phase to fail. Are we interfering with the file system lookup of the path on disk somewhere? I didn't find any evidence. The framework map comes directly form otool, which outputs a path thus preserving whatever file system preference there was at the time of linking and leaving the re-interpretation of the path to be handled by the file system where If I'm right, I don't think there should be any fix. |
This is probably due to some CI script which incorrectly upload the file based on the project name.
My file system is the default case insensitive, APFS (Encrypted)
This worked because in the regular copy-framework phase the output file was listed as One last observation: When running But maybe this behaviour is what we want since the iOS file system is case sensitive? |
@typfel was your error a compile time or a "failed to load library" from dyld? |
"failed to load library" since it's not copied to the framework directory. |
could you please |
|
OK, this is a problem. The current implementation in this PR silently ignores missing frameworks. I verified by renaming a framework in Carthage/Build/iOS by changing its case. The build succeeded but the framework was not bundled. IMO this needs to be fixed so that a missing library causes |
@typfel I'm curious... in the situation where your app links against |
Let me explain current implementation and it's limitations in the give context: Workaround is to ask user to place |
Just to add: in order to verify that all of the dependencies are in place I'm running a carthage-verify script. Would be good to make it a built-in tool. Anyway, verification is needed. |
Fair enough. In that case I vote to merge. |
I think we can all agree that the actual casing on the file system should be |
Any update on this? |
This comment has been minimized.
This comment has been minimized.
Any updates about review? :| |
Sooo, is there anything else left to add to merge this PR finally? :) |
This PR resolves #2605.
Tested on a test project as well as two production projects - works as expected. However, I need your advise on:
otool -L
. I don't know for sure, but MachO format seems to be quite stable, so no changes from otool expected.frameworksInDirectory
returns both static and dynamic frameworks. Attempt to filter them out by checking their MachHeader.fileType doesn't help, because both returnMH_EXECUTE
. I end up checking whether framework nested in theStatic
folder or not but is that ok?