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

running sync() locally results in errors #1370

Closed
sdondley opened this issue Jul 1, 2020 · 3 comments · Fixed by #1372
Closed

running sync() locally results in errors #1370

sdondley opened this issue Jul 1, 2020 · 3 comments · Fixed by #1372

Comments

@sdondley
Copy link
Contributor

sdondley commented Jul 1, 2020

Describe the bug

Doing a local rsync with the sync() command results in errors.

How to reproduce it

From comm

sync('/Users/me/workshop/rsync1', '/Users/me/workshop/rsync2' );

Steps to reproduce the behavior:

  1. Create a task called 'sync_task' with sync command that syncs between two local directories.
  2. Run rex sync_task

Errors:

[2020-07-01 18:08:07] ERROR - Can't use string ("") as a HASH ref while "strict refs" in use at /Users/stevedondley/git_repos/Rex_fork/Rex/lib/Rex/Commands/Rsync.pm line 166, <> line 5.

Shortest code example that demonstrates the bug:

use strict;
use warnings;
use Rex -feature => [ qw( 1.4 ) ];
use Rex::Commands::Rsync;
no_ssh;

task test_sync => sub {
  sync('/Users/me/some_dir',
    '/Users/me/some_other_dir', { download => 1, parameters =>
    '--rsync-path="nice -n 19 rsync" --archive' });
};

Expected behavior

No errors.

Circumstances

  • Rex version: 1.11
  • Perl version: 5.24
  • OS running rex: Darwin
  • OS managed by rex: Darwin
@ferki
Copy link
Member

ferki commented Jul 2, 2020

Local sync support was recently added added after 1.11.0 and is unreleased yet, and functionality is tested in t/rsync.t. Could you be more precise what exactly might cause it to fail in your case, please?

Is it triggered by no_ssh; or by download => 1? Or does it only happen when these two are combined? Could you perhaps demonstrate the minimum failing case by adding a failing new (sub)test to t/rsync.t?

EDIT: if caused by no_ssh;, does it also happen when no_ssh task ... form is used, or only when no_ssh is on its own?

[2020-07-01 18:08:07] ERROR - Can't use string ("") as a HASH ref while "strict refs" in use at /Users/stevedondley/git_repos/Rex_fork/Rex/lib/Rex/Commands/Rsync.pm line 166, <> line 5.

That line is

if ( !$local_connection ) {

currently in the master branch, so I don't yet see how it could cause an error about HASH ref. Can it be that you might have some other local modifications in place in your fork that interferes with the line numbers and/or the behavior?

@ferki
Copy link
Member

ferki commented Jul 2, 2020

Alright, I'm not sure if download => 1 makes any sense in the context of local rsync operations.

In remote rsync operations, it controls whether the source or the destination path gets prefixed with $user@$servername:. Perhaps, in local context it could switch source and destination pathes, but I find that confusing, so perhaps throwing an error about local+download would be a better approach there.

EDIT: or download => 1 in local context could be a no-op, and would still just transfer source path to destination path.

Based on my tests in the local_rsync_download branch, there are at least the following factors at play here:

  • download => 1 with local connection still tries to set the $user@$servername: prefix for the source => that's a bug, and the fix is to move the line a few lines below into the if block
  • local connection is determined by checking if servername is defined or not, which is good enough for the tests, but in tasks, the correct way to check that is either calling is_local or checking if the servername is <local> => that's a bug, seems easy to fix the logic, but preferably there should be tests about it
  • no_ssh doesn't seem to be in play here

@ferki ferki mentioned this issue Jul 2, 2020
5 tasks
@ferki
Copy link
Member

ferki commented Jul 2, 2020

@sdondley: if you have a chance to do it early, could you test the code from #1372, please?

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