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

Use defaultLinuxLogsLocation for /var/log #150

Merged
merged 2 commits into from
Feb 3, 2014
Merged

Conversation

hfs
Copy link
Contributor

@hfs hfs commented Feb 2, 2014

Log directory logs in the installation directory is soft-linked to /var/log/app-name/. Instead of hard coding /var/log use the already provided variable defaultLinuxLogsLocation.

I would also offer to add a test and have prepared one in 3f81f5b, but I only know how to check symbolic links using Java 7’s NIO2 and I think you try to keep Java 6 compatibility.

Log directory 'logs' in the installation directory is soft-linked to
/var/log/app-name/. Instead of hard coding /var/log use the already
provided variable defaultLinuxLogsLocation.
@muuki88
Copy link
Contributor

muuki88 commented Feb 2, 2014

IMHO for tests java7 is okay. Only for the plugin itself we keep the java 6 compatibility.
Thanks for this fix and especially for adding a test case 👍

@jsuereth
Copy link
Member

jsuereth commented Feb 2, 2014

Actually, tests still cause an issue. The only way to ensure Java6 compatibility is to build directly (and test) on Java6 (yay for rt.jar that isn't really compatible). However, you can use any linux utiltiy in your test to validate symlinks and we can ensure this is available on our test machine(s).

Thanks for the fix! Looks great to me.

@muuki88
Copy link
Contributor

muuki88 commented Feb 2, 2014

So, replacing the java7 code with something like

import scala.sys.process._
val link = Paths.get(args(0))
val absolutePath = ("readlink -f " + link).!!
assert(...)

@jsuereth
Copy link
Member

jsuereth commented Feb 2, 2014

@muuki88 exactly.

Add a new test 'debian/log-directory' to check that a custom setting for
defaultLinuxLogsLocation is correctly applied.
@hfs
Copy link
Contributor Author

hfs commented Feb 3, 2014

Ok, I’ve added a test based on readlink -m

muuki88 added a commit that referenced this pull request Feb 3, 2014
Use defaultLinuxLogsLocation for /var/log
@muuki88 muuki88 merged commit 6d95423 into sbt:master Feb 3, 2014
@hfs hfs deleted the wip/log-directory branch February 3, 2014 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants