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

LOGBACK-796 RollingFileAppender should have error when File matches FileNamePattern #67

Merged
merged 1 commit into from Mar 27, 2013
Merged

Conversation

The-Alchemist
Copy link
Contributor

Why

Ceki said:

File property and the archived files should not point to the same
file path. When you wish them to point to the same path, then File
property should be omitted.

From http://mailman.qos.ch/pipermail/logback-user/2013-January/003655.html

I've added the code / test for this.

What

  • I've logged the issue at http://jira.qos.ch/browse/LOGBACK-796
  • I wasn't sure where to add the check, but I added it to RollingFileAppender.start() because that's where other checks where (as opposed to RollingPolicyBase, where it seemed like the checks should be)
  • I wanted to make the least changes to the code, so I didn't want to add the concept of validation to file name patterns.
  • The additional test went into RollingFileAppenderTest
  • All tests in logback-core (except Scala tests) still pass. :)

Other

Feedback, comments, etc. welcome.

@The-Alchemist
Copy link
Contributor Author

I'm not sure whether I covered all the situations, or whether my validation logic is even correct.

I'm just doing a regex comparison to see if File matches FileNamePattern, which is OK but has some limitations:

  • if File is x-1000.log and the file name pattern is x-%d{yyyy}.log, there technically shouldn't be problems because the system clock will (hopefully) never be set to the year 1000. However, the regex will match and with this pull request, logback will error out. I'm not sure if a loggin framework should be /that/ smart, though.

ceki added a commit that referenced this pull request Mar 27, 2013
LOGBACK-796 RollingFileAppender should have error when File matches FileNamePattern
@ceki ceki merged commit f572714 into qos-ch:master Mar 27, 2013
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.

None yet

2 participants