Making WordPress.org

Opened 4 years ago

Closed 7 months ago

#5645 closed enhancement (fixed)

Feature Request: Update on reversion to `trunk/`

Reported by: rumperuu's profile rumperuu Owned by: dd32's profile dd32
Milestone: Priority: normal
Component: Plugin Directory Keywords:
Cc:

Description (last modified by dd32)

Hi,

We encountered an issue yesterday when I accidentally triggered the release of a broken development version of our Plugin. A full post-mortem is available at https://github.com/markcheret/footnotes/issues/55, but in short it was due to me not realising that changing the ‘Stable Tag’ field to the development version (2.5.9d1) in trunk/readme.txt would cause the WP Plugin Directory to parse it as a new version, look for the corresponding tags/2.5.9d1/ and, upon failing to find it, revert to trunk/.

We've reviewed the issue and changed our release process to avoid it happening again in future, but I'm willing to bet that I'm neither the first nor the last person to make such a mistake.

Part of the issue was that none of us received any notice that the SVN had defaulted to using the code in trunk/ (as we usually do whenever a new tag or revision is committed), which would have quickly alerted us to the fact that something was wrong. I would like to request that some sort of fallback notification email be implemented in order to help developers to catch such cases.

Without knowing the technical ins and outs of the Directory system, I've thought of a couple possible pseudocode implementations of this:

if ((stable_tag !== 'trunk' or null) and parsed_stable_tag == 'trunk') send email

or

