Problem/Motivation

The new route match is a concept which is not bound to drupal itself. On the other hand it is a great tool to decouple your
code from needing the request in all kind of places.

On the other hand this makes code less compatible with other PHP folks code out there.

Proposed resolution

Move the Route match into the \code>\Drupal\component\Routingnamespace, so one day it can be shared using composer.

Remaining tasks

User interface changes

API changes

Original report by @effulgentsia

From #2294157-7: Switch getOptions() and getRouteParameters() within LocalActionInterface and LocalTaskInterface to use RouteMatch:

Did we actually ever considered to move the routeMatch interface into Drupal/Component? This could be really useful for other code as well and allows people to write generic libraries without pulling in all of drupal.

CommentFileSizeAuthor
#67 interdiff-2296205-61-67.txt585 bytesmartin107
#67 2296205-67.patch241.51 KBmartin107
#61 interdiff-60-61.txt2.16 KBmartin107
#61 2296205-61.patch241.45 KBmartin107
#60 interdiff-2296205-59-60.txt1010 bytesmartin107
#60 2296205-60.patch240.13 KBmartin107
#59 interdiff-2296205-57-59.txt1.84 KBmartin107
#59 2296205-59.patch240.13 KBmartin107
#57 interdiff-2296205-56-67.txt1.62 KBmartin107
#57 2296205-57.patch238.29 KBmartin107
#56 interdiff-2296205-55-56.txt704 bytesmartin107
#56 2296205-56.patch236.68 KBmartin107
#55 2296205-55.patch236.33 KBshaktik
#53 2296205-53.patch246.62 KBmartin107
#52 interdiff-2296205-50-52.txt1.72 KBmartin107
#52 2296205-52.patch12.9 MBmartin107
#50 2296205-49.patch245.35 KBmartin107
#45 intediff-2296205-44-45.txt1.38 KBmartin107
#45 2296205-45.patch245.69 KBmartin107
#44 interdiff-2296205-43-44.txt2.88 KBmartin107
#44 2296205-44.patch245.39 KBmartin107
#43 interdiff-42-43-2296205.txt15.26 KBmartin107
#43 2296205-43.patch243.49 KBmartin107
#42 interdiff-40-41.txt25.24 KBmartin107
#42 2296205-41.patch233.39 KBmartin107
#40 2296205-40.patch214.29 KBmartin107
#40 interdiff-2296205-38-40.txt5.6 KBmartin107
#38 interdiff-2296205-37-38.txt15.72 KBmartin107
#38 2296205-38.patch221.71 KBmartin107
#37 routing-2296205-37.patch211.65 KBmartin107
#11 routing-2296205-11.patch79.58 KBmartin107
#11 interdiff-9-11.txt436 bytesmartin107
#9 routing-2296205-8.patch79.64 KBmartin107
#9 interdiff-6-8.txt4.57 KBmartin107
#6 routing-2296205-6.patch75.38 KBmartin107
#3 routing-2296205-3.patch73.79 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Assigned: Unassigned » dawehner

Thanks for creating the issue!

dawehner’s picture

Here is a list of the available bits, just in Core/Routing:

  • AcceptHeaderMatcher.php: Useful for all kind of symfony based routing, no dependency
  • AdminContext.php: No dependency but not useful outside drupal
  • CompiledRoute.php: Coupled to the Drupal routing implementation
  • ContentTypeHeaderMatcher.php: Not really useful
  • CurrentRouteMatch.php: Really useful together with the other route match files.
  • GeneratorNotInitializedException.php: Not useful
  • LazyLoadingRouteCollection.php: There is an issue in symfony CMF routing to include it
  • MatcherDumper.php: Drupal specific implementation
  • MatcherDumperInterface.php: Drupal specific implementation
  • MatchingRouteNotFoundException.php: Drupal specific implementation
  • NullGenerator.php: Just used in the installer
  • NullMatcherDumper.php: just used in the installer
  • NullRouteMatch.php: Just used in the installer
  • RequestHelper.php: There is an symfony issue to include a faster duplicate with different path method
  • RouteBuilder.php: Drupal specific implementation
  • RouteBuilderInterface.php: Drupal specific implementation
  • RouteBuilderStatic.php: Drupal specific implementation
  • RouteBuildEvent.php: Drupal specific implementation
  • RouteCompiler.php: Drupal specific implementation
  • RouteMatch.php: useful
  • RouteMatchInterface.php: useful
  • RoutePreloader.php: Drupal specific
  • RouteProvider.php: Drupal specific
  • RouteProviderInterface.php: Drupal specific, maybe worth to port to symfonfy CMF
  • RouteSubscriberBase.php: Drupal specific
  • RoutingEvents.php: Drupal specific
  • UrlGenerator.php: Drupal specific
  • UrlGeneratorInterface.php: Drupal specific
  • UrlMatcher.php: Drupal specific

