Item9699: LocalSite.cfg is continuously appended

Minor, but LocalSite.cfg is appended to with each configuration commit. I don't remember this happen when running TWiki.

It just became confusing when I went to reset the admin password, and deleted the first definition of the password not realizing there 10 more definitions of it in the rest of the file.

Thanks.

-- KiltBear - 16 Sep 2010

Sorry George, but I had to go to bed I let you do the merge if needed.

So, thanks for reporting that. The fix is as simple as adding a m to the regex which tests for existence of the key I've committed it, so you can grab the patch at the bottom of this bug report, but I doubt we will ever release it. It's fixed on trunk and Release01x01.

Closing this bug... Putting it in waiting for release, and I'm sure Kenneth will hate that, but what else should I do?

-- OlivierRaginel - 16 Sep 2010

Note that the fix stops new duplication, but it does not correct previously duplicated lines. They should be manually deleted.

-- GeorgeClark - 16 Sep 2010

Actually the fix makes it worse. The ongoing duplication is resolved, but the update now changes only the first instance. But when the config is read by Foswiki, it's the last instance that is used. Changing the regex to /msg will change all occurrences to prevent inconsistencies.

-- GeorgeClark - 16 Sep 2010

I've got the following patch for 1.0.10 that will find and remove duplicate configuration entries.

diff --git a/core/lib/Foswiki/Configure/FoswikiCfg.pm b/core/lib/Foswiki/Configure/FoswikiCfg.pm
index 1e86518..8429556 100644
--- a/core/lib/Foswiki/Configure/FoswikiCfg.pm
+++ b/core/lib/Foswiki/Configure/FoswikiCfg.pm
@@ -73,6 +73,8 @@ use Foswiki::Configure::Value     ();
 use Foswiki::Configure::Pluggable ();
 use Foswiki::Configure::Item      ();
 
+my %dupItem;
+
 =begin TML
 
 ---++ ClassMethod new()
@@ -409,7 +411,10 @@ sub startVisit {
         $txt =~ s/VAR1/Foswiki::cfg$keys/;
 
         # Substitute any existing value, or append if not there
-        unless ( $this->{content} =~ s/^\s*?\$(Foswiki::)?cfg$keys\s*=.*?;\n/$txt/m )
+        #unless ( $this->{content} =~ s/^\s*?\$(Foswiki::)?cfg$keys\s*=.*?;\n/$txt/ms )
+        #
+        # SMELL:  The _updateEntry call is needed to fix up configs broken by Item9699.
+        unless ( $this->{content} =~ s/^\s*?\$(?:Foswiki::)?cfg($keys)\s*=.*?;\n/&_updateEntry($1,$txt)/msge )
         {
             $this->{content} .= $txt;
         }
@@ -417,6 +422,14 @@ sub startVisit {
     return 1;
 }
 
+sub _updateEntry {
+    my $keys = shift;
+    my $newentry = shift;
+    return '' if $dupItem{"$keys"} ;
+    $dupItem{"$keys"} = 1;
+    return $newentry;
+}
+
 sub endVisit {
     my ( $this, $visitee ) = @_;

-- GeorgeClark - 16 Sep 2010

You guys sure work fast!

Do you know who I might talk to to find a workaround for Tasks.Item9686? It's a real show-stopper for me.

-- KiltBear - 17 Sep 2010

Unfortunately I don't use Kinosearch. Best to ask on IRC - #foswiki. Have you by any chance tested the patched file? The patched file for 1.0.10 is here: http://trac.foswiki.org/export/9187/branches/Release01x00/core/lib/Foswiki/Configure/FoswikiCfg.pm (Please back up your LocalSite.cfg first.)

-- GeorgeClark - 17 Sep 2010

Looks good. I took one of the LocalSite.cfg files with all the dups in it, and the script reduced it appropriately. I then copied my saved good LocalSite.cfg and ran it through the saving config again. Also looks good. A few lines in different order, but that's it.

-- KiltBear - 20 Sep 2010

Lavr, I've committed this "fixup" to 1.1 as well. If corrupted configs with duplicate entries slip through, the latest regex will result in configure not saving changes. This is safer, though a bit more overhead on save.

-- GeorgeClark - 21 Sep 2010

I agree on that assessment. People may be trying to upgrade using an old LocalSite.cfg and then we need the new configure code to clean up the mess.

I set this waiting for release as it fixed a released bug.

-- KennethLavrsen - 26 Sep 2010

Commenting is disabled while not logged in