INI_SCANNER_RAW change

[Marcelo Gornstein <ma...@gmail.com>]

Hello all,

I've been using log4php via the .ini files to configure the loggers
and appenders. Up to 2.1.0 (maybe every version up to and not
including 2.2.0), a configuration like:

log4php.appender.default.file = ROOT_APPLICATION_PATH "/runtime/log/log.log"

would work as expected, replacing a php constant definition with the
correct value transparently.

Now, in 2.2.0 I noted this change in
configurators/LoggerConfigurationAdapterINI.php:

$properties = @parse_ini_file($url, true, INI_SCANNER_RAW);

I understand the reasons behind this change (according to the comments
in the code), but it's kind of a b/c breaker in this kind of use
cases.

Is this change irreversible? Is there anything that can be done to
restore this behaviour? It seems to me that it is a benefit to expect
the log4php ini files behave exactly as the php ini files (or it
should be documented otherwise if they ARE NOT php ini files from
2.2.0 and up).

I'm willing to work on a patch but I wanted to ask first if there is
any consensus about this that I should be aware of.

Thanks in advance,


--
// I don't sleep. I coffee.
"Make everything as simple as possible, but not simpler." -- Albert Einstein
"The class object inherits from Chuck Norris."
"Chuck Norris can divide by zero and can unit test an entire
application with a single assert."
"There’s a lot of work happening behind the scenes, courtesy of the
Spring AOP framework"
"Why do I have this nagging hunch that you have no idea what you're doing?"
"Any society that would give up a little liberty to gain a little
security will deserve neither and lose both" - Benjamin Franklin
"Delivering stupid software faster just get's you more stupid
software" - Jeff Patton
"Utilitas, Venustas, Firmitas"
"Stop breaking tests. Stop it. Just, stop."

Re: Re: INI_SCANNER_RAW change

[Ivan Habunek <iv...@gmail.com>]

Hi,

Since it turns out that log4php v2.2 does not parse INI config files
when using PHP 5.2.x we will be going ahead with a bugfix release
quite soon. It will take a little time to make sure we're not breaking
anything else by doing.

I have created an issue for this in our tracker:
https://issues.apache.org/jira/browse/LOG4PHP-167

Thanks Marcelo and Sven for reporting this. I appreciate it.

Regards,
Ivan

Re: Re: INI_SCANNER_RAW change

[Ivan Habunek <iv...@gmail.com>]

On 28 January 2012 12:17, Sven Rautenberg <sv...@rtbg.de> wrote:

> I really hate to say it, but the third optional parameter to
> parse_ini_file() is a PHP 5.3-only feature. See changenotes section of
> the docs page: http://de.php.net/manual/en/function.parse-ini-file.php
>
> I assume you created duplicate behaviour for installations with 5.2 vs.
> 5.3, one behaving as tested and advertised, and one not.
>
> I'd suggest going back to INI_SCANNER_NORMAL, as this is the unchangable
> default for PHP 5.2.
>
>
Darn, totally missed that. I guess we will be removing this option for next
release. And it will have to be quite soon because INI configuration is
broken in v2.2. Great...

Regards,
Ivan

Re: Re: INI_SCANNER_RAW change

[Sven Rautenberg <sv...@rtbg.de>]

Am 20:59, schrieb Ivan Habunek:
> On 27 January 2012 21:55, Marcelo Gornstein <ma...@gmail.com> wrote:
> 
>> <snip>
>>
>> Is this change irreversible? Is there anything that can be done to
>> restore this behaviour? It seems to me that it is a benefit to expect
>> the log4php ini files behave exactly as the php ini files (or it
>> should be documented otherwise if they ARE NOT php ini files from
>> 2.2.0 and up).
>>
>> I'm willing to work on a patch but I wanted to ask first if there is
>> any consensus about this that I should be aware of.
>>
>>
> Hi Marcelo,
> 
> Frankly, I did not consider this use case when writing the new parser. You
> have a valid point, and I'll consider bringing this back as it was. I will
> first have to consider what else this action might break...
> 
> Originally, this is where the idea came from:
> https://issues.apache.org/jira/browse/LOG4PHP-114
> 
> To avoid the whole parsing issue, have you considered using the PHP config
> format? Since 2.2. it is possible to provide it inline as well as in a
> file.
> http://logging.apache.org/log4php/docs/configuration.html
> 
> We're trying to push the PHP and XML formats instead of INI since they're
> more versatile and easier for us to parse and to document (working on an
> XSD for next release).

I really hate to say it, but the third optional parameter to
parse_ini_file() is a PHP 5.3-only feature. See changenotes section of
the docs page: http://de.php.net/manual/en/function.parse-ini-file.php

I assume you created duplicate behaviour for installations with 5.2 vs.
5.3, one behaving as tested and advertised, and one not.

I'd suggest going back to INI_SCANNER_NORMAL, as this is the unchangable
default for PHP 5.2.

