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

j2objcXcode now dependency of j2objcBuild #524

Merged
merged 1 commit into from
Oct 23, 2015

Conversation

brunobowden
Copy link
Contributor

j2objcXcode:

  • skips task if xcodeProjectDir is not set
  • @input for project dependencies
  • PodspecDetails is now serializable so it can be used for @input
  • Cocoapods 0.39.0 is now installed in Travis build on OS X

Other changes:

  • xcodeTargetsIos added to multiProject1
  • j2objcPodspec now depends on j2objcPreBuild
  • j2objcXcode now depends on j2objcPodspec
  • j2objcAssemble now depends on j2objcXcode

@brunobowden
Copy link
Contributor Author

PTAL

@@ -277,7 +277,7 @@ class J2objcPlugin implements Plugin<Project> {
}
lateDependsOn(project, 'build', 'j2objcBuild')

// TODO: Where shall we fit this task in the plugin lifecycle?
// TODO: make this a dependency of j2objcBuild
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to just go ahead and do this?

@brunobowden brunobowden force-pushed the improve branch 2 times, most recently from 1438925 to 3b087fb Compare October 21, 2015 22:59
@advayDev1
Copy link
Contributor

Please add a comment to CHANGELOG.md also; LGTM when passes.

@brunobowden brunobowden changed the title XcodeTask @Input for project dependencies j2objcXcode now dependency of j2objcBuild Oct 22, 2015

@Input
// List of all dependencies
List<PodspecDetails> getPodspecDependencies() {
Copy link
Contributor

Choose a reason for hiding this comment

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

class PodspecDetails must be modified as follows to be an input:

@EqualsAndHashCode
static class PodspecDetails implements Serializable {
...
private static final long serialVersionUID = 1L
...
}

@brunobowden
Copy link
Contributor Author

@advayDev1 - it also required implementing writeObject() and calling defaultWriteObject(). I added deserialization as well, so that I could test it end it end. Curiously, the @EqualsAndHashCode wasn't needed, so I wanted your opinion on that.

@advayDev1
Copy link
Contributor

  1. Not throwing without EqualsAndHashCode makes sense. However, I think you use it anyway: while serialization will work, you need it to make sure up-to-date checks are correct and even in Groovy, equality is referential unless overridden.
  2. I'm not understanding why writeObject/defaultWriteObject were needed for you. I don't have them. Instead of doing that, consider using @AutoExternalize: http://docs.groovy-lang.org/2.4.5/html/gapi/groovy/transform/AutoExternalize.html

@advayDev1
Copy link
Contributor

Aha: you don't have a default no-arg constructor for PodspecDetails. Ok this makes sense now

@brunobowden
Copy link
Contributor Author

I expect that Gradle is comparing Serialized binary to Serialized binary, that way that can get consistent behaviour over time and across daemon runs - they can't keep the object around in memory. The writeDefaultObject is a necessary part of serialization.

@advayDev1
Copy link
Contributor

I'd still suggest using @EqualsAndHashCode - we don't know what the implementation detail of Gradle is (I looked briefly, couldn't find it). Understood about writeDefaultObject, the point of @AutoExternalizable is to do that for you. (I'm still trying to figure out why I don't need AutoExternalizable nor implementations of Serializable - but that is an aside).

j2objcXcode:
- skips task if xcodeProjectDir is not set
- @input for project dependencies
- PodspecDetails is now serializable so it can be used for @input
- Cocoapods 0.39.0 is now installed in Travis build on OS X

Other changes:
- xcodeTargetsIos added to multiProject1
- j2objcPodspec now depends on j2objcPreBuild
- j2objcXcode now depends on j2objcPodspec
- j2objcAssemble now depends on j2objcXcode
@brunobowden
Copy link
Contributor Author

I've added back @EqualsAndHashCode. Since the serialization is working (and is tested), I'm fine with doing it the Java way with writeDefaultObject().

One concern I have is that getPodspecDependencies() is called 2-3 times during the gradle run. I'm surprised that this happens. Dependending on how large the dependency trees become, this may need caching.

I'll merge this once the build is green.

@advayDev1
Copy link
Contributor

sg - LGTM

brunobowden added a commit that referenced this pull request Oct 23, 2015
j2objcXcode now dependency of j2objcBuild
@brunobowden brunobowden merged commit f6def79 into j2objc-contrib:master Oct 23, 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