-
Notifications
You must be signed in to change notification settings - Fork 445
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
RpmNoReplaceplugin and LinuxMappingDSL for "noreplace" configs #896
Conversation
* This plugin automatically updates all config files in a RPM to "noreplace". Those files will not be overwritten | ||
* during an update if they have been changed on disk. This is useful for server apps. | ||
*/ | ||
object RpmNoReplaceConfigPlugin extends AutoPlugin with LinuxKeys with LinuxMappingDSL { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not convinced of my initial idea implementing this as an autoplugin. It only makes sense when enabled by default and for debian as well.
This will break backwards compatibility, which I usually prefer to avoid. If you can make a good case ( maybe some best practices, blog posts on this issue? ) I'm fine with adding this.
From a code perspective:
- instead of extending the
LinuxKeys
use the "global" Keys object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it is not enabled by default. Only when I explicitly use the plugin it will change the behaviour, so I don't see backwards compatibility being broken. However, using the helper from the DSL is definitely more explicit, and you don't save too much boilerplate by using the plugin. So I am also fine in removing the plugin and using only the helper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess starting with the helper is enough. We can make this a default later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I remove the plugin for now. And I'll add some docs then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. When you have added some docs this is ready to be merged. Thanks for your time and patience :)
/** | ||
* Created by carsten on 13.10.16. | ||
*/ | ||
class LinuxMappingDSLSpec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Have you run sbt test:scalafmt
on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, yes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
@@ -312,13 +312,14 @@ case class RpmSpec(meta: RpmMetadata, | |||
if (symlinks.isEmpty) | |||
None | |||
else { | |||
val checkUninstall = "if [ $1 -eq 0 ] ;\nthen" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that slipped in, I thought I created the branch based on master. I'll try to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the commit from the pull request.
…re correctly marked as `noconfig`, but `/etc/default` files provided by JavaServerApp archetype not.
…pPackaging plugins. It marks all config files as "noreplace" in the RPM, thus preventing them from being replaced during updates. The main method was added to the LinuxMappingDSL and can also be used in build.sbt for non server projects.
I removed the plugin and added some docs. Please let me know if any further changes are required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM.
I just realized that the |
Ah. This is as scoping issue. The server archetype scopes the etc default template to linuxPackageMappings in Rpm := ... |
Ah... hmm.. updating adds a merge commit. Let's see how that plays out when using the "squash & merge" button. |
I finally had the time to try it out, but when I scope the linuxPackageMappings to Rpm, then |
I can confirm this ( for The
And this is with the configWithNoReplace
For me, this look like a more general bug and not in your implementation. |
Sorry. This is again, a configuration issue. If you replace the linuxPackageMappings in Rpm := configWithNoReplace((linuxPackageMappings in Rpm).value) Would you mind making a small PR and add this to the docs? |
Just fixed the docs: |
Added a RpmConfigNoReplacePlugin that depends on the Rpm and ServerAppPackaging plugins. It marks all config files as "noreplace" in the RPM, thus preventing them from being replaced during updates. The main method was added to the LinuxMappingDSL and can also be used in build.sbt for non server projects.
Fixes #572.