JElchison / format-udf

Bash script to format a block device (hard drive or Flash drive) in UDF. The output is a drive that can be used for reading/writing across multiple operating system families: Windows, macOS, and Linux. This script should be capable of running in macOS or in Linux.
GNU General Public License v2.0
518 stars 48 forks source link

format-udf does not work without sudo #36

Closed tloimer closed 7 years ago

tloimer commented 7 years ago

The format-udf script makes the assumptions that (1) it is run as unprivileged user and (ii) sudo is available. I wished that format-udf also works without sudo, run as root.

JElchison commented 7 years ago

Hi @tloimer, thanks for reporting.

Are you willing to test my fix for this issue, using your environment? The fix exists in the develop branch, currently at 1ca76af. Travis result here.

The specific commit that implements your fix is 89835bc. Please review.

Please report back. Thanks!

tloimer commented 7 years ago

Great, works. I simply formatted again the same device that I had formatted before (where all occurences of "sudo" were deleted from the script).

However, I would suggest to add a warning if the script is not run as root, something along the lines of

diff -u a/format-udf.sh b/format-udf.sh --- a/format-udf.sh 2017-07-26 16:40:11.636131851 +0200 +++ b/format-udf.sh 2017-07-26 16:38:36.706030054 +0200 @@ -27,6 +27,10 @@

setup sudo

SUDO='' if [[ $(id -u) -ne 0 ]]; then

Otherwise, on my machine, if the script is mistakenly run as ordinary user, the output is $ ./format-udf.sh sdb GE [+] Validating arguments... [+] Testing dependencies... [+] Looking for drive info tool... [-] Dependencies unmet. Please verify that at least one of the following are installed, executable, and in the PATH: blockdev, ioreg [*] Exiting without changes to /dev/sdb

With the change as above, the output would be $ ./format-udf.sh sdb GE [-] Please run this script as root.

Thanks a lot.

JElchison commented 7 years ago

Thanks, @tloimer. I'm on board with your patch. It will be useful in cases where sudo is not available.

However, I want to ensure that I understand your scenario before merging in your patch. Concerning the following error message:

[-] Dependencies unmet.  Please verify that at least one of the following are installed, executable, and in the PATH:  blockdev, ioreg

The script is behaving as if blockdev is not in your unprivileged user's PATH, but it is in the root PATH. Is that what's happening?

Thanks!

tloimer commented 7 years ago

Yes, $su; which blockdev /sbin/blockdev

JElchison commented 7 years ago

Thanks, @tloimer.

It turns out that your recommended patch bypasses the core issue, but did not fix the core issue. The core issue is that the calls to which also need $(SUDO) if the usage of the tool uses $(SUDO). Can you please review b6aee7d, as well as 6690130?

Also, are you willing to re-test using your environment? The fix exists in the develop branch, currently at b6aee7d. Travis result here.

Thanks!

tloimer commented 7 years ago

Tested both. Both give now error messages which describe the issue in this case and propose a solution. Thanks a lot!

JElchison commented 7 years ago

Thanks again for reporting, and thanks for your help testing! I'll generate a new release shortly...