Skip to content

test_cp_parents_2_dirs fails on android #4079

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

Closed
sylvestre opened this issue Oct 24, 2022 · 9 comments · Fixed by #4099
Closed

test_cp_parents_2_dirs fails on android #4079

sylvestre opened this issue Oct 24, 2022 · 9 comments · Fixed by #4099
Labels

Comments

@sylvestre
Copy link
Contributor


---- test_cp::test_cp_parents_2_dirs stdout ----
current_directory_resolved: 
mkdir_all: /data/data/com.termux/files/usr/tmp/.tmpLVaxT2/a/b/c
mkdir: /data/data/com.termux/files/usr/tmp/.tmpLVaxT2/d
run: /data/data/com.termux/files/home/coreutils/target/debug/coreutils cp -a --parents a/b/c d
thread 'test_cp::test_cp_parents_2_dirs' panicked at 'Command was expected to succeed.
stdout = 
 stderr = cp: Permission denied (os error 13)
', tests/common/util.rs:176:9

test introduced in:
ac3fcca

cc @jfinkels

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

Looking into this.

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

Looks like the problem is in attributes, if I remove this line here - https://github.com/uutils/coreutils/blob/main/src/uu/cp/src/cp.rs#L739, it passes.

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

After a bit more digging, looks like Xattr is the one causing the permission denied error here.

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

Here is a simpler test case that you can run in an Android emulator in Termux:

touch ~/a
cp -a ~/a ~/b
# no error
touch ~/a
./coreutils/target/debug/coreutils cp -a ~/a ~/b
# permission denied

This is probably because of selinux attributes, but I'm not sure.

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

Something else that is interesting. With GNU cp, when we specify -a the preserve_xattr is set to true, but this doesn't set the require_preserve_xattr to true. But, if we do --preserve=xattr, the require_preserve_xattr is set to true. Then, when I run cp --preserve=xattr ~/a ~/b on this AVD, I get cp: cannot preserve extended attributes, cp is built without xattr support. So, it looks like the GNU cp doesn't even try setting the attributes in this case.

@sssemil
Copy link
Contributor

sssemil commented Oct 25, 2022

I can think of three solutions here:

  1. Remove the -a parameter from the test.
  2. Change the attributes setting code to set only if the value is not set to what you want it to be (because all of the values are default in this case, the problem is trying to set them even to the same value).
  3. Ignore the error. Should be similar to what GNU cp does.

What do you think?

@tertsdiepraam
Copy link
Member

Interesting! I think we should match GNU behaviour as much as possible. One thing I'm curious about, is it possible too build cp with xattr support and what is the behaviour then?

@sssemil
Copy link
Contributor

sssemil commented Oct 27, 2022

@tertsdiepraam looking at https://github.com/coreutils/coreutils/blob/master/src/copy.c#L1427-L1432:

    if (preserve_xattr)
    {
      if (!copy_attr (src_name, source_desc, dst_name, dest_desc, x)
          && x->require_preserve_xattr)
        return_val = false;
    }

If we select -a, we set preserve_xattr to true, and require_preserve_xattr remains at false. Then, when copy attribute fails, since the require_preserve_xattr is set to false, we do not get an error. So, I guess I would ignore the error in case of -a but not if --preserve=xattr is set. I think this calls for a few additional test cases.

I will try building GNU cp with xattr enabled on android.

@jtracey
Copy link
Contributor

jtracey commented Nov 9, 2022

Again, this is because of #3477.

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