os: cleanup APIs returning !bool to either return ! or bool (#16111)

cmd/tools/vdoctor.v

@@ -257,8 +257,8 @@ fn (mut a App) report_info() {
 }
 
 fn is_writable_dir(path string) bool {
-	res := os.is_writable_folder(path) or { false }
+	os.ensure_folder_is_writable(path) or { return false }
-	return res
+	return true
 }
 
 fn main() {

vlib/io/util/util.v

@@ -19,7 +19,7 @@ pub fn temp_file(tfo TempFileOptions) !(os.File, string) {
 	if d == '' {
 		d = os.temp_dir()
 	}
-	os.is_writable_folder(d) or {
+	os.ensure_folder_is_writable(d) or {
 		return error(@FN +
 			' could not create temporary file in "$d". Please ensure write permissions.')
 	}
@@ -46,31 +46,28 @@ pub struct TempDirOptions {
 	pattern string
 }
 
+fn error_for_temporary_folder(fn_name string, d string) !string {
+	return error('$fn_name could not create temporary directory "$d". Please ensure you have write permissions for it.')
+}
+
 // temp_dir returns an uniquely named, writable, directory path
 pub fn temp_dir(tdo TempFileOptions) !string {
 	mut d := tdo.path
 	if d == '' {
 		d = os.temp_dir()
 	}
-	os.is_writable_folder(d) or {
+	os.ensure_folder_is_writable(d) or { return error_for_temporary_folder(@FN, d) }
-		return error(@FN +
-			' could not create temporary directory "$d". Please ensure write permissions.')
-	}
 	d = d.trim_right(os.path_separator)
 	prefix, suffix := prefix_and_suffix(tdo.pattern) or { return error(@FN + ' $err.msg()') }
 	for retry := 0; retry < util.retries; retry++ {
 		path := os.join_path(d, prefix + random_number() + suffix)
 		os.mkdir_all(path) or { continue }
 		if os.is_dir(path) && os.exists(path) {
-			os.is_writable_folder(path) or {
+			os.ensure_folder_is_writable(path) or { return error_for_temporary_folder(@FN, d) }
-				return error(@FN +
-					' could not create temporary directory "$d". Please ensure write permissions.')
-			}
 			return path
 		}
 	}
-	return error(@FN +
+	return error('${@FN} could not create temporary directory "$d". Retry limit ($util.retries) exhausted.')
-		' could not create temporary directory "$d". Retry limit ($util.retries) exhausted. Please ensure write permissions.')
 }
 
 // * Utility functions

vlib/io/util/util_test.v

@@ -81,11 +81,10 @@ fn test_temp_dir() {
 		return
 	}
 	assert os.is_dir(path)
-	mut writable := os.is_writable_folder(path) or {
+	os.ensure_folder_is_writable(path) or {
 		assert false
 		return
 	}
-	assert writable
 	mut prev_path := path
 	// Test pattern
 	path = util.temp_dir(
@@ -114,11 +113,10 @@ fn test_temp_dir() {
 	}
 	assert path != prev_path
 	assert os.is_dir(path)
-	writable = os.is_writable_folder(path) or {
+	os.ensure_folder_is_writable(path) or {
 		assert false
 		return
 	}
-	assert writable
 	assert path.contains(tfolder)
 	filename = os.file_name(path)
 	for c in filename {

vlib/os/dir_expansions_test.v

@@ -13,13 +13,12 @@ fn test_tmpdir() {
 	os.rm(tfile) or { panic(err) }
 }
 
-fn test_is_writable_folder() {
+fn test_ensure_folder_is_writable() {
 	tmp := os.temp_dir()
-	f := os.is_writable_folder(tmp) or {
+	os.ensure_folder_is_writable(tmp) or {
 		eprintln('err: $err')
-		false
+		assert false
 	}
-	assert f
 }
 
 fn test_expand_tilde_to_home() {

vlib/os/filelock/lib_nix.c.v

@@ -17,21 +17,19 @@ pub fn (mut l FileLock) unlink() {
 	C.unlink(&char(l.name.str))
 }
 
-pub fn (mut l FileLock) acquire() !bool {
+pub fn (mut l FileLock) acquire() ! {
 	if l.fd != -1 {
-		// lock already acquired by this instance
+		return error_with_code('lock already acquired by this instance', 1)
-		return false
 	}
 	fd := open_lockfile(l.name)
 	if fd == -1 {
-		return error('cannot create lock file $l.name')
+		return error_with_code('cannot create lock file $l.name', -1)
 	}
 	if C.flock(fd, C.LOCK_EX) == -1 {
 		C.close(fd)
-		return error('cannot lock')
+		return error_with_code('cannot lock', -2)
 	}
 	l.fd = fd
-	return true
 }
 
 pub fn (mut l FileLock) release() bool {
@@ -44,7 +42,7 @@ pub fn (mut l FileLock) release() bool {
 	return false
 }
 
-pub fn (mut l FileLock) wait_acquire(s int) !bool {
+pub fn (mut l FileLock) wait_acquire(s int) bool {
 	fin := time.now().add(s)
 	for time.now() < fin {
 		if l.try_acquire() {

vlib/os/filelock/lib_windows.c.v

@@ -15,17 +15,15 @@ pub fn (mut l FileLock) unlink() {
 	C.DeleteFileW(t_wide)
 }
 
-pub fn (mut l FileLock) acquire() !bool {
+pub fn (mut l FileLock) acquire() ! {
 	if l.fd != -1 {
-		// lock already acquired by this instance
+		return error_with_code('lock already acquired by this instance', 1)
-		return false
 	}
 	fd := open(l.name)
 	if fd == -1 {
-		return error('cannot create lock file $l.name')
+		return error_with_code('cannot create lock file $l.name', -1)
 	}
 	l.fd = fd
-	return true
 }
 
 pub fn (mut l FileLock) release() bool {
@@ -39,7 +37,7 @@ pub fn (mut l FileLock) release() bool {
 	return false
 }
 
-pub fn (mut l FileLock) wait_acquire(s int) !bool {
+pub fn (mut l FileLock) wait_acquire(s int) bool {
 	fin := time.now().add(s)
 	for time.now() < fin {
 		if l.try_acquire() {

vlib/os/os.js.v

@@ -177,3 +177,10 @@ pub fn file_last_mod_unix(path string) int {
 
 	return mtime
 }
+
+pub fn ensure_folder_is_writable(path string) ! {
+	fpath := join_path(path, 'some_newfile')
+	#try { $fs.writeFileSync(fpath); $fs.unlinkSync(fpath) } catch(e) { return error(new string('could not write to ' + path)) }
+
+	_ := fpath
+}

vlib/os/os.v

@@ -856,3 +856,9 @@ pub fn config_dir() !string {
 	}
 	return error('Cannot find config directory')
 }
+
+[deprecated: 'use os.ensure_folder_is_writable instead']
+pub fn is_writable_folder(folder string) !bool {
+	ensure_folder_is_writable(folder)!
+	return true
+}

vlib/os/os_js.js.v

@@ -1,15 +1,15 @@
 module os
 
-pub fn mkdir(path string, params MkdirParams) !bool {
+pub fn mkdir(path string, params MkdirParams) ! {
 	$if js_node {
 		if path == '.' {
-			return true
+			return
 		}
 		#$fs.mkdirSync(path.valueOf())
 
-		return true
+		return
 	} $else {
-		return false
+		return error('could not create folder')
 	}
 }
 

vlib/os/os_nix.c.v

@@ -281,51 +281,16 @@ pub fn ls(path string) ![]string {
 	return res
 }
 
-/*
-pub fn is_dir(path string) bool {
-	//$if linux {
-		//C.syscall(4, path.str) // sys_newstat
-	//}
-	dir := C.opendir(path.str)
-	res := !isnil(dir)
-	if res {
-		C.closedir(dir)
-	}
-	return res
-}
-*/
-
 // mkdir creates a new directory with the specified path.
-pub fn mkdir(path string, params MkdirParams) !bool {
+pub fn mkdir(path string, params MkdirParams) ! {
 	if path == '.' {
-		return true
+		return
 	}
-	/*
-	mut k := 0
-	defer {
-		k = 1
-	}
-	*/
 	apath := real_path(path)
-	// defer {
-	// apath.free()
-	//}
-	/*
-	$if linux {
-		$if !android {
-			ret := C.syscall(sys_mkdir, apath.str, params.mode)
-			if ret == -1 {
-				return error(posix_get_error_msg(C.errno))
-			}
-			return true
-		}
-	}
-	*/
 	r := unsafe { C.mkdir(&char(apath.str), params.mode) }
 	if r == -1 {
 		return error(posix_get_error_msg(C.errno))
 	}
-	return true
 }
 
 // execute starts the specified command, waits for it to complete, and returns its output.
@@ -423,18 +388,18 @@ pub fn (mut c Command) close() ! {
 	}
 }
 
-pub fn symlink(origin string, target string) !bool {
+pub fn symlink(origin string, target string) ! {
 	res := C.symlink(&char(origin.str), &char(target.str))
 	if res == 0 {
-		return true
+		return
 	}
 	return error(posix_get_error_msg(C.errno))
 }
 
-pub fn link(origin string, target string) !bool {
+pub fn link(origin string, target string) ! {
 	res := C.link(&char(origin.str), &char(target.str))
 	if res == 0 {
-		return true
+		return
 	}
 	return error(posix_get_error_msg(C.errno))
 }
@@ -449,14 +414,6 @@ pub fn (mut f File) close() {
 		return
 	}
 	f.is_opened = false
-	/*
-	$if linux {
-		$if !android {
-			C.syscall(sys_close, f.fd)
-			return
-		}
-	}
-	*/
 	C.fflush(f.cfile)
 	C.fclose(f.cfile)
 }
@@ -475,14 +432,15 @@ pub fn debugger_present() bool {
 
 fn C.mkstemp(stemplate &u8) int
 
-// `is_writable_folder` - `folder` exists and is writable to the process
+// ensure_folder_is_writable checks that `folder` exists, and is writable to the process
+// by creating an empty file in it, then deleting it.
 [manualfree]
-pub fn is_writable_folder(folder string) !bool {
+pub fn ensure_folder_is_writable(folder string) ! {
 	if !exists(folder) {
-		return error('`$folder` does not exist')
+		return error_with_code('`$folder` does not exist', 1)
 	}
 	if !is_dir(folder) {
-		return error('`$folder` is not a folder')
+		return error_with_code('`$folder` is not a folder', 2)
 	}
 	tmp_perm_check := join_path_single(folder, 'XXXXXX')
 	defer {
@@ -491,12 +449,11 @@ pub fn is_writable_folder(folder string) !bool {
 	unsafe {
 		x := C.mkstemp(&char(tmp_perm_check.str))
 		if -1 == x {
-			return error('folder `$folder` is not writable')
+			return error_with_code('folder `$folder` is not writable', 3)
 		}
 		C.close(x)
 	}
 	rm(tmp_perm_check)!
-	return true
 }
 
 [inline]

vlib/os/os_test.v

@@ -469,7 +469,7 @@ fn test_realpath_absolutepath_symlink() {
 	file_name := 'tolink_file.txt'
 	symlink_name := 'symlink.txt'
 	create_file(file_name)!
-	assert os.symlink(file_name, symlink_name)!
+	os.symlink(file_name, symlink_name)!
 	rpath := os.real_path(symlink_name)
 	println(rpath)
 	assert os.is_abs_path(rpath)

vlib/os/os_windows.c.v

@@ -190,16 +190,15 @@ pub fn ls(path string) ![]string {
 }
 
 // mkdir creates a new directory with the specified path.
-pub fn mkdir(path string, params MkdirParams) !bool {
+pub fn mkdir(path string, params MkdirParams) ! {
 	if path == '.' {
-		return true
+		return
 	}
 	apath := real_path(path)
 	if !C.CreateDirectory(apath.to_wide(), 0) {
 		return error('mkdir failed for "$apath", because CreateDirectory returned: ' +
 			get_error_msg(int(C.GetLastError())))
 	}
-	return true
 }
 
 // Ref - https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle?view=vs-2019
@@ -378,7 +377,7 @@ pub fn raw_execute(cmd string) Result {
 	}
 }
 
-pub fn symlink(origin string, target string) !bool {
+pub fn symlink(origin string, target string) ! {
 	// this is a temporary fix for TCC32 due to runtime error
 	// TODO: find the cause why TCC32 for Windows does not work without the compiletime option
 	$if x64 || x32 {
@@ -397,12 +396,12 @@ pub fn symlink(origin string, target string) !bool {
 		if !exists(target) {
 			return error('C.CreateSymbolicLinkW reported success, but symlink still does not exist')
 		}
-		return true
+		return
 	}
-	return false
+	return error('could not symlink')
 }
 
-pub fn link(origin string, target string) !bool {
+pub fn link(origin string, target string) ! {
 	res := C.CreateHardLinkW(target.to_wide(), origin.to_wide(), C.NULL)
 	// 1 = success, != 1 failure => https://stackoverflow.com/questions/33010440/createsymboliclink-on-windows-10
 	if res != 1 {
@@ -411,7 +410,6 @@ pub fn link(origin string, target string) !bool {
 	if !exists(target) {
 		return error('C.CreateHardLinkW reported success, but link still does not exist')
 	}
-	return true
 }
 
 pub fn (mut f File) close() {
@@ -494,19 +492,21 @@ pub fn loginname() string {
 	return unsafe { string_from_wide(&loginname[0]) }
 }
 
-// `is_writable_folder` - `folder` exists and is writable to the process
+// ensure_folder_is_writable checks that `folder` exists, and is writable to the process
-pub fn is_writable_folder(folder string) !bool {
+// by creating an empty file in it, then deleting it.
+pub fn ensure_folder_is_writable(folder string) ! {
 	if !exists(folder) {
-		return error('`$folder` does not exist')
+		return error_with_code('`$folder` does not exist', 1)
 	}
 	if !is_dir(folder) {
-		return error('`folder` is not a folder')
+		return error_with_code('`folder` is not a folder', 2)
 	}
 	tmp_folder_name := 'tmp_perm_check_pid_' + getpid().str()
 	tmp_perm_check := join_path_single(folder, tmp_folder_name)
-	write_file(tmp_perm_check, 'test') or { return error('cannot write to folder "$folder": $err') }
+	write_file(tmp_perm_check, 'test') or {
+		return error_with_code('cannot write to folder "$folder": $err', 3)
+	}
 	rm(tmp_perm_check)!
-	return true
 }
 
 [inline]

vlib/v/builder/compile.v

@@ -31,7 +31,7 @@ fn check_if_output_folder_is_writable(pref &pref.Preferences) {
 	if odir.len == pref.out_name.len {
 		output_folder = os.getwd()
 	}
-	os.is_writable_folder(output_folder) or {
+	os.ensure_folder_is_writable(output_folder) or {
 		// An early error here, is better than an unclear C error later:
 		verror(err.msg())
 	}