Problem/Motivation

In #3016252: Handle Major API changes for d8/d9 for localize.drupal.org urls/files we updated the localize.drupal.org setup to respond to /all/ in the URL of 8.x+ translations. That helps us move towards removing core compatibility from the requested URLs, especially as they would be inaccurate with Drupal 9 and not applicable to semantic versioned contributed modules anymore.

Proposed resolution

Remove the /8.x/ core compatibility component from the URL and include the server supported /all/ compatibility.

Remaining tasks

Commit.

User interface changes

None.

API changes

None.

Data model changes

None. However a project data cache clear was needed on update to clear off the old version info.

Release notes snippet

N/A

CommentFileSizeAuthor
#21 3016471-21.patch6.1 KBtedbow
#21 interdiff-19-21.txt455 bytestedbow
#19 3016471.patch5.66 KBdrumm
#17 3016471.patch3.67 KBdrumm
#11 3016471-11.patch3.72 KBGábor Hojtsy
#8 3016471-8.patch3.75 KBGábor Hojtsy
#8 interdiff.txt514 bytesGábor Hojtsy
#7 3016471-7.patch3.25 KBGábor Hojtsy
#7 interdiff.txt860 bytesGábor Hojtsy
#5 interdiff.txt1.51 KBGábor Hojtsy
#5 3016471-5.patch2.41 KBGábor Hojtsy
#2 3016471-2.patch928 bytesGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy created an issue. See original summary.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Active » Needs review
FileSize
3016471-2.patch928 bytes
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Related issues: +#3016252: Handle Major API changes for d8/d9 for localize.drupal.org urls/files

Status: Needs review » Needs work

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

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Needs review
FileSize
3016471-5.patch2.41 KB
interdiff.txt1.51 KB

Fix local server test data.

Status: Needs review » Needs work

The last submitted patch, 5: 3016471-5.patch, failed testing. View results

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Needs review
FileSize
interdiff.txt860 bytes
3016471-7.patch3.25 KB

One more test place I forgot to make the change apparently.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
FileSize
interdiff.txt514 bytes
3016471-8.patch3.75 KB

Change the one place left in the installer. I don't believe this was deemed testable.

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.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented 
+++ b/core/modules/locale/locale.compare.inc
@@ -69,7 +69,8 @@ function locale_translation_build_projects() {
+      // @todo this is a quick test

Is this still relevant?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
FileSize
3016471-11.patch3.72 KB

No, that was from my first patch. Then the tests got updated. Regardless of this passing tests, all the d.o side need to be ready to deal with this, so this cannot be committed without d.o supporting it fully. I believe @mixologic added a symlink so that /all/ works but @drumm said more changes are needed on the caching layer, etc. That would be in #3016252: Handle Major API changes for d8/d9 for localize.drupal.org urls/files.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Make localization file download major version agnostic » [PP-1] Make localization file download major version agnostic
Status: Needs review » Postponed

Ah, thanks!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Priority: Normal » Critical
Issue tags: +Drupal 9
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

#13 triggered me to do #3016252-9: Handle Major API changes for d8/d9 for localize.drupal.org urls/files.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented
Status: Postponed » Reviewed & tested by the community

#3016252: Handle Major API changes for d8/d9 for localize.drupal.org urls/files is fixed, and this looks good.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs re-roll

Needs a re-roll.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented
Status: Needs work » Needs review
FileSize
3016471.patch3.67 KB

In core/modules/locale/locale.compare.inc, the ternary operation was removed from 'core' => isset($data['info']['core']) ? $data['info']['core'] : \Drupal::CORE_COMPATIBILITY,, so this patch is more simple now.

Status: Needs review » Needs work

The last submitted patch, 17: 3016471.patch, failed testing. View results

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented
Status: Needs work » Needs review
FileSize
3016471.patch5.66 KB

There is a new test in core/modules/locale/tests/src/Kernel/LocaleBuildTest.php that also needs updating.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs review » Needs work

The issue summary has

None. However a project data cache clear will be needed on update to clear off the old version info.

Seems like we still need an update hook to clear this cache.

Otherwise it looks good

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Title: [PP-1] Make localization file download major version agnostic » Make localization file download major version agnostic
Status: Needs work » Needs review
FileSize
interdiff-19-21.txt455 bytes
3016471-21.patch6.1 KB

Adding update hook to clear Locale project storage

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.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -Needs re-roll

Discussed this today with @drumm and @mixologic. This only changes the /8.x/ component in URLs to /all/. Sent for a retest for now. This would still need to be fixed in Drupal 8 and 9 as well.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Reviewed & tested by the community

Patch still applies and tests still prove this works in the test suite. Since this is about an external service where results also even depend on the version of core you are running with I manually tested with a 8.x dev to see if the fallback worked, and it did. I also tried then with 8.8 alpha1. Installation in a foreign language and adding a new foreign language later or fresh on an English site all worked. While there is no way to test this with Drupal 9 yet, we agreed with @drumm and @mixologic that putting up hacks on the infrastructure side to test this with 9.x-dev for the (hopefully) short time remaining before Drupal 9 alpha1 could cause more problems that it solves.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue summary: View changes
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

The patch is just making simple URL changes, and after reading up on the other issue, I'm satisfied with the testing in #24. Committing this now will tell us soon enough if it breaks for 9.x and more work is needed.

Committed and pushed to 9.0.x; 8.9.x; 8.8.x. Thanks!

  • webchick committed 201ea5b on 8.8.x 
    Issue #3016471 by Gábor Hojtsy, drumm, tedbow: Make localization file...

  • webchick committed dce54d0 on 8.9.x 
    Issue #3016471 by Gábor Hojtsy, drumm, tedbow: Make localization file...

  • webchick committed d3d7180 on 9.0.x 
    Issue #3016471 by Gábor Hojtsy, drumm, tedbow: Make localization file...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Version: 8.9.x-dev » 8.8.x-dev

Remember to set the branch selector to the lowerst branch that receives the backport.

Status: Fixed » Closed (fixed)

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