Update cp command

[AbelA72]

• Add a flag for recursively copying directories
• Add progress bars that show the file size and then update it as chunks are copied
• Add a flag to print a message telling the user the file name and destination that just got copied

[jcmendoza2]

I can code review this.

[n-t-eastham]

I'll code review this project.

[n-t-eastham]

I'm currently experiencing a make error due to a missing target f2 required for fs.img. Could you please check the Makefile for any rules related to these components? Additionally, some documentation on how to run the command and what expected output should look like would be appreciated.

[n-t-eastham]

High Level Checks:

• [✓] PR fully resolves the task/issue
• [✓] Does the PR have tests?
• [✓] Does the PR follow project code formatting standards?
• [✓] Does the OS still compile and run?
• [✗] Is documentation included?

Code Checks:

• [✓] Does the code achieve its intended purpose?
• [✗] Were all edge cases considered?
• [✓] Is the code organized and maintainable?
• [✓] Does the code maintain the same level of performance? (no performance regressions)

The request resolves each proposed task from Issue #68, however, the default functionality of copy makes the -v redundant. While the request includes test files they only contain a few words each. With that being said, when adding more robust files to the OS to copy no bugs were found. I saw no issue in the project code formatting standards. The OS still compiles and runs only if a small adjustment is made to the make file. While the code provides help comments within files, the project lacks any formal documentation. The code absolutely achieves its intended purpose. While most edge cases where considered, hard coding the buffers used in copy.c could cause some issues. For the most part the code is organized and maintainable, stylistically some logic could be removed from copy_directory and placed into helper functions. Also, mkfifo.c could be removed because it serves no purpose. The code doesn't technically maintain the same level of performance as the linux command but I believe this is because you are displaying the byte chucks copied. If this where something omitted from the default functionality, I believe there would be no noticeable performance regressions.

[jcmendoza2]

High Level Checks:

[✓] PR fully resolves the task/issue
[✓] Does the PR have tests?
[✓] Does the PR follow project code formatting standards?
[✓] Does the OS still compile and run? // Yes if you make changes to the Makefile
[✓] Is documentation included?

Code Checks:

[✓] Does the code achieve its intended purpose?
[ ] Were all edge cases considered?
[✓] Is the code organized and maintainable?
[✓] Does the code maintain the same level of performance? (no performance regressions)

Hi, this is a good implementation towards updating the cp command. The PR does achieve the suggested purpose specified in the PR outline. However, while there is some documentation outlining what a function does and intends to accomplish, I would suggest more in depth documentation depicting the functions along with guidance on how to test the PR: like /copy [flag] [source_destination] or how to create many directories in QEMU in order to test directories that contain sub-directories etc. It was challenging as a first time user to navigate and figure out how to test the program accordingly so adding more robust documentation would be helpful. The code will compile only after making a few changes to the Makefile such as rm f2 in fs.img and mkfs/mkfs. And in agreement with the other reviewer, a possible suggestion would be to remove mkfifo.c entirely as the entire file is commented out and is serving no purpose to the implementation (no dependence).

A few suggestions:
During testing I tried:
/copy r (invalid format of flag) test.txt
Result: cp: could not stat r
suggestion: cp: could not stat r: invalid flag provided: -r or -v
/copy -r helloooo.txt (file doesn't exist and no directory specified)
Result: cp: could not stat helloooo.txt
/copy -v hello.txt (file doesn't exist)
Result: exec /copy failed
Suggestion: verbose failed, invalid [filename or filepath] provided

The main() for argument parsing seems to be assuming a specific order of arguments
/copy other.txt -t
Result: Copying: 100 bytes out of total
/copy other.txt -v
Result: Copying: 100 bytes out of total

Buffer Size Inconsistency:
The buffer size in copy.c is buf[2048] but in copy_directory() the buf is buf[512]. I would suggest a consistent buffer size to mitigate any passing problems (if directory paths are longer than 512) ensuring that the directory paths in copy_directory() aren't surpassing the buffer size when adding to directory entry names.

Besides a few user output guidance suggestions and documentation on how to thoroughly test the PR, great work!

[malensek]

There is great feedback above, you should take all of it into account if you resubmit.

• There are several parts where your error messages are going to stderr (great!) but a few say "ls" instead of "copy". Also, maybe rename your utility cp or update the messages?
• The buffer thing definitely needs to be updated to be consistent.
• When verbose is turned off, you shouldn't write however many bytes you've written thus far.
• Recursive implies you'll recurse into subdirectories, but this will only go 1 directory deep.

You can get +1 back to your project score with the updates listed above.