WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#17173 closed defect (bug) (fixed)

Review the usage of _copy_dir() introduced in #14484

Reported by: dd32 Owned by: dd32
Milestone: 3.2 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: 3.3-early
Focuses: Cc:

Description

Review the usage of _copy_dir() introduced in #14484;

Back story: Due to enhancements made to copy_dir() in 3.2 development which #14484 took advantage of, in order to bring immediate relief to users, a temporary function was utilised.

We should be able to remove that in 3.3, However, Removing it will harm those upgrading from pre-3.2 to 3.3, for the majority of users this will not be a problem (As long as they don't skip a version).

Change History (10)

comment:1 dd323 years ago

  • Status changed from new to accepted

comment:2 nacin3 years ago

  • Milestone changed from Future Release to 3.2

Mark suggested we should just build and serve a special package for upgrades on dotorg and revert the changes. Makes sense considering we're also doing partials. Let's talk Wednesday.

comment:3 westi3 years ago

@dd32 @nacin - Do we need to do anything here for WordPress 3.2 ?

comment:4 dd323 years ago

I'm not sure if Nacin has completed the work which would've mean't removing this from core before release. At this stage, I believe this is to stay for 3.2 due how close a release is..

comment:5 follow-up: sterlo3 years ago

Introducing code that would keep people using < 3.2 from upgrading to 3.3 could cause some serious issues.

"Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes."

"Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API. It MAY include minor and patch level changes."

http://semver.org/

Thoughts?

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

Replying to sterlo:

Introducing code that would keep people using < 3.2 from upgrading to 3.3 could cause some serious issues.

"Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes."

"Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API. It MAY include minor and patch level changes."

http://semver.org/

Thoughts?

I'm not sure what this has to do with this ticket at all.

One of our design goals is backwards compatibility and we don't inflate version numbers.

comment:7 dd323 years ago

Thoughts?

There is no huge "Backwards incompatible change" here, There is a bug, which has affected the last half a dozen releases which is fixed for 3.2 onwards, those upgrading who skip the 3.1-3.2 step might will trigger it again, however, it's not a harmful bug, it's one that people have lived with over time (akismet getting re-installed upon upgrade). Even then, That might be nulled by a API change that's in the works.

to make it clear: Those upgrading from 2.9 to 3.3 will be able to do it, this change will not prevent that, They will however, get akismet re-installed if they've removed it.

furthermore, That version numbering is not the version numbering WordPress utilises. (x.y) is the major version, (x.y.z) is the "patch" version(We don't have a "minor" release, the latter is generally called a minor release, or a point release), 2.8->2.9 holds the same weight as 2.9->3.0, which will be the same as 3.9->4.0.

comment:8 aaroncampbell3 years ago

Honestly I don't see why we can't just make the changes to the API, rely on them completely, and remove the function right now. This temporary fix seems kind of messy (not the code, just the process of adding it in and taking it back out).

I guess the main reason is so we can announce partial upgrades as a 3.2 feature? If that's the case it seems to me like partial upgrades are really only a "faster, lighter" feature if the download is smaller, right?

comment:9 nacin3 years ago

I spent a long time with a giant whiteboard with koopersmith and PeteMall at WordCamp Reno dealing with some of the roadblocks I've been hitting implementing a new API and build process.

While working on the new API, I had been under the impression that we could just wire in the new no-plugins-no-themes zip into the existing API version, and avoid our _copy_dir() hack for 3.2.

The issue is that our current API is inadequate. Here it is:

upgrade
http://wordpress.org/download/
http://wordpress.org/wordpress-3.1.3.zip
3.1.3
en_US
4.3
4.1.2

In particular, on update-core.php, there's a button to download the newest version. It presumably once linked to /download/, the second line returned. But it now links directly to the zip, which is where we'd need to shoehorn in our own custom zip. So you see the problem: The same URL is used for two different things. No shoehorning allowed.

So, here is what I am doing and proposing:

  • I have been working on a new build script on WP.org for content-less builds and also partial builds. This is quite complicated, as there's an entirely different build process for languages as well. The main one should be done soon, and I'm still looking into the languages complication. At worst, we flip a switch when that is done.
  • We need a new API. We were going to do this for partial builds anyway, but we need one now. So, I'll be posting a patch to switch over to /1.6/ hopefully as early as today, which will be a serialized response that can scale to support partial builds, and importantly will support content-less builds.
  • We need to leave _copy_dir() in for one or two release cycles. We can leave this in now for the immediate 3.1 to 3.2 (maybe 3.3) benefit. It isn't grandfathered code we need to keep forever -- it's okay if someone updating from 3.1 to 3.4 (or 3.3) gets their Akismet and Hello Dolly stomped on.
  • Partial builds require some changes to core, particularly due to a decision we made on md5'ing files. Otherwise, it's just the matter of choosing a zip to download. I have code from dd32 that I've been massaging into a patch. My goal is to have everything working by tomorrow.

So, for 3.2, here's what I'm potentially thinking:

  • Leave in _copy_dir().
  • Have a new API that returns whatever we need or will need. Just a matter of wiring it in.
  • No UI changes at this time.
  • No partial build support. However, we'll have the ability to tell core to instead use the partial build URL through a temporary constant or filter. Thus we can test this through the 3.2.x cycles, and include md5 checking in 3.3.

That said:

The new API, once fully functional, can be wired in at any time. Can probably do it today. But if partial builds are near and dear to anyone, then you're either out of luck or we need to scrap md5 implementation as a project requirement.

I think md5 is a nice-to-have, rather than a requirement. A button "Update to latest" can trigger the partial build, and "Full re-install" would trigger a full upgrade/reinstall. This would be easy to implement -- it requires the new API, which we're doing anyway, and a new button (if we can do a partial build).

comment:10 nacin3 years ago

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

_copy_dir() stays until further notice, as indicated in the above comment.

Everything else has been carried into #10611.

Note: See TracTickets for help on using tickets.