You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "FreeAndNil (via GitHub)" <gi...@apache.org> on 2026/10/10 21:55:26 UTC
[PR] use a dated file name when the date pattern names files like backups (logging-log4net)
FreeAndNil opened a new pull request, #336:
URL: https://github.com/apache/logging-log4net/pull/336
Fixes #335. With `StaticLogFileName`, a `DatePattern` such as `.dd` names the file of the 10th `log.txt.10`, which is also backup 10, and a roll overwrote one with the other.
* warns at activation and sets `StaticLogFileName` to false, so the live file is the dated one and its backups are numbered after the date
* no report for a count-down limit below the smallest dated name (`.dd` with 5 backups)
* existing static file and its backups left untouched
* startup scan of a dated name requires a dot after the name, so with `.d` the files of the 11th no longer count as backups of the 1st
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] use a dated file name when the date pattern names files like backups (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #336:
URL: https://github.com/apache/logging-log4net/pull/336#discussion_r4242713038
##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -1000,6 +1012,37 @@ public void InitializeRollBackups(string baseFile, IList<string> arrayFiles)
}
}
+ /// <summary>
+ /// With a static name, a date that formats as a dot and up to four digits (<c>.dd</c> writes
+ /// <c>.10</c>) is also the name of backup 10. Counting down to a limit below the smallest such
+ /// name never reaches it.
+ /// </summary>
+ private bool DoesDatePatternCollideWithBackupNames()
+ {
+ if (DatePattern is not string datePattern || !IsBaseFileRolledByDate() || !HasNumberedBackups())
+ {
+ return false;
+ }
+
+ string sample = _sDate1970.ToString(datePattern, DateTimeFormatInfo.InvariantInfo);
+ return IsDotAndUpToFourDigits(sample) && CanBackupNumberEqualDate(sample);
Review Comment:
The 1970 sample is the narrowest formatting of every numeric field except 12-hour `h`, which is unusable without `tt`. `.Md` over-reports a count-down limit of exactly 10, accepted for an ambiguous pattern. The five-digit threshold is where `IsDateSuffix` stops counting dated files as backups, so detection and counting agree. The advice now names a separator only, see 8590be00.
##########
src/changelog/3.5.1/335-short-date-pattern-collides-with-backup-names.xml:
##########
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="335" link="https://github.com/apache/logging-log4net/issues/335"/>
+ <description format="asciidoc">With `StaticLogFileName` a `DatePattern` such as `.dd` named dated
+ files like numbered backups, so a roll could overwrite one with the other. Such an appender is now
+ reported when activated and uses the dated file name as with `StaticLogFileName` false, so its
+ backups are numbered after the date, and an existing static file is left untouched, including
+ its numbered backups. Use a
+ `DatePattern` with a separator or at least five digits to keep the static name (reported by
Review Comment:
Changed to recommend a separator, with the default `.yyyy-MM-dd` as the example, see 8590be00.
##########
src/site/antora/modules/ROOT/pages/manual/configuration/appenders/rollingfileappender.adoc:
##########
@@ -88,6 +88,13 @@ For each day only the last 10 files of 1MB will be kept.
</appender>
----
+[NOTE]
+====
+With `staticLogFileName` a `datePattern` that formats as a dot and up to four digits, such as `.dd`, names dated files like numbered backups.
+Such an appender is reported when activated and uses the dated file name as with `staticLogFileName` false, so its backups are numbered after the date.
+Use a pattern with a separator or at least five digits to keep the static name.
Review Comment:
Changed to recommend a separator, with the default `.yyyy-MM-dd` as the example, see 8590be00.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] use a dated file name when the date pattern names files like backups (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on PR #336:
URL: https://github.com/apache/logging-log4net/pull/336#issuecomment-6112864326
Implementation plan for #335, as approved before the work started, kept here for the record. The merged change took a different route than step 1 of the plan: it switches `StaticLogFileName` off instead of making no numbered backups (see the PR description).
<details>
<summary>Plan</summary>
# Plan for #335: short date pattern collides with size backup names
## Context
Issue #335 (split out of #334, reported by @Snotface): with `StaticLogFileName` true, a
`DatePattern` that formats as a short number (`.dd`, `.MM`, `.HH`, `.d`) names dated files like
numbered size backups: `log.txt.10` is both the file for the 10th and backup 10.
Why files get destroyed (`src/log4net/Appender/RollingFileAppender.cs`, at `d0efa655`):
- `RollFile` deletes its target before `File.Move`, so a size roll onto `log.txt.10` destroys the
dated file, and a time roll onto `log.txt.10` destroys backup 10.
- `DetermineCurSizeRollBackups` counts the dated files as backups, so `RollOverRenameFiles`
deletes the "oldest backup" `log.txt.10` and `RollOverTime` moves `log.txt.1..Cur` as backups.
- Affected: `Composite`, and `Date` with `AppendToFile` false (the startup roll-out in
`ExistingInit` makes numbered backups when `MaxSizeRollBackups` is not 0).
Decision (user, 2026-10-10): **report at activation and make no numbered backups** for such an
appender. Date rolling keeps working, the file grows within the period. Accepted cost from the
plan review: with `AppendToFile` false the file of the same period is overwritten on restart, as
with `MaxSizeRollBackups` 0. The changelog states that.
Not a collision, and must not be reported (plan review): the default count-down setup with a finite
limit. With `CountDirection` -1 and `MaxSizeRollBackups` N only `.1` to `.N` exist, and the dated
names of a k-digit sample start at `10^(k-1)` (`.dd` gives `.01`, which is not `.1`). So `.dd`
with 5 backups is fine, `.dd` with 10 collides, `.d` always collides, `CountDirection` 0 or 1 and
`MaxSizeRollBackups` -1 always collide.
## Steps
### 0. Branch
`git switch -c Feature/335-short-date-pattern-backup-collision master` (done, at `d0efa655`).
### 1. `RollingFileAppender`: detect, report, behave as `MaxSizeRollBackups` 0
No new field. In `ActivateOptions`, after the `ComputeCheckPeriod` block and before
`ExistingInit()`:
```csharp
if (DoesDatePatternCollideWithBackupNames())
{
LogLog.Warn(_declaringType, $"""
DatePattern [{DatePattern}] of [{Name}] names dated files like numbered backups, so none are
made. Use a DatePattern with a separator or more digits, such as .yyyyMMdd, or set
StaticLogFileName to false.
""");
MaxSizeRollBackups = 0;
_rollSize = false;
}
```
`MaxSizeRollBackups = 0` makes every existing path do the right thing: `InitializeFromOneFile`
stays at zero, so the time roll moves nothing; `ExistingInit` takes its overwrite branch instead
of `RollOverRenameFiles`; `OpenFile` raises no INTERNAL ERROR. `_rollSize = false` keeps a size
roll from truncating the file, which is what 0 backups would otherwise do at `MaxFileSize`. Neither
needs undoing: reconfiguration creates a new appender. `LogLog.Warn`, not `ErrorHandler.Error`: the default
`OnlyOnceErrorHandler` reports only its first error, so the activation report would mute a later
write failure; `LogLog` reaches the `ConfigureAndWatch` message list just the same.
Detection, a private helper next to `ComputeCheckPeriod`, which already formats `_sDate1970`:
```csharp
/// <summary>
/// With a static name, a date that formats as a dot and up to four digits (<c>.dd</c> writes
/// <c>.10</c>) is also the name of backup 10. Counting down to a limit below the smallest such
/// name never reaches it. Five or more digits never count as a backup, see <see cref="IsDateSuffix"/>.
/// </summary>
private bool DoesDatePatternCollideWithBackupNames()
```
True when `_rollDate && StaticLogFileName && DatePattern is not null && MaxSizeRollBackups != 0 &&
(_rollSize || !AppendToFile)`, the sample `_sDate1970.ToString(DatePattern, InvariantInfo)` is a `.`
followed by one to four digits (a `for` loop with `c is < '0' or > '9'`, no `Regex`), and
`CountDirection >= 0 || MaxSizeRollBackups < 0 || MaxSizeRollBackups >= (int)Math.Pow(10, digits - 1)`.
Docs: one sentence on the `DatePattern` property remarks, saying that such an appender is
reported when activated, makes no numbered backups and then reads `MaxSizeRollBackups` as 0. That
makes the changed public property visible; a second `ActivateOptions` on the same instance sees 0
and skips the warning, which is acceptable for a minimal fix.
### 2. Tests, `src/log4net.Tests/Appender/RollingFileAppenderTest.cs`
Shape: `RemoteSyslogAppenderTest.IdentityWithControlCharactersIsReported`
(`src/log4net.Tests/Appender/RemoteSyslogAppenderTest.cs:256`): `List<LogLog> messages`,
`LogLog.ExecuteWithoutEmittingInternalMessages` around `using LogLog.LogReceivedAdapter _ =
new(messages);` plus the activation, assert on `messages.ConvertAll(m => m.Message)`. The adapter
hooks a static event, so `[NonParallelizable]`. Folder from `using AutoTempFolder folder = new();`,
close the appender before the scope ends. Doc comment on each test.
1. `ShortDatePatternMakesNoBackups`: `Composite`, `.dd`, static, `MaxSizeRollBackups` 10,
`MaxFileSize` 100, `File` in the temp folder, `DateTimeStrategy` a `MockDateTime`. Assert one
warning containing `DatePattern [.dd]`, log past the size a few times, assert the folder holds
only `log.txt` and that its length is above `MaxFileSize`. Fails at `d0efa655` (a `log.txt.1`
appears).
2. `ShortDatePatternIsReportedOnlyWhenBackupsCollide`, `[TestCase]`s shortest to longest source
line, arguments (pattern, static, maxBackups, countDirection, expected): `.dd` 10 -1 true;
`.dd` 5 -1 false; `.d` 5 -1 true; `.dd` 5 1 true; `.dd` -1 -1 true; `.yyyy-MM-dd` 10 -1 false;
`.dd` with `StaticLogFileName` false → false. `Composite`, `AppendToFile` true, so no file is
touched beyond creation. `.yyyy` is not a case: `ComputeCheckPeriod` already throws for it.
3. `ShortDatePatternRestartWithoutAppendMakesNoBackups`, the second affected path: `Date`, `.d`,
static, `AppendToFile` false, `MaxSizeRollBackups` 3, an existing `log.txt` in the folder with
its last-write time set to the mocked date, so `RollOverIfDateBoundaryCrossing` sees the same
period and does not time roll. After `ActivateOptions` the folder holds no `log.txt.1`. Fails
at `d0efa655`, where `ExistingInit` rolls the file to `log.txt.1`.
### 3. Changelog
`src/changelog/3.5.1/335-short-date-pattern-collides-with-backup-names.xml`, `type="fixed"`,
`<issue id="335" link="https://github.com/apache/logging-log4net/issues/335"/>`. Prose, no
bullets: what collided, that such an appender is now reported at activation and makes no numbered
backups (with `AppendToFile` false the file of the same period is overwritten on restart, as with
`MaxSizeRollBackups` 0), and the opt-out (a `DatePattern` with a separator or at least five digits,
or `StaticLogFileName` false). Close with `(reported by @Snotface, fixed by @FreeAndNil)`.
### 4. Manual
`src/site/antora/modules/ROOT/pages/manual/configuration/appenders/rollingfileappender.adoc`: one
NOTE paragraph after the Composite example (its `yyyyMMdd` has no dot and is fine). No ` -- `.
### 5. Drafts into `.git/plans/335/`
`commit.txt` in the style of the #334 one (subject with `#335`, bullets, no attribution line) and
a short `pr.md`.
## Verification
```
S=/tmp/claude-1002/-apache-logging-log4net/bae2ed16-61bf-4ff2-908d-7be0718e13b1/scratchpad
dotnet build src/log4net.sln > $S/build.txt 2>&1 # read the whole file, 0 warnings
dotnet test src/log4net.Tests/log4net.Tests.csproj --filter "FullyQualifiedName~RollingFileAppenderTest" > $S/test.txt 2>&1
```
Revert to fail: copy the appender source to `$S/RFA.fixed.cs`, `git checkout d0efa655 --
src/log4net/Appender/RollingFileAppender.cs`, rerun with `--filter
"FullyQualifiedName~ShortDatePattern"` and confirm tests 1 and 3 fail, copy the saved file back. Then grep
the touched files for `[—–]` and ` -- `.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] use a dated file name when the date pattern names files like backups (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #336:
URL: https://github.com/apache/logging-log4net/pull/336
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] use a dated file name when the date pattern names files like backups (logging-log4net)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #336:
URL: https://github.com/apache/logging-log4net/pull/336#discussion_r4241401479
##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -1000,6 +1012,37 @@ public void InitializeRollBackups(string baseFile, IList<string> arrayFiles)
}
}
+ /// <summary>
+ /// With a static name, a date that formats as a dot and up to four digits (<c>.dd</c> writes
+ /// <c>.10</c>) is also the name of backup 10. Counting down to a limit below the smallest such
+ /// name never reaches it.
+ /// </summary>
+ private bool DoesDatePatternCollideWithBackupNames()
+ {
+ if (DatePattern is not string datePattern || !IsBaseFileRolledByDate() || !HasNumberedBackups())
+ {
+ return false;
+ }
+
+ string sample = _sDate1970.ToString(datePattern, DateTimeFormatInfo.InvariantInfo);
+ return IsDotAndUpToFourDigits(sample) && CanBackupNumberEqualDate(sample);
Review Comment:
A single 1970 sample does not establish the set of numeric names a pattern can produce. For example, `.Md` samples as `.11`, so this treats 10 as the minimum and changes `StaticLogFileName` with a count-down limit of 10, although `.10` is impossible and the first colliding name is `.11`. Conversely, `.yyyyM` is ignored because its sample has five digits, but count-up backups are unbounded and can reach the dated name (for example `.20261`), recreating the overwrite after a restart. Determine whether an actually reachable formatted value intersects the reachable backup indices rather than inferring this from sample length.
This issue also appears on line 1140 of the same file.
##########
src/changelog/3.5.1/335-short-date-pattern-collides-with-backup-names.xml:
##########
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="335" link="https://github.com/apache/logging-log4net/issues/335"/>
+ <description format="asciidoc">With `StaticLogFileName` a `DatePattern` such as `.dd` named dated
+ files like numbered backups, so a roll could overwrite one with the other. Such an appender is now
+ reported when activated and uses the dated file name as with `StaticLogFileName` false, so its
+ backups are numbered after the date, and an existing static file is left untouched, including
+ its numbered backups. Use a
+ `DatePattern` with a separator or at least five digits to keep the static name (reported by
Review Comment:
The five-digit recommendation is unsafe for count-up configurations, whose indices keep increasing past the retention limit. A numeric `.yyyyM` date can still collide with backup 20261, so the release note should recommend a nonnumeric separator instead.
##########
src/site/antora/modules/ROOT/pages/manual/configuration/appenders/rollingfileappender.adoc:
##########
@@ -88,6 +88,13 @@ For each day only the last 10 files of 1MB will be kept.
</appender>
----
+[NOTE]
+====
+With `staticLogFileName` a `datePattern` that formats as a dot and up to four digits, such as `.dd`, names dated files like numbered backups.
+Such an appender is reported when activated and uses the dated file name as with `staticLogFileName` false, so its backups are numbered after the date.
+Use a pattern with a separator or at least five digits to keep the static name.
Review Comment:
At least five digits is not sufficient to prevent a collision because count-up backup indices are unbounded. For example, `.yyyyM` can collide with backup 20261. A nonnumeric separator is the safe recommendation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org