feat(bin/oli): support cp to dir by asukaminato0721 · Pull Request #6140 · apache/opendal

asukaminato0721 · 2025-05-01T18:56:42Z

Which issue does this PR close?

Closes #2795.
part of #422.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

asukaminato0721 · 2025-05-01T19:09:29Z

No cargo test in CI...

yihong0618 · 2025-05-02T09:38:27Z

I think we can drop obvious commnets

.github/workflows/ci_bin_oli.yml

Copilot

Pull Request Overview

This PR enhances the CI workflow for the bin/oli directory to support the "cp to dir" feature, ensuring that code linting and tests are executed in the CI process.

Renames the CI job from "check_clippy" to "check_clippy_and_test"
Adds a caching step using the sccache-action to speed up build times
Combines cargo clippy and test executions with sccache environment support

.github/workflows/ci_bin_oli.yml

xxchan

Others LGTM

bin/oli/src/commands/cp.rs

bin/oli/tests/cp.rs

xxchan · 2025-05-13T07:02:56Z

@asukaminato0721 Would you like to try the testing style with #6174? I'm fine to either merge that one before or after this PR

asukaminato0721 · 2025-05-13T11:38:11Z

aha, I know the reason now, I enable the cargo test in CI.

asukaminato0721 · 2025-05-13T12:19:55Z

I can't understand

cargo test
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.33s
     Running unittests src/lib.rs (target/debug/deps/oli-e6bc33088fff7ae4)

running 7 tests
test config::tests::test_load_from_env ... ok
test config::tests::test_parse_s3_location3 ... ok
test config::tests::test_load_from_toml ... ok
test config::tests::test_load_config_from_file_and_env ... ok
test config::tests::test_parse_fs_location ... ok
test config::tests::test_parse_s3_location2 ... ok
test config::tests::test_parse_s3_location ... ok

test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/bin/oli.rs (target/debug/deps/oli-598fecbf05f25d6d)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/integration/main.rs (target/debug/deps/integration-fd61fe36475b972e)

running 14 tests
test cp::test_basic_cp ... ok
test cat::test_basic_cat ... ok
test cat::test_cat_for_path_in_current_dir ... ok
test ls::test_basic_ls ... ok
test cp::test_cp_file_to_existing_dir ... ok
test cp::test_cp_for_path_in_current_dir ... ok
test cp::test_recursive_cp_dir_to_new_dir ... ok
test stat::test_basic_stat ... ok
test mv::test_basic_mv ... ok
test mv::test_move_a_file_to_a_dir ... ok
test stat::test_stat_for_path_in_current_dir ... ok
test rm::test_basic_rm ... ok
test rm::test_rm_for_path_in_current_dir ... ok
test mv::test_mv_with_recursive ... ok

test result: ok. 14 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.20s

   Doc-tests oli

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

asukaminato0721 · 2025-05-13T12:23:34Z

seems that the size of folder diffs.

xxchan · 2025-05-13T12:37:10Z

seems that the size of folder diffs.

maybe we can ignore size of folder

asukaminato0721 · 2025-05-13T13:06:06Z

well...

────────────┬───────────────────────────────────────────────────────────────────
    1     1 │ exit_code: 0
    2     2 │ ----- stdout -----
    3     3 │ [TEMP_DIR]/
    4     4 │ dst_3.txt
          5 │+dst_1.txt
    5     6 │ dst_2.txt
    6       │-dst_1.txt
    7     7 │ 
    8     8 │ ----- stderr -----
────────────┴───────────────────────────────────────────────────────────────────

asukaminato0721 · 2025-05-13T13:32:55Z

another problem is that

