WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#11796 closed enhancement (fixed)

Deprecate VHOST in favor of a boolean

Reported by: Denis-de-Bernardy Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

or maybe change the checks to something like this, so as to give plugin authors the time to update their code:

if ( VHOSTS && VHOSTS != 'no' )

Or maybe add some kind of function call that returns true|false so that plugin devs don't need to worry about the constant's value.

Attachments (3)

11796.diff (5.5 KB) - added by nacin 4 years ago.
11796.2.diff (8.2 KB) - added by nacin 4 years ago.
Would introduce SUBDOMAIN_INSTALL and deprecate VHOST
11796.3.diff (5.9 KB) - added by xibe 4 years ago.
First try at updated patch.

Download all attachments as: .zip

Change History (20)

comment:1 andrea_r4 years ago

Just an explanation so you know wher eit comes from:

the vhost check is set on install in MU to determine the type of sub-blogs. VHOST=yes is for subdomain blogs via wildcard subdomains set on the server. VHOST=no is for subdirectory blogs.

comment:2 filosofo4 years ago

But the question is whether VHOST ever has more than two possible values. In core it doesn't.

"yes" and "no" are better expressed by a boolean, as Denis suggests.

comment:3 wpmuguru4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [12674]) Add is_subdomain_install() to ms code - Fixes #11796

comment:4 nacin4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

As Denis suggested, I think it would makea lot of sense to standardize this on define('VHOST', true) and define('VHOST', false), instead of keeping "yes" and "no".

is_subdomain_install() should be switched to this:
if ( defined('VHOST') && VHOST && VHOST != 'no' )

comment:5 follow-up: Denis-de-Bernardy4 years ago

adding to nacin's comment, maybe the function ought to be called is_vhost_enabled() or something. is_subdomain_install() makes it sound like we're on a subdomain.

comment:6 in reply to: ↑ 5 nacin4 years ago

Replying to Denis-de-Bernardy:

adding to nacin's comment, maybe the function ought to be called is_vhost_enabled() or something. is_subdomain_install() makes it sound like we're on a subdomain.

Possibly, but in the context of MU alone, this makes sense. VHOST is for a subdomain install, otherwise MU operates with subdirectories. Perhaps is_ms_vhost(), is_multisite_subdomain(), is_multisite_on_vhost(), etc.

nacin4 years ago

comment:7 follow-up: nacin4 years ago

  • Keywords has-patch added

Patch attached to convert VHOSTS to a boolean.

This will break any plugins (including super cache etc.) that rely on a straight VHOST = 'yes' or VHOST == 'no' check, but 3.0 is the best time to do that. An MU config file will still work, as is_subdomain_install() is adjusted to account for the deprecated 'yes' and 'no' value.

I'm thinking we should possibly throw a deprecated argument warning if they're defining VHOST as 'yes' or 'no' in config.

comment:8 in reply to: ↑ 7 wpmuguru4 years ago

Replying to nacin:

Patch attached to convert VHOSTS to a boolean.

This will break any plugins (including super cache etc.) that rely on a straight VHOST = 'yes' or VHOST == 'no' check, but 3.0 is the best time to do that. An MU config file will still work, as is_subdomain_install() is adjusted to account for the deprecated 'yes' and 'no' value.

Please have consideration for plugin authors when requesting semantic changes. In addition, backward compatibility, the MU plugin API, etc. were discussed in the meetup on January 7, 2010.

https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2010-01-07#m51727

comment:9 nacin4 years ago

As an MU administrator I am fully prepared for my blog to break on the next upgrade, but you're right.

The more I think about this, I think we should deprecate the VHOST constant all together. We'll create a new boolean SUBDOMAIN_INSTALL constant and encourage use of the API, is_subdomain_install().

If VHOST is defined, we'll throw a deprecated warning and inform them of SUBDOMAIN_INSTALL and is_subdomain_install(). We'll also define VHOST for back compat when we're using the new constant. If both is defined for some reason, we'll use SUBDOMAIN_INSTALL and throw a deprecated warning, unless of course they are set to different things, in which case we pick SUBDOMAIN_INSTALL and throw a louder error.

Not only does this get us away from yes/no to a bool, it also will encourage very quickly the use of is_subdomain_install().

Let me put together a patch.

comment:10 dd324 years ago

  • Keywords needs-patch added; has-patch removed

I think we should deprecate the VHOST constant all together

That'd be my preference as well.

Simply for the fact its using yes/no, which should be avoided for cleaner typed bools and/or the API.

Breaking backwards compatibility for a simple constant change doesnt seem worth it to me, but a better constant name & value is still needed

nacin4 years ago

Would introduce SUBDOMAIN_INSTALL and deprecate VHOST

comment:11 nacin4 years ago

  • Keywords has-patch added; needs-patch removed

A backwards compatible patch is attached.

It relies on some constant mapping in default-constants.php, to ensure that both end up being defined.

If VHOST and SUBDOMAIN_INSTALL are both already defined and they conflict, then a warning is triggered (and it uses SUBDOMAIN_INSTALL). Otherwise, a notice is triggered if VHOST is already defined.

comment:12 wpmuguru4 years ago

(In [12836]) Change VHOST check to is_subdomain_install(), See #11796

comment:13 wpmuguru4 years ago

  • Milestone changed from 3.0 to Future Release
  • Summary changed from Deprecate VHOSTS in favor of a boolean to Deprecate VHOST in favor of a boolean

This constant is being included in the merge for backward compatability with WordPress MU. The terminology changes being made as part of the merge reflect change in the range and scope of architectural implementations of MU (and multiple sites).

The VHOST constant imposes a limitation on what can be done with the WP network code. I'm not introducing a second constant to be worked around, and, potentially have to deprecate out in the future.

Changing the milestone so that this can be discussed at the next multisite component scope meeting.

comment:14 nacin4 years ago

The VHOST constant imposes a limitation on what can be done with the WP network code. I'm not introducing a second constant to be worked around, and, potentially have to deprecate out in the future.

Can you explain that a little further? The subdirectory route (VHOST = no) is a choice some make even if they have the capability to implement subdomains. We're not dealing with a limitation or workaround, we're dealing with an option. I'm not sure why it would need to be deprecated out -- are you saying the thought is that the option between subdomains and subdirectories might eventually be stored at the database level?

comment:15 nacin4 years ago

  • Keywords needs-refresh added
  • Milestone changed from Future Release to 3.0

xibe4 years ago

First try at updated patch.

comment:16 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [14452]) Deprecate VHOST in favor of a boolean, SUBDOMAIN_INSTALL. Core will keep VHOST defined for plugins' sake, but you should only define SUBDOMAIN_INSTALL. Throws a notice if VHOST is defined, and a warning if they somehow conflict. Sunrise can still handle them. fixes #11796.

comment:17 kawauso3 years ago

  • Keywords needs-refresh removed
Note: See TracTickets for help on using tickets.