@mbabker approved this pull request.

What? without any comment :-) my FIRST approved PR without someone asking for changes hahahaha

However, as we are about to revert the change, Joomla no longer needs parse_ini_file :-)

Imho we can't make it a requirement in J3, but we should in J4.

However we can show it as a recommend setting for J3

I have tested this item ✅ successfully on 4d99cda

RTC

RTC? are you sure?

changed to pending

I am the one who pointed to this issue and the solution in the maintainers chat.

I take off RTC indeed ( was too fast) as you used && instead of ||

The case we have is with people who do have parse_ini_string but not parse_ini_file, therefore please upgrade this PR to fit.

I am the one who pointed to this issue and the solution in the maintainers chat.

A chat I dont have party to and though know nothing about !

I take off RTC indeed ( was too fast) as you used && instead of ||

Proves that at least 2 people who reviewed this PR both missed the test I set (ok the brain fart I did) ... Joomla has to improve code quality somehow, if we cant trust two senior people then who can we trust :) :) :) :)

The case we have is with people who do have parse_ini_string but not parse_ini_file, therefore please upgrade this PR to fit.

However Joomla 3.7.0 REQUIRES people to have parse_ini_file therefore the PR with && was correct !

However Joomla 3.7.1 will NOT require parse_ini_file (if PR #15620 is accepted) and therefore this PR doesnt need to be made anyway and can be discarded.

semver says we cannot change the minimum technical requirements, and so hard requiring something that was not hard required before is naughty really.

However Joomla 3.7.1 will NOT require parse_ini_file (if PR #15620 is accepted) and therefore this PR doesnt need to be made anyway and can be discarded.

Wrong imho, we still need to inform people at Installation time that something is wrong concerning their parse_ini whether it is file OR string.

If you change https://github.com/PhilETaylor/joomla-cms/blob/4d99cdaa107ac8242d87809646697322ea53b722/installation/model/setup.php#L291 to have a non-null value then when getPhpOptionsSufficient runs for "hard" requirements the INI parser availability won't cause it to fail over. Basically the same way we deal with it for ext/mcrypt.

Don't forget that a large percentage of new installation are not created with the installer

Ya well we can only cater for what we can control.

Even if we revert the "hard" requirement, having the check raise a notice if one of the two are missing would be good. That said, it should probably hard fail if both are missing unless we're willing to accept the dependency to the FOF code, which it seems some aren't willing to use a library we ship within core.

which it seems some aren't willing to use a library we ship within core.

That is shipped with the core and NEVER EVER used .... /facepalm

Crazy to ship AND Use a library but prevent it's use

It is used

FOF is used, the FOF ini parser class is not used :)

I tested in real world and && works We get a "No" while || does not (after disabling parse_ini_file in php.ini)
Therefore this PR works here.

Any other debate concerns #15620
I change again my test to OK

I have tested this item ✅ successfully on 4d99cda

To my knowledge we required the parse_ini_string always in J3. So if that isn't allowed, Joomla wouldn't have worked properly already in J3.
The FOF class isn't really needed at all as we safely can use that native PHP method in J3.

I dont understand the thinking behind going from the old way (string) to the new way (file) was - I was not involved with that. There must have been a reason someone thought it was a "better" way.

The reason was likely that you can use one function call instead of several and you don't need to do the QQ string replacing as parse_ini_file can handle that itself.
Plus on a sidenote it had better handling of "bad" files.
All in all a minor performance thing, nothing ground breaking.

No need to merge this as #15620 has been merged.

Let's consider it for 4.0 with some changes including new lang string AND default to hardcoded English phrase as otherwise user may only get the constant:
If we do, I suggest to separate the check for parse_ini_string from the one for parse_ini_file to make it clear at that time that it is the lack of this specific method which prevents installing.

Concerning update, we may also need to do something generally speaking, for example displaying after update a hardcoded error specifying the issue.

Even with #15620 merged this should still go into 3.x in some form. Right now our install checks are only looking for parse_ini_string() support. In theory if one put that method into the disabled functions list but left parse_ini_file() out of it (I could see it happening, people do dumb stuff), our install checks would fail even though they are probably in a usable configuration.

@mbabker
I have no problem implementing this in a different way. Separating both parse_ini methods and default to harcoded message in case of.