Last modified: 2014-09-22 23:23:45 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42362, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40362 - [Regression] deleteArchivedFiles.php broken
[Regression] deleteArchivedFiles.php broken
Status: RESOLVED WORKSFORME
Product: MediaWiki
Classification: Unclassified
Maintenance scripts (Other open bugs)
1.21.x
All All
: Low normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: code-update-regression
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-19 15:53 UTC by Krinkle
Modified: 2014-09-22 23:23 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Krinkle 2012-09-19 15:53:39 UTC
On a wiki where I know are some archived files:

> > $ php deleteArchivedFiles.php --delete
> Searching for and deleting archived files...
> PHP Notice:  Uninitialized string offset: 0 in /home/jqadmin/jqdocs_repo_stage/www/mediawiki-core/includes/filerepo/FileRepo.php on line 1326
> PHP Notice:  Uninitialized string offset: 1 in /home/jqadmin/jqdocs_repo_stage/www/mediawiki-core/includes/filerepo/FileRepo.php on line 1326
> PHP Notice:  Uninitialized string offset: 2 in /home/jqadmin/jqdocs_repo_stage/www/mediawiki-core/includes/filerepo/FileRepo.php on line 1326
> Notice - file '' not found in group 'deleted'

...etc..etc..etc..

> Done! [0 file(s)]
Comment 1 Aaron Schulz 2012-09-20 20:49:12 UTC
This will be an exception with https://gerrit.wikimedia.org/r/#/c/24489/. The storage key column was probably NULL or something.
Comment 2 Krinkle 2012-09-20 21:05:23 UTC
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?
Comment 3 Aaron Schulz 2012-09-20 21:13:59 UTC
(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.
Comment 4 Andre Klapper 2013-03-24 19:12:54 UTC
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?
Comment 5 Andre Klapper 2013-04-11 14:11:57 UTC
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?
Comment 6 Andre Klapper 2013-06-25 13:10:07 UTC
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?
Comment 7 Krinkle 2014-06-24 18:10:07 UTC
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).
Comment 8 Krinkle 2014-06-24 18:11:53 UTC
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.
Comment 9 Bawolff (Brian Wolff) 2014-06-24 18:13:01 UTC
(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)
Comment 10 Krinkle 2014-06-24 18:13:21 UTC
https://gerrit.wikimedia.org/r/#/c/17512/ made it throw an exception. That's a lot better.
Comment 11 Gerrit Notification Bot 2014-06-24 18:19:17 UTC
Change 141741 had a related patch set uploaded by Aaron Schulz:
Avoid warnings for empty file sha1 keys

https://gerrit.wikimedia.org/r/141741
Comment 12 Umherirrender 2014-06-24 18:27:44 UTC
(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
Comment 13 Bawolff (Brian Wolff) 2014-06-24 18:29:48 UTC
> 
> PopulateFilearchiveSha1 should run by the updater, see
> DatabaseUpdater::$postDatabaseUpdateMaintenance

Whoops. I must have made a typo when grepping for it.
Comment 14 Gerrit Notification Bot 2014-06-26 20:17:29 UTC
Change 141741 merged by jenkins-bot:
Avoid warnings for empty file sha1 keys

https://gerrit.wikimedia.org/r/141741
Comment 15 Andre Klapper 2014-08-17 11:26:48 UTC
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?
Comment 16 Krinkle 2014-09-22 23:23:45 UTC
I don't have a MediaWiki 1.12 wiki to test this one.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links