WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#18629 closed enhancement (fixed)

Filter for SVN revision in 'right now' (and footer)

Reported by: Ipstenu Owned by:
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.3
Component: Administration Keywords: has-patch close
Focuses: Cc:
PR Number:

Description

Right now, if you're running WordPress via svn, there's no quick way to know what version of SVN you're on, without checking 'svn version' - That adds an extra layer on to people who are beta testing.

Yes, you could use the beta tester plugins, but that doesn't work on all servers and lets face it, some of us really just like to be in control (Nacin, you can make the tinfoil hat jokes).

Adding into rightnow_end mostly works (see https://gist.github.com/1206932 for Pete Mall's awesome catch), but it doesn't show it 'inline' with the rest. Even the show SVN version only puts it in the footer: http://wordpress.org/extend/plugins/show-svn-revision/

That's not 'visual' enough in the right way. It should be inline right there with 'You are using WordPress version ...'

So I made a patch for it, leveraging Pete's work and then tweaking the updates.

Attachments (6)

add-svn.diff (2.6 KB) - added by Ipstenu 8 years ago.
Adding SVN to 'Right Now' and Footer.
18629.patch (531 bytes) - added by SergeyBiryukov 8 years ago.
18629.2.patch (2.3 KB) - added by SergeyBiryukov 8 years ago.
18629.3.patch (2.3 KB) - added by SergeyBiryukov 8 years ago.
Same as previous, with proper whitespace
18629.4.patch (2.3 KB) - added by SergeyBiryukov 7 years ago.
18629.5.patch (1.6 KB) - added by SergeyBiryukov 7 years ago.

Download all attachments as: .zip

Change History (31)

@Ipstenu
8 years ago

Adding SVN to 'Right Now' and Footer.

#1 @Ipstenu
8 years ago

  • Type changed from defect (bug) to enhancement

#2 @nacin
8 years ago

We can probably do this without a new global.

The trick would probably be to use the bloginfo filter and switch these to get_bloginfo('version', 'display'). Then have a function hook into there to add the SVN version. By default, it should probably only do said hooking if is_admin() and is_super_admin().

#3 @Ipstenu
8 years ago

I made the new global because I couldn't see where bloginfo might be able to grab the SVN version (since it's in a file outside of WP).

#5 @jane
8 years ago

Whenever you guys agree on code approach, I'm down with putting the notice in Right Now, per discussion earlier on Twitter.

#6 @dd32
8 years ago

I'm pretty sure there's been a ticket on this before, But I can't seem to find it..

My prefered method would be something such as this:

wp-includes/version.php

$wp_version = '3.2.1';
if ( file_exists(ABSPATH . '.svn/entries') ) {
  $svn = file(ABSPATH . '.svn/entries');
  $wp_version .= '-r' . $svn[3];
  unset($svn);
}

That has the advantage of the revision showing up everywhere, it shouldn't affect version_compare() either as that should hopefully ignore the non-"versiony" extension.

