-
Notifications
You must be signed in to change notification settings - Fork 441
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
Relocatable rpm symlink fix #685
Conversation
7b6a72d
to
bc19116
Compare
@@ -284,6 +284,8 @@ object JavaServerAppPackaging extends AutoPlugin { | |||
Some(script) | |||
} | |||
|
|||
protected def makePostInstallScript(): Option[String] = ??? | |||
|
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.
Why was this removed?
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.
Oops... I thought I've put that in, I'll put it back.
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.
Not implemented? what's missing here?
LGTM |
Manual test successfully done on Redhat 7 VM for both relocated and default installation. Test is done using ConductR rpms:
|
preun.fold("")("\n%preun\n" + _ + "\n\n") | ||
|
||
def postunContent(tearDownSymlinkScript: Option[String]): String = { | ||
val scripts = Seq(postun, tearDownSymlinkScript).collect { |
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.
this can be a simple flatten
, or?
I'm still confused why the Nevertheless code looks okay. And if the relocate feature isn't used, nothing of these bash-magic will make it into the |
bc19116
to
96ab8d0
Compare
@muuki88, unfortunately the When rpm install is called with relocate option (i.e. Before this change, the symlink is created during the build time and registered as part of For example, a symlink from This change will manage the symlink creation as part of post-install and post-uninstall steps using The additional complexity introduced in the Hopefully it explains why this change is done. |
cb915f8
to
f8eac6c
Compare
When creating relocatable RPM packages, the symlink needs to take into account the relocated file paths. RPM does not provide out of the box support for this, hence the pull request to manage symlinks as part of %post and %postun scriptlet. This will ensure the symlink will be created and removed as part of post-install and post-uninstall step. When creating the symlink, both the source path and the symlink itself need to take into account the relocated path.
f8eac6c
to
e8f49a2
Compare
Ensure actor system is shutdown only after the sent message is received by the actor under test as expected in the test
system.shutdown | ||
(router ? "SUCCESS!!").foreach { _ => | ||
system.shutdown | ||
} |
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.
@muuki88 - this is not related to the RPM symlink issue, but I have intermittent failure on this test today, and hence the fix.
The failure is originally caused by the race condition where the actor system may be shutdown before the actor under test has the chance to print the expected message.
@muuki88 - if you don't have any further feedback, can you merge this and release another milestone version please? |
Released in |
Thanks! |
Thank you @muuki88! |
When creating relocatable RPM packages, the symlink needs to take into account the relocated file paths.
RPM does not provide out of the box support for this, hence the pull request to manage symlinks as part of
%post
and%postun
scriptlet. This will ensure the symlink will be created and removed as part of post-install and post-uninstall step.When creating the symlink, both the source path and the symlink itself need to take into account the relocated path.