This will be an exception with https://gerrit.wikimedia.org/r/#/c/24489/. The storage key column was probably NULL or something.

I ran this on a wiki that was upgraded from MediaWiki 1.8.2 to MediaWiki 1.20alpha using the update.php script.

Assuming these errors don't appear on a freshly installed wiki I'll assume this is caused by an inconsistency between the installer and the updater?

(In reply to comment #2)

I ran this on a wiki that was upgraded from MediaWiki 1.8.2 to MediaWiki
1.20alpha using the update.php script.

Assuming these errors don't appear on a freshly installed wiki I'll assume this
is caused by an inconsistency between the installer and the updater?

I was thinking old bad rows due to some problem a long time ago. I noticed the person adding the fa_sha1 column in gerrit (https://gerrit.wikimedia.org/r/#/c/17512/) made the populate script skip NULL entries, so assume there are some floating around in the DB.

Is it clear up to which older version to upgrade from this could happen (as Krinkle upgraded from 1.8.2 to 1.20alpha)?

Krinkle / Aaron: Is this something still worth to fix, or even important?
If not, should the currently high priority be lower?

Is it clear up to which older version to upgrade from this could happen (as
Krinkle upgraded from 1.8.2 to 1.20alpha)?

Krinkle / Aaron: Is this something still worth to fix, or even important?
If not, should the currently high priority be lower?

Is it clear up to which older version to upgrade from this could happen (as
Krinkle upgraded from 1.8.2 to 1.20alpha)?

Krinkle / Aaron: Is this something still worth to fix, or even important?
If not, should the currently high priority be lower?

I initially set this at a high priority because it was causing errors in production.

The case of a wiki upgrading from stable version to a recent release and subsequently running a clean up script.

The errors were PHP notices which indicate there is clearly a state that the maintenance script did not account for. What matters is that that state is handled explicitly and not implicitly. Whether the implicit handling right now falls back or actively caused images to be corrupted or orphaned is unknown to me.

What matters is that the maintenance script should not be spitting out PHP errors when ran. I don't know the File backend enough to know what it needs in terms of code handling, but it obviously isn't being handled at all right now which could cause all kinds of issues as nothing is handling or catching these errors (and since they're not fatal errors, it can continue to operate under false assumptions).

As for the actual data at hand, whether we want to support upgrading from 1.8 not, I don't know, but the main point here is that the script should in that case be modified to explicitly reject or warn against it and not move on under false assumptions causing unexpected code to be executed and things to be left in an unexpected state.

(In reply to Andre Klapper from comment #4)

Is it clear up to which older version to upgrade from this could happen (as
Krinkle upgraded from 1.8.2 to 1.20alpha)?

Krinkle / Aaron: Is this something still worth to fix, or even important?
If not, should the currently high priority be lower?

In theory, could happen for people upgrading from 1.20 and lower (fa_sha1 introduced in 1.21. populateFilearchiveSha1.php is not configured to be run by the updater)

https://gerrit.wikimedia.org/r/#/c/17512/ made it throw an exception. That's a lot better.

Change 141741 had a related patch set uploaded by Aaron Schulz:
Avoid warnings for empty file sha1 keys

https://gerrit.wikimedia.org/r/141741

(In reply to Bawolff (Brian Wolff) from comment #9)

(In reply to Andre Klapper from comment #4)

Is it clear up to which older version to upgrade from this could happen (as
Krinkle upgraded from 1.8.2 to 1.20alpha)?

Krinkle / Aaron: Is this something still worth to fix, or even important?
If not, should the currently high priority be lower?

In theory, could happen for people upgrading from 1.20 and lower (fa_sha1
introduced in 1.21. populateFilearchiveSha1.php is not configured to be run
by the updater)

PopulateFilearchiveSha1 should run by the updater, see DatabaseUpdater::$postDatabaseUpdateMaintenance

PopulateFilearchiveSha1 should run by the updater, see
DatabaseUpdater::$postDatabaseUpdateMaintenance

Whoops. I must have made a typo when grepping for it.

Change 141741 merged by jenkins-bot:
Avoid warnings for empty file sha1 keys

https://gerrit.wikimedia.org/r/141741

Patch by Aaron mentioned in this report was merged - is there more work left to do here (if yes: please reset the bug report status to NEW or ASSIGNED), or can you close this ticket as RESOLVED FIXED?

I don't have a MediaWiki 1.12 wiki to test this one.