+-----------------------------------------------+
| Path                     Type   Content       |
+===============================================+
| [TEMP_DIR]               DIR                  |
| [TEMP_DIR]/dest_root     DIR                  |
| [TEMP_DIR]/dest_dir      DIR                  |
| [TEMP_DIR]/file1.txt     FILE   file1_content |
| [TEMP_DIR]/file3.txt     FILE   file3_content |
| [TEMP_DIR]/sub_dir       DIR                  |
| [TEMP_DIR]/file2.txt     FILE   file2_content |
| [TEMP_DIR]/source_root   DIR                  |
| [TEMP_DIR]/source_dir    DIR                  |
| [TEMP_DIR]/file1.txt     FILE   file1_content |
| [TEMP_DIR]/file3.txt     FILE   file3_content |
| [TEMP_DIR]/sub_dir       DIR                  |
| [TEMP_DIR]/file2.txt     FILE   file2_content |
+-----------------------------------------------+

this is not good, since the src and dst become the same.

asukaminato0721 · 2025-05-13T13:49:45Z

+----------------------------------------------------------------------------+
| Path                                                  Type   Content       |
+============================================================================+
| [TEMP_DIR]                                            DIR                  |
| [TEMP_DIR]/dest_root                                  DIR                  |
| [TEMP_DIR]/dest_root/dest_dir                         DIR                  |
| [TEMP_DIR]/dest_root/dest_dir/file1.txt               FILE   file1_content |
| [TEMP_DIR]/dest_root/dest_dir/file3.txt               FILE   file3_content |
| [TEMP_DIR]/dest_root/dest_dir/sub_dir                 DIR                  |
| [TEMP_DIR]/dest_root/dest_dir/sub_dir/file2.txt       FILE   file2_content |
| [TEMP_DIR]/source_root                                DIR                  |
| [TEMP_DIR]/source_root/source_dir                     DIR                  |
| [TEMP_DIR]/source_root/source_dir/file1.txt           FILE   file1_content |
| [TEMP_DIR]/source_root/source_dir/file3.txt           FILE   file3_content |
| [TEMP_DIR]/source_root/source_dir/sub_dir             DIR                  |
| [TEMP_DIR]/source_root/source_dir/sub_dir/file2.txt   FILE   file2_content |
+----------------------------------------------------------------------------+

now pretty well.

asukaminato0721 · 2025-05-13T13:54:45Z

ping @xxchan

xxchan · 2025-05-13T14:13:59Z

+----------------------------------------------------------------------------+
| Path                                                  Type   Content       |
+============================================================================+
| [TEMP_DIR]                                            DIR                  |
| [TEMP_DIR]/dest_root                                  DIR                  |
| [TEMP_DIR]/dest_root/dest_dir                         DIR                  |
| [TEMP_DIR]/dest_root/dest_dir/file1.txt               FILE   file1_content |
| [TEMP_DIR]/dest_root/dest_dir/file3.txt               FILE   file3_content |
| [TEMP_DIR]/dest_root/dest_dir/sub_dir                 DIR                  |
| [TEMP_DIR]/dest_root/dest_dir/sub_dir/file2.txt       FILE   file2_content |
| [TEMP_DIR]/source_root                                DIR                  |
| [TEMP_DIR]/source_root/source_dir                     DIR                  |
| [TEMP_DIR]/source_root/source_dir/file1.txt           FILE   file1_content |
| [TEMP_DIR]/source_root/source_dir/file3.txt           FILE   file3_content |
| [TEMP_DIR]/source_root/source_dir/sub_dir             DIR                  |
| [TEMP_DIR]/source_root/source_dir/sub_dir/file2.txt   FILE   file2_content |
+----------------------------------------------------------------------------+

now pretty well.

Ideally we can have sth like [TEMP_DIR1], [TEMP_DIR2], but I guess this is not achievable via regexp? The workaround above (only 1 tempdir as root) looks fine to me. Alternatively, maybe we use named tempdir and only replaces the common prefix of the path? 🤔

bin/oli/tests/integration/ls.rs

bin/oli/tests/integration/test_utils.rs

xxchan

Thanks!

Xuanwo · 2025-05-14T07:28:09Z

Thank you @asukaminato0721 for working on this and thank you @xxchan for the review!

