Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Issue tags: +Update manager

Tagging so this doesn't get lost/forgotten.

dww’s picture

Title: Add some garbage collection to the update manager when there are failures » Add some garbage collection to the update manager
Category: task » bug
Priority: Normal » Major
hass’s picture

Looks like there is more detailed info in the duplicate case than here :-)

mermentau’s picture

@ hass Could be the wrong one was closed and the result is that it's not critical anymore. To me a smooth module update process is a key feature of Drupal 7.

hass’s picture

Yes, I'm for critical but kept it as dww marked it... :-)

dww’s picture

If you read the handbook page about issue status values you'll see that for something to be critical it has to basically render the entire system unusable. However, "major" is defined as such:

For issues which have significant repercussions, but do not render the whole system unusable. For example a PHP error which is only triggered under rare circumstances or which affects only a small percentage of all users.

Update manager is optional functionality. Furthermore, the lack of garbage collection is only breaking support for installing -dev releases. Official releases (strongly recommended all over the place) still work. Therefore, the bug here only affects a small percentage of all users.

So, while a big deal, and something we should address ASAP, this bug isn't actually critical and shouldn't block 7.0 if no one gets to it in time...

That said, instead of spending energy on deciding which issue is the right one or how high the priority should be set, it'd be nicer to see that energy spent actually resolving the bug. ;)

Cheers,
-Derek

JoshOrndorff’s picture

I agree, let's focus on getting this one worked out. I'm running a drupal7-beta2 install with styles7.x-1.0-alpha4. Update manager suggests that I update to styles7.x-1.0-alpha1. Alpha 4 is the very first and only version of styles that I've installed.

I'm working on porting another module right now, but whenever there is a patch, I'll be ready to test it out.

-Josh

verbosity’s picture

Subscribing.

I'd mark this as assigned to me, but at the moment I've no idea if i can make any real difference on this issue.

EvanDonovan’s picture

Priority: Critical » Major

Subscribing.

If it turns out we can't fix this in two weeks, could we make it so that Update Manager would not allow the installation of dev releases (unless perhaps a hidden setting was added to the $conf array in settings.php)?

That way, we would have the usability benefits of the Update Manager and could possibly fix its behavior in a point release.

EDIT: Removed hass' quote since justinrandell quoted it below.

verbosity’s picture

Priority: Major » Critical

Changing to critical on the basis of : #971546 [meta] Battle plan for Update Manager being critical and this ( seemingly ) being the most important of the issues listed in #971546

Anonymous’s picture

i'm looking at this. i thought i'd bring some context over from #951426: Update-cache is never cleared and holds stale module downloads. in that issue, hass said:

From a very quick review update_manager_file_get() looks like it only downloads the files, but never clears / deletes the "cached" files. I was also not able to find any other code that references update-cache. Therefore I suspect this cleanup is completely missing and locally cached release downloads are never ever deleted from cache.

I do not like to guess, but it looks like someone missed to implement it or it's a design flaw if this is by design.

Logic wise we need to clear the update-cache from time to time. I guess a good time is once or twice per day. Maybe only block re-downloads if a release has been downloaded within the last hour - but no longer or compare the time stamp inside the .info file with the timestamp on d.o and only download if there are newer versions on d.o. Otherwise we will always run in such type of issues. Additional I do not see a reason why my update-cache should grow for ages and keep all releases I have ever installed in my drupal site.

how long should we wait before treating a cached file as out of date? maybe a variable_get() and an admin form?

also, i guess we need a call from update_cron() to something like (but probably better named) update_manager_garbage_collect_downloads() ?

babbage’s picture

Priority: Major » Critical

If it turns out we can't fix this in two weeks, could we make it so that Update Manager would not allow the installation of dev releases.

I think this is an extremely sensible suggestion... you could make an argument for that as being a "feature", even if it wasn't required for other reasons. You probably wouldn't win that argument, given how often -dev releases are used in our community... but you could at least *make* the argument. :)

geerlingguy’s picture

I think not allowing dev releases via the Update Manager would be a very amenable solution. Those who most often complain about the manager are those who would (a) be more prone to using dev releases, and (b) would probably not use Update Manager regardless.