It could even be changed to simply if ( is_admin() && file_exists()..) (Assuming is_admin() exists there, else just fall back to if ( defined('WP_ADMIN') && ...

#7 @Ipstenu
8 years ago

+1 to dd32's suggestion. If that doesn't bugger version_compare(), that's hella more elegant.

#8 @SergeyBiryukov
8 years ago

  • Keywords has-patch added

load.php is included earlier, so is_admin() is fine. Added trim() to remove trailing line break.

#9 follow-up: @dd32
8 years ago

Unfortunately I was wrong. Suffixing the version will have issues with version_compare(), To quote php.net:

Then it compares the parts starting from left to right. If a part contains special version strings these are handled in the following order: any string not found in this list < dev < alpha = a < beta = b < RC = rc < # < pl = p. This way not only versions with different levels like '4.1' and '4.1.2' can be compared but also any PHP specific version containing development state.

As such, 3.2.1 is Greater than 3.2.1-r12345, and 3.2.1-r12345 is greater than 3.2.1-RC1. As such, this will be problematic for users who checkout stable versions, They'll get a update notification for the same version they're running (if $wp_version is suffixed in version.php, if it was done anywhere else, it should be fine for updates, but might have issues elsewhere).

#10 @dd32
8 years ago

3.2.1 is Greater than 3.2.1-r12345, and 3.2.1-r12345 is greater than 3.2.1-RC1

Got part of that wrong, 3.2.1-RC1 > 3.2.1-r12345

#11 in reply to: ↑ 9 @SergeyBiryukov
8 years ago

18629.2.patch implements Nacin's idea of using get_bloginfo().

#12 @dd32
8 years ago

18629.2.patch implements Nacin's idea of using get_bloginfo().

I think that's the best way forward :)

#13 @Ipstenu
8 years ago

Nice, I like it!

@SergeyBiryukov
8 years ago

Same as previous, with proper whitespace

#14 follow-up: @Ipstenu
8 years ago

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

This is in 3.3 now. So I'm closing this as fixed, to get one more done before 3.3

#15 in reply to: ↑ 14 @SergeyBiryukov
8 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to Ipstenu:

This is in 3.3 now.

It isn't: "You are using WordPress 3.3-beta3".

#16 @Ipstenu
8 years ago

Interesting. It showed up as WordPress3.3-beta3 with an SVN yesterday(?) ... I noticed it showed Beta2 for a while, but then oozed into SVN again.

#17 @SergeyBiryukov
8 years ago

It shows a version manually bumped in version.php, not current SVN revision:
http://core.trac.wordpress.org/browser/trunk/wp-includes/version.php

#18 @nacin
7 years ago

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

Patch needs refresh, in particular to replace bumpbot's revision if it is present. Also, current_user_can( 'update_core' ) is a more specific check than is_super_admin(). I think you should also only get a revision if you are running a non-stable build. Easiest way to test that is look for the presence of a hyphen.

#19 @bpetty
7 years ago

I wonder if bumpbot revision should be shown, when this patch is applied, even if .svn/entries isn't there. We won't know for sure whether a nightly/beta tester is giving us version numbers that were generated from bumpbot (possibly a stale revision number from what they are actually running) or with the real SVN revision number.

Or maybe nightly builds should also trigger bumpbot for revision version update even if there are no changes to CSS/JS needing recompiled just before being built so they always are using the exact reported revision.

#20 @SergeyBiryukov
7 years ago

  • Keywords needs-refresh removed

18629.4.patch is the refreshed patch.

However, the method of getting the revision number by parsing .svn/entries no longer works since Subversion 1.7. It seems that now there's no easy way to get SVN revision number from PHP without calling external programs.

I guess we should still replace $GLOBALS['wp_version'] with get_bloginfo(), so that a plugin could implement get_svn_revision() and hook into there. 18629.5.patch does that.

#21 @nacin
7 years ago

In [21986]:

Use get_bloginfo('version', 'display') for displaying the WP version number in the admin. Can allow for filtering -- for example, showing the SVN revision. props SergeyBiryukov, see #18629.

#22 follow-up: @nacin
7 years ago

  • Keywords close added

Plugin material otherwise, for now? Probably worth even adding it to the Beta Tester plugin. We've talked about making that plugin about more than just updates to nightly builds. (Imagine if a dashboard widget gave you diagnostics, instructions, and links to Trac; we could also send additional info to an API easily, such as usage of PHP modules and such, to see what kind of test coverage we are getting.)

#23 in reply to: ↑ 22 ; follow-up: @bpetty
7 years ago

Replying to nacin:

Plugin material otherwise, for now? Probably worth even adding it to the Beta Tester plugin. We've talked about making that plugin about more than just updates to nightly builds. (Imagine if a dashboard widget gave you diagnostics, instructions, and links to Trac; we could also send additional info to an API easily, such as usage of PHP modules and such, to see what kind of test coverage we are getting.)

Definitely plugin material for most of what was just mentioned there, but I still think it's a bug that the SVN revision shown (when it is shown) still isn't the actual SVN revision most of the time without any plugin installed that uses the hook added in r21986 to override the bumpbot revision number.

#24 in reply to: ↑ 23 ; follow-up: @nacin
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
  • Summary changed from Show SVN revision in 'right now' (and footer) to Filter for SVN revision in 'right now' (and footer)

Replying to bpetty:

Definitely plugin material for most of what was just mentioned there, but I still think it's a bug that the SVN revision shown (when it is shown) still isn't the actual SVN revision most of the time without any plugin installed that uses the hook added in r21986 to override the bumpbot revision number.

The only reasons why bumpbot changes this version are A) we used to do it manually when we felt like it, and usually only once we got to beta time, and B) we rewrote the script-loader to use a global version rather than individual versions.

It's not a bug. It *might* be confusing, but really, who is looking at that for the SVN version number? If we did 3.5-beta2-r22222 ("r" or "rev") then I'd understand why I'd call it a bug. Essentially, it's a nightly build version — it's only there to differentiate between two nightly builds, as well as a nightly build that follows a beta. It's designed so someone using the beta tester plugin can update and they realize things have updated, rather than "updating" to the same beta 2 over the course of a week.

Also, bumpbot runs immediately before the nightly build is generated, which means the build itself is going to be accurate. Unfortunately since SVN 1.7 made this more difficult, there's no compelling reason to add this to core for now.

#25 in reply to: ↑ 24 @bpetty
7 years ago

Replying to nacin:

Also, bumpbot runs immediately before the nightly build is generated, which means the build itself is going to be accurate. Unfortunately since SVN 1.7 made this more difficult, there's no compelling reason to add this to core for now.

Actually, bumpbot might run right before nightlies, but unless there's actually changes to CSS/JS, it still doesn't bother bumping the revision even when it has changed. So no, the nightlies aren't even accurate. This is actually what brought my attention to this ticket in the first place -- someone using nightlies thought they weren't actually updating.

I know there's still a way to at least safely run "svn info --xml" within core if it's there (rather than the .svn/entries check that was first proposed).

Note: See TracTickets for help on using tickets.