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

install: fix bug #1823 #1848

Merged
merged 6 commits into from
Mar 20, 2021
Merged

install: fix bug #1823 #1848

merged 6 commits into from
Mar 20, 2021

Conversation

nomius10
Copy link
Contributor

@nomius10 nomius10 commented Mar 20, 2021

TIL path manipulation in rust is intentionally basic due to security concerns.

fixes #1823

Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

Looks good but would it be great to add a test !

src/uu/install/src/install.rs Show resolved Hide resolved
- add a test for copying a file from one directory to another
- add the desired behavior

Fixes uutils#1823
Copy link
Contributor Author

@nomius10 nomius10 left a comment

Choose a reason for hiding this comment

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

For some reason I can't run any of the tests for install:

cargo test test_install_copy_file_leading_dot 
cargo test --bin=install --package=uu_install 
running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out 

I think that has to do with the fact that currently install is not included in the busybox-like binary

src/uu/install/src/install.rs Show resolved Hide resolved
src/uu/install/src/install.rs Show resolved Hide resolved
@nomius10 nomius10 requested a review from sylvestre March 20, 2021 14:17
@sylvestre
Copy link
Contributor

cargo test --features feat_os_unix
is a way to run tests

@nomius10
Copy link
Contributor Author

nomius10 commented Mar 20, 2021

I've snuck in a refactoring commit: I've renamed the file & directory names throughout the tests, since they gave a false impression that they all execute within the same directory (at_and_ucmd!() macro creates a new temporary directory for each individual test).

Please do tell if I should revert

@nomius10 nomius10 requested a review from sylvestre March 20, 2021 19:25
@sylvestre sylvestre merged commit 45acb08 into uutils:master Mar 20, 2021
@sylvestre
Copy link
Contributor

great, thanks :)

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.

install: does not remove path from files
3 participants