Add debian support with 1.8 #51
Conversation
…for a central place for adding conventions.
…scription 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.
…ot use them at the top extension. Support installUtils for deb.
…s strings or files.
…lues put onto the CopySpec, by correcting the type and reading from the instance instead of the class.
| @@ -0,0 +1,69 @@ | |||
| /** | |||
There was a problem hiding this comment.
Attempting to apply via -I results in a "You can't change a configuration which is not in unresolved state!" for me. It doesn't like the classpath on line 13.
There was a problem hiding this comment.
Fixed in f1421ee. There's some magic required, but I tested it and it works after far as I can tell, though I didn't do a full publish.
|
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
|
|
Ah. It's because both have been renamed. group -> permissionGroup and description -> packageDescription. Probably not a bad idea, but a breaking change. -Spencer
|
| * _release_ - RPM Release | ||
| * _version_ - Version field, defaults to project.version | ||
| * _user_ - Default user to permission files to | ||
| * _permissionGroup_ - Default group to permission files to, "group" is used by Gradle for the display of tasks |
There was a problem hiding this comment.
This is a migration change (not necessarily a bad thing) but would need to be account for, so it's not a drop-in replacement plugin for older versions.
There was a problem hiding this comment.
I should be more explicit about the breakage. I found that group and description conflicted in some scenarios, so I wanted to remove the possibility of it being ignored for this plugin. I added note in the History file in 07cd29c
…ble. Then iterated over some groovyc errors which could only be solved by not implementing CopySpec anymore. It's clearly not ideal, but I tried a dozen alternatives and not worked. At least the @DeleGate is gone.
…script gradle/nexus.gradle build
|
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. |
|
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
|
|
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. |
Without this dependency, use of os-package-plugin gave the following error: Execution failed for task ':buildDeb'. > Could not call SystemPackagingTask.copy() on task ':buildDeb' due to java.lang.NoClassDefFoundError: org.apache.commons.lang.time.DateFormatUtils
3.1 has been out since November 2011, so it is time to upgrade from 2.x. Ref: http://commons.apache.org/proper/commons-lang/article3_0.html
commons-lang dependency improvements
|
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 RpmPluginTest > files RpmPluginTest.groovy:93 Since createDirectoryEntry() is part of how the "files" test should work, it's probably related to the decoration issue above. -Spencer |
|
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 |
|
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 |
|
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. |
Users, groups and permissions should apply properly with different copyspecs, and these tests check that.
Tests that should pass
| * | ||
| * Support some additional properties on CopySpecs. Using CopySpecInternal since its the parent of both DefaultCopySpec | ||
| * and RelativizedCopySpec. The old way used the metaClass on the Class, which effected all CopySpecs in the build. This | ||
| * limits the scope to just without our from/into/rename methods. This style also gives us type safety. |
There was a problem hiding this comment.
Limiting the scope breaks the CopySpec.with(CopySpec) implementation. CopySpecs should be able to be defined outside of a copy task and re-used across copy tasks. This is the functionality the "with" method supports and throws exceptions if the needed properties are not available to all copy specs. I have a common use case where I want to define a copy spec and re-use it for RPM, TAR and other basic copy tasks. If a copy task cannot be shared with RPM, we have a great deal of duplication and bloated build scripts. I submitted a pull request previously to fix this and would like to see this functionality pulled into the plugin so that we are not stuck maintaining our own implementation.
|
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. |
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.)