asukaminato0721 changed the title ~~cp dir~~ feat(bin/oli): support cp to dir May 1, 2025

asukaminato0721 marked this pull request as ready for review May 1, 2025 19:15

asukaminato0721 requested a review from Xuanwo as a code owner May 1, 2025 19:15

asukaminato0721 force-pushed the oli-cp-dir branch from 0bfd6ca to d6843e7 Compare May 6, 2025 10:10

xxchan reviewed May 11, 2025

View reviewed changes

.github/workflows/ci_bin_oli.yml Outdated Show resolved Hide resolved

asukaminato0721 force-pushed the oli-cp-dir branch from d6843e7 to fac962c Compare May 11, 2025 09:52

dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 11, 2025

xxchan mentioned this pull request May 11, 2025

Bump mozilla-actions/sccache-action from 0.0.4 to 0.0.9 apache/infrastructure-actions#144

Merged

dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels May 11, 2025

asukaminato0721 requested a review from xxchan May 11, 2025 19:11

xxchan requested a review from Copilot May 12, 2025 06:14

Copilot AI reviewed May 12, 2025

View reviewed changes

.github/workflows/ci_bin_oli.yml Show resolved Hide resolved

.github/workflows/ci_bin_oli.yml Show resolved Hide resolved

xxchan reviewed May 12, 2025

View reviewed changes

bin/oli/src/commands/cp.rs Outdated Show resolved Hide resolved

bin/oli/src/commands/cp.rs Outdated Show resolved Hide resolved

bin/oli/tests/cp.rs Outdated Show resolved Hide resolved

asukaminato0721 marked this pull request as draft May 12, 2025 16:33

asukaminato0721 force-pushed the oli-cp-dir branch from 1c3c881 to 523b1a8 Compare May 13, 2025 06:44

asukaminato0721 added 6 commits May 13, 2025 20:23

cp dir

5ef366c

keep origin

104808d

cargo test

9c159df

cache

6bc1125

path

b1b7594

rm some comment

5b5d73d

asukaminato0721 force-pushed the oli-cp-dir branch from 523b1a8 to 2bcea19 Compare May 13, 2025 11:24

asukaminato0721 added 2 commits May 13, 2025 20:26

fmt

05db6b0

update test

608ac3e

asukaminato0721 added 2 commits May 13, 2025 20:42

update test

24deec4

fix test

da74d8f

asukaminato0721 marked this pull request as ready for review May 13, 2025 11:48

asukaminato0721 requested a review from xxchan May 13, 2025 12:20

asukaminato0721 added 3 commits May 13, 2025 21:39

try size -> len

3560cce

ignore size

a2c0a67

fix ls test

fc42f45

fix folder regex

64258d3

fix stat

c6d9206

xxchan reviewed May 13, 2025

View reviewed changes

bin/oli/tests/integration/ls.rs Outdated Show resolved Hide resolved

xxchan reviewed May 13, 2025

View reviewed changes

bin/oli/tests/integration/test_utils.rs Outdated Show resolved Hide resolved

fix regex && check ls output

a3c72e3

asukaminato0721 requested a review from xxchan May 13, 2025 19:39

xxchan approved these changes May 14, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label May 14, 2025

Xuanwo merged commit 1b953a5 into apache:main May 14, 2025
16 checks passed

asukaminato0721 deleted the oli-cp-dir branch May 14, 2025 08:02

Comments

Conversation

asukaminato0721 commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

asukaminato0721 commented May 1, 2025

Uh oh!

yihong0618 commented May 2, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

xxchan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xxchan commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

xxchan commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

asukaminato0721 commented May 13, 2025

Uh oh!

xxchan commented May 13, 2025

Uh oh!

Uh oh!

Uh oh!

xxchan left a comment

Choose a reason for hiding this comment

Uh oh!

Xuanwo commented May 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asukaminato0721 commented May 1, 2025 •

edited

Loading