Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change in v7.3.2 & v7.3.3 for cachetool task #3741

Closed
claytonrcarter opened this issue Dec 5, 2023 · 5 comments · Fixed by #3742
Closed

Breaking change in v7.3.2 & v7.3.3 for cachetool task #3741

claytonrcarter opened this issue Dec 5, 2023 · 5 comments · Fixed by #3742

Comments

@claytonrcarter
Copy link

claytonrcarter commented Dec 5, 2023

  • Deployer version: 7.3.2, 7.3.3
  • Deployment OS: Debian 11

Hi, we recently updated from 7.3.1 to 7.3.3 and found that our very "default" cachetool usage stopped working. (The only cachetool config we have set is set('bin/cachetool', 'vendor/bin/cachetool') so use the composer installed version.) The issue was introduced by #3684 – where is caused a loud failure – and then tweaked by cb28eb8 – where it now causes a silent failure.

The issue is that the cachetool setting is '' (empty string) by default (see https://github.com/deployphp/deployer/blob/master/contrib/cachetool.php#L50) but the new changes in cachetool_options (see diff of 7.3.1...master at v7.3.1...master#diff-3ff1b6689a573050ee941e9cf0474c9f96a4ff7697a386d9c1af6b320fc10ce6) mean that this default value leads to no invocations of cachetool:

  1. cachetool starts as ''
  2. it is then cast to an array, turning into ['']
  3. the empty element is skipped (by the foreach ... if)
  4. an empty array is returned from cachetool_options
  5. task cachetool:clear:opcache iterates over this array, doing nothing because it's empty

Based on the code, it seems that this will affect any of the cachetool:* tasks, as they all iterate over cachetool_options, but I have only experienced it with cachetool:clear:opcache

Workaround
I have found that set('cachetool_args', ' '); is an effective workaround for the time being. Note that it needs the second arg to be a non-empty string of only whitespace.

Possible Fixes
I'm sorry, but I don't have the time or wherewithal to test and submit a fix for this at this time. The options that jump out at me, though, are:

  1. don't cast (array)get('cachetool');, and do if ($options === '') { return [''] }, then cast (array) $options when it's given to the foreach
  2. leave the cast in place, but change the foreach to something like $return[] = empty($option) ? '' : "--fcgi={$option}"

Thank you!

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@antonmedv
Copy link
Member

@infabo please, take a look.

@infabo
Copy link
Contributor

infabo commented Dec 5, 2023

I gonna submit a PR patch later today. I already was afraid of a side effect like this one.

@infabo
Copy link
Contributor

infabo commented Dec 6, 2023

@claytonrcarter Please have a look at #3742 if this PR fixes the issue.

@claytonrcarter
Copy link
Author

Thank you both for the fix!

@antonmedv Is there any chance we'll be getting a new point release soon to fix this publicly?

@antonmedv
Copy link
Member

Yes, will release Deployer today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants