Paths

Table of Contentst

Differential D28940

usr.sbin/newsyslog: Implement E newsyslog.conf entry flag
ClosedPublic
Actions

Authored by otis on Feb 26 2021, 4:36 PM.

Tags

None

Referenced Files

	F102751040: D28940.id84878.diff
	Sat, Nov 16, 4:35 PM

	Unknown Object (File)
	Sat, Oct 19, 4:22 PM

	Unknown Object (File)
	Oct 10 2024, 9:12 PM

	Unknown Object (File)
	Sep 23 2024, 1:56 PM

	Unknown Object (File)
	Sep 18 2024, 10:13 AM

	Unknown Object (File)
	Sep 9 2024, 3:02 AM

	Unknown Object (File)
	Sep 6 2024, 3:21 AM

	Unknown Object (File)
	Sep 2 2024, 6:33 PM

View All 97 Files

Subscribers

Details

Reviewers

dvl
eugen_grosbein.net
markj
dab
delphij
cem
• ian
rpokala

Group Reviewers

Commits

rG7e863cb9893b: newsyslog(8): Implement a new 'E' flag to not rotate empty log files
rGffdcad758e3e: newsyslog(8): Implement a new 'E' flag to not rotate empty log files
rG78619cc2ff1b: newsyslog(8): Implement a new 'E' flag to not rotate empty log files
rGc7d27b225df8: newsyslog(8): Implement a new 'E' flag to not rotate empty log files

Summary

Based on a conversation with @dvl (https://twitter.com/DLangille/status/1364944587647836166) where an idea came from his work colleague, Laszlo Danielisz:

Implement E flag for newsyslog.conf entries.

This flag instructs newsyslog to not rotate a logfile when its size is zero bytes.

Its most frequent use will be along with N and/or B flags.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37436
Build 34325: arc lint + arc unit

Event Timeline

otis created this revision.Feb 26 2021, 4:36 PM

Herald added a reviewer: manpages. · View Herald TranscriptFeb 26 2021, 4:36 PM

Herald added a subscriber: imp. · View Herald Transcript

otis requested review of this revision.Feb 26 2021, 4:36 PM

Harbormaster completed remote builds in B37409: Diff 84751.Feb 26 2021, 4:36 PM

otis edited the summary of this revision. (Show Details)Feb 26 2021, 5:02 PM

This arose from a Tweet: https://twitter.com/DLangille/status/1364944587647836166

My question came from a work colleague, Laszlo Danielisz. It was his idea.

otis edited the summary of this revision. (Show Details)Feb 26 2021, 7:07 PM

Adding reviewers who did some work on newsyslog over the past years.

• ian accepted this revision.Feb 26 2021, 7:32 PM

This revision is now accepted and ready to land.Feb 26 2021, 7:32 PM

Seems reasonable to me, just a couple of minor comments.

usr.sbin/newsyslog/newsyslog.c
535	The parentheses around `(ent->fsize == 0)` are redundant.
usr.sbin/newsyslog/newsyslog.conf.5
291	I'd suggest adding a sentence noting that this option is not very useful without 'B'. It would be cool if we could also avoid rotation if the file only contains the message from the previous rotation, but I'm not sure how best to approach that.

I also think

usr.sbin/newsyslog/newsyslog.conf.5
291	Perhaps also mention: don't use compression. e.g. JXYZ

otis marked an inline comment as done.Feb 26 2021, 8:04 PM

otis added inline comments.

usr.sbin/newsyslog/newsyslog.conf.5
291	I'd suggest adding a sentence noting that this option is not very useful without 'B'. It would be cool if we could also avoid rotation if the file only contains the message from the previous rotation, but I'm not sure how best to approach that. A mention of `B` has been added I was thinking about it, it can be subject to another RR in near future.
291	Perhaps also mention: don't use compression. e.g. JXYZ As has been mentioned on IRC, compression does not matter in this case, as `B` ensures that the new file is empty and rotated file can be compressed anyway.

rpokala requested changes to this revision.Feb 27 2021, 2:49 AM

rpokala added a subscriber: rpokala.

rpokala added inline comments.

usr.sbin/newsyslog/newsyslog.c
1303	The description and manpage say `E`, but you're checking for `e`.

This revision now requires changes to proceed.Feb 27 2021, 2:49 AM

otis marked an inline comment as done.Feb 27 2021, 10:56 AM

otis added inline comments.

usr.sbin/newsyslog/newsyslog.c
1303	The description and manpage say `E`, but you're checking for `e`. There is `switch (tolowerch(*q))` so yes, `e` is apropriate.

otis marked an inline comment as done.Feb 27 2021, 10:57 AM

The new revision wasn't uploaded, but this seems ok to me.

Update PR

Harbormaster completed remote builds in B37436: Diff 84796.Feb 27 2021, 4:16 PM

In D28940#648492, @markj wrote:

The new revision wasn't uploaded, but this seems ok to me.

Now it can be accepted. RR has been updated.

otis marked 2 inline comments as done.Feb 27 2021, 4:18 PM

markj added inline comments.Feb 27 2021, 5:12 PM

usr.sbin/newsyslog/newsyslog.conf.5
291	New sentences should start on new lines.

Start new sentence on a new line.

otis marked an inline comment as done.Feb 27 2021, 5:18 PM

Harbormaster completed remote builds in B37439: Diff 84801.Feb 27 2021, 5:18 PM

rpokala accepted this revision.Feb 28 2021, 12:17 AM

rpokala added inline comments.

usr.sbin/newsyslog/newsyslog.c
1303	Ugh, it is! That's Gross 🤮 Not your fault 😜 Bearing that in mind, LGTM.

This revision is now accepted and ready to land.Feb 28 2021, 12:17 AM

Closed by commit rGc7d27b225df8: newsyslog(8): Implement a new 'E' flag to not rotate empty log files (authored by otis). · Explain WhyFeb 28 2021, 10:32 PM

This revision was automatically updated to reflect the committed changes.

otis added a commit: rGc7d27b225df8: newsyslog(8): Implement a new 'E' flag to not rotate empty log files.

otis added a commit: rG78619cc2ff1b: newsyslog(8): Implement a new 'E' flag to not rotate empty log files.Mar 15 2021, 12:11 AM

otis added a commit: rGffdcad758e3e: newsyslog(8): Implement a new 'E' flag to not rotate empty log files.Mar 15 2021, 9:26 PM

otis added a commit: rG7e863cb9893b: newsyslog(8): Implement a new 'E' flag to not rotate empty log files.May 23 2021, 1:41 PM

Revision Contents
Changeset List

Path

Size

usr.sbin/

newsyslog/

2 lines

newsyslog.conf.5

10 lines

Diff 84796

usr.sbin/newsyslog/newsyslog.c

Loading...

usr.sbin/newsyslog/newsyslog.conf.5

Loading...