Problem/Motivation

In #2801777-63: Give users the option to prevent drupal from automatically marking unused files as temporary + #2801777-66: Give users the option to prevent drupal from automatically marking unused files as temporary , it was pointed out that the temporary_maximum_age setting in system.file really belongs in file.settings. It's something that's relevant only for the File module, not the System module.

One small complication is that the Update module also uses this.

Proposed resolution

  1. Make Update module use a hardcoded max age (which then also matches their documentation).
  2. Move the setting from system.file to file.settings.
  3. Provide update path.

Remaining tasks

TBD.

User interface changes

TBD.

API changes

TBD.

Data model changes

TBD.

CommentFileSizeAuthor
#3 2894194-3.patch2.79 KBwim leers
#3 interdiff.txt2.09 KBwim leers
#2 2894194-2.patch747 byteswim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2894194-2.patch747 bytes

Implements step 1 in the proposed resolution.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
StatusFileSize
new interdiff.txt2.09 KB
new 2894194-3.patch2.79 KB

Implements step 2 in the proposed resolution.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: file system » file.module
Issue tags: +Needs update path, +Needs update path tests, +Needs change record

Fixing issue metadata.

Would like a review of this before doing the boatload of work that is required for step 3.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented 
+++ b/core/modules/update/update.module
@@ -766,7 +766,7 @@ function update_clear_update_disk_cache() {
-    $max_age = \Drupal::config('system.file')->get('temporary_maximum_age');
+    $max_age = 6 * 3600;

Note that the docblock for this function is very explicit about this:

/**
 * Deletes stale files and directories from the update manager disk cache.
 *
 * Files and directories older than 6 hours and development snapshots older than
 * 5 minutes are considered stale. We only cache development snapshots for 5
 * minutes since otherwise updated snapshots might not be downloaded as
 * expected.
 *
 * When checking file ages, we need to use the ctime, not the mtime
 * (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. We want to see the last time the file was changed on disk, which is
 * left alone by tar and correctly set to the time the archive file was
 * unpacked.
 *
 * @param $path
 *   A string containing a file path or (streamwrapper) URI.
 */
function update_delete_file_if_stale($path) {

This is why I think it's fine to hardcode it now.

dawehner’s picture

dawehner
Primary language German
commented

Just a theoretical question: Are we actually able to move config keys, given that random custom code might rely on its existence?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I'd say we're able to do that, because config is owned by a particular module, and config is part of the module's internal design. The module always must provide an upgrade path of course.

However, in this case… this config is owned by the system module. Which means it's a fairly special case, because every single Drupal module is safe to assume that the system module is installed, therefore can also use the system module's configuration.

So… I don't know.

However: if we can't do this, then http://buytaert.net/making-drupal-upgrades-easy-forever is already proving to not be true.

The last submitted patch, 2: 2894194-2.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 3: 2894194-3.patch, failed testing. View results

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review

I guess for moving config around, we could for example provide some system in the config system, which throws a deprecation warning when accessing the old value, and then point to the new value.

However: if we can't do this, then http://buytaert.net/making-drupal-upgrades-easy-forever is already proving to not be true.

Are you really surprised by that?

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes
Status: Needs review » Needs work

Found this via a Google search after reading #2492171-183: Provide options to sanitize filenames (transliterate, lowercase, replace whitespace, etc) from @alexpott.

I'm really confused. ;) system.module provides the settings form for these settings. file.module provides no settings form (that I can find).

What does it mean to move config from system for file for these cases?

How does anyone set values for these things once they move to file.settings? The "party line" is that there's no longer a UI and you must set it manually in your site's config? @alexpott suggested a form_alter() for this. Yuck. ;) Seems like trading one DrupalWTF for another.

Why are *any* of the things from core/modules/system/src/Form/FileSystemForm.php the responsibility of system.module? Isn't all of that really file.module's responsibility?

If we're going to move this one setting, why not move everything? And then keep the settings form (but change who registers it).

I don't necessarily want to untangle this whole mess in #2492171 itself, and I'd love to punt "those settings are really file.module's responsibility" to a more appropriate follow-up. Is this the right issue? ;) Is there a better place? Should we re-scope this issue to 'Move FileSystemForm from system.module to file.module"? Should I open that as a separate thing and mark this postponed on it?

Help? ;)

Thanks!
-Derek

p.s. Updated the summary. A lot of 'None' answers were lies. ;) Set them to 'TBD' for now...

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.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.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.