AssetAdapter - If the visibility is already correct, do not try to re-set it.

[nathanbrauer]

AssetAdapter.php - If the visibility is already correct, do not try to re-set it.

Attempting to set the visibility of a file/folder when the filesystem is already set correctly causes "Operation not permitted" errors on some Linux servers in /dev/build. This often leads to a fatal error preventing the completion of the /dev/build.

Issue

• AssetAdapter attempts to set the visibility of a file/folder when the filesystem is already set correct #564

[nathanbrauer]

This fix is for inclusion in the next minor release of silverstripe/silverstripe-framework:^4.x which uses silverstripe/silverstripe-assets:^1.x

However, I've already checked and this commit is also safe to rebase/cherry-pick/whatever into silverstripe/silverstripe-assets:^2.x as well.

[GuySartorelli]

Hi,
Thanks for this. Please retarget this to the 1.13 branch so that it can be released as a patch.

[nathanbrauer]

My pleasure! Let me know if I did that right! :)

[GuySartorelli]

Yes, looks like you have, thank you!
You'll notice I've created an issue that I've associated with this PR - that will let us track this on our internal board. It's in our backlog to be properly reviewed when we have capacity to do so.

[emteknetnz]

Code looks good, I'm running an extra test of this PR on running against the silverstripe/installer CI - if that goes green I'm happy to merge https://github.com/emteknetnz/silverstripe-installer/actions/runs/5947561612

Update:

This phpunit test failure is existing

• https://github.com/emteknetnz/silverstripe-installer/actions/runs/5947561612/job/16160418456 << PR branch
• https://github.com/silverstripe/silverstripe-installer/actions/runs/5941845031/job/16113582409 << existing failure

However this behat test failure is not

• https://github.com/emteknetnz/silverstripe-installer/actions/runs/5947561612/job/16160418605

Saving HTML into /home/runner/work/silverstripe-installer/silverstripe-installer/artifacts/screenshots/zz_insert-an-image.feature_122.html And the "Content" HTML field should contain "My alt updated & saved" # SilverStripe\Framework\Tests\Behaviour\CmsFormsContext::theHtmlFieldShouldContain() The string "My alt updated & saved" should be found in the HTML of the element matching name "Content". Actual content: "<p>[image src="/assets/folder1/3d0ef6ec37/file1.jpg" id="2" width="50" height="50" class="leftAlone ss-htmleditorfield-file image" title="My title text updated &amp;amp;amp;amp; saved" alt="My alt updated &amp;amp;amp;amp; saved"]My awesome content</p>" (Behat\Mink\Exception\ElementHtmlException)

Issues appears to be & getting turned into &, presumably by multiple saves?

I've replicated the behat failure locally and have confirmed that it does not happen with the most recent version of silverstripe/assets - 53e8331. I tried rebase the PR branch on top of 53e8331 on my local however that didn't resolve the behat failure.

[emteknetnz]

Updated installer test using pull-request - if this goes green then I'm happy with this pull-request https://github.com/emteknetnz/silverstripe-installer/actions/runs/6116321876

[emteknetnz]

I'm getting really weird results with this branch, on the test I did running this branch against the silverstripe/installer CI https://github.com/emteknetnz/silverstripe-installer/actions/runs/6116321876 there is a behat test failure. I could replicate this locally.

Running this locally, and rebasing this branch top of assets 1.13 didn't do anything. However if I recreated the branch from 1.13 and then copy pasted the contents of AssetAdapter.php in (essentially recreating the PR), then the behat test did pass.

So would you be able to recreate this PR on top of 1.13 please?

[emteknetnz]

@nathanbrauer Have you had a chance to look at this one. I'll look to close this PR in a week or so if there's no further activity on it

[nathanbrauer]

It's on my radar and I'll come back to it soon (currently working through some other priorities).

[emteknetnz]

Looks like there hasn't been any work done on this PR for a while. I'll close this for now. Feel free to reopen this if further work is done on it.