Skip to content

Conversation

@sgross-emlix
Copy link
Contributor

@sgross-emlix sgross-emlix commented Nov 10, 2025

Description

Implement -r and [SOURCES...] [DESTINATION] for scp
Collegues requested this feature since it matches the expectation on how to use the widely known scp command.

Checklist

  • Documentation for the feature
  • PR has been tested

to Target

labgrid@phoebe:~/client$ lgc scp dummy/bar dummy/foo :/tmp
bar                                                                                                                                                                                100%    3     1.2KB/s 00:00
foo                                                                                                                                                                                100%    5     3.3KB/s   00:00
labgrid@phoebe:~/client$ lgc scp -r dummy :/tmp
foo                                                                                                                                                                                100%    5     2.6KB/s   00:00
bar                                                                                                                                                                                100%    3     1.7KB/s   00:00
labgrid@phoebe:~/client$ lgc ssh


BusyBox v1.34.1 () built-in shell (ash)

# cd /tmp
# ls
WebKitCache          bar                  env.yml              foo                  mesa_shader_cache    weston
avahi-daemon.socket  dummy                fontconfig           hsts-storage.sqlite  update-tool.lock
# cd dummy/
# ls
bar  foo

from Target

labgrid@phoebe:~/client$ mkdir rev
labgrid@phoebe:~/client$ lgc scp :/tmp/dummy/foo :/tmp/dummy/bar rev/
foo                                                                                                                                                                                100%    5     1.0KB/s   00:00
bar                                                                                                                                                                                100%    3     0.8KB/s   00:00
labgrid@phoebe:~/client$ lgc scp -r :/tmp/dummy rev
bar                                                                                                                                                                                100%    3     0.4KB/s   00:00
foo    
labgrid@phoebe:~/client$ find rev
rev
rev/dummy
rev/dummy/foo
rev/dummy/bar
rev/foo
rev/bar
  • Man pages have been regenerated

@sgross-emlix sgross-emlix force-pushed the feature/ssh-option-recursive branch from f6b4349 to 1cebc8b Compare November 10, 2025 10:30
@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.2%. Comparing base (2f0bdf1) to head (1b2997c).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
labgrid/remote/client.py 0.0% 3 Missing ⚠️
labgrid/driver/sshdriver.py 92.8% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1769     +/-   ##
========================================
+ Coverage    45.1%   45.2%   +0.1%     
========================================
  Files         172     172             
  Lines       13618   13626      +8     
========================================
+ Hits         6144    6163     +19     
+ Misses       7474    7463     -11     
Flag Coverage Δ
3.10 45.2% <76.4%> (+0.1%) ⬆️
3.11 45.2% <76.4%> (+0.1%) ⬆️
3.12 45.2% <76.4%> (+0.1%) ⬆️
3.13 45.2% <76.4%> (+0.1%) ⬆️
3.14 45.2% <76.4%> (+0.1%) ⬆️
3.9 45.2% <76.4%> (+0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sgross-emlix
Copy link
Contributor Author

I would like to update tests, but I have no idea where to start. test_sshdriver looks confusing to me

Copy link
Member

@jluebbe jluebbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pull out bb72bc0 into a separate PR so we can merge that early.

Comment on lines 1925 to 2027
subparser.add_argument("src", help="source path (use :dir/file for remote side)")
subparser.add_argument("dst", help="destination path (use :dir/file for remote side)")
subparser.add_argument("--recursive", "-r", action="store_true", help="copy recursive")
subparser.add_argument(
"files", nargs="+", metavar="SRC/DST", help="source and destination path (use :dir/file for remote side)"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to just use nargs="+" for src and keep the separate dst argument.

"-S", self._ssh,
"-F", "none",
"-o", f"ControlPath={self.control.replace('%', '%%')}",
src, dst,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use *src to avoid the insert loop below.

@step(args=['src', 'dst'])
def scp(self, *, src, dst):
@step(args=['src', 'dst', 'recursive'])
def scp(self, *, src, dst, recursive=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to consider backwards compatibility here. So, if src is a string, wrap it in a list.

if src.startswith(':'):
src = '_' + src
if dst.startswith(':'):
if all([f.startswith(':') for f in src]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to check that either all src elements start with ':' or none of them do.

So perhaps first something like

remote_src = [f.startswith(':') for f in src]
if any(remote_src) != all(remote_src):
   raise ValueError("All sources must be consistently local or remote (start with :)")

before the other checks?

@jluebbe
Copy link
Member

jluebbe commented Dec 17, 2025

I would like to update tests, but I have no idea where to start. test_sshdriver looks confusing to me

Use test_local_put() as an example. To run those tests, you first need to have ssh localhost working outside of pytest (via pubkey auth). Then run the tests with --ssh-username=<your username>.

In the test, make two directories via in the tmpdir fixture and transfer beween them.

@sgross-emlix sgross-emlix force-pushed the feature/ssh-option-recursive branch from 1cebc8b to edfc3c3 Compare December 18, 2025 07:52
@sgross-emlix sgross-emlix marked this pull request as draft December 18, 2025 08:23
@sgross-emlix sgross-emlix force-pushed the feature/ssh-option-recursive branch 2 times, most recently from 70e0c94 to 3f312be Compare December 19, 2025 06:50
sbngross and others added 4 commits December 19, 2025 07:50
Users are accustomed to scp having an option for copying recursively as
well as to accept multiple source files.
Meet user expectation by adding well known `-r` option and support for
multiple source files.

Signed-off-by: Sebastian Gross <[email protected]>
Signed-off-by: Sebastian Gross <[email protected]>
Use features of multifile scp driver

Signed-off-by: Sebastian Gross <[email protected]>
Signed-off-by: Sebastian Gross <[email protected]>
@sgross-emlix sgross-emlix force-pushed the feature/ssh-option-recursive branch from 3f312be to 1b2997c Compare December 19, 2025 06:50
@sgross-emlix sgross-emlix marked this pull request as ready for review December 19, 2025 06:51
@sgross-emlix sgross-emlix requested a review from jluebbe December 19, 2025 06:51
@sgross-emlix sgross-emlix removed their assignment Dec 19, 2025
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 this pull request may close these issues.

3 participants