hass’s picture

Status: Active » Needs review
FileSize
848 bytes

Here is a patch that downloads locally cached files again after 6 hours, but does not implement the garbage collection. It worked in a very short test on my Windows box.

I guess we all have some more general ideas in mind, but this is a workaround if we are not able to implement the big solution within the required time.

dww’s picture

#14 is a reasonable start. A few things:

A) I'd rather this new behavior only happened on -dev releases, although we don't really have a reliable way to know other than parsing the filename (which isn't super elegant). However, if people aren't using standard d.o release tarballs, it's an edge case, and they can be careful not to incorrectly name them so they end in ".x-dev.tar.gz". It really is pointless to re-download an official release if we have it locally, even after 6 hours. However, maybe that's over-complicating things at this stage, so I'd be willing to retract this concern. That's why I'm not setting this to needs work.

B) If #14 (or something based on it) is committed, we still can't mark this issue fixed. Even if we're successfully re-downloading -dev releases, we'll just slowly accumulate more junk in the update manager temp folders over time, slowly filling up disks all over the web. So, we really need some garbage collection, not just a work-around to get -dev releases functional. If the only thing this issue was concerned with was making sure installing/updating -dev works, I wouldn't call this critical.

hass’s picture

A) Yeah, we can also filter for dev in the filename. Shouldn't be a big trick. The patch is only a quick hack.

B) Yes, as said I also do not see this patch as a solution as the disks are filled with garbage and we need to free up the used disk space. I'm only asking me where we should do this? system_cron()? What rules apply? Simply kill all files older than 6 hours (DRUPAL_MAXIMUM_TEMP_FILE_AGE) or any other constant?

I know we also have 100% the same garbage collection issue with JS and CSS caches in #721400: Order JS files according to weight, don't change filenames for aggregated JS/CSS, #174. So we need to have a better solution.

hass’s picture

Aside, I guess we should also check the file size or MD5 hash. Sometimes things are rotten and than a file is only downloaded half or with 0kb. In this case a file exists is TRUE, but the TAR could be corrupt. Not sure how we can grab the MD5 generated with every release from d.o... I only say this as I do expect that such issues may also occur (e.g. if d.o goes down while downloading) at least in theory... You never know, anything can happen :-)

dww’s picture

@hass: Adding MD5 verification is outside the scope of this issue, but we should discuss that at #974676: Verify MD5 hashes when downloading release tarballs for the Update manager. Instead of verifying the MD5 during garbage collection, we should check it as soon as we download it.

rschwab’s picture

So what are we looking for here? Deleting files for failed installs? Clearing out the cache_update table? Lets articulate a plan.

dww’s picture

This issue is *only* about the filesystem. No one needs to touch the {cache_update} table in the database (and if they do, they'll probably break something).

Furthermore, we can't just remove files on failed installs, since that would leave the tarballs around for successful installs, too. Maybe we should just remove any file in this cache directory older than 6 hours inside update_cron(). And, it wouldn't hurt to keep something like the code from #14 as a defensive measure just in case hook_cron() isn't working for some reason.

dww’s picture

Status: Needs review » Needs work

We can return to #14 if nothing else materializes, but at least for now, this issue needs more work to flesh out and implement #20.

Welsby’s picture

Is it not worth doing a CRC check rather than just redownloading based on duration passed?

dww’s picture

@welsby: The problem is that we never remove any of the files we download. In the normal case, once we download something and install it, there'd never be a reason to try to re-install it. The Update manager doesn't let you remove code from the filesystem, only install new stuff or upgrade it. So, the only time you'd ever hit the "redownload" case is the edge case of a failure during installation. And, it's not worth bloating the filesystem forever just in case there was a chance you might need to re-install something you already downloaded once.

hass’s picture

Assigned: Unassigned »
hass’s picture

Status: Needs work » Needs review
FileSize
2.51 KB

Final patch that removes all stale files from 'temporary://update-cache' and 'temporary://update-extraction'.

hass’s picture

@dww: Aside of this case, could you take a look to http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_de..., please? It uses filemtime() and I guess this is a bug if I understood your filectime() comments correctly...

mermentau’s picture

I tested the patch with an attempt to update the dev version of the Views module. It threw this message:

Downloading updates failed:

* Archivers can only operate on local files: temporary://update-cache/views-7.x-3.x-dev.tar.gz not supported

I checked the /update-cache/ directory and the old tar of Views was deleted. I ran the update again and everything worked with no warning.

In the end /update-cache/ had no tar file for Views. The /update-extraction/ directory in the end was left with the updated files for the Views module. Going to Administration>Reports>Available Updates shows that Views is the latest Dev version now.

hass’s picture

The download/archiving failure sounds not related to this patch. Also see my patch in #977408: Subdirectory in "update-extraction" is not deleted before extraction of other versions, please.

hass’s picture

After some more thinking about the patch I think we need to exclude DEV versions from the 6 hours check. Otherwise people may download at 2am and will not receive the version build from 1am.

@dww: Any chance that you find some time for a review? :-)

