Can't say something about the code but is this also catching the issue where cron.php is not running because of the missing APC/APCu?

I tested this and with the proposed change it will also fix the cron stuff. Maybe adjust the log message then to CLI instead of occ

Screw you too Jenkins

@owncloud-bot retest this please

Since it's not recommended to enable memcache with APCu on CLI, why don't we simply test directly in the factory?

Something like that:

if (\OC::$CLI && $memcache === "APCu") {
    // APCu memcache is not recommended by upstream for CLI.
    return null;
}
else {
    // all good.
    return new Memcache;
}

We could even check for apc.encable_cli so that if the user really want to activate it anyway, we just send a warning saying it's not recommended by upstream, but it's been forced so we obliged.

@MightyCreak The problem here isn't specific to APCu, XCache will also have the same problem (it runs in-process too). What's more, the CLI should never need a memcache for operation. The only reason the exception throwing was introduced was due to the idiom that any unavailable service should prevent the OC instance from running, be that LDAP, DB or (now) memcache. For CLI this isn't a problem, so memcache failures shouldn't be a hard-fail.

@Xenopathic Ok, but then why should we catch an exception from \OC\Memcache\Factory? The ifin the catch block should directly be in the Factory function, don't you think?

Catching an exception to handle a case that is not an error seems like a workaround to me. But if everyone is fine with that, then it's fine by me ;)

@MightyCreak Hmm, that might be a better idea. Avoiding catch is always a good thing!

Now get back to Mesamatrix and implement new features! 😆

So I moved the checking logic to the memcache factory, removing the try catch statements. Also modified the wording of the exception text/logging text:

Memcache {class} not available for {use} cache

instead of

Missing memcache class {class} for {use} cache

Also pre-emptively fixed the issue I mentioned in the original post, with the class name vs object.

In case this is worse than what was here before, I'm leaving this commit ID to revert back to: a629f16cacc55227f769acde3c9c46a24a36c600

A new inspection was created.

👍 ^^

Jenkins PR: #17681

Tested and works 👍

@karlitschek Should we backport this to fix the broken behaviour of APCu on CLI for the occ tool? The original ticket was against 8.1.1

I think a backport would be good 👍

@Xenopathic Can you please backport? Keep in mind: stable8.1 freeze is planned for monday evening ... ;)