Skip Menu |

This queue is for tickets about the Config-IniFiles CPAN distribution.

Report information
The Basics
Id: 104763
Status: resolved
Priority: 0/
Queue: Config-IniFiles

People
Owner: Nobody in particular
Requestors: DBOOK [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 2.86
Fixed in: (no value)



Subject: Importing config which is not written to file fails after re-reading
When using the -import constructor option to import another Config::IniFiles object, any configuration that has not been written to a file is "imported" the first time the configuration is read from file, but is not imported if the configuration is reread. I've attached a test case to demonstrate this issue occurring in a few different ways.
Subject: config-inifiles-import.t.txt
use strict; use warnings; use Config::IniFiles; use Test::More; use File::Temp 'tempfile'; my $config_nofile = Config::IniFiles->new(-allowempty => 1); $config_nofile->newval('section','param',1); my ($fh, $filename) = tempfile; ## Import config, set filename, read config, then read again my $config = Config::IniFiles->new(-import => $config_nofile, -allowempty => 1); ok($config->val('section','param'), 'Configuration is imported'); $config->SetFileName($filename); $config->ReadConfig; ok($config->val('section','param'), 'Configuration is still imported'); $config->ReadConfig; ok($config->val('section','param'), 'Configuration is still imported'); # fails ## Import config that has already been imported $config = Config::IniFiles->new(-import => $config_nofile, -allowempty => 1, -file => $filename); ok($config->val('section','param'), 'Configuration is imported again'); # fails $config_nofile = Config::IniFiles->new; $config_nofile->newval('section','param',1); ## Import config and set filename in constructor, then read again $config = Config::IniFiles->new(-import => $config_nofile, -allowempty => 1, -file => $filename); ok($config->val('section','param'), 'Configuration is imported'); $config->ReadConfig; ok($config->val('section','param'), 'Configuration is still imported'); # fails ## Import config that is written to file, but with parameters not written to file, then read again my ($fh2, $filename2) = tempfile; my $config_file = Config::IniFiles->new(-allowempty => 1, -file => $filename2); $config_file->newval('section','param2',1); $config_file->RewriteConfig; $config_file->newval('section','param',1); $config = Config::IniFiles->new(-import => $config_file, -allowempty => 1, -file => $filename); ok($config->val('section','param'), 'Configuration is imported'); $config->ReadConfig; ok($config->val('section','param'), 'Configuration is still imported'); # fails done_testing;
On Fri May 29 00:31:27 2015, DBOOK wrote: Show quoted text
> When using the -import constructor option to import another > Config::IniFiles object, any configuration that has not been written > to a file is "imported" the first time the configuration is read from > file, but is not imported if the configuration is reread. > I've attached a test case to demonstrate this issue occurring in a few > different ways.
Hi Dan! This is an acknowneldgement that I saw your test report, and will try to investigate it soon. Regards, -- Shlomi Fish
I've looked at this a little bit and I've identified the root of the problem. The issue is with the 'firstload' / re-load logic. On the first call to $config->ReadConfig, the contents of {v} are cleared and then copied back from the imported $config_nofile. At the end of this call, $config->{firstload} is still true (since we return before setting it to false, because $config has no filename). Then we call $config->SetFilename. Calling $config->ReadConfig again goes through the same process as before, except that this time, since we _do_ have a filename, we set $config->{firstload} to false and _then_ return. The trap is set! One of the first things that happens the next time $config->ReadConfig is called, is that $config->{v} is cleared. Since $config has an import, and we just set $config->{firstload} to false, we call $config->{import}->ReadConfig (which we haven't done before). This clears {v} in the imported $config_nofile, and since that object has neither import or filename, it has nowhere to re-read its configuration from. When we return, {v} is still empty. This is when we've lost the data. When $config tries to re-populate its {v} by copying it from the imported object (with _deepcopy), all it gets is an empty hash. In part because I'm not that familiar with the distribution, I'm not sure how much of this is expected behaviour. But this can be fixed pretty trivially by removing the line that sets 'firstload' to false in ReadConfig. I'm assuming, however, that that logic is there for a reason. If I can get a better idea of what is the expected behaviour in this case, I'll be happy to put together a pull request. (This was done as part of the CPAN pull request challenge). On Fri May 29 03:01:48 2015, SHLOMIF wrote: Show quoted text
> On Fri May 29 00:31:27 2015, DBOOK wrote:
> > When using the -import constructor option to import another > > Config::IniFiles object, any configuration that has not been written > > to a file is "imported" the first time the configuration is read from > > file, but is not imported if the configuration is reread. > > I've attached a test case to demonstrate this issue occurring in a > > few > > different ways.
> > Hi Dan! > > This is an acknowneldgement that I saw your test report, and will try > to investigate it soon. > > Regards, > > -- Shlomi Fish
Subject: Re: [rt.cpan.org #104763] Importing config which is not written to file fails after re-reading
Date: Sat, 10 Mar 2018 19:55:34 +0200
To: bug-Config-IniFiles [...] rt.cpan.org
From: Shlomi Fish <shlomif [...] shlomifish.org>
On Sat, 10 Mar 2018 11:39:03 -0500 "José Joaquín Atria via RT" <bug-Config-IniFiles@rt.cpan.org> wrote: Show quoted text
> Queue: Config-IniFiles > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104763 > > > I've looked at this a little bit and I've identified the root of the problem. > > The issue is with the 'firstload' / re-load logic. > > On the first call to $config->ReadConfig, the contents of {v} are cleared and > then copied back from the imported $config_nofile. > > At the end of this call, $config->{firstload} is still true (since we return > before setting it to false, because $config has no filename). > > Then we call $config->SetFilename. > > Calling $config->ReadConfig again goes through the same process as before, > except that this time, since we _do_ have a filename, we set > $config->{firstload} to false and _then_ return. > > The trap is set! > > One of the first things that happens the next time $config->ReadConfig is > called, is that $config->{v} is cleared. > > Since $config has an import, and we just set $config->{firstload} to false, > we call $config->{import}->ReadConfig (which we haven't done before). > > This clears {v} in the imported $config_nofile, and since that object has > neither import or filename, it has nowhere to re-read its configuration from. > When we return, {v} is still empty. > > This is when we've lost the data. > > When $config tries to re-populate its {v} by copying it from the imported > object (with _deepcopy), all it gets is an empty hash. > > In part because I'm not that familiar with the distribution, I'm not sure how > much of this is expected behaviour. But this can be fixed pretty trivially by > removing the line that sets 'firstload' to false in ReadConfig. > > I'm assuming, however, that that logic is there for a reason. > > If I can get a better idea of what is the expected behaviour in this case, > I'll be happy to put together a pull request. > > (This was done as part of the CPAN pull request challenge). >
Hi Jose! If the change passes all existing tests and also allows the new tests to pass, then I am fine with incorporating it. C::IF has gone through a lot of cruft, and some of it makes no sense any more. Show quoted text
> On Fri May 29 03:01:48 2015, SHLOMIF wrote:
> > On Fri May 29 00:31:27 2015, DBOOK wrote:
> > > When using the -import constructor option to import another > > > Config::IniFiles object, any configuration that has not been written > > > to a file is "imported" the first time the configuration is read from > > > file, but is not imported if the configuration is reread. > > > I've attached a test case to demonstrate this issue occurring in a > > > few > > > different ways.
> > > > Hi Dan! > > > > This is an acknowneldgement that I saw your test report, and will try > > to investigate it soon. > > > > Regards, > > > > -- Shlomi Fish
Great! I've just created https://github.com/shlomif/perl-Config-IniFiles/pull/2 which should fix this. I'll be happy to receive comments about it. On Sat Mar 10 13:17:22 2018, shlomif@shlomifish.org wrote: Show quoted text
> On Sat, 10 Mar 2018 11:39:03 -0500 > "José Joaquín Atria via RT" <bug-Config-IniFiles@rt.cpan.org> wrote: >
> > Queue: Config-IniFiles > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104763 > > > > > I've looked at this a little bit and I've identified the root of the > > problem. > > > > The issue is with the 'firstload' / re-load logic. > > > > On the first call to $config->ReadConfig, the contents of {v} are > > cleared and > > then copied back from the imported $config_nofile. > > > > At the end of this call, $config->{firstload} is still true (since we > > return > > before setting it to false, because $config has no filename). > > > > Then we call $config->SetFilename. > > > > Calling $config->ReadConfig again goes through the same process as > > before, > > except that this time, since we _do_ have a filename, we set > > $config->{firstload} to false and _then_ return. > > > > The trap is set! > > > > One of the first things that happens the next time $config-
> > >ReadConfig is
> > called, is that $config->{v} is cleared. > > > > Since $config has an import, and we just set $config->{firstload} to > > false, > > we call $config->{import}->ReadConfig (which we haven't done before). > > > > This clears {v} in the imported $config_nofile, and since that object > > has > > neither import or filename, it has nowhere to re-read its > > configuration from. > > When we return, {v} is still empty. > > > > This is when we've lost the data. > > > > When $config tries to re-populate its {v} by copying it from the > > imported > > object (with _deepcopy), all it gets is an empty hash. > > > > In part because I'm not that familiar with the distribution, I'm not > > sure how > > much of this is expected behaviour. But this can be fixed pretty > > trivially by > > removing the line that sets 'firstload' to false in ReadConfig. > > > > I'm assuming, however, that that logic is there for a reason. > > > > If I can get a better idea of what is the expected behaviour in this > > case, > > I'll be happy to put together a pull request. > > > > (This was done as part of the CPAN pull request challenge). > >
> > Hi Jose! > > If the change passes all existing tests and also allows the new tests > to pass, > then I am fine with incorporating it. C::IF has gone through a lot of > cruft, and > some of it makes no sense any more. > >
> > On Fri May 29 03:01:48 2015, SHLOMIF wrote:
> > > On Fri May 29 00:31:27 2015, DBOOK wrote:
> > > > When using the -import constructor option to import another > > > > Config::IniFiles object, any configuration that has not been > > > > written > > > > to a file is "imported" the first time the configuration is read > > > > from > > > > file, but is not imported if the configuration is reread. > > > > I've attached a test case to demonstrate this issue occurring in > > > > a > > > > few > > > > different ways.
> > > > > > Hi Dan! > > > > > > This is an acknowneldgement that I saw your test report, and will > > > try > > > to investigate it soon. > > > > > > Regards, > > > > > > -- Shlomi Fish
Subject: Re: [rt.cpan.org #104763] Importing config which is not written to file fails after re-reading
Date: Fri, 16 Mar 2018 12:01:16 +0200
To: bug-Config-IniFiles [...] rt.cpan.org
From: Shlomi Fish <shlomif [...] shlomifish.org>
On Thu, 15 Mar 2018 19:21:27 -0400 "José Joaquín Atria via RT" <bug-Config-IniFiles@rt.cpan.org> wrote: Show quoted text
> Queue: Config-IniFiles > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104763 > > > Great! > > I've just created https://github.com/shlomif/perl-Config-IniFiles/pull/2 > which should fix this. I'll be happy to receive comments about it. >
thanks! I'll take a look. Show quoted text
> On Sat Mar 10 13:17:22 2018, shlomif@shlomifish.org wrote:
> > On Sat, 10 Mar 2018 11:39:03 -0500 > > "José Joaquín Atria via RT" <bug-Config-IniFiles@rt.cpan.org> wrote: > >
> > > Queue: Config-IniFiles > > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104763 > > > > > > > I've looked at this a little bit and I've identified the root of the > > > problem. > > > > > > The issue is with the 'firstload' / re-load logic. > > > > > > On the first call to $config->ReadConfig, the contents of {v} are > > > cleared and > > > then copied back from the imported $config_nofile. > > > > > > At the end of this call, $config->{firstload} is still true (since we > > > return > > > before setting it to false, because $config has no filename). > > > > > > Then we call $config->SetFilename. > > > > > > Calling $config->ReadConfig again goes through the same process as > > > before, > > > except that this time, since we _do_ have a filename, we set > > > $config->{firstload} to false and _then_ return. > > > > > > The trap is set! > > > > > > One of the first things that happens the next time $config-
> > > >ReadConfig is
> > > called, is that $config->{v} is cleared. > > > > > > Since $config has an import, and we just set $config->{firstload} to > > > false, > > > we call $config->{import}->ReadConfig (which we haven't done before). > > > > > > This clears {v} in the imported $config_nofile, and since that object > > > has > > > neither import or filename, it has nowhere to re-read its > > > configuration from. > > > When we return, {v} is still empty. > > > > > > This is when we've lost the data. > > > > > > When $config tries to re-populate its {v} by copying it from the > > > imported > > > object (with _deepcopy), all it gets is an empty hash. > > > > > > In part because I'm not that familiar with the distribution, I'm not > > > sure how > > > much of this is expected behaviour. But this can be fixed pretty > > > trivially by > > > removing the line that sets 'firstload' to false in ReadConfig. > > > > > > I'm assuming, however, that that logic is there for a reason. > > > > > > If I can get a better idea of what is the expected behaviour in this > > > case, > > > I'll be happy to put together a pull request. > > > > > > (This was done as part of the CPAN pull request challenge). > > >
> > > > Hi Jose! > > > > If the change passes all existing tests and also allows the new tests > > to pass, > > then I am fine with incorporating it. C::IF has gone through a lot of > > cruft, and > > some of it makes no sense any more. > > > >
> > > On Fri May 29 03:01:48 2015, SHLOMIF wrote:
> > > > On Fri May 29 00:31:27 2015, DBOOK wrote:
> > > > > When using the -import constructor option to import another > > > > > Config::IniFiles object, any configuration that has not been > > > > > written > > > > > to a file is "imported" the first time the configuration is read > > > > > from > > > > > file, but is not imported if the configuration is reread. > > > > > I've attached a test case to demonstrate this issue occurring in > > > > > a > > > > > few > > > > > different ways.
> > > > > > > > Hi Dan! > > > > > > > > This is an acknowneldgement that I saw your test report, and will > > > > try > > > > to investigate it soon. > > > > > > > > Regards, > > > > > > > > -- Shlomi Fish
-- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ Summer Glau Facts - http://shlom.in/sglau-facts A man in love is incomplete until he is married. Then he is finished. — Zsa Zsa Gabor, "Newsweek" Please reply to list if it's a mailing list post - http://shlom.in/reply .
On Thu Mar 15 19:21:27 2018, JJATRIA wrote: Show quoted text
> Great! > > I've just created https://github.com/shlomif/perl-Config- > IniFiles/pull/2 which should fix this. I'll be happy to receive > comments about it. >
This pull-req was merged and is part of the 2.95 release. Resolving this ticket. Thanks all, and feel free to contribute more!