dww’s picture

Status: Needs review » Needs work

Not sure about the -dev exclusion yet, I'd need to think about that.

No time for a careful review, but one quick concern:

+ * Clear the temporary files and directories based on file age from disk.
+ */
+function update_clear_update_cache() {

This function has a very confusing name. There's already a {cache_update} DB cache table. I'd prefer if this function's name referenced the fact we're talking about the filesystem or disk, not the database. Something like:

function update_clear_update_disk_cache() {

(That's not great, but it's better.)

I also have my recurring fear about functions that call file_unmanaged_delete_recursive($path) without any attempts to ensure we're not about to do something incredibly destructive or malicious, although I'm not sure exactly what to suggest.

hass’s picture

Status: Needs work » Needs review
FileSize
2.82 KB

New patch with function name change to update_clear_update_disk_cache() and DEV versions are deleted after 5 minutes what seems to be an appropiate timeframe to me.

I do not really understand your comment about file_unmanaged_delete_recursive($path) as we know what files and directory structures are inside temporary://update-cache and temporary://update-extraction. For me the function works like a charm.

hass’s picture

I hate this buggy test exclusion... D7 == 7.x-dev and there is nothing in the description that D7's are ignored like D5 and D6 if the case is set to 7.x-dev.

cosmicdreams’s picture

Is the only thing holding this patch back a test?

dww’s picture

Yes. This needs testing on a variety of platforms and configurations, since the actual operation of the update manager can't be tested via simpletest alone.

hass’s picture

We should not hold back this patch any longer - it can only become better than today! I don't see a reason why this should fail on a platform as it's only using existing core functions. If there is a problem other core code must have a bug... :-)

rschwab’s picture

5 minutes passes and my update-cache directory was cleared out of -dev tarballs as expected, but not update-extraction. Maybe this will happen in 6 hours? I'll let you know when its tomorrow (and too late, doh!).

This setup:
MAMP server 1.7.2
php 5.2.6
mysql: 5.0.41

cosmicdreams’s picture

I can try to give this ago tonight as well.

carlos8f’s picture

subscribing.

hass’s picture

@rschwab: yes, update-extraction subdir is only killed after 6 hours and on install with patch #977408: Subdirectory in "update-extraction" is not deleted before extraction of other versions.

carlos8f’s picture

subscribing

rschwab’s picture

in that case +1 for #32. It worked like a charm on my system. I'm not going to RTBC this though - we need more people verifying this works as intended.

threewestwinds’s picture

I was trying to test this yesterday, but ran into http://drupal.org/node/935036 and put it to rest for the night. I'm trying again right now. I'll be back here in bit with a report, and hopefully RTBC.

bendiy’s picture

I can verify that #32 is working for me.

  • xampp-1.7.4-beta2
  • PHP 5.3.3
  • PostgreSQL 8.2

Steps used to test:

  1. Checked out HEAD from cvs.
  2. Setup new database on PostgreSQL 8.3
  3. Installed Drupal.
  4. Check for updates.
  5. Installed Token Alpha-2 via upload form.
  6. The file token-7.x-1.0-alpha2.tar.gz is in /tmp.
  7. The "update-extraction" folder is made and token folder is extracted inside it.
  8. Checked for updates.
  9. Updated Token to Alpha-3 through update manager. Ran update.php etc...
  10. Token version in "update-extraction" folder is replaced with the new one.
  11. The "update-cache" folder is created with token-7.x-1.0-alpha3.tar.gz file inside it.
  12. Ran Cron, everything stayed in "update-cache" folder.
  13. Cleared cache, everything stayed in "update-cache" folder.
  14. Change system time forward 6.5 hours.
  15. Copied new file, "Copy of token-7.x-1.0-alpha2.tar.gz", into "update-cache" folder so it's created time is now().
  16. Ran Cron, everything stayed in "update-cache" folder.
  17. Cleared cache, everything stayed in "update-cache" folder.
  18. Applied patch #32.
  19. Ran Cron, everything was removed from "update-cache" folder except my newly created file, "Copy of token-7.x-1.0-alpha2.tar.gz", that was only minutes old. The "update-extraction" folder is empty.
idflood’s picture

patch #32 works fine for me ( I only checked the 5 minutes update-cache by installing new modules from url ).
Tested on:
mamp 1.7.1 ( Apache/2.0.59 (Unix) PHP/5.2.5 DAV/2 MYSQL/5.0.41-log )
ovh ( Apache/2.2.X (Unix) PHP/5.2.14 MYSQL/5.0.90-log )

cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

sounds like a RTBC to me.

carlos8f’s picture

Status: Reviewed & tested by the community » Needs review

This is missing the "R" in RTBC :) Let's make sure the code is sound, too.

