Fix typo in GOOMPH_PDE_UPDATE_SITE property#48
Conversation
|
Ha! Sorry, this sort of bug is very frustrating to find. I think we need to at least throw a warning if "UDPATE" is defined. There are users who are using this now, typo and all - don't want to break them silently. |
|
good point, i can add this. do you know how to get a offline update site zip to provide the pde update site let's say for 4.6.3? |
|
with this i do not get a valid eclipse update-site-archive which can then be used as UPDATE_SITE. |
|
ok, i extracted the package and faked the 'installed' file. now PDEInstallation does not redownload from eclipse update site. would be awesome to have the p2-bootstrap feature with pde,too |
|
I am confused :) Happy to take any PRs though. |
…pde-install-typo
added some unit tests
Added logger warning if a user uses the old property
|
i have added the downwards compatibility to not break current behavior and added a warn message for the user. |
| Project project = ProjectBuilder.builder().build(); | ||
| ExtraPropertiesExtension props = project.getExtensions().getExtraProperties(); | ||
|
|
||
| props.set("GOOMPH_PDE_VER", "1.0.0"); |
There was a problem hiding this comment.
This unit test fails on Mac because of the check in PdeInstallation constructor:
java.lang.IllegalArgumentException: On mac, must be 4.5.0 (Mars) or later, because of folder layout problems.
at com.diffplug.gradle.pde.PdeInstallation.<init>(PdeInstallation.java:122)
at com.diffplug.gradle.pde.PdeInstallation.from(PdeInstallation.java:110)
at com.diffplug.gradle.pde.PdeInstallation.fromProject(PdeInstallation.java:105)
at com.diffplug.gradle.pde.PdeInstallationPropertiesTest.testUpdateSiteProperty(PdeInstallationPropertiesTest.java:39)
com.diffplug.gradle.pde.PdeInstallationPropertiesTest > testUpdateSitePropertyDownwardsCompatibilityDueTypo FAILED
java.lang.IllegalArgumentException: On mac, must be 4.5.0 (Mars) or later, because of folder layout problems.
at com.diffplug.gradle.pde.PdeInstallation.<init>(PdeInstallation.java:122)
at com.diffplug.gradle.pde.PdeInstallation.from(PdeInstallation.java:110)
at com.diffplug.gradle.pde.PdeInstallation.fromProject(PdeInstallation.java:105)
at com.diffplug.gradle.pde.PdeInstallationPropertiesTest.testUpdateSitePropertyDownwardsCompatibilityDueTypo(PdeInstallationPropertiesTest.java:57)
Probably the check is wrong and the test is OK. I don't think it makes sense that GOOMPH_PDE_VER must match Eclipse version numbers?
There was a problem hiding this comment.
The check is actually important. eclipse moved the locations of a lot of files around in eclipse 4.5.0 on mac. Supporting pre<4.5.0 on mac is basically supporting a fourth OS. Better to change 1.0.0 to 4.5.0 I think.
Also, the eclipse releases are looked-up manually in this: https://github.com/diffplug/goomph/blob/master/src/main/java/com/diffplug/gradle/pde/EclipseRelease.java
3 participants
No description provided.