This change has been reverted pending POLA dicusssions. Refer rS479438

Given we got slapped over the head last time we went forward with this, I see this new proposed change is trying to be more careful. Please see detailed comments and questions therein.

@mat preferred to delete the old file if the new file already exists and has been modified from the default. See further description below.

mat@ choice is unacceptable because it can destroy data. In rare circumstances, but it's still possible.

We are not keeping around the reference sample files for each and every version, so when the user moved the file of .sample file we install now with mailman-2.1.29_3 changes in the future into the new location so as to make mailman work on 12.0, the logic will misfire, will assume the .conf file (unmodified from 2.1.29_3) in the new place was altered, and will remove the old possibly modified file. Oops. We cannot assume when or where the user will upgrade the package, which versions he will skip, or how thorough he will read through the half dozen screen walls full of text and act accordingly.

There is no harm in leaving the old file around that gets ignored by FreeBSD 12+'s newsyslog, and I will not scratch configuration files. We can afford 1...3 blocks and 1 inode wasted if the system ignores them.

The only situation where I will tolerate removing the old file is if it the old file's contents are identical to either the new one or to the sample file (so no information lost). Meaning I will require to compare the contents of the file we are about to remove, and not decide it can go because of how two other files unrelated to the "rm" compare against one another.

So let me as the port maintainer rule, the rm has to be made more careful or I will not accept the patch. And to avoid another screenful of logic, I'd propose to just warn the user he needs to choose which file he wants to keep in the new place.

Sorry.

(and yes it's only newsyslog, not Postfix's main.cf or your Samba PDC configuration)

Makes sense to me.

The problem is that if you do not remove the file, and have foo and foo.conf both saying rotate xxx, both will be used, and I'm sure the behavior is undocumented and probably won't be what you expect.

In D17088#389354, @mat wrote:

The problem is that if you do not remove the file, and have foo and foo.conf both saying rotate xxx, both will be used, and I'm sure the behavior is undocumented and probably won't be what you expect.

I understand that "both will be used" only applies to FreeBSD <= 11.x. Anyways, we're stuck and need to find another approach, and I am proposing that we look into pre-install scripts to move the configuration file providing that there is no target file - in that situation a "ln && rm" combo might work (not mv which is potentially destructive). Opinions?

As per the existing code, if the contents of ${newfile} are the same as ${samplefile} then there is no destruction to move ${oldfile} to ${newfile}. If the user wants to recover the content of the clobbered newfile - it is fully available in samplefile.

Which leaves us only with the question of what to do if the contents of newfile and samplefile differ. As @mat has pointed out, we can’t leave both around as it will cause double rotation of FreeBSD < 12. Given these are typically 1 line files, we are not talking about a large amount of data, and the option to delete oldfile suggested by @mat is a good one. Alternatively, we could move oldfile to /var/backups/newsyslog-mailman.conf.bak?

Thanks for the review team.
For the record, the old filename is printed out in the warning message if the user needs to do the cleanup - the echo command before the "if cmp" check.

Ben, thanks for handling this and pointing me back to the OLD: output line.
The one incremental improvement I have made afterwards is to expand the ${PREFIX} - %%PREFIX%% does that magic when substituting placeholders by real strings from files/foo.in to foo.