dww’s picture

Status: Needs review » Needs work

Only problem I see in the patch is this:

+ * Delete stale files and directories older than 6 hours and development
+ * snapshots older than 5 minutes from disk.

The PHPDoc summary for the function needs to be only one line.

I'll quickly re-roll for this, then it's RTBC.

dww’s picture

Status: Needs work » Needs review
FileSize
3.13 KB

Well, while I was at it, I fixed a few other things:

A) The PHPDoc summary I mentioned in #47.

B) Improved and reorganized the rest of the comments for update_delete_file_if_stale().

C) update_delete_file_if_stale() used to always try to compute the ctime() on a file, even if it didn't exist. Fixed the logic to test for the file existing first.

D) Moved these two new functions outside of the update_status_cache defgroup, since they're not related to the database cache functions that defgroup is documenting.

Someone else should probably RTBC, since this is a bit more extensive of a change than a trivial PHPDoc summary fix.

aspilicious’s picture

Functions should start with 3th person verb...
But I won't start a fight a day before RC ;)

dww’s picture

@aspilicious: No fight needed. Both of the functions added with this patch begin (after the module namespace, of course) with a 3rd person verb: "clear" and "delete".

webchick’s picture

This is the last remaining critical bug in the queue. I'd like to commit it, and then put a "code freeze" on HEAD for a few hours. I'd also like to eat supper. How close do you feel we are? Looks like many people tested it and confirmed it working.

carlos8f’s picture

I'd like clarification on this: the regex checking for dev snapshots matches .tar.gz/.zip files, but would it also be good to match *-dev directories as well, so file_unmanaged_delete_recursive() can clean up the unpacked dev snapshot, too?

carlos8f’s picture

Or maybe there is no "-dev" on the directory to check for...

from update_manager_install_form_submit()

  // Unfortunately, we can only use the directory name for this. :(
  $project = drupal_substr($files[0], 0, -1);
  $directory = _update_manager_extract_directory();
  $project_location = $directory . '/' . $project;

Does that mean it's impossible to separately stage multiple versions of a module within the garbage collection period? If you've used update manager to install foo-2.0 and then you go to foo-2.x-dev, the folder "foo" is used to stage both versions, without clearing anything in between?

dww’s picture

Re:

Does that mean it's impossible to separately stage multiple versions of a module within the garbage collection period? If you've used update manager to install foo-2.0 and then you go to foo-2.x-dev, the folder "foo" is used to stage both versions, without clearing anything in between?

That particular case doesn't work at all. See #977414: Allow updating to a -dev or new major branch with Update manager

However, I think the patch you care about would be this: #977408: Subdirectory in "update-extraction" is not deleted before extraction of other versions

carlos8f’s picture

Assigned: » carlos8f