TL;DR: The route match is quite useful, other ones not really.

dawehner’s picture

FileSize
73.79 KB

Drupal 8 installed with this.

dawehner’s picture

Assigned: dawehner » Unassigned
Status: Active » Needs review

Try to learn things again.

herom’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
martin107’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
75.38 KB

This issue seems like a good idea, so :-

Reroll.. had to update \Drupal\shortcut\Form\SwitchShortcutSet to get it to install, I think testbot will expose others which need updating.

Status: Needs review » Needs work

The last submitted patch, 6: routing-2296205-6.patch, failed testing.

dawehner’s picture

Issue summary: View changes
martin107’s picture

Status: Needs work » Needs review
FileSize
4.57 KB
79.64 KB

This should fix things

dawehner’s picture

thank you so much for rerolling this patch!

martin107’s picture

FileSize
436 bytes
79.58 KB

Removed unused use statement.

Crell’s picture

While I'm generally in favor of more Drupal-agnostic code, this is all code that depends on Symfony's Routing component either directly or indirectly. The rules for Drupal\Component are currently *no* dependencies outside of Drupal\Component, and preferably not even then. So this wouldn't be moveable right now unless we change the policy for Drupal\Component.

dawehner’s picture

While I'm generally in favor of more Drupal-agnostic code, this is all code that depends on Symfony's Routing component either directly or indirectly. The rules for Drupal\Component are currently *no* dependencies outside of Drupal\Component, and preferably not even then. So this wouldn't be moveable right now unless we change the policy for Drupal\Component.

I am sorry but this is just wrong in reality:

  • \Drupal\Component\Annotation\Plugin\Discovery\AnnotatedClassDiscovery relies on Doctrine\Common\Annotations\SimpleAnnotationReader
  • \Drupal\Component\Bridge\ZfExtensionManagerSfContainer relies on symfony and Zend, obviously
  • \Drupal\Component\Discovery\YamlDiscovery does not actually work
  • \Drupal\Component\Serialization\Yaml relies on Symfony\compotent\yaml
Crell’s picture

Well, then people haven't been reading their README files:

http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Component/README.txt

sun’s picture

Those examples are perfectly in line with that README.txt. If the README text was supposed to be interpreted differently, then it (1) should have used unambiguous wording and (2) would have to include a rationale for why such an extreme stance of not depending on anything else at all is a good idea in any way.

My interpretation of that README.txt is as simple as this: Drupal\Component components MUST NOT depend on Drupal\Core components or any other Drupal code. But aside from that, anything that can be specified in a hypothetical composer.json file of the component is a legit, appropriate, and possibly even desired dependency.

dawehner’s picture

Opened a different issue as it is independent from this issue: #2303777: Allow drupal components to depend on other components outside Drupal

dawehner’s picture

So given that we just need an update to README.txt can we move this issue forward?

dawehner’s picture

Priority: Normal » Major
Issue tags: +Needs reroll

@crell

There is simply no reason on earth to block that given that also #1972300: Write a more scalable dispatcher introduced a new class in there which does depends on an external component.

znerol’s picture

Status: Needs review » Needs work
Crell’s picture

