In registry.inc we have:

      $modified_file = !isset($file['filectime']) || !isset($file['filemtime'])
                  || $filectime != $file['filectime'] || $filemtime != $file['filemtime'];

as our method for detecting changed files, which may cause weird problems with multiple web nodes with separate file systems, or when transferring DB dumps and code to and from development and produciton.

Let's swithc back to using more robust hashing of files for this.

CommentFileSizeAuthor
#3 819388-registry-file-hashes-3.patch6.39 KBpwolanin

Comments

Anonymous’s picture

Anonymous (not verified) commented

for reference, here's the issue where we changed from md5 to file times: #513592: Simpletest 7 much slower than 6.

i'd support moving back, as the current checks don't handle the case of different contents, same ctime/mtime. that's clearly an edge case, but given that a registry error can hose your entire site, probably worth covering.

catch’s picture

catch
he/him
Primary language English
commented

Subscribing. #797024: Moving a class between files can cause SQL errors on registry rebuild just doubled the time it takes to install / run simpletests, reverting #513592 looks like it'd add an extra 1/5 (1/5 of what I have not idea), and of course we'll be required to use the more expensive drupal_hash_64() here since md5() isn't cool enough for government bureaucrats.

However the file check already caused one fatal error at #605374: Dont add nonexisting files to the registry so I'd agree with trying to go back to some form of hashing if we can do so without making Drupal uninstallable.

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new 819388-registry-file-hashes-3.patch6.39 KB

Here's a first pass at it.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

looks good to me, very simple patch, and makes the logic a bit simpler. RTBC.

#2, we've fixed the install speed regression bug.

dries’s picture

dries commented
Project: Drupal core » HEAD to HEAD
Version: 7.x-dev »
Component: base system » Code

Committed to CVS HEAD.

Moving this to the HEAD to HEAD project.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Project: HEAD to HEAD » Drupal core
Version: » 7.x-dev
Component: Code » base system
Status: Reviewed & tested by the community » Active

IMO it distorts the history quite a lot to take an issue which is a Drupal issue (fix something) and after it's run its course, to assign it to another project with a different purpose (fix what we broke by fixing the other thing).

Can we just open new Head2Head issues for these and leave these things closed?

Our issue queues are hard enough to understand without things hopping projects.

I'm taking the liberty of doing that, if you don't mind. If you disagree, I don't mind.

I took the liberty of creating #850070: Fix: Switch registry back to using hashes not file mtime and ctime

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Active » Fixed
pwolanin’s picture

pwolanin commented

Thanks Dries.

I agree with rfay though - nodes are cheap - let's not move issues around. if you note the need, one of us will happily open a relevant head2head issue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.