Closed (outdated)
Project:
Flysystem
Version:
3.0.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Aug 2021 at 00:13 UTC
Updated:
12 Jun 2026 at 14:44 UTC
Jump to comment: Most recent
Comments
Comment #2
benjifisherI think the correct status for this issue is Postponed.
Comment #3
bradjones1Comment #4
dieterholvoet commentedAnother option is
m2mtech/flysystem-stream-wrapper, it's a fork of thetwistor/flysystem-stream-wrapperpackage we're currently using and seems to have more downloads on Packagist.Comment #5
lisa.rae commentedSwitching this for 2.1.x branch, let's apply there and work toward a total replacement using the updated Flysystem V3 for 3.0.x
Comment #7
lisa.rae commentedComment #8
lisa.rae commentedUnfortunately just switching out the dependency is not an option without significant refactoring because ` m2mtech/flysystem-stream-wrapper` is a complete rewrite of `twistor/flysystem-stream-wrapper` with multiple change to the API.
Attached is a patch to port the code for the third party libraries that are dependencies for this module, effectively:
* twistor/flysystem-stream-wrapper
* league/flysystem
* league/flysystem-replicate-adapter
And remediating the ported code for PHP 8.1 compliance and D10 compliance.
Would love to be able to go back to utilizing external libraries, as this adds alot of additional code to maintain, and I'm not yet convinced the test coverage is here to support the ported code (I did also port several of the tests, but may have missed some).
It's a good starting point. Failing tests have been remediated, but ported code is not Drupal coding standards compliant..
Comment #9
lisa.rae commentedComment #10
dieterholvoet commentedI'm not sure copying the code of those libraries to this module is a good solution. We'll have to migrate to Flysystem V2 or V3 at some point, might as well do it now, right?
Comment #11
lisa.rae commentedOh, I agree, I'm looking to get the 2.1 branch stable for D10, then start working on the 3.0.x branch and using either Flysystem v2 or v3. That will be a breaking change because the APIs on V2 and V3 changed significantly from V1, with alot of refactoring along the way to eliminate the dependencies on abandoned third party packages.
Comment #12
dieterholvoet commentedOkay, makes sense.
Comment #13
lisa.rae commentedRan a code coverage report after applying this patch:
* Code Coverage for the original module code (not ported) is very good, exceeding 90%.
More test coverage could be used:
For the Optimizers, the original maintainer extended Drupal Core classes instead of implementing Interfaces, so these statistics may be misleading, as test coverage may exist in Drupal Core for Methods that are part of the parent class.
At this time, I'm not going to port all of the tests for the ported code, primarily because the goal is to stabilize the module for D10, and focus on a rewrite for a new major version release using league/flysystem v2 or v3.
I'm going to merge this patch in "As Is" to the dev branch, but hold off for a bit on generating another release until this has had some time to percolate a bit.
Same with Coding Standards compliance for the ported code. Would rather focus time and energy on v3.0. However if someone wants to tackle coding standards, I'll accept patches!
Comment #15
lisa.rae commentedComment #16
dieterholvoet commentedAre you sure all this is actually necessary? Because I have a project which is running Drupal 10.1.1, PHP 8.1 and Flysystem 2.1.0-rc4 and everything is working as expected. The only patches I have applied are these:
twistor/flysystem-stream-wrapper
flysystem
Comment #17
dieterholvoet commentedI think it would be enough to fix this on the 3.x branch.
Comment #18
lisa.rae commentedRethinking this one, which is why it's not yet part of a release.
Comment #19
lisa.rae commentedComment #20
dieterholvoet commentedI don't think this issue has anything to do with Drupal 10 / PHP 8.1 compatibility, as far as I know 2.1.0-rc5 is completely D10 compatible, except #3376399: Support new lazy asset collection optimizers & deprecate the serve_* options. This is just about getting rid of an abandoned dependency. Let's remove those tags and change this issue to target 3.x. If this does impact D10/PHP8.1 compatibility, please tell me how and maybe we can figure out another way to fix it.
Comment #21
lisa.rae commentedI moved this back to "Needs Work", i have a revert patch running through tests currently, took a bit to make sure that I didn't lose other MRs that had been merged. I'll be updating the 2.1.x-dev branch with the revered code as soon as I get through with my team meetings today, in a couple of hours tops.
Comment #23
lisa.rae commentedReverted the refactoring while keeping other merged MRs. Closing this issue.
Comment #24
lisa.rae commentedComment #25
dieterholvoet commentedWhy close? This still needs to happen in the 3.0.x branch, right?
Comment #26
lisa.rae commentedReopening this issue, as it is now relevant to the 3.0.x branch work. Will evaluate to determine what changes may be needed.
Comment #27
lisa.rae commentedClosing as outdated. See latest 3.0.x dev branch code as of 2026 June 12, which is a complete rewrite of the existing codebase. Notes are in the README.md file as well as the DeveloperNotes.md file.