Regards,

Sven Rautenberg

Re: INI_SCANNER_RAW change

[Ivan Habunek <iv...@gmail.com>]

Hi guys,

The 2.2.1 release is being voted on on the dev mailing list:
http://markmail.org/thread/ictidelq5selh4tm

This issue has been resolved by removing the INI_SCANNER_RAW flag.

You are welcome to check it out and give your vote.

Regards,
Ivan


On 28 January 2012 12:37, Marcelo Gornstein <ma...@gmail.com> wrote:
> Hi Ivan,
>
>> INI files support will not be removed for the foreseeable future so you can
>> continue using them if you're more comfortable that way. However, some
>> features may not be configurable using the INI format (currently only
>> filters). The reason for this is that INI format is increasingly harder to
>> interpret as the configuration gets more complex.
> I understand. Thanks for the clarification.
>
>> There is an easy way of converting your INI configuration to PHP:
>> $conf = new LoggerConfiguratorDefault();
>> $conf->parse('config.ini');
>>
>> This will return the equivalent PHP configuration array.
> Excellent, that's very good to know.
>
>> Now, looking through the code base, there used to be an (undocumented) way
>> of inserting constants in configuration params. But it seems that it was...
>> erm... misplaced when I rewrote the configurator for v2.2. :)
>>
>> Prior to 2.2., this would have worked:
>> log4php.appender.default.file =
>> "${ROOT_APPLICATION_PATH}/runtime/log/log.log"
>>
>> The constant delimited by ${ } would be resolved during configuration. I can
>> fix this to work for 2.3. and I would prefer doing this instead of removing
>> the SCANNER_RAW option.
>>
>> Would this solve all your problems?
> Well, it is still a b/c break :) But it would be nice to have a chance
> to use constants in the ini files again.
>
> I'll wait until the next release to make a decision based on how this turns out.
>
> Thank you very much for your efforts, Ivan.
>
> Best,
>
>
> --
>
> --
> // I don't sleep. I coffee.
> "Make everything as simple as possible, but not simpler." -- Albert Einstein
> "The class object inherits from Chuck Norris."
> "Chuck Norris can divide by zero and can unit test an entire
> application with a single assert."
> "There’s a lot of work happening behind the scenes, courtesy of the
> Spring AOP framework"
> "Why do I have this nagging hunch that you have no idea what you're doing?"
> "Any society that would give up a little liberty to gain a little
> security will deserve neither and lose both" - Benjamin Franklin
> "Delivering stupid software faster just get's you more stupid
> software" - Jeff Patton
> "Utilitas, Venustas, Firmitas"
> "Stop breaking tests. Stop it. Just, stop."

Re: INI_SCANNER_RAW change

[Marcelo Gornstein <ma...@gmail.com>]

Hi Ivan,

> INI files support will not be removed for the foreseeable future so you can
> continue using them if you're more comfortable that way. However, some
> features may not be configurable using the INI format (currently only
> filters). The reason for this is that INI format is increasingly harder to
> interpret as the configuration gets more complex.
I understand. Thanks for the clarification.

> There is an easy way of converting your INI configuration to PHP:
> $conf = new LoggerConfiguratorDefault();
> $conf->parse('config.ini');
>
> This will return the equivalent PHP configuration array.
Excellent, that's very good to know.

> Now, looking through the code base, there used to be an (undocumented) way
> of inserting constants in configuration params. But it seems that it was...
> erm... misplaced when I rewrote the configurator for v2.2. :)
>
> Prior to 2.2., this would have worked:
> log4php.appender.default.file =
> "${ROOT_APPLICATION_PATH}/runtime/log/log.log"
>
> The constant delimited by ${ } would be resolved during configuration. I can
> fix this to work for 2.3. and I would prefer doing this instead of removing
> the SCANNER_RAW option.
>
> Would this solve all your problems?
Well, it is still a b/c break :) But it would be nice to have a chance
to use constants in the ini files again.

I'll wait until the next release to make a decision based on how this turns out.

Thank you very much for your efforts, Ivan.

Best,


-- 

--
// I don't sleep. I coffee.
"Make everything as simple as possible, but not simpler." -- Albert Einstein
"The class object inherits from Chuck Norris."
"Chuck Norris can divide by zero and can unit test an entire
application with a single assert."
"There’s a lot of work happening behind the scenes, courtesy of the
Spring AOP framework"
"Why do I have this nagging hunch that you have no idea what you're doing?"
"Any society that would give up a little liberty to gain a little
security will deserve neither and lose both" - Benjamin Franklin
"Delivering stupid software faster just get's you more stupid
software" - Jeff Patton
"Utilitas, Venustas, Firmitas"
"Stop breaking tests. Stop it. Just, stop."

Re: INI_SCANNER_RAW change

[Ivan Habunek <iv...@gmail.com>]

On 28 January 2012 00:10, Marcelo Gornstein <ma...@gmail.com> wrote:

