Unify maintainerScripts #625
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| # ___) | __/ | \ V / __/ | / ___ \| | | (__| | | | __/ |_| |_| | |_) | __/ | ||
| # |____/ \___|_| \_/ \___|_| /_/ \_\_| \___|_| |_|\___|\__|\__, | .__/ \___| | ||
| # |___/|_| | ||
| # ------------------------------------------------------------------------------------ |
There was a problem hiding this comment.
Hmm, is this part of the server archetype?
|
I like the direciton a lot. Should clean up some confusion across keys. Ideally, I'd like to see if we can stretch this to not break binary compatibility, yet keep the same underlying structure... |
Me too. I probably will add the keys back. Throwing exceptions doesn't break BC, right ;) This is one of the things I will probably add with this PR as well. A setting describing how to merge maintainerScripts. |
muuki88
changed the title
| @@ -37,23 +37,32 @@ trait RpmKeys { | |||
| val rpmConflicts = SettingKey[Seq[String]]("rpm-conflicts", "Packages this RPM conflicts with.") | |||
| val rpmDependencies = SettingKey[RpmDependencies]("rpm-dependencies", "Configuration of dependency info for this RPM.") | |||
|
|
|||
| // MAINTAINER SCRIPTS | |||
| @deprecated("Use maintainerScripts in RPM and RpmConstants.Pretrans instead.", "1.1.x") | |||
There was a problem hiding this comment.
Just an FYI -
Deprecated warnings aren't really flagged well in sbt (Maybe we should open a ticket about that).
Additionally, we may want to issue the warning when someone sets these settings, i.e. something like:
val keyDeprecationWarnings: SettingKey[Seq[SettingKey]]
def warnIfSet(k: SettingKey[_]): Setting[_] = {
warn ++= k.?.value match {
case None => Seq()
case Some(_) => Seq(k)
}
}This should be good enough for now, but we should probably check to see what the experience is upgraded.
There was a problem hiding this comment.
That looks pretty nice. Could you do this in another PR 😍
|
Really like the cleanups and the ASCII art. LGTM. One caveat on deprecated error messages though. |
muuki88
force-pushed
the
wip/unify-maintainerscripts
branch
from
df80cf9 to
14a07e7
Compare
|
@fsat would you be able to test this? If this doesn't break anything with the rpm relocatable packages? |
|
Yes, I should be able to test this. I'll get back to you in a day or two. |
|
@muuki88 - I've manually tested the RPM build & install using the The For the ConductR manual test, I did a The ConductR project however failed the |
|
@fsat thanks a lot! You have really short days ;) So my changes make the rpmbuild calling |
|
Yes it was :) Now it's turned long - sorry for not getting back to you sooner. There's a diff between this branch's I have pasted the specs file from failing vs successful build. The And the specs file when the build is passing: |
muuki88
force-pushed
the
wip/unify-maintainerscripts
branch
2 times, most recently
from
398c219 to
81753b2
Compare
muuki88
force-pushed
the
wip/unify-maintainerscripts
branch
from
81753b2 to
f70d59c
Compare
| * @param replacements | ||
| */ | ||
| protected def rpmScriptletContents(scriptDirectory: File, scripts: Map[String, Seq[String]], replacements: Seq[(String, String)]): Map[String, Seq[String]] = { | ||
| import RpmConstants._ |
There was a problem hiding this comment.
…redudant settings
muuki88
force-pushed
the
wip/unify-maintainerscripts
branch
from
f70d59c to
a38b34f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the initial implementation for a unified
maintainerScriptsAPI, which at the current state servers forpostinst,preinst, etc.Usage
The basic task is
TaskKey[Map[String, Seq[String]]]. Every script (String Key) is mapped to the content ( Seq[String] ). For debian the taskoutput could look like this:Every package-plugin should scope this setting and use it to generate it's custom output.
Using the helper:
State of this PR
Currently the DebianPlugin / RpmPlugin implements this new structure. We will need additional helper as well to make it easy adding custom lines or using files ( already started in
MaintainerScriptHelper).