Remove usages of conf_path()

Needs patch.

This is un-postponed but note the comments on #2384675-136: Deprecate conf_path().

Re-postponed on #2503015: Remove usages of conf_path() in InstallerExistingSettingsNoProfileTest.

Once https://www.drupal.org/node/2503015 is committed.

@joshi.rohit100 This would have been a good novice issue. It needs a change record or a change record update.

I think, CR (https://www.drupal.org/node/2275139) already contains this.

I see. That is probably the only relevant CR. I think for a removal like this we usually post a CR, something like "conf_path() has been removed". We can even reference the other CR for the new usage, to not be repetitive. But we could go either way. Actually the title of the older CR could simply be edited to indicate conf_path() is removed. That would also work for me.

Postponed on #2487592: CMI: don't ship with a default "active" directory that is empty in most Drupal installations

Opening this up so it's not overlooked. We should still work on #2487592: CMI: don't ship with a default "active" directory that is empty in most Drupal installations

Added beta eval.

#2487592: CMI: don't ship with a default "active" directory that is empty in most Drupal installations is in so this issue is unblocked.

Is everyone ok with this CR: https://www.drupal.org/node/2275139? conf_path() has been deprecated for three months. The other option is a new CR specifying conf_path() is now removed and linking to the detailed CR on its replacement code.

Change request looks good - it explicitly mentions what to do if you're calling conf_path().

#19 is fine with me if it passes.

There are still some occurrences of conf_path in example.sites.php and Drupal\KernelTests\KernelTestBase.

Patch looks good. All occurrences of conf_path() are gone so RTBC.

So this will break drush which will break my workflow for developing patches which will slow down D8 development. Let's fix drush before we do this.

Agree, there are hosting platforms which rely on drush and have a really "rapid" deployment cycle of new drush versions.

Opened https://github.com/drush-ops/drush/issues/1607

My reading of @alexpott's comment is that he'd be happy to review/commit this as soon as a version of drush without this dependency is available. We can't really afford to wait for hosting providers to catch up, if we want this in before RC1.

The blocking issue https://github.com/drush-ops/drush/issues/1607 has a clear next-step if anyone wants to try it. I will do it if I can find a little time.

We can't really afford to wait for hosting providers to catch up, if we want this in before RC1.

a) This is why drush is a bad dependency for a maintainer especially, and why we should have as much as possible of that behavior in an internal-to-Drupal console with tests.

b) Un-removed deprecations should be a blocker on RC1, or they were a lie when they were marked as @deprecated for removal before 8.0.0.

The deprecations (or at least the dates) were always an aspiration. If people updated their modules and then we didn't end up removing the function then nothing's lost, they've still upgraded to the new, recommended API. If we didn't deprecate the function and then removed it then we'd have broken a promise we made to module maintainers.

We should do our best to remove the functions, but if we can't then we have to ask if we're prepared to accept their maintenance burden until Drupal 9. Only if we're not do they need to block Drupal 8.

The deprecations (or at least the dates) were always an aspiration.

Disagree.

The deprecations (or at least the dates) were always an aspiration.

Disagree.

Especially (but not only) early in the use of @deprecated, a lot of functions were deprecated before all conversions had been finished.

Some functions were deprecated before they had any viable replacement in core at all, and before any conversions had been done at all, just because someone really hated them and was hoping to get rid of them.

Later on, we realised that this was an awful way to handle deprecation and tidied things up a lot (in fact ianthomas_uk did a lot of that work), but aspirational is a really, really polite way to describe some of those earlier @deprecated patches.

The fact that this patch removes two calls to conf_path() is a symptom of exactly that. Certainly not the worst one, but the fact that conf_path() isn't a single line wrapper around something else means this patch might not have been straightforward to roll - had something in core been relying on that specific behaviour. The only way to know for sure would be for conf_path() to be completely uncalled dead code.

So yes, 'aspirational'.

It is certainly postponed on #2573443: Remove conf_path() from core and effectively postponed on https://github.com/drush-ops/drush/issues/1607. We typically remove all usages, then remove the deprecated function, true?

Removing the Novice tag because this is not actionable right now.

Seems like it would have been easier to rescope here and then create a new deletion issue.

+++ b/core/includes/bootstrap.inc
@@ -122,47 +122,6 @@
-function conf_path($require_settings = TRUE, $reset = FALSE, Request $request = NULL) {

We're not actually removing conf_path() here due to rescope.

Re #34, Duh, true. Should I repurpose the other issue for the removal?

Oh, you got there. THanks.

Working on a patch.

Assigning to @luukyb

Working on a patch.

Hi,
Here is an updated patch to remove the calls to "conf_path()" and replace them with "\Drupal::service('site.path')".
Documentation updated too on example.sites.php.
Cheers,
Luc

I will review the patch and confirm that there are no other instances of conf_path() in the code.

The attached interdiff confirms that the only difference with the patch in #41 and the patch in #22 is the reinstating of the conf_path() function which is expected.

The successfully test run of the patch in #22 confirms that Drupal core tests pass without the conf_path() being present in the PHP code. I also searched the codebase for uses of the function and couldn't find any.

This looks good to me!

Committed 37bcc5a and pushed to 8.0.x. Thanks!