if (parsed_stable_tag == 'trunk' and tags/* exists) send email

Thanks,
Ben

Change History (7)

#1 follow-up: @dd32
4 years ago

  • Description modified (diff)
  • Keywords security UX logging removed
  • Priority changed from high to normal

Hi Ben,

in short it was due to me not realising that changing the ‘Stable Tag’ field to the development version (2.5.9d1) in trunk/readme.txt would cause the WP Plugin Directory to parse it as a new version, look for the corresponding tags/2.5.9d1/ and, upon failing to find it, revert to trunk/.

May I ask, what did you expect would be the result of this? For simply no change to occur?
(Do read on, I'm actually okay with that behaviour and it's something that's been discussed not-on-trac)

but I'm willing to bet that I'm neither the first nor the last person to make such a mistake.

You're definitely not the first, there's been a few cases where larger plugins have had the same thing happen, where trunk was packaged as the tag didn't get committed in the proper format.

But I also know several plugin authors who rely upon the behaviour.

You may want to enable Release Confirmations for your plugin, #5352, you can do that here: https://wordpress.org/plugins/footnotes/advanced/

As part of that, a few of us discussed if disabling plugin releases from trunk entirely would be viable, #5484 is a step towards that.

Another thing worth noting, is that since Github is your primary development platform, syncing every commit to plugins.svn might not be needed, and partially lead to this situation. Personally I use the 10up Github action which deploys any releases I tag on Github to be pushed over to plugins.svn instead.

#2 in reply to: ↑ 1 @rumperuu
4 years ago

Replying to dd32:

Hi Ben,

in short it was due to me not realising that changing the ‘Stable Tag’ field to the development version (2.5.9d1) in trunk/readme.txt would cause the WP Plugin Directory to parse it as a new version, look for the corresponding tags/2.5.9d1/ and, upon failing to find it, revert to trunk/.

May I ask, what did you expect would be the result of this? For simply no change to occur?
(Do read on, I'm actually okay with that behaviour and it's something that's been discussed not-on-trac)

I didn't expect the versioning info in readme.txt to have any effect, just as the version number in the Plugin base file docblock doesn't (https://plugins.svn.wordpress.org/footnotes/trunk/footnotes.php). Coming from a string of Node.js projects, I expect I assumed the only important version number was stored in some sort of config file like package.json or composer.json, or that trunk/ was for dev. versions and tags/* was for releases (as this is how this particular Plugin is laid out, but I see that there is no necessity for other projects to use tags).

This behaviour is mentioned in the Plugin Handbook (https://developer.wordpress.org/plugins/wordpress-org/how-your-readme-txt-works/#how-the-readme-is-parsed), but only as a one-liner in the second paragraph of the section of how readme.txt is parsed. Not realising that trunk/readme.txt would be parsed in trunk/, I didn't think to look at how it might be parsed.

but I'm willing to bet that I'm neither the first nor the last person to make such a mistake.

You're definitely not the first, there's been a few cases where larger plugins have had the same thing happen, where trunk was packaged as the tag didn't get committed in the proper format.

But I also know several plugin authors who rely upon the behaviour.

You may want to enable Release Confirmations for your plugin, #5352, you can do that here: https://wordpress.org/plugins/footnotes/advanced/

Ah! Definitely a good idea, thanks for the tip.

As part of that, a few of us discussed if disabling plugin releases from trunk entirely would be viable, #5484 is a step towards that.

I think allowing releases from trunk/ where ‘Stable Tag’ is either ‘trunk’ or nonexistent should be fine (and disabling it would presumably break those projects you mentioned that rely on the behaviour), but having it fallback to trunk/ when it can't find tags/stable_tag/ is a footgun waiting to happen, especially as it is currently silent. I would expect a non-‘trunk’ stable tag that did not have a matching folder in tags/ to be considered an error and the last-known-good version to be reverted to.

Another thing worth noting, is that since Github is your primary development platform, syncing every commit to plugins.svn might not be needed, and partially lead to this situation. Personally I use the 10up Github action which deploys any releases I tag on Github to be pushed over to plugins.svn instead.

Thanks for the heads-up, we've only just started shifting over to GitHub so I've still got the release process workflow to hammer out.

Last edited 4 years ago by rumperuu (previous) (diff)

#3 @rumperuu
4 years ago

As a follow-on, I notice (though I hadn't looked at this menu until now) that in the Advanced View for the Plugin (https://wordpress.org/plugins/footnotes/advanced/), the ‘Previous Version’ selector lists the version in trunk/ as the ‘Development Version’, which as I just found out is not necessarily the case depending on what you put in trunk/readme.txt.

#4 @Ipstenu
4 years ago

Word of warning: If you sync every commit from Git to SVN, you'll eventually get a stern email from the plugins team asking you to stop, because (in the past) we used to regenerate all your zips when you did that. Now we only rezip the one branch you're on (say, Trunk) but still, there's no reason to do that.

I didn't expect the versioning info in readme.txt to have any effect, just as the version number in the Plugin base file docblock doesn't

This is actually documented. https://developer.wordpress.org/plugins/wordpress-org/how-your-readme-txt-works/#how-the-readme-is-parsed

If there's no tag, it reverts to trunk.

Also if you change the version in the plugin base but not the readme, you could cause your plugin to act 'out of date' and constantly try to update.

tl;dr? Readme and main plugin file must match if you want things to work as expected.

The ‘Previous Version’ selector lists the version in trunk/ as the ‘Development Version’, which as I just found out is not necessarily the case depending on what you put in trunk/readme.txt.

No, trunk is always your dev version. Even if you don't use it like that, that's the system setting. Always has been.

Edit: For someone wondering what I mean by a constantly trying to update, here's the sitch :) If you set the version in the main plugin file to 1.0, but the readme has 2.0, then WP will see your 1.0 as out of date, try to update, reinstall a version that claims it's 1.0, and on and on. So, y'know, sync versions is best :)

Last edited 4 years ago by Ipstenu (previous) (diff)

#5 @pewgeuges
4 years ago

@dd32, @Ipstenu,

Another thing worth noting, is that since Github is your primary development platform, syncing every commit to plugins.svn might not be needed, and partially lead to this situation. Personally I use the 10up Github action which deploys any releases I tag on Github to be pushed over to plugins.svn instead.

And:

Word of warning: If you sync every commit from Git to SVN, you'll eventually get a stern email from the plugins team asking you to stop, because (in the past) we used to regenerate all your zips when you did that. Now we only rezip the one branch you're on (say, Trunk) but still, there's no reason to do that.

The reason why trunk/ has been updated is the transition to a WordPress Coding Standards conformant code base, that @rumperuu has worked out. In order for SVN to be able to keep generating meaningful diffs, we needed a first commit solely dedicated to replacing CRLF with LF, because the code base had been developed on Windows and needed to get its EOL normalised. The diffs show totally changed files, where any other changes would be hidden. A second commit was then dedicated to committing the standardised codebase as prepared on GitHub. The diffs look great and allow to exactly localise the WP-compliance changes. In a last go, the version number in trunk/ was then synced with the current release.

And the reason why trunk/ needs to be updated more often than just for releases is the need to make development versions available. Support responses on the Forum often linked to trunk/ where the current development version enabled the user to help testing and to evaluate the fitness for use.

Anyway, thanks for the hint about syncing version numbers in Stable Tag and Plugin Main. Thankfully that is what we always did, but that is only true at release time and inside each stable tag.

Later on, depending on the need to make development versions available, trunk/ would have the version number (in Plugin Main) incremented (d series appended to incremented bugfix version number), whereas the Stable Tag must remain stable.

Following @rumperuu, I now think that the Stable Tag field in the readme.txt should not be parsed for vital release information. It should be no less informative than the version number in Main.php.

As @Ipstenu explains, WordPress enables a bug occurring when the Stable Tag is greater than the version number.

Some issues could be mitigated if the Plugin Directory would:

  • Not pay attention to the version number in the Main.php header;
  • Add a Package Version field in the readme.txt header;
  • Add a Tagged Version field in the readme.txt header.

As the first change would disrupt current algorithms, it must be dropped, but the other two are informative and help users see in a single place (as opposed to two files, one of which is a PHP file):

  • The package version, that can be a development version fixing a bug or testing a feature;
  • The tagged version allowing to fix bugs as fast as possible regardless of release schedules.

These two version numbers add to the stable tag that is used for release configuration and may remain stable over a period of time exceeding the time elapsed until the next tagged bugfix version is made available.

Last edited 4 years ago by pewgeuges (previous) (diff)

#6 @dd32
7 months ago

This probably is superseded by #6380 - forcing the use of tags and not allowing trunk.

Throwing a warning when the Stable Tag: doesn't exist and it falls back to trunk is something that's worth doing in the meantime - although currently that wouldn't trigger any emails, only an alert on the plugins page. Email can be handled via #6108.

#7 @dd32
7 months ago

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

In 13726:

Plugin Directory: Import: When importing from SVN, if the plugin specifies a Stable Tag that does not exist, and we fall back to trunk, warn them.

See #6380, #6108.
Fixes #5645.

Note: See TracTickets for help on using tickets.