I wrote a graph library, and made it available via composer. We now want to pull this library in for at least two patches:

#1762204: Introduce Assetic compatibility layer for core's internal handling of assets
#2024083: [META] Improve the extension system (modules, profiles, themes)

I've talked this over with chx, and I believe he's OK with replacing the existing graph class with what I've written (though please, do reconfirm that here, chx). There is another graph library, but that takes a generic approach to its graph implementation that trades performance for graph features we don't need.

I currently have gliph in as part of the assets patch already, but as it seems to be becoming a shared dependency, it seems optimal to pull it in directly now, so that we can iterate elsewhere.

The risk: while Gliph is 100% unit tested, it is very young, and I am still actively adding features to it. All that really means is that we should expect to probably have a few issues in the future where we update it to newer versions.

Patch in the first comment...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

I am fine with it. I wanted to rewrite the thing anyways. It's really not a generic class and that's biting us in the ass.

fubhy’s picture

Yes, please.. I need this for ModuleHandler!

sdboyer’s picture

attached is a patch containing gliph 0.1.3. it would be fine to put this in, but based on discussions i'm having with fubhy, i'm adding a new algo that we'll probably end up needing. that'll be 0.1.4.

sdboyer’s picture

Status: Active » Needs review
RobLoach’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal8.base-system.2098375-2.patch, failed testing.

sdboyer’s picture

Status: Needs work » Needs review

#3: drupal8.base-system.2098375-2.patch queued for re-testing.

fubhy’s picture

Status: Needs review » Reviewed & tested by the community
RobLoach’s picture

sdboyer’s picture

Assigned: sdboyer » Dries

reassigning to Dries, at catch's suggestion/request.

sdboyer’s picture

FileSize
92.49 KB

updated the patch to gliph 0.1.4, which i pushed last night. it adds an implementation of Tarjan's strongly connected components algorithm, for nifty cycle detection. Test coverage remains >95%.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, gliph.2098375-11.patch, failed testing.

sdboyer’s picture

Status: Needs work » Reviewed & tested by the community

eh, i musta screwed up the patch. let's just use the 0.1.3 one from #3 that went green - that works fine. the important thing is that it's in - patches needing more recent versions can update it in composer.

back to RTBC, since we're disregarding #11.

sdboyer’s picture

FileSize
91.13 KB

oh, doi. i accidentally included some experimental changes to ModuleHandler in there. yeah, that'll break shit.

RobLoach’s picture

FileSize
91.09 KB

Hmm, interesting. Testbot?

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Discussed with sam at the code sprint and committed to 8.x. Thanks!

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

sdboyer’s picture

sdboyer’s picture