dww explained the situation in IRC, and now I understand that the shortcomings I mentioned are ongoing issues with this module. Even so, I'm going to investigate this patch tonight and see if there's anything to improve.

webchick’s picture

Priority: Critical » Major

While this is indeed a nasty little critter, it doesn't meet the actual definition of a critical bug; Drupal as a whole is not unusable without this, though if you hit this behaviour it certainly seems buggy. There's also nothing in here that affects strings, so this can still be committed post-RC1. Downgrading to (solidly) "major."

Looks like we're pretty close here! Needs more testing though, methinks.

carlos8f’s picture

Assigned: carlos8f » Unassigned

Drupal as a whole is not unusable

That sums it up nicely ... not incredibly usable either! ;)

I tried to review this but got frustrated by the fact that none of my dev environments even support Update manager (SSH with pubkey access only, no FTP daemon, webserver doesn't own any module files). And seeing that this is no longer blocking an RC, It's a bit lower on my priorities so I'm unassigning for now.

Tor Arne Thune’s picture

I would be happy to test the patch, as I have a local FTP server running. Unfortunately, #988898: Can never use my local FTP server or SSH login. stops me from testing it.

Tor Arne Thune’s picture

FileSize
4.69 KB
4.87 KB
13.27 KB
73.15 KB

Tested #48 and it is working for me. I used bendiy's test notes as a base and ran through them with the new patch.

Xubuntu 10.10
Apache 2.2.16
PHP 5.3.3
MySQL 5.1.49

Steps used to test:

  1. Checked out HEAD from CVS.
  2. Set up new database on MySQL 5.1.49.
  3. Installed Drupal.
  4. Checked for available updates.
  5. Applied patch in #48 successfully.
  6. Installed Devel module beta-2 by using Install from a URL.
    The "update-cache" folder was created in /tmp and the file "devel-7.x-1.0-beta2.tar.gz" was contained within (see Screengrab 1.)
    The "update-extraction" folder was created in /tmp and "devel" folder was extracted inside it (see Screengrab 1.)
  7. Enabled Devel module.
  8. Checked for updates.
  9. Updated Devel from beta-2 to rc-1 through update manager.
    The file "devel-7.x-1.0-rc1.tar.gz" was in the "update-cache" folder. (see Screengrab 2.)
    Devel folder in "update-extraction" folder was replaced with the new one (see Screengrab 2.)
  10. Ran Cron, everything stayed in "update-cache" folder.
  11. Cleared cache, everything stayed in "update-cache" folder.
  12. Changed system time forward to a time (18:18) 6h1m after the creation time of "devel-7.x-1.0-beta2.tar.gz", but before the creation time of "devel-7.x-1.0-rc1.tar.gz".
  13. Ran Cron.
    Folder "update-cache" contained only "devel-7.x-1.0-rc1.tar.gz" (see Screengrab 3.)
    The "update-extraction" folder contained the "devel" folder.
  14. Changed system time forward to 1 minute after the creation time of "devel-7.x-1.0-rc1.tar.gz" and "devel" folder (18:33).
  15. Ran Cron.
    The "update-cache" folder was empty (see Screengrab 4.)
    The "update-extraction" folder was empty (see Screengrab 4.)
bojanz’s picture

1V, you are officially awesome.

Read through the patch and noticed this:

+ * (modification time) since many (all?) tar implementations will go out of
+ * their way to set the mtime on the files it creates to the timestamps
+ * recorded in the tarball.

That's not really grammatically correct (tar implementations -> it creates, and the "will" is not needed), let's change it to:

+ * (modification time) since many (all?) tar implementations go out of
+ * their way to set the mtime on the files they create to the timestamps
+ * recorded in the tarball.

Might not be a bad idea to replace "files" with "tarballs" or "archives", makes it more clear.

Thoughts?

dww’s picture

Normally I'm glad core patches get such close gramatical scrutiny for code comments. But this is a major/critical bug that might not get into 7.0 at this point, and if I reroll the patch for such tiny details, the maintainers might want it to all be tested again, etc. Plus, no, this is incorrect:

Might not be a bad idea to replace "files" with "tarballs" or "archives", makes it more clear.

You misunderstand the comment. When you ask tar to untar an archive file, it goes out of its way to set the modification times on the files it creates for you on your filesystem to match the modification times on those files that were true when the archive/tarball was first created. We don't care about modification times on tarball files themselves. We care about the modification times on the files that are "in" the tarball once they show up on your disk.

Anyway, here's a trivial re-roll of #48 (and interdiff) to fix s/it/they/. I didn't try to re-word the rest of it since I still think it's pretty clear.

bojanz’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
3.12 KB

Okay, thanks.

I tend to reroll myself when nitpicking about comments, didn't do so here because I wasn't sure about the proposed changes. Sorry!
I now see "they creates", changed to "they create", and setting status to RTBC.
I think RTBC is justified, we've seen many iterations and tests of the patch, including the latest in #59.

geerlingguy’s picture

@bojanz - is that another re-roll of dww's patch above, or is it simply a re-upload?

bojanz’s picture

As I said, I changed "they creates" to "they create" in one of the comments. So it's *almost* a reupload :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

I'd really like to see tests for this, but as I understand it that's not possible with update manager and all the various testbot environments. Thanks to bendly and 1V for doing an awesome walkthrough of everything they tried. :)

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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