*sigh*. Fine. Because godforbid we actually follow our own written rules and policies. It's so much easier to just conveniently ignore them rather than changing them explicitly.

effulgentsia’s picture

Why is this Major priority?

While I think this is a nice thing to do, I don't think it brings enough benefit to Drupal 8.0.0 to justify spending more time on it prior to D8's release and breaking other patches in the queue. How about postponing it to 8.1? It'll mean needing to retain the \Core classes as wrappers for BC, but at this point in the 8.0 cycle, we would need to do that for 8.0 as well.

dawehner’s picture

Version: 8.0.x-dev » 8.1.x-dev
Priority: Major » Normal

@effulgentsia
You know sometimes sharing with other people seemed to be a good idea, though I don't remember why I set it to major. Maybe
this was a moment of rage. I do agree that moving to normal and 8.1.x is the right idea.

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

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now 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.

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

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.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.

dawehner’s picture

I think noone cares about this task anymore. Any objections against just closing down this issue?

Ivan Berezhnov’s picture

Issue tags: +CSKyiv18

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.

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.

vsujeetkumar’s picture

Assigned: Unassigned » vsujeetkumar
vsujeetkumar’s picture

Assigned: vsujeetkumar » Unassigned
prabha1997’s picture

I am not able to reroll a patch [#11 patch]

saurabh-2k17’s picture

I am getting this for Patch #11

git apply --check routing-2296205-11.patch

error: patch failed: core/core.services.yml:288
error: core/core.services.yml: patch does not apply
error: patch failed: core/includes/theme.inc:18
error: core/includes/theme.inc: patch does not apply
error: patch failed: core/lib/Drupal/Core/Routing/CurrentRouteMatch.php:2
error: core/lib/Drupal/Core/Routing/CurrentRouteMatch.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Routing/NullRouteMatch.php:2
error: core/lib/Drupal/Core/Routing/NullRouteMatch.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Routing/RouteMatch.php:2
error: core/lib/Drupal/Core/Routing/RouteMatch.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Routing/RouteMatchInterface.php:2
error: core/lib/Drupal/Core/Routing/RouteMatchInterface.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php:29
error: core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php:9
error: core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php: patch does not apply
error: core/lib/Drupal/Core/Cache/ThemeCacheContext.php: No such file or directory
error: patch failed: core/lib/Drupal/Core/EventSubscriber/MaintenanceModeSubscriber.php:11
error: core/lib/Drupal/Core/EventSubscriber/MaintenanceModeSubscriber.php: patch does not apply
error: core/lib/Drupal/Core/EventSubscriber/ThemeNegotiatorRequestSubscriber.php: No such file or directory
error: patch failed: core/lib/Drupal/Core/Site/MaintenanceMode.php:6
error: core/lib/Drupal/Core/Site/MaintenanceMode.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Theme/ThemeNegotiator.php:7
error: core/lib/Drupal/Core/Theme/ThemeNegotiator.php: patch does not apply
error: patch failed: core/modules/action/action.module:5
error: core/modules/action/action.module: patch does not apply
error: patch failed: core/modules/aggregator/aggregator.module:8
error: core/modules/aggregator/aggregator.module: patch does not apply
error: patch failed: core/modules/ban/ban.module:5
error: core/modules/ban/ban.module: patch does not apply
error: patch failed: core/modules/basic_auth/basic_auth.module:5
error: core/modules/basic_auth/basic_auth.module: patch does not apply
error: patch failed: core/modules/block/block.module:6
error: core/modules/block/block.module: patch does not apply
error: core/modules/block/src/EventSubscriber/NodeRouteContext.php: No such file or directory
error: core/modules/block/src/Plugin/DisplayVariant/FullPageVariant.php: No such file or directory
error: core/modules/block/tests/src/Plugin/DisplayVariant/FullPageVariantTest.php: No such file or directory
error: patch failed: core/modules/block_content/block_content.module:5
error: core/modules/block_content/block_content.module: patch does not apply
error: patch failed: core/modules/book/book.module:9
error: core/modules/book/book.module: patch does not apply
error: patch failed: core/modules/book/src/BookBreadcrumbBuilder.php:11
error: core/modules/book/src/BookBreadcrumbBuilder.php: patch does not apply
error: patch failed: core/modules/breakpoint/breakpoint.module:8
error: core/modules/breakpoint/breakpoint.module: patch does not apply
error: patch failed: core/modules/ckeditor/ckeditor.module:5
error: core/modules/ckeditor/ckeditor.module: patch does not apply
error: patch failed: core/modules/color/color.module:8
error: core/modules/color/color.module: patch does not apply
error: patch failed: core/modules/comment/comment.module:18
error: core/modules/comment/comment.module: patch does not apply
error: patch failed: core/modules/comment/src/CommentBreadcrumbBuilder.php:9
error: core/modules/comment/src/CommentBreadcrumbBuilder.php: patch does not apply
error: patch failed: core/modules/config/config.module:5
error: core/modules/config/config.module: patch does not apply
error: patch failed: core/modules/config_translation/config_translation.module:7
error: core/modules/config_translation/config_translation.module: patch does not apply
error: patch failed: core/modules/contact/contact.module:5
error: core/modules/contact/contact.module: patch does not apply
error: patch failed: core/modules/content_translation/content_translation.module:10
error: core/modules/content_translation/content_translation.module: patch does not apply
error: patch failed: core/modules/contextual/contextual.module:7
error: core/modules/contextual/contextual.module: patch does not apply
error: patch failed: core/modules/datetime/datetime.module:7
error: core/modules/datetime/datetime.module: patch does not apply
error: patch failed: core/modules/dblog/dblog.module:11
error: core/modules/dblog/dblog.module: patch does not apply
error: patch failed: core/modules/editor/editor.module:8
error: core/modules/editor/editor.module: patch does not apply
error: core/modules/entity/entity.module: No such file or directory
error: patch failed: core/modules/entity_reference/entity_reference.module:10
error: core/modules/entity_reference/entity_reference.module: patch does not apply
error: patch failed: core/modules/field/field.module:9
error: core/modules/field/field.module: patch does not apply
error: patch failed: core/modules/field_ui/field_ui.module:7
error: core/modules/field_ui/field_ui.module: patch does not apply
error: patch failed: core/modules/file/file.module:8
error: core/modules/file/file.module: patch does not apply
error: patch failed: core/modules/filter/filter.module:12
error: core/modules/filter/filter.module: patch does not apply
error: patch failed: core/modules/forum/forum.module:10
error: core/modules/forum/forum.module: patch does not apply
error: patch failed: core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php:10
error: core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php: patch does not apply
error: patch failed: core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php:7
error: core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php: patch does not apply
error: patch failed: core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php:7
error: core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php: patch does not apply
error: core/modules/forum/tests/src/Breadcrumb/ForumBreadcrumbBuilderBaseTest.php: No such file or directory
error: core/modules/forum/tests/src/Breadcrumb/ForumListingBreadcrumbBuilderTest.php: No such file or directory
error: core/modules/forum/tests/src/Breadcrumb/ForumNodeBreadcrumbBuilderTest.php: No such file or directory
error: patch failed: core/modules/help/help.module:5
error: core/modules/help/help.module: patch does not apply
error: patch failed: core/modules/help/src/Controller/HelpController.php:8
error: core/modules/help/src/Controller/HelpController.php: patch does not apply
error: patch failed: core/modules/history/history.module:11
error: core/modules/history/history.module: patch does not apply
error: patch failed: core/modules/image/image.module:7
error: core/modules/image/image.module: patch does not apply
error: patch failed: core/modules/language/language.module:7
error: core/modules/language/language.module: patch does not apply
error: patch failed: core/modules/link/link.module:6
error: core/modules/link/link.module: patch does not apply
error: patch failed: core/modules/locale/locale.module:15
error: core/modules/locale/locale.module: patch does not apply
error: core/modules/menu_link/menu_link.module: No such file or directory
error: patch failed: core/modules/menu_ui/menu_ui.module:11
error: core/modules/menu_ui/menu_ui.module: patch does not apply
error: patch failed: core/modules/node/node.module:11
error: core/modules/node/node.module: patch does not apply
error: patch failed: core/modules/options/options.module:7
error: core/modules/options/options.module: patch does not apply
error: patch failed: core/modules/path/path.module:9
error: core/modules/path/path.module: patch does not apply
error: patch failed: core/modules/quickedit/quickedit.module:13
error: core/modules/quickedit/quickedit.module: patch does not apply
error: patch failed: core/modules/rdf/rdf.module:6
error: core/modules/rdf/rdf.module: patch does not apply
error: patch failed: core/modules/responsive_image/responsive_image.module:6
error: core/modules/responsive_image/responsive_image.module: patch does not apply
error: patch failed: core/modules/rest/rest.module:5
error: core/modules/rest/rest.module: patch does not apply
error: patch failed: core/modules/search/search.module:7
error: core/modules/search/search.module: patch does not apply
error: patch failed: core/modules/serialization/serialization.module:5
error: core/modules/serialization/serialization.module: patch does not apply
error: patch failed: core/modules/shortcut/shortcut.module:7
error: core/modules/shortcut/shortcut.module: patch does not apply
error: patch failed: core/modules/shortcut/src/Form/SwitchShortcutSet.php:10
error: core/modules/shortcut/src/Form/SwitchShortcutSet.php: patch does not apply
error: patch failed: core/modules/simpletest/simpletest.module:4
error: core/modules/simpletest/simpletest.module: patch does not apply
error: patch failed: core/modules/statistics/statistics.module:7
error: core/modules/statistics/statistics.module: patch does not apply
error: patch failed: core/modules/syslog/syslog.module:5
error: core/modules/syslog/syslog.module: patch does not apply
error: patch failed: core/modules/system/src/Form/ModulesListForm.php:17
error: core/modules/system/src/Form/ModulesListForm.php: patch does not apply
error: patch failed: core/modules/system/src/PathBasedBreadcrumbBuilder.php:13
error: core/modules/system/src/PathBasedBreadcrumbBuilder.php: patch does not apply
error: patch failed: core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php:10
error: core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php: patch does not apply
error: core/modules/system/src/Plugin/Block/SystemHelpBlock.php: No such file or directory
error: patch failed: core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php:10
error: core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php: patch does not apply
error: patch failed: core/modules/system/system.api.php:903
error: core/modules/system/system.api.php: patch does not apply
error: patch failed: core/modules/system/system.module:8
error: core/modules/system/system.module: patch does not apply
error: core/modules/system/tests/src/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php: No such file or directory
error: patch failed: core/modules/taxonomy/src/TermBreadcrumbBuilder.php:8
error: core/modules/taxonomy/src/TermBreadcrumbBuilder.php: patch does not apply
error: patch failed: core/modules/taxonomy/taxonomy.module:9
error: core/modules/taxonomy/taxonomy.module: patch does not apply
error: patch failed: core/modules/telephone/telephone.module:5
error: core/modules/telephone/telephone.module: patch does not apply
error: patch failed: core/modules/text/text.module:7
error: core/modules/text/text.module: patch does not apply
error: patch failed: core/modules/toolbar/toolbar.module:7
error: core/modules/toolbar/toolbar.module: patch does not apply
error: patch failed: core/modules/tour/tour.module:6
error: core/modules/tour/tour.module: patch does not apply
error: patch failed: core/modules/tracker/tracker.module:7
error: core/modules/tracker/tracker.module: patch does not apply
error: patch failed: core/modules/update/update.module:11
error: core/modules/update/update.module: patch does not apply
error: patch failed: core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php:7
error: core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php: patch does not apply
error: patch failed: core/modules/user/src/Theme/AdminNegotiator.php:10
error: core/modules/user/src/Theme/AdminNegotiator.php: patch does not apply
error: patch failed: core/modules/user/user.module:4
error: core/modules/user/user.module: patch does not apply
error: patch failed: core/modules/views/views.module:14
error: core/modules/views/views.module: patch does not apply
error: patch failed: core/modules/views_ui/views_ui.module:5
error: core/modules/views_ui/views_ui.module: patch does not apply
error: core/modules/xmlrpc/xmlrpc.module: No such file or directory
error: patch failed: core/tests/Drupal/Tests/Core/Routing/CurrentRouteMatchTest.php:2
error: core/tests/Drupal/Tests/Core/Routing/CurrentRouteMatchTest.php: patch does not apply
error: core/tests/Drupal/Tests/Core/Routing/RouteMatchBaseTest.php: No such file or directory
error: patch failed: core/tests/Drupal/Tests/Core/Routing/RouteMatchTest.php:2
error: core/tests/Drupal/Tests/Core/Routing/RouteMatchTest.php: patch does not apply
error: patch failed: core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php:42
error: core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php: patch does not apply
error: patch failed: core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php:7
error: core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php: patch does not apply
martin107’s picture

Assigned: Unassigned » martin107
FileSize
211.65 KB

It is good to activity on this issue ... Here is a reroll.

With over 130 CONFLICTing files in the reroll ... I expect a few error. which I will fix.

martin107’s picture

Status: Needs work » Needs review
FileSize
221.71 KB
15.72 KB

site install now works.

Status: Needs review » Needs work

The last submitted patch, 38: 2296205-38.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

martin107’s picture

Not yet complete just posting while I work

martin107’s picture

Issue tags: -Needs reroll

To speak to Crell's objections....

automated tests now, a few years later, are present which ensure that component files no longer depend on Core objects.

Additionally other test enforce the presence of

LICENSE.txt and README.txt files.

which I have added here.

As a note for myself linting of files can be done in this command

find . -name "*.php" -print0 | xargs -0 -n1 -P8 php -l

When this come back green, I am going to test against 9.1.x which is where I think this need to be

martin107’s picture

FileSize
233.39 KB
25.24 KB
martin107’s picture

FileSize
243.49 KB
15.26 KB

More tests pass.

martin107’s picture

more fixes.

martin107’s picture

Regarding the failing tests

RouteMatchTest and CurrentRouteMatchTest

through RouteMatchTestBase they depend on Drupal\Tests\UnitTestCase

which is a prohibited core object ( given the conversion )

The actual dependence is fake, so we can just drop back to depending on the lesser PHPUnit\Framework\TestCase

A happy accident.

martin107’s picture

Assigned: martin107 » Unassigned

vsujeetkumar, prabha1997 or Saurabh_sgh

if you want to work on this ... I guide you through tasks.

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.

shaktik’s picture

Hi @martin107,

Patch #45 no longer to apply on 8.9 and 9.1 need re-reroll wokinrg on it.

martin107’s picture

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

This reroll is a tricky.

The original patch was developed on the 8.9.x branch

The currently active branch 9.1.x is so different that the patch cannot be rerolled using the conventional method.

So the plan is in 2 stages .. first reroll to the head of 8.9.x and then we will proceed from there.

martin107’s picture

Status: Needs review » Needs work

The last submitted patch, 50: 2296205-49.patch, failed testing. View results

martin107’s picture

Version: 8.9.x-dev » 9.1.x-dev
Status: Needs work » Needs review
FileSize
12.9 MB
1.72 KB

Fixing tests

martin107’s picture

The last interdiff is correct -- it show the changes I made.

BUT the last patch was diffed against 9.1.x ( 8.9.x was intended )

More haste less speed.

martin107’s picture

Assigned: Unassigned » martin107

next steps

git rebase 9.0.x

the rebase to 9.1.x

shaktik’s picture

Assigned: martin107 » Unassigned
FileSize
236.33 KB

Re-roll patch of #45

martin107’s picture

Nice work shaktik .. thanks for the reroll... that was a really big task.

There is a minor fix which seems to be a the centre of all the failing tests. Here is the fix.

martin107’s picture

That reduced the failure count ... alot .. but not perfectly

This time, I did a search replace on

use Drupal\Core\Routing\RouteMatchInterface;

Status: Needs review » Needs work

The last submitted patch, 57: 2296205-57.patch, failed testing. View results

martin107’s picture

All these failures also appear to have a small common set of causes.

martin107’s picture

This is slowly coming good.

-use Drupal\comment\Routing\RouteMatchInterface;

is not a thing

martin107’s picture

Status: Needs work » Needs review
FileSize
241.45 KB
2.16 KB

I have fixed the obvious failing test

The one remaining functional test will take some detailed bug hunting ... I will take a look on Sunday.

Status: Needs review » Needs work

The last submitted patch, 61: 2296205-61.patch, failed testing. View results

tim.plunkett’s picture

martin107’s picture

Hmm ...

I ran out of time this evening ..

The only useful thing I have to add is to describe test failure.

What is asserted is that the 403 page is generated using the seven theme but after debugging...

The problem is the the expected page is correctly returned but what was returned was built using the stable theme not the seven theme...

Last time we had passing tests it was on the 8.9.x branch

So I need to understand how the theme selection has changed between D8 and D9. Any suggestions welcome

shaktik’s picture

Hi @martin107,

getting error after applied #61, Kindly check.

The website encountered an unexpected error. Please try again later.
Error: Class 'Drupal\Core\Routing\CurrentRouteMatch' not found in Drupal\Component\DependencyInjection\Container->createService() (line 257 of core/lib/Drupal/Component/DependencyInjection/Container.php).
Drupal\Component\DependencyInjection\Container->createService(Array, 'current_route_match') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('current_route_match', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'route_processor_current') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('route_processor_current', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 271)
Drupal\Component\DependencyInjection\Container->createService(Array, 'route_processor_manager') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('route_processor_manager', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'private__zpo0hTnDUvjD5VJF99cYFfEg3efThaha5qz1wHgNfRA') (Line: 447)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'url_generator') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('url_generator', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'router.no_access_checks') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('router.no_access_checks', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'router') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('router', 1) (Line: 432)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 235)
Drupal\Component\DependencyInjection\Container->createService(Array, 'router_listener') (Line: 171)
Drupal\Component\DependencyInjection\Container->get('router_listener') (Line: 148)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->getListeners('kernel.request') (Line: 121)
Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy->getListeners('kernel.request') (Line: 71)
Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy->dispatch(Object, 'kernel.request') (Line: 134)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 705)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
martin107’s picture

