Page MenuHomePhabricator

composer.json should require pear/mail_mime
Closed, ResolvedPublic

Description

HTML mail is supported by MediaWiki if the PEAR module is installed. Therefore it should be included from Composer by default.

Event Timeline

Legoktm subscribed.

Which version are you referring to? This should already be the case in 1.31.

Requiring a new composer dependency in an already released MediaWiki version is a pretty clear breaking change...

Legoktm writes:

Requiring a new composer dependency in an already released MediaWiki version is a pretty clear breaking change...

I don't understand the definition you're using of "breaking change".

This is pretty clearly fixing a bug: "Providing missing dependencies
to allow MediaWiki's HTML email support to work."

Adding the package to the tarball of maintenance releases wouldn't stop
any of the current functionality.

Adding the package would not introduce any bugs into current
functionality.

It would add support for HTML email.

It is possible that "adding support for HTML email" fails into the
category of breaking change for you, but given that I have repeatedly
run into problems with the PEAR package not being installed (when
MediaWiki would use it if it had been installed), I think "add missing
support for HTML email" would pretty clearly be a bug fix.

Err, let's take a step back. Ever since T97454 / https://gerrit.wikimedia.org/r/c/mediawiki/vendor/+/231611/ (landed in 1.27), the pear mail libraries have been bundled in the tarball. So if you're using the tarball, then the library is already there.

Like I asked earlier, which MediaWiki version are you talking about?

Legoktm writes:

Requiring a new composer dependency in an already released MediaWiki version is a pretty clear breaking change...

I don't understand the definition you're using of "breaking change".

You proposed backporting ee5a9b788a2fa3d98aaadbe0284e38ec5f9fdbfa, which adds a new *requirement*, and drops support for using the PEAR-installed libraries. That seems like a breaking change to me.

As should be obvious by now, I'm not using the tarball. I installed MW from git. In another case, the wiki I was working with (1.28.2) had been installed from git.

You proposed backporting ee5a9b788a2fa3d98aaadbe0284e38ec5f9fdbfa, which adds a new *requirement*, and drops support for using the PEAR-installed libraries. That seems like a breaking change to me.

What is breaking in that change? It is designed to more consistently use Mail_Mime by providing it via composer. That fixes a problem (MW isn't capable of HTML email) that is possible without pulling in the library via composer.

As should be obvious by now, I'm not using the tarball. I installed MW from git. In another case, the wiki I was working with (1.28.2) had been installed from git.

It's not obvious, since you mentioned the tarball in your comments earlier. And, 1.28 is unsupported. No fixes are going to be made to that branch.

You proposed backporting ee5a9b788a2fa3d98aaadbe0284e38ec5f9fdbfa, which adds a new *requirement*, and drops support for using the PEAR-installed libraries. That seems like a breaking change to me.

What is breaking in that change? It is designed to more consistently use Mail_Mime by providing it via composer. That fixes a problem (MW isn't capable of HTML email) that is possible without pulling in the library via composer.

It feels like we're just talking past each other. I'll just quote myself again:

which adds a new *requirement*, and drops support for using the PEAR-installed libraries

Adding a new requirement that previously didn't exist is a breaking change. Dropping support for something that previously worked is a breaking change.

At this point I'd recommend 1) upgrading to MediaWiki 1.31, or if that's not feasible 2) adding the pear libraries you want to composer.local.json and pulling them in that way.

Legoktm writes:

At this point I'd recommend 1) upgrading to MediaWiki 1.31, or if
that's not feasible 2) adding the pear libraries you want to
composer.local.json and pulling them in that way.

I wasn't thinking of my situation or asking for a fix to my problem.

I was not aware that pear/mail_mime was included in the tarball. If I
had checked, I would have seen that it was and not mentioned it. I did
mention the tarball because I was projecting my experience on them
without verifying it.

With the information you've provided and cross-checking with the Version
Lifecycle page on mw.o, I would return to the title ("composer.json
should require pear/mail_mime") and add the phrase "for 1.27.x and
1.30.x".

The good news, though, is that both of those are at the end of their
lifecycle, and the problem is fixed in current MW.

Re: breaking change. The only people who would see a "break" in
functionality are those who don't want HTML mail but haven't set
$wgAllowHtmlEmail to faise. Oh, and the people who are using the PEAR
module but don't run "composer update" after "git pull".

(I'm sure there are others, but those are the only two cases I could
come up with right now.)

Vvjjkkii renamed this task from composer.json should require pear/mail_mime to tfaaaaaaaa.Jul 1 2018, 1:03 AM
Vvjjkkii removed MarkAHershberger as the assignee of this task.
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
CommunityTechBot renamed this task from tfaaaaaaaa to composer.json should require pear/mail_mime.Jul 2 2018, 10:39 AM
CommunityTechBot assigned this task to MarkAHershberger.
CommunityTechBot raised the priority of this task from High to Needs Triage.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added a subscriber: Aklapper.
Reedy subscribed.

pear/mail_mime from composer is default from >= 1.31