The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.In the code for the sql-sync command (/commands/sql/sync.sql.php), at line 215, when the age of the local dump file is calculated via
filemtime($local_file) - time().
However, the modification-time will always be in the past, and hence a smaller integer than what's returned from time(), hence the result of the expression will always be negative - as it's small-integer - large-integer.
Hence the complete age-check ((filemtime($local_file) - time()) < ($cache * 60 * 60)) will always fail as it's negative-integer < positive-integer.
It should have been the other way around - time() - filemtime($local_file);. This will get you the age of the local file in seconds, and can then be used in the age-check..
Attached is a patch that reverses the order of the two functions.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | sql-sync-cache.patch | 9.5 KB | greg.1.anderson |
| drush-sqlsync-filemtime.patch | 220 bytes | fangel |
Comments
Comment #1
kotnik commentedYes, ((filemtime($local_file) - time()) < ($cache * 60 * 60)) will always return TRUE regardless of $cache time set.
This seems like an error, and this patch fixes it.
Comment #2
greg.1.anderson commentedShocking. I was sure that I had tested this; it seemed impossible that I could have made an error of such magnitude (or direction, at least), and I was ready to conclude that the only possible explanation was that time() must have been monotonically decreasing on my computer when I went over this the first time. However, upon reviewing the rest of the code, I saw that the logic for caching on the remote system was also obviously borked (although it does appear that time does, at least, flow forward on my remote servers), and there were some old variables like
$source_is_tmpand$target_is_tmpthat I thought I had gotten rid of back when I introduceddrush_sql_dump_file.Since there was no way to figure out what happened, there was nothing for it but to repent of these sign errors and logic errors and fix everything up once and for all.
An updated patch is included.
Comment #3
moshe weitzman commentedHehe. No worries. Just one more example of our recklessness in proceeding without unit tests.
This code is pretty dense. I'm happy to just give you the green light once you are happy with it.
Comment #4
greg.1.anderson commentedCommitted. "Once and for all" might prove to be too optimistic, but this should work better than what was there before. :)