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.

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

Multi-project dependencies in Xcode #504

Merged
merged 1 commit into from
Oct 13, 2015

Conversation

brunobowden
Copy link
Contributor

Multi-project dependencies in Xcode

  • j2objcXcode finds all podspecs in dependency tree and applies them to Podfile
  • “podFile” => “podfile” to match CocoaPods documentation
  • .gitignore the Pods directory for CocoaPods

PodspecTask

  • j2objcPodspec separated from j2objcXcode to create podspec
  • j2objcAssemble{Debug|Release} now depends on j2objcAssembleResources
  • j2objcResources now depends on j2objcPodspec
  • Podfile podspec paths are now relative instead of absolute
  • Podspec headers moved from public_header_files => HEADER_SEARCH_PATHS

multiProject1 test

  • Rename projects and workspace to match FAQ example
  • Project with two targets: IOS-APP and IOS-APPTests

Other Changes:

  • FAQ: Add .gitignore entry, revise folder structure
  • Condense warnings on unclean git and SNAPSHOT build
  • Clean up a few redundant imports
  • Copyright notices for Xcode project

Fixes #432 - Xcode podspec dependencies
Fixes #502 - Split j2objcPodspec task from j2objcXcode task

@brunobowden
Copy link
Contributor Author

@advayDev1 - this still needs some more work but this is close enough to be worth getting your feedback.

