handle restore of multiple asset manifests on rollback

[lennart]

Handle multiple manifest files gracefully.

Related to #123 this PR will fix issues with multiple sprockets asset manifest files lying around in #{shared_path}/public/assets/ and #{release_path}/assets_manifest_backup on cap <stage> deploy:rollback.

Currently this will fail due to multiple filepaths being interpolated into a single shell if condition:

if test "[[ -f #{source} && -f #{target} ]]"

see https://github.com/capistrano/rails/blob/master/lib/capistrano/tasks/assets.rake#L93 and #204 (comment)

To deal with this restore_manifest will explicitly copy each of these files in a separate cp command.

Feedback on how to improve this PR is very welcome!

[capistrano-bot]

	1 Warning
⚠️	Please update CHANGELOG.md with a description of your changes. If this PR is not a user-facing change (e.g. just refactoring), you can disregard this.

Thanks for the PR! This project lacks automated tests, which makes reviewing and approving PRs somewhat difficult. Please make sure that your contribution has not broken backwards compatibility or introduced any risky changes.

Here's an example of a CHANGELOG.md entry (place it immediately under the * Your contribution here! line):

* [#226](https://github.com/capistrano/rails/pull/226): handle restore of multiple asset manifests on rollback - [@lennart](https://github.com/lennart)

Generated by 🚫 Danger

[lennart]

since this is neither a user-facing change nor can I see how to "test" this (apart from using this, and it seems to work).

is it possible to merge this pr?

[mattbrictson]

Hi, thanks for the PR and sorry for the lack of a timely response.

The reason this is a hard PR for me to review is that even though I am a maintainer, I do not use the "rollback" feature of Capistrano. That makes it hard for me to intuitively judge if this change is needed and whether it works as intended.

That said, your explanation of why -f fails makes sense, and your code is easy to follow. Thanks for that!

Before I merge, could you post output of a cap rollback that demonstrates this code working? I don't have a means of testing it at the moment.

[lennart]

@mattbrictson I resolved the issues you had in code and made a demo-run of deploy and rollback handling multiple-manifest files without breakage, see:

https://gist.github.com/lennart/d2eab716d5b589da869a7c6f86ec6cc2

[lennart]

um, when I take a look at this now, I realize there's a bug. targets and sources are mixed up... I'll fix this

[lennart]

ok, so now, starting at: https://gist.github.com/lennart/d2eab716d5b589da869a7c6f86ec6cc2#file-capistrano-deploy-and-rollback-log-L511

it lists the files in the assets_manifest_backup folder and then restores all backed-up manifests to public/assets/

[mattbrictson]

This looks good. Thanks for your patience and for following up!

I have one more small change to request before merging this. Let me know what you think.

[lennart]

@mattbrictson sounds good, I accepted your suggestion!