Fix Clippy warnings for rustc version 1.65

[mohamedasaker-arm]

Two warnings have been introduced, and both are from the category: needless_borrow
This commit is to remove the needless borrow

error: this expression borrows a value the compiler would automatically borrow
   --> src/back/backend_handler.rs:101:40
    |
101 |                 unwrap_or_else_return!((&app).as_ref().ok_or(ResponseStatus::NotAuthenticated));
    |                                        ^^^^^^ help: change this to: `app`
    |
    = note: `-D clippy::needless-borrow` implied by `-D clippy::all`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

error: the borrowed expression implements the required traits
   --> src/key_info_managers/on_disk_manager/mod.rs:398:56
    |
398 |                     let mut key_info_file = File::open(&key_name_file_path).with_context(|| {
    |                                                        ^^^^^^^^^^^^^^^^^^^ help: change this to: `key_name_file_path`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

error: could not compile `parsec-service` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `parsec-service` due to 2 previous errors

Signed-off-by: Mohamed Omar Asaker mohamed.omarasaker@arm.com

[ionut-arm]

Thanks!!

[anta5010]

Firstly, why do we set 777 if we need write permissions?
Secondly, I'm not sure the new rust deployment procedure would require write permissions for everyone to the rust registry.

[mohamedasaker-arm]

Firstly, why do we set 777 if we need write permissions? Secondly, I'm not sure the new rust deployment procedure would require write permissions for everyone to the rust registry.

Why do we set 777? we need read and write permissions (not sure about the execute, but I believe we need it as well).
For the second question, it seems so (recently, it fails when parsec-client-2 tries to run cargo command)

[anta5010]

Why do we set 777? we need read and write permissions (not sure about the execute, but I believe we need it as well).
For the second question, it seems so (recently, it fails when parsec-client-2 tries to run cargo command)

I think if we need to change permissions for the rust registry to by open for everyone then we do something wrong with either rust installation or build procedure. We need to fix that instead of adding a work-around. Using "su" for running cargo also looks strange.

For example, a similar issue was fixed without setting 777 permissions:
vibbits/rust-node-ci#1

[mohamedasaker-arm]

Why do we set 777? we need read and write permissions (not sure about the execute, but I believe we need it as well).
For the second question, it seems so (recently, it fails when parsec-client-2 tries to run cargo command)

I think if we need to change permissions for the rust registry to by open for everyone then we do something wrong with either rust installation or build procedure. We need to fix that instead of adding a work-around. Using "su" for running cargo also looks strange.

For example, a similar issue was fixed without setting 777 permissions: vibbits/rust-node-ci#1

I guess the benefit of using a common registry is not to download and build the same stuff again. By having a separate registry, we will lose this benefit. (as we are running the same test but for different users)

Not sure why su looks strange. Could you elaborate?

[anta5010]

I guess the benefit of using a common registry is not to download and build the same stuff again. By having a separate registry, we will lose this benefit. (as we are running the same test but for different users)

Not sure why su looks strange. Could you elaborate?

Ignore my comment about "su", I didn't scroll enough to the side to see the whole command.
But, instead of using 777 permissions I would prefer to see a common group created for parsec-client-1 and parsec-client-2 with using the group to set permissions.

[mohamedasaker-arm]

I guess the benefit of using a common registry is not to download and build the same stuff again. By having a separate registry, we will lose this benefit. (as we are running the same test but for different users)
Not sure why su looks strange. Could you elaborate?

Ignore my comment about "su", I didn't scroll enough to the side to see the whole command. But, instead of using 777 permissions I would prefer to see a common group created for parsec-client-1 and parsec-client-2 with using the group to set permissions.

Yea, it's mentioned in the slack discussion here.

I don't mind either approach, but the thing is, having a common group might (with unmask) allow users to access each other files (through group permission) might not be a good idea. I will take a look at the tests closely.
Also, what is the reason you don't prefer granting permissions to the registry during testing?

@ionut-arm, Could you give us your insights on the common group?

[anta5010]

I don't mind either approach, but the thing is, having a common group might (with unmask) allow users to access each other files (through group permission) might not be a good idea. I will take a look at the tests closely.
Also, what is the reason you don't prefer granting permissions to the registry during testing?

My reason is that for me permissions 777 always look like a hack saying "we didn't care to think about defining correct permissions". It's not a good example of using Parsec even if it's only in a test container

Parsec book recommends to use parsec-clients group for client applications that wish to use the Parsec service: https://parallaxsecond.github.io/parsec-book/getting_started/installation_options.html

So, instead of using 777 on the registry in ci.sh I would prefer to see in the docker image

1. parsec-clients group is created
2. parsec-client-1 and parsec-client-2 added into the group
3. the registry permissions set to parsec-cleints group ownership and 660 (or 664) if required

[ionut-arm]

I think, if possible, using the group approach might be better. I think the tests we have which require multiple users are less about how the users can mess with each other, and more about showing that we can - in the Parsec service - facilitate user ID-based authentication. What the users can then do to each other outside of the service isn't really a concern (unless, of course, that has some implication on the actual tests).

[mohamedasaker-arm]

Hey @anta5010, @ionut-arm
I did the changes and enabled the docker build, and everything is passing now.
Please check it out and if no further comments I'll push the docker and then disable the docker build in this PR.

[anta5010]

LGTM