@@ -18,6 +18,7 @@ Paste the results below, replacing existing contents.
- [How do I develop with Xcode?](#how-do-i-develop-with-xcode)
- [How can I speed up my build?](#how-can-i-speed-up-my-build)
- [What libraries are linked by default?](#what-libraries-are-linked-by-default)
- [What should I add to .gitignore?](what-should-i-add-to-gitignore)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a hashtag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@advayDev1
Copy link
Contributor

cool. 2 main comments, rest are minor:

  • Use the j2objcLinkage dependencies and/or NativeCompilation.dependsOnProjects instead of decoding the task dependencies. The dependencies/dependsOnProjects are the spec; while the Task's dependsOn trees are an implementation detail. They can easily change (I've had to change them in the past already). Also, (as in my app), users can customize the dependency tree to force things to build in a particular order.
  • If this commit is requiring --no-package-directories, we need to fix that.

I assume you're going to take a look at your logs as well before this is ready.

@brunobowden
Copy link
Contributor Author

Thanks. I'll take a look at this tomorrow.

What did you mean by "look at my logs"?

On Fri, Oct 9, 2015 at 11:14 PM Advay Mengle [email protected]
wrote:

cool. 2 main comments, rest are minor:

  • Use the j2objcLinkage dependencies and/or
    NativeCompilation.dependsOnProjects instead of decoding the task
    dependencies. The dependencies/dependsOnProjects are the spec; while the
    Task's dependsOn trees are an implementation detail. They can easily change
    (I've had to change them in the past already). Also, (as in my app), users
    can customize the dependency tree to force things to build in a particular
    order.
  • If this commit is requiring --no-package-directories, we need to fix
    that.

I assume you're going to take a look at your logs as well before this is
ready.


Reply to this email directly or view it on GitHub
#504 (comment)
.

@advayDev1
Copy link
Contributor

logs: you added a number of logger.error and .warns that should probably be deleted.

@brunobowden
Copy link
Contributor Author

@advayDev1 - PTAL. I've addressed the first major issue of using the j2objcLinkage / NativeCompilation. There's a lot of refactoring there which has put it in to a much better place. Still need to test out if no-package-dependencies is really needed. Also, please mark any logger statements that I should remove. Just look at the second commit to make it more manageable for you.

@brunobowden
Copy link
Contributor Author

P.S. The podspec paths are now relative rather than absolute, so no more 'brunobowden' in that file. I believe that the other 'brunobowden' mentions in the Xcode project may be unavoidable.

@brunobowden
Copy link
Contributor Author

The 'brunobowden' mentions are completely removed.

I'm still having an issue with --no-package-dependencies. I can get the j2objc builds to work. When doing the import in Xcode, it appears that CocoaPods flattens all the header includes. So doing an import of "com/example/Cube.h" fails as it has to look for "Cube.h" instead.

My suggestion is to merge the PR as it stands, then work separately on this issue. Are you ok with that? If so, then do a final pass through the changes and give me an LGTM.

Here's an example of the Header directory as it's copied over to Pods:

Brunos-Air:multiProject1 brunobowden$ ls -l ios/Pods/Headers/Public/j2objc-extended-debug
total 480
lrwxr-xr-x  1 brunobowden  staff  104 Oct 10 23:36 $Gson$Preconditions.h -> ../../../../../extended/build/j2objcOutputs/src/main/objc/com/google/gson/internal/$Gson$Preconditions.h
lrwxr-xr-x  1 brunobowden  staff   96 Oct 10 23:36 $Gson$Types.h -> ../../../../../extended/build/j2objcOutputs/src/main/objc/com/google/gson/internal/$Gson$Types.h
lrwxr-xr-x  1 brunobowden  staff  106 Oct 10 23:36 ArrayTypeAdapter.h -> ../../../../../extended/build/j2objcOutputs/src/main/objc/com/google/gson/internal/bind/ArrayTypeAdapter.h
...
lrwxr-xr-x  1 brunobowden  staff   84 Oct 10 23:36 ExtendedCube.h -> ../../../../../extended/build/j2objcOutputs/src/main/objc/com/example/ExtendedCube.h

@brunobowden brunobowden added this to the 0.5 Release milestone Oct 11, 2015
List<Project> getBeforeProjects() {
return nativeCompilation.beforeProjects
}
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

String libName = "${project.name}-j2objc"

// podspec creation
// TODO: allow custom list of libraries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add a Todo for handling the OSX (x86_64) directories.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added TODO

@advayDev1
Copy link
Contributor

@brunobowden - the only important thing i'd like to still understand is what you are doing with the DO NOT COMMITs.

re: --no-package-directories, I guess this PR can sustain that, but I don't think we can release 0.5 until that is resolved.

@brunobowden
Copy link
Contributor Author

This should now be ready to go.

  • Resolve the --no-package-directories by switching the podspecs from public_header_files to HEADER_SEARCH_PATHS

Main thing I'd like feedback on is the dependency changes. I noticed that j2objcPodspec wasn't being built. Curiously, j2objcBuild also doesn't depend on the j2objcAssembleResources tasks, so that was being missed too. I've changed j2objcAssemble{Debug|Release} to depend on j2objcAssembleResources, which in turn depends on j2objcPodspec. You could argue that the podspec is a 'resource to be assembled'. I'm not completely comfortable with that description but it appears the best with the existing dependencies.

@advayDev1
Copy link
Contributor

@brunobowden

  1. did you repush? for example, i'm still seeing all the incorrect copyright notices.
  2. AssembleResourcesTask has as its output directory getDestSrcDirFile('main', 'resources').
    PodSpecTask has as its input (I know it not an input file) getDestSrcDirFile('main', 'resources').
    AssembleResourcesTask always occurs after PodspecTask.
    Does gradle incremental build handle this correctly? If you ./gradlew build twice in a row, on the second run does PodspecTask run (it shouldn't).

LGTM if 1 and 2 are good.

@advayDev1
Copy link
Contributor

oh you might want to add 'Handling of multi-project dependencies in Cocoa Pods' to the Changelog

Multi-project dependencies in Xcode
- j2objcXcode finds all podspecs in dependency tree and applies them to Podfile
- “podFile” => “podfile” to match CocoaPods documentation
- .gitignore the Pods directory for CocoaPods

PodspecTask
- j2objcPodspec separated from j2objcXcode to create podspec
- j2objcAssemble{Debug|Release} now depends on j2objcAssembleResources
- j2objcResources now depends on j2objcPodspec
- Podfile podspec paths are now relative instead of absolute
- Podspec headers moved from public_header_files => HEADER_SEARCH_PATHS

multiProject1 test
- Rename projects and workspace to match FAQ example
- Project with two targets: IOS-APP and IOS-APPTests

Other Changes:
- FAQ: Add .gitignore entry, revise folder structure
- Condense warnings on unclean git and SNAPSHOT build
- Clean up a few redundant imports
- Copyright notices for Xcode project

Fixes j2objc-contrib#432 - Xcode podspec dependencies
Fixes j2objc-contrib#502 - Split j2objcPodspec task from j2objcXcode task
@brunobowden
Copy link
Contributor Author

  1. I've pushed the changes with the copyright notices. I've also rebased the PR as well.
  2. Gradle handles the dependencies fine. The ordering of events doesn't matter, only the @input and @output. Since I changed it from @InputDirectory to @input, it now works just fine. I didn't notice one failed UP-TO-DATE (issue j2objcAssembleDebug fails UP-TO-DATE due to shared directory for debug / release build #505), how this is a pre-existing issue.

Given the complexity involved, I'll wait for all the checks to pass before merging.

@brunobowden brunobowden changed the title Xcode / podspec dependencies Multi-project dependencies in Xcode Oct 13, 2015
brunobowden added a commit that referenced this pull request Oct 13, 2015
Multi-project dependencies in Xcode
@brunobowden brunobowden merged commit 8c4ea6f into j2objc-contrib:master Oct 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants