Skip to content
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

Refresh PR: Treat symlinks as normal files in spec file #1039

Merged
merged 1 commit into from Oct 22, 2017

Conversation

erickpintor
Copy link
Contributor

I'm trying to bring back to life a very old PR: #916

Here I'm adding back the original test made by @dpennell and I'm reapplying his changes on top of the current master branch.

An interesting improvement is that by describing symbolic links as part of RPM's spec file, it can remove them during uninstall, leaving no empty folders behind. Previously, the links were removed in the postun hook, after RPM tried to rid of its managed folders, resulting in empty directories whenever there was a symbolic link inside them.

I've tested this with a project I'm working on using a CentOS VM.

@lightbend-cla-validator

Hi @erickpintor,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@muuki88
Copy link
Contributor

muuki88 commented Sep 28, 2017

I will be offline for 3 weeks ( traveling through New Zealand ), but will review and merge these changes as soon as I am back home.

Thanks for the very quick response and your time working on native-packager 😍😍

@erickpintor
Copy link
Contributor Author

Sure. No problem. I've noticed some broken tests on Travis that might be caused by this PR. I'll take a look at them next week since I'll be also traveling.

I also signed the Lightbend Contributors License Agreement.

Enjoy your vacation.

@dpennell
Copy link
Contributor

Thanks for resurrecting this. A series of life events has prevented me from doing so.

@erickpintor
Copy link
Contributor Author

Hi everyone. I fixed the failing test that was caused by this PR. There are still 2 failing tests suites: bash and ash. I think there were already broken before and they don't seem to be related to this PR.

Anyway, let me know if there is anything else to change / improve.

@muuki88
Copy link
Contributor

muuki88 commented Oct 1, 2017

Awesome 😎
You are correct. The bash and ash scripts are currently failing. I assume travis changed something

I'll take a look as soon as I'm home ( 22.10 )

@muuki88 muuki88 added the rpm label Oct 21, 2017
Copy link
Contributor

@muuki88 muuki88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot for reviving this pull request ❤️

@muuki88 muuki88 merged commit 5dcce82 into sbt:master Oct 22, 2017
@erickpintor erickpintor deleted the rpm-symlinks branch October 23, 2017 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants