TrigonicSolutions / gradle-rpm-plugin

Gradle plugin for constructing RPM packages.
Apache License 2.0
38 stars 19 forks source link

Add debian support with 1.8 #51

Closed quidryan closed 10 years ago

quidryan commented 11 years ago

CopySpec completely changed in 1.8, separating out CopySpec and CopyActions. This pull request aligns with 1.8, and won't work with anything earlier. There's some other changes to make it easier for me to work with, like removing nexus and bumping the version. I can see what is required to get those back if needed. (Though I don't believe in putting publishing code into the build, it's factor specific to the user, and they can injected via -I.)

merscwog commented 11 years ago

I'm going to test this in a more complex scenario later today, but the plugin code looks really good.  Better than the hack-a-path I was going through to remap everything to the new internal types, although some of that nasty hackish-ness is unavoidable and is still present.

-Spencer

merscwog commented 11 years ago

I haven't tracked it down yet, but the description field comes out as (none) every time, but more importantly, the group is not being applied to files/directories, so only my user assignments are still honored.  Plus, I now have to explicitly depend upon guava.

I have not tested the deb pathways, just the rpm stuff.

-Spencer


From: Justin Ryan notifications@github.com To: TrigonicSolutions/gradle-rpm-plugin gradle-rpm-plugin@noreply.github.com Sent: Thursday, September 26, 2013 1:35 AM Subject: [gradle-rpm-plugin] Add debian support with 1.8 (#51)

CopySpec completely changed in 1.8, separating out CopySpec and CopyActions. This pull request aligns with 1.8, and won't work with anything earlier. There's some other changes to make it easier for me to work with, like removing nexus and bumping the version. I can see what is required to get those back if needed. (Though I don't believe in putting publishing code into the build, it's factor specific to the user, and they can injected via -I.)


You can merge this Pull Request by running git pull https://github.com/quidryan/gradle-rpm-plugin add-debian-support-with-1.8 Or view, comment on, or merge it at:   https://github.com/TrigonicSolutions/gradle-rpm-plugin/pull/51 Commit Summary

  • Extract out more general parts, for future use by other plugins, and for a central place for adding conventions.
  • Start using ConventionMapping to calculate values only if we need to.
  • Creating an extension that can be used to inherit all values from. description had to be changed to differentiate between the task description and a package description, and is hence now named packageDescription. Copy Specs are not supported yet.
  • Support a Project extension to also drive the CopySpec.
  • Clean up imports, and introduce a base plugin to modify CopySpec.
  • Porting the pkg4j code to use the same control as the gradle-rpm-plugin
  • Start work on the plugin to unite deb and rpm creation.
  • Write DEB file format test
  • Set defaults that jdeb expects
  • Push some of the aliases up to the plugin, still needs to be shared in extension
  • Move tests to proper package
  • Start testing the high level plugin
  • Expose task classes onto the Project.
  • Start looking at issue #30
  • Isolate platform specific features into their respective tasks, and not use them at the top extension. Support installUtils for deb.
  • Initial work to use Categories to modify CopySpecImpl
  • Add support for multiple pre/post/install commands. To be specified as strings or files.
  • Upgrade Gradle and shift version to gradle.properties.
  • Push some values up to the extension. Fix some issues reading from values put onto the CopySpec, by correcting the type and reading from the instance instead of the class.
  • Additional test to flush out behavior on Jenkins.
  • Lock down to 1.1.10 of redline, since 1.1.11 and above requires Java 7 or above.
  • Isolate Nexus setup, so that it can be used when it's needed, otherwise its left out.
  • Extract out aliasing, and then apply to the extension in addition to any tasks.
  • Reduce explicit dependency on Groovy, since it brings in too much.
  • Adding version to extension and consequently tasks.
  • Organizing imports.
  • Shrinking epoch time to seconds instead of milliseconds.
  • Use string instead of BigDecimal for version from project
  • Fix issue #30
  • Adding documentation for all the plugins.
  • Fixed spelling on formal
  • Limited the dependencies needed for jdeb.
  • List all excludes, so that they show up in the pom.
  • Protect against null values for the control replacements
  • Fix packageGroup for deb packages. And start to reveal Architecture too.
  • Merge commit '6386954' into add-debian-support to add 1.7 support.
  • Merge commit '406fe47' into add-debian-support-with-1.7
  • Fix typo where postInstall script specified with an equals would become a preInstall script
  • Disable ancillary plugins that don't have to do with releasing.
  • Bump version for 1.7 compatible version
  • Incrementing because Jenkins doesn't have permissions to do it automatically.
  • Migrate to 1.8 CopyActions
  • Bump version to 2.8 to reflect that this works with 1.8 and above File Changes
  • M .gitignore (1)
  • M History.md (5)
  • A Plugin-Deb.md (143)
  • A Plugin-Rpm.md (160)
  • M README.md (115)
  • M build.gradle (43)
  • A gradle.properties (1)
  • A gradle/nexus.gradle (69)
  • M gradle/wrapper/gradle-wrapper.jar (0)
  • M gradle/wrapper/gradle-wrapper.properties (12)
  • A settings.gradle (1)
  • A src/main/groovy/com/trigonic/gradle/plugins/deb/DataProducerDirectorySimple.groovy (21)
  • A src/main/groovy/com/trigonic/gradle/plugins/deb/DataProducerFileSimple.groovy (24)
  • A src/main/groovy/com/trigonic/gradle/plugins/deb/Deb.groovy (66)
  • A src/main/groovy/com/trigonic/gradle/plugins/deb/DebCopyAction.groovy (275)
  • A src/main/groovy/com/trigonic/gradle/plugins/deb/DebPlugin.groovy (36)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/AbstractPackagingCopyAction.groovy (181)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/AliasHelper.groovy (38)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/CommonPackagingPlugin.groovy (29)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/CopySpecEnhancement.groovy (113)
  • R src/main/groovy/com/trigonic/gradle/plugins/packaging/Dependency.groovy (4)
  • R src/main/groovy/com/trigonic/gradle/plugins/packaging/Link.groovy (2)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/ProjectPackagingExtension.groovy (62)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingBasePlugin.groovy (58)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingExtension.groovy (187)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingPlugin.groovy (46)
  • A src/main/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingTask.groovy (179)
  • M src/main/groovy/com/trigonic/gradle/plugins/rpm/Rpm.groovy (155)
  • A src/main/groovy/com/trigonic/gradle/plugins/rpm/RpmCopyAction.groovy (158)
  • D src/main/groovy/com/trigonic/gradle/plugins/rpm/RpmCopySpecVisitor.groovy (149)
  • M src/main/groovy/com/trigonic/gradle/plugins/rpm/RpmPlugin.groovy (42)
  • A src/main/resources/META-INF/gradle-plugins/deb.properties (1)
  • A src/main/resources/META-INF/gradle-plugins/os-package-base.properties (1)
  • A src/main/resources/META-INF/gradle-plugins/os-package.properties (1)
  • A src/main/resources/deb/control.ftl (15)
  • A src/main/resources/deb/postinst.ftl (22)
  • A src/main/resources/deb/postrm.ftl (5)
  • A src/main/resources/deb/preinst.ftl (9)
  • A src/main/resources/deb/prerm.ftl (5)
  • A src/test/groovy/com/trigonic/gradle/plugins/deb/DebPluginTest.groovy (238)
  • A src/test/groovy/com/trigonic/gradle/plugins/deb/Scanner.groovy (184)
  • A src/test/groovy/com/trigonic/gradle/plugins/packaging/CopySpecEnhancementTest.groovy (27)
  • A src/test/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingBasePluginTest.groovy (115)
  • A src/test/groovy/com/trigonic/gradle/plugins/packaging/SystemPackagingPluginTest.groovy (67)
  • M src/test/groovy/com/trigonic/gradle/plugins/rpm/RpmCopySpecVisitorTest.groovy (19)
  • M src/test/groovy/com/trigonic/gradle/plugins/rpm/RpmPluginTest.groovy (179)
  • M src/test/groovy/com/trigonic/gradle/plugins/rpm/Scanner.groovy (39) Patch Links:
  • https://github.com/TrigonicSolutions/gradle-rpm-plugin/pull/51.patch
  • https://github.com/TrigonicSolutions/gradle-rpm-plugin/pull/51.diff
merscwog commented 11 years ago

Ah.  It's because both have been renamed.  group -> permissionGroup and description -> packageDescription.

Probably not a bad idea, but a breaking change.

-Spencer


From: Spencer Allain spencer_allain@yahoo.com To: TrigonicSolutions/gradle-rpm-plugin reply@reply.github.com Sent: Thursday, September 26, 2013 12:46 PM Subject: Re: [gradle-rpm-plugin] Add debian support with 1.8 (#51)

I haven't tracked it down yet, but the description field comes out as (none) every time, but more importantly, the group is not being applied to files/directories, so only my user assignments are still honored.  Plus, I now have to explicitly depend upon guava.

I have not tested the deb pathways, just the rpm stuff.

quidryan commented 11 years ago

Thank you @merscwog for the comments! I really appreciate someone looking at this. I pushed the changes I mentioned so far. We do use this internally for RPM and DEBs, and feel pretty good about it. Before this PR gets merged, let me test some more in the real world, but I think the code is now stable.

merscwog commented 11 years ago

My thanks to you is much greater.  I wish to use this plugin, and you've done the heavy lifting of making it work under 1.8.

I also saw your post on the convention mapping stuff.  I had noted that annotation on the DefaultCopySpec, but had no idea what it really meant.

-Spencer


From: Justin Ryan notifications@github.com To: TrigonicSolutions/gradle-rpm-plugin gradle-rpm-plugin@noreply.github.com Cc: Spencer Allain spencer_allain@yahoo.com Sent: Friday, September 27, 2013 1:11 PM Subject: Re: [gradle-rpm-plugin] Add debian support with 1.8 (#51)

Thank you @merscwog for the comments! I really appreciate someone looking at this. I pushed the changed I mentioned so far. We do use this internally for RPM and DEBs, and feel pretty good about it. Before this PR gets merged, let me test some more in the real world, but I think the code is now stable. — Reply to this email directly or view it on GitHub.

glyn commented 11 years ago

Thanks for this PR Ryan! I was faced with upgrading the plugin to gradle 1.8 and this hopefully saves me a lot of time. Well I got a clean build which is a good sign. Meanwhile I'll certainly feed back any issues or improvements I find and hope to see this PR merged before too long.

merscwog commented 10 years ago

So, this might already be known, but using the nightly (gradle-1.10-2013017220032+0000) with this pull request fails on two tests:

RpmPluginTest > verifyCopySpecCanComeFromExtension RpmPluginTest.groovy:275 groovy.lang.MissingMethodException: No signature of method: org.gradle.api.internal.file.copy.CopySpecWrapper_Decorated.createDirectoryEntry() is applicable for argument types: (java.lang.Boolean) values: [true]

RpmPluginTest > files RpmPluginTest.groovy:93 java.lang.AssertionError: expected:<[./a/path/not/to/create/alone, ./opt/bleah, ./opt/bleah/apple, ./opt/bleah/banana]> but was:<[./a/path/not/to/create/alone, ./opt/bleah/apple, ./opt/bleah/banana]>

Since createDirectoryEntry() is part of how the "files" test should work, it's probably related to the decoration issue above.

-Spencer

merscwog commented 10 years ago

And, no surprise. Gradle-1.9-rc-1 exhibits identical behavior. Maybe we could request that extensions against this internal class work until a public API can be exposed?

-Spencer

merscwog commented 10 years ago

I believe I've found an issue with at least the rpm portion of this plugin enhancement. Individual CopySpec values seem to be applying to the task overall, so that the "last" assignment wins. Applying the user, fileMode, or group/permissionGroup within individual from() blocks results in the latest assignment from the ordered individual blocks and being applied to all files/directories.

There are not enough unit tests to highlight the issue, and the files() unit test works under Gradle 1.8 because there is an interaction between the addParentDirs and the createDirectoryEntry that results in the desired outcome. Under Gradle 1.9 and above, the attempt to use createDirectoryEntry for a directory that does not exist at all no longer works, because it appears non-existent sources are not ever passed to any visit function.

I need to add some more comprehensive unit tests (I guess as a slightly different pull to each branch because of permissionGroup and packageDescription, but that's easy enough).

Easiest test: Assign user "A" at task level. Create from block with no adjustments. Create from block that adjusts user to be "B". Files from first from block should have "A" as owner. Files from second block should have "B" as owner.

Slightly more complex test: Assign user and fileMode at task level. Create from block that adjusts just user. Create from block that adjusts just fileMode. Create from block that adjusts user and fileMode. Create from block with no adjustments.

-Spencer

merscwog commented 10 years ago

Need to make a pull request (hopefully this evening), but there is definitely an issue in the 1.8-debian pull request with users and groups and different copyspecs. Interestingly enough it handles filemodes just fine between them, so I need to look at that more closely.

quidryan commented 10 years ago

I forked the plugin and did a release as com.netflix.gradle#gradle-ospackage-plugin;1.8.4 to bintray. The code can be found at https://github.com/nebula-plugins/gradle-ospackage-plugin. The version implies 1.8 support. Eventually I'll put out versions for 1.6 and 1.7.

@melodious I will look at the other pull request and try to integrate it over there.