> Hello Ivan,
>
> Thank you very much for your quick response!
>
> Is the decision of deprecating the ini files final / irrevocable?
>
>
INI files support will not be removed for the foreseeable future so you can
continue using them if you're more comfortable that way. However, some
features may not be configurable using the INI format (currently only
filters). The reason for this is that INI format is increasingly harder to
interpret as the configuration gets more complex.

There is an easy way of converting your INI configuration to PHP:
$conf = new LoggerConfiguratorDefault();
$conf->parse('config.ini');

This will return the equivalent PHP configuration array.

Now, looking through the code base, there used to be an (undocumented) way
of inserting constants in configuration params. But it seems that it was...
erm... misplaced when I rewrote the configurator for v2.2. :)

Prior to 2.2., this would have worked:
log4php.appender.default.file =
"${ROOT_APPLICATION_PATH}/runtime/log/log.log"

The constant delimited by ${ } would be resolved during configuration. I
can fix this to work for 2.3. and I would prefer doing this instead of
removing the SCANNER_RAW option.

Would this solve all your problems?

Regards
Ivan

Re: INI_SCANNER_RAW change

[Marcelo Gornstein <ma...@gmail.com>]

Hello Ivan,

Thank you very much for your quick response!

Is the decision of deprecating the ini files final / irrevocable?

If yes, then I guess I'll have to migrate to one of the options you
mentioned (even if you could revert the change, it would be in a
deprecated feature anyway).

Is this decision because of lack of interest in having them, or lack
of mantainment? What would be required to keep them? I think I'd like
to cooperate if there still some interest in not loosing this feature.

Best!


On Fri, Jan 27, 2012 at 6:30 PM, Ivan Habunek <iv...@gmail.com> wrote:
> On 27 January 2012 21:55, Marcelo Gornstein <ma...@gmail.com> wrote:
>>
>> <snip>
>>
>>
>> Is this change irreversible? Is there anything that can be done to
>> restore this behaviour? It seems to me that it is a benefit to expect
>> the log4php ini files behave exactly as the php ini files (or it
>> should be documented otherwise if they ARE NOT php ini files from
>> 2.2.0 and up).
>>
>> I'm willing to work on a patch but I wanted to ask first if there is
>> any consensus about this that I should be aware of.
>>
>
> Hi Marcelo,
>
> Frankly, I did not consider this use case when writing the new parser. You
> have a valid point, and I'll consider bringing this back as it was. I will
> first have to consider what else this action might break...
>
> Originally, this is where the idea came from:
> https://issues.apache.org/jira/browse/LOG4PHP-114
>
> To avoid the whole parsing issue, have you considered using the PHP config
> format? Since 2.2. it is possible to provide it inline as well as in a
> file.
> http://logging.apache.org/log4php/docs/configuration.html
>
> We're trying to push the PHP and XML formats instead of INI since they're
> more versatile and easier for us to parse and to document (working on an XSD
> for next release).
>
> Regards,
> Ivan



-- 

--
// I don't sleep. I coffee.
"Make everything as simple as possible, but not simpler." -- Albert Einstein
"The class object inherits from Chuck Norris."
"Chuck Norris can divide by zero and can unit test an entire
application with a single assert."
"There’s a lot of work happening behind the scenes, courtesy of the
Spring AOP framework"
"Why do I have this nagging hunch that you have no idea what you're doing?"
"Any society that would give up a little liberty to gain a little
security will deserve neither and lose both" - Benjamin Franklin
"Delivering stupid software faster just get's you more stupid
software" - Jeff Patton
"Utilitas, Venustas, Firmitas"
"Stop breaking tests. Stop it. Just, stop."

Re: INI_SCANNER_RAW change

[Ivan Habunek <iv...@gmail.com>]

On 27 January 2012 21:55, Marcelo Gornstein <ma...@gmail.com> wrote:

> <snip>
>
> Is this change irreversible? Is there anything that can be done to
> restore this behaviour? It seems to me that it is a benefit to expect
> the log4php ini files behave exactly as the php ini files (or it
> should be documented otherwise if they ARE NOT php ini files from
> 2.2.0 and up).
>
> I'm willing to work on a patch but I wanted to ask first if there is
> any consensus about this that I should be aware of.
>
>
Hi Marcelo,

Frankly, I did not consider this use case when writing the new parser. You
have a valid point, and I'll consider bringing this back as it was. I will
first have to consider what else this action might break...

Originally, this is where the idea came from:
https://issues.apache.org/jira/browse/LOG4PHP-114

To avoid the whole parsing issue, have you considered using the PHP config
format? Since 2.2. it is possible to provide it inline as well as in a
file.
http://logging.apache.org/log4php/docs/configuration.html

We're trying to push the PHP and XML formats instead of INI since they're
more versatile and easier for us to parse and to document (working on an
XSD for next release).

Regards,
Ivan