Alan D.’s picture

Version: 7.x-dev » 8.x-dev
Priority: Major » Normal
Status: Closed (fixed) » Needs review
FileSize
1.35 KB

I was just trying to use these functions for the first time and noticed a logical error:

If the file does not exist locally, the system will download it, otherwise it will use the existing file. There is nothing returned by update_delete_file_if_stale(). So there is potential for a stale file to be deleted and update_manager_file_get() returning the local non-existent path.

The patch add a return value for update_delete_file_if_stale().

For D7, to avoid an API change, just move the function call before the if statement.

Note: unlink() should flush the internal PHP cache, so file_exists() shouldn't be using cached values and clearstatcache() shouldn't be needed, unless there is a core PHP bug hanging around.

function update_manager_file_get($url) {
  ....
  update_delete_file_if_stale($local);
  if (!file_exists($local)) {
    return system_retrieve_file($url, $local, FALSE, FILE_EXISTS_REPLACE);
  }
  else {
    return $local;
  }
}

I'm guessing that either the internal poor-man-cron runs enough to avoid this use case or that other internal error checking is preventing this from creating issues.

Leeteq’s picture

(Coming from #613230: Allow upgrading -dev with update manager, comment #9.)

For the sake of perspective:
Is D7 still a long way away from being able to update modules form older -dev versions to the newest -dev version of contrib. modules and themes using D7 core's Update Manager?

AFAICT, this is still not possible in the latest D7 release(s), or is there a way to make this possible that I am not aware of? (this question is about the GUI/Update Manager, not about Drush)

[FYI: User name changed from DanielTheViking, Feb. 2012.]

kscheirer’s picture

kscheirer’s picture

Retesting against latest HEAD since it has been over a year.

Status: Needs review » Needs work
Issue tags: -Update manager

The last submitted patch, allow-use-of-cached-non-stale-file-605318-66.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 72e8023 on 8.3.x
    #605318 by hass, dww, bendly, 1V: Add garbage collection to the update...

  • webchick committed 72e8023 on 8.3.x
    #605318 by hass, dww, bendly, 1V: Add garbage collection to the update...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 72e8023 on 8.4.x
    #605318 by hass, dww, bendly, 1V: Add garbage collection to the update...

  • webchick committed 72e8023 on 8.4.x
    #605318 by hass, dww, bendly, 1V: Add garbage collection to the update...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

  • webchick committed 72e8023 on 9.1.x
    #605318 by hass, dww, bendly, 1V: Add garbage collection to the update...
dww’s picture

Issue summary: View changes
Status: Needs work » Fixed

Re: #67: In the intervening years since your patch, this bug was independently rediscovered at #2303323: update_delete_file_if_stale() must return a value or update_manager_file_get() will always fetch remote files. The latest patch over there includes test coverage. This issue has been open way too long as it is, so let's re-close this and focus on #2303323 instead.

Thanks,
-Derek

dww’s picture

Status: Fixed » Closed (fixed)