Ignore visibility by default on copy/move in Filesystem

[jnoordsij]

With the current behavior, when creating a Filesystem instance with default visibility set (through passing the config); this will be enforced on any copyor move operation, where I would expect the default the be like the default adapter behavior (e.g. for AwsS3V3Adapter: keep the current visibility). This is due to the forced config merge taking place e.g. here:

flysystem/src/Filesystem.php

Line 122 in 532bb63

$config = $this->config->extend($config);

The following is a quick illustration of the somewhat confusing behavior:

// File foo.txt with public visibility
$adapter = new AwsS3V3Adapter(...);
$adapter->copy('foo.txt', 'adapter-copy.txt', new Config([])); // adapter-copy.txt has the same visibility as foo.txt (public)
$filesystem = new Filesystem($adapter, ['visibility' => 'private']);
$filesystem->copy('foo.txt', 'filesystem-copy.txt'); // filesystem-copy.txt will have private visibility

I noticed this within the context of Laravel, which by default creates a Filesystem wrapper around any adapter before exposing it to the application; see also https://github.com/laravel/framework/blob/10.x/src/Illuminate/Filesystem/Filesystem.php.

This is marked as draft as I'd like to have some input about possible approaches and wether or not this is something that is open for behavioral changes. Some ideas about this:

• the current implementation is slightly hackish, by force-setting visibility to null
• alternatively, some method could be added to the Config class which unsets an option, which could be called here

[frankdejonge]

Hi, I think this makes sense, I'll touch up the PR after merging before the release 👌 I've added a method to removeSettings on the main branch and will overlay this behaviour with the retain_visibility option that was merged yesterday. Thanks for contributing!

[jnoordsij]

Thanks a lot for your quick response (both here and on the other PRs), much appreciated!