Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#5652 closed defect (bug) (fixed)

root_blog_id problem since trunk has been "gruntified" for website using trunk version

Reported by: imath Owned by: DJPaul
Milestone: 2.1 Priority: high
Severity: normal Version: 2.0
Component: Core Keywords:
Cc:

Description

In bp-loader.php, the root blog id is by default the current blog id.

If the config is multisite, we get the network active plugins and check if BuddyPress is in, if so the root blog id becomes the get_current_site()->blog_id.

The problem appears when a multisite config is using the trunk version of BuddyPress or the git repo of BuddyPress. In that case checking if BuddyPress is in the network active plugins can* fail because the basename checked is buddypress/src/bp-loader.php and the one in the network active plugins is buddypress/bp-loader.php.

As a result, the root blog id is always the current blog id and Directory pages are created in child blogs etc... So in a way, for this config, we're forcing BP_ENABLE_MULTIBLOG.

So i suggest the attached patch to avoid this situation.

*this isn't happening if buddypress/src is symlinked in /wp-content/plugins/buddypress

Attachments (3)

5652.patch (567 bytes) - added by imath 6 years ago.
5652.ray.patch (2.8 KB) - added by r-a-y 5 years ago.
5652.02.patch (1.0 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (35)

@imath
6 years ago

#1 follow-up: @r-a-y
6 years ago

This might be related to #5489 as well.

#2 @DJPaul
6 years ago

  • Keywords commit added

Yep, fix looks correct.

#3 @imath
6 years ago

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

In 8433:

Remove build & src parts from BuddyPress plugin basename

In case the trunk version or the git repo of BuddyPress are used, make sure the build & src parts are removed so that the root blog id is correctly set in multisite configs.

Fixes #5652

#4 in reply to: ↑ 1 @imath
6 years ago

Replying to r-a-y:

This might be related to #5489 as well.

Hi r-a-y,
Well i'm using symlinks for BuddyPress and i don't have troubles with root blog id. So i'm not sure it's related.

#5 @johnjamesjacoby
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Let's reopen this. Not a fan of having BuddyPress core have hard-coded references to build or src directories. Those are important to us, not to millions of other installations. Would like to work towards a more elegant solution.

#6 follow-up: @boonebgorges
6 years ago

Let's reopen this. Not a fan of having BuddyPress core have hard-coded references to build or src directories. Those are important to us, not to millions of other installations. Would like to work towards a more elegant solution.

IMHO, some clever solution that manages to do the same thing without explicitly referencing 'build' or 'src' is bound to be *less* elegant, because it will rely on some funky string manipulation techniques and will be harder to understand. The bug is that the string doesn't work with our build tools, so it seems fine to me if the fix directly addresses the build tools. If there's a way to fix this that doesn't require jumping through hoops, that'd be fine, but let's please not waste lots of brain cycles on what is, in my view, a non-issue.

#7 @imath
6 years ago

In 8434:

Revert r8433

see #5652

#8 @imath
6 years ago

  • Keywords commit removed

#9 in reply to: ↑ 6 @johnjamesjacoby
6 years ago

Replying to boonebgorges:

Let's reopen this. Not a fan of having BuddyPress core have hard-coded references to build or src directories. Those are important to us, not to millions of other installations. Would like to work towards a more elegant solution.

IMHO, some clever solution that manages to do the same thing without explicitly referencing 'build' or 'src' is bound to be *less* elegant, because it will rely on some funky string manipulation techniques and will be harder to understand. The bug is that the string doesn't work with our build tools, so it seems fine to me if the fix directly addresses the build tools. If there's a way to fix this that doesn't require jumping through hoops, that'd be fine, but let's please not waste lots of brain cycles on what is, in my view, a non-issue.

I didn't say: "let's come up with an overly architected clever solution" so it's unhelpful to jump to that conclusion. Instead, let's collectively agree to strictly avoid polluting the core codebase with hardcoded references to any build processes. (The only exception in my mind here is minified CSS and JS assets.)

I suspect there's a better way to think of and address this problem. It looks like it boils down to supporting the activation of BuddyPress living in somewhat unpredictable locations (subdirectories, mu-plugins maybe, some other custom plugins directory?) with a custom bootstrap loader. If that's the case, BuddyPress would currently work on a few different installations I've seen recently, with non-conventional wp-content directories or plugin loader/routers.

#10 @imath
6 years ago

In 8436:

Remove build & src parts from BP plugin basename : the return

Untill / if a better solution is found, removing the build & src parts from the plugin basename will meanwhile make sure the root blog id is correctly set in multisite configs.

See #5652

@r-a-y
5 years ago

#11 @r-a-y
5 years ago

ray.patch is my attempt at streamlining the whole path / URL variables.

In the stub bp-loader.php:

  • BP_PLUGIN_DIR and BP_PLUGIN_URL are defined so these constants take precedence over the ones in the main bp-loader.php file.
  • BP_PLUGIN_URL uses a slightly different algorithm to be nice to symlinked directories as plugin_dir_url() doesn't work properly in these instances.
  • A new define - BP_LOAD_MODE - is added to determine what type of source is being loaded.

In the main bp-loader.php file:

  • BP_PLUGIN_DIR and BP_PLUGIN_URL are tweaked to take advantage of newer fixes in WP core. I haven't copied over the BP_PLUGIN_URL changes from the stub loader because chances are devs aren't symlinking the regular directory when BP is downloaded from the wp.org plugin repo. Open to discussing this.
  • The file and basename are calculated based off the BP_PLUGIN_DIR constant
  • plugin_dir and plugin_url are set to whatever build is running

This works in both trunk and build modes. Bonus is BP is now symlink-compatible without having to manually define the BP_PLUGIN_DIR and BP_PLUGIN_URL constants.

Last edited 5 years ago by r-a-y (previous) (diff)

#12 @DJPaul
5 years ago

I haven't tested, but I think I follow the logic through, and it looks OK. I see you removed the "If we're using https, update the protocol" bit -- I can't remember why we added that, but don't we still need that?

You say "take advantage of newer fixes in WP core" which sounds to me like this would cause a bump in the minimum supported WordPress version for BuddyPress. I don't mind doing that, but I just wanted to surface that here.

#13 @DJPaul
5 years ago

r-a-y do you have time this next week to get this into trunk?

#14 @r-a-y
5 years ago

You say "take advantage of newer fixes in WP core" which sounds to me like this would cause a bump in the minimum supported WordPress version for BuddyPress. I don't mind doing that, but I just wanted to surface that here.

According to our readme.txt, we support down to WP 3.6. WP 3.6 has the fixes in plugin_dir_url() and plugin_dir_path() that I reference in src/bp-loader.php.

I see you removed the "If we're using https, update the protocol" bit -- I can't remember why we added that, but don't we still need that?

plugin_dir_url() uses plugins_url(), which should switch to SSL automatically.

do you have time this next week to get this into trunk?

I think the patch works, but could use some additional feedback before committing.

#15 @DJPaul
5 years ago

  • Owner changed from imath to DJPaul
  • Status changed from reopened to assigned

OK. Let's poke people about it at dev chat tomorrow so we get eyes on it. Thanks!

This ticket was mentioned in IRC in #buddypress-dev by paulgibbs. View the logs.


5 years ago

#17 @DJPaul
5 years ago

  • Keywords dev-feedback added

Core devs, please give any feedback on this ticket for r-a-y; it makes some changes to Grunt and -- as Grunt is new to 2.1 and as we're still pre-beta -- I would really, really like this to get into 2.1.

As far as I'm concerned, the patch looks pretty clever. :)

#18 @boonebgorges
5 years ago

As far as I'm concerned, the patch looks pretty clever. :)

Yes, logic looks good to me. Only small quibble is that 'BP_LOAD_MODE' is a not-entirely-semantic name for this constant. Strictly speaking, the "load mode" would be something like: 'build', 'src', or 'distribution' (the wp.org version). An empty "load mode" doesn't seem like a mode at all. What the constant really is is the mode-subdirectory. That said, I can't think of a great alternative off the top of my head, so maybe we can just have an extra line of inline documentation on what BP_LOAD_MODE stands for.

#19 @r-a-y
5 years ago

Only small quibble is that 'BP_LOAD_MODE' is a not-entirely-semantic name for this constant.

How about 'BP_SOURCE_SUBDIRECTORY' instead?

#20 @boonebgorges
5 years ago

I think that's more descriptive, yes. If you're OK with it, that'd be my preference.

This ticket was mentioned in IRC in #buddypress-dev by paulgibbs. View the logs.


5 years ago

#22 @r-a-y
5 years ago

I'm fine with the change. Will commit it in a bit.

Thanks for the feedback everyone!

#23 @r-a-y
5 years ago

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

In 8740:

Streamline internal path and plugin directory properties.

Due to the introduction of Grunt (#5160), a few issues have popped up with
BP's internal properties (file, basename, plugin_dir, and plugin_url) being
incorrect for those running trunk.

To address this:

In the stub bp-loader.php file:

  • We introduce a new constant - BP_SOURCE_DIRECTORY - to determine what source folder is being run from trunk.
  • BP_PLUGIN_DIR and BP_PLUGIN_URL are defined so these constants take precedence over the ones in the main bp-loader.php file.
  • BP_PLUGIN_URL uses a slightly different algorithm to be nice to symlinked directories as plugin_dir_url() does not work properly in these instances.

In the main `bp-loader.php file:

  • BP_SOURCE_DIRECTORY is set to a blank string.
  • The file and basename are calculated based off the BP_PLUGIN_DIR constant
  • plugin_dir and plugin_url are set to whatever build is running

Fixes #5652.

#24 @DJPaul
5 years ago

  • Keywords has-patch dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

If you have a build folder generated, the following error happens:

Warning: require(/media/psf/UPVoNH7f8vJO/example.com/src/wp-content/plugins/buddypress/bp-core/bp-core-wpabstraction.php): failed to open stream: No such file or directory in /media/psf/UPVoNH7f8vJO/example.com/src/wp-content/plugins/buddypress/build/bp-loader.php on line 416

#25 @DJPaul
5 years ago

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

Groan. My bad; I had an out-of-date build folder.

#26 @imath
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi,

I'm sorry but just noticed i have a problem :( And i think it's due to r8740, as when i revert to this version of bp-loader.php, everything work fine on my config.

My config is WordPress Multisite and buddypress/src symlinked to buddypress into plugins directory. In this case, when setting the root blog id

  • in get_site_option( 'active_sitewide_plugins' ), the plugin basename is buddypress/bp-loader.php
  • & $basename at line 262 is : src/bp-loader.php

As the result, every blog is a root blog id, and must update permalinks, create pages for components .. Applying the 5652.02.patch i'm attaching to this ticket seems to fix the issue for my config.

@imath
5 years ago

#27 @r-a-y
5 years ago

  • Keywords reporter-feedback added

My config is WordPress Multisite and buddypress/src symlinked to buddypress into plugins directory.

I'm trying to understand why you would symlink 'plugins/buddypress/src' to 'plugins/buddypress'. Are you trying to simulate a production environment? Why not use the grunt build mode if you're trying to do that?

I usually symlink 'plugins/buddypress' so I can use the same 'buddypress' folder across multiple dev environments on different WP instances.

#28 @imath
5 years ago

Well, if it's a problem using plugin_basename instead of basename, i'll adapt :)

I put all my plugins in ~/plugins

then i symlink the ones i want to test into ~/sites/wordpress/src/wp-content/plugins.

#29 @imath
5 years ago

  • Keywords reporter-feedback removed

#30 follow-up: @r-a-y
5 years ago

Well, if it's a problem using plugin_basename instead of basename, i'll adapt :)

Switching to plugin_basename() breaks symlink compatibility if you're using trunk and you symlink the 'buddypress' folder like I do ;)

Try symlinking the main 'buddypress' folder instead of 'buddypress/src' and let me know if that works for you.

#31 in reply to: ↑ 30 @imath
5 years ago

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

Replying to r-a-y:

Try symlinking the main 'buddypress' folder instead of 'buddypress/src' and let me know if that works for you.

Ok thanks, i'll do that from now on, it fixes the trouble.

#32 @r-a-y
5 years ago

Cool, glad that worked!

Note: See TracTickets for help on using tickets.