Problem/Motivation
Failing tests such as https://git.drupalcode.org/project/drupal/-/jobs/1622602
I looked into this a bit. It started with #3445211: Composer tests fail because of invalid Drupal version Where the version was changed from 11.0.0-dev to 11.0-dev
With the drupal version change in that issue, the test downloads the translation file as drupal-11.0-dev.xx-lolspeak.po instead of drupal-11.0.x.xx-lolspeak.po
I don’t know what’s supposed to happen here with this version, but this line https://git.drupalcode.org/project/drupal/-/blob/11.x/core/includes/inst... would normally change 11.0.0-dev to 11.0.x but it can’t match 11.0-dev, so it’s just leaving it as 11.0-dev. the translation server seems to have files for both 11.x and 11.0-dev so regardless, it gets a file.
But the file translator here: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co... won’t match either of those two translation file patterns. So the installer sees the language request, sees it doesn’t have a translation file, orders it downloaded, the download succeds, installer_download_translation reloads the page (purportedly in the new language), and on reload, the installer sees a language request, but doesn’t see the file that had been downloaded, and starts the process over again until something times out.
Good news is this should only affect the 11.x branch, and not the alpha or beta releases.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3448036
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mikelutzI’m not sure where it should be fixed, (more specifically I’m not sure what translation file name format we WANT to use for 11.0-dev, but once we know, we need to adjust either of those two regexes above to download and/or find the right file)
Comment #4
mikelutzHere's the simplest fix I see for this test, making the patch value optional in the regex that filetranslation uses to detect files.
Also, I was trying to figure out why the version change was committed 9 days ago, and we are just seeing this test fail now. The drupal-11.0-dev.xx-lolspeak.po file that is downloaded has metadata indicating it's the file for 11.0.0-beta1. I suspect that file was added to the server with the beta release late last week. The test only tests that a 200 status is returned after selecting a language. If the download fails, the installer displays a requirements page stating that the file is missing rather than trying to reload the page, so the test still passes. So it wouldn't have been a problem until the download succeeded, but the installer couldn't detect the downloaded file.
Comment #5
mikelutzComment #6
mikelutzI'm wondering if we should do something more robust here, like add a query parameter on the redirect indicating that we downloaded the translation file, so when we go back through we see that we thought we downloaded it, but can't find it and report the error instead of an endless redirect. Or a more structured way of ensuring that whatever is set as the drupal version will pass the FileTranslation scanner. I'm really not sure, It's been a while since I got deep in the installer code, and I'm not that familiar with the deep parts of the string translation system and po files to know what else could be screwed up by changing the FileTranslation scanner. I suspect the change here is probably safe, but still don't know if it's the right thing to do.
Comment #7
mikelutzMoving to critical per https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-...
Comment #8
catchI think we should open a follow-up for #6 to make things more robust, but also let's go ahead here to un-break HEAD again. All of this will change again once we can use a
mainbranch instead of 11.x.Comment #11
larowlanThanks, committed to 11.x
Comment #12
quietone commentedTagging per #8
Comment #14
mikelutzOpened #3506133: Harden installer translation file download against potential infinite redirects as a follow-up