Looking at the stack trace

When I see

Class 'Drupal\Core\Routing\CurrentRouteMatch' not found in relation to

a call to createService()

->createService(Array, 'current_route_match')

It makes me think that the definition for current_route_match in core.services.yml is invalid. but the patch looks correct.

current_route_match:current_route_match:
     class: Drupal\Component\Routing\CurrentRouteMatch
     arguments: ['@request_stack']

infact searching all files in my core directory tells me the legacy string "Drupal\Core\Routing\CurrentRouteMatch" does not exists

so I think something in your environment did not get rebuilt correctly

Can I ask you to press the "Clear all caches" button - that should invalidate the kernel

as it calls
drupal_flush_all_caches() which has this line

// Invalidate the container.
  \Drupal::service('kernel')->invalidateContainer();

Locally I can reproduce the failing test after a clear cache. ( testbot report 2 failing test .. but it can't count)

martin107’s picture

Status: Needs work » Needs review
FileSize
241.51 KB
585 bytes

I finally found the time to unclog this.

Status: Needs review » Needs work

The last submitted patch, 67: 2296205-67.patch, failed testing. View results

martin107’s picture

Ha, when I test locally ... I can get all listed failing tests to pass.... I am retesting

shaktik’s picture

Thanks, @martin107 it's working fine after the cache clear of #61 patch and finally, issue fixed.

shaktik’s picture

Status: Needs work » Needs review
tim.plunkett’s picture

Status: Needs review » Postponed
Related issues: +#2917331: Decouple from Symfony CMF

This is looking pretty good. But I believe this should be postponed on the CMF issue, as that adds new classes to \Drupal\Core\Routing.

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.