in octal notation that is 040777
(anyone can read/write/execute)
and 040555 - which indeed lacks write permissions
Thanks Robin… after a bit more research it seems that S_IWUSR is usually defined to be _S_IWRITE which (in Windows) had historically been defined as 0x0080 and (in Linux) as 0x0200 - so I’m wondering if Windows has recently brought itself in line and maybe that’s now screwing us up with our older version of glib?? I’ve no idea if that’s true. It’s a total guess.
I think you’re confusing octal and hexadecimal numbers. The write flag is “2”
octal 200 for user write (which is 0x0080 in hex)
Maybe, but in any case… if Microsoft had changed its mind about that value, why would it only be causing problems for one particular folder? It’s confusing alright 
Perhaps the solution is to test the .ardour session file instead?
Can you check what the other, later tests, return on Windows?
does g_access (p.c_str(), W_OK) return true?
Good suggestion (more about this later…)
The online descriptions for g_stat() and g_access() suggest that they’re only intended for 32-bit Windows and they recommend to use the official Microsoft versions. So I tested here both with 32-bit _stat and also _stat64 but the results and performance were identical to their glib equivalents - so I’d guess the documentation might be out of date.
As to your request Robin, if you’re suggesting this, it works fine here - both for OneDrive and non-OneDrive sessions:-
if (g_stat (p.c_str(), &statbuf) != 0) {
DEBUG_TRACE (DEBUG::FileUtils, string_compose("exists_and_writable stat '%1': failed\n", p));
/* doesn't exist - not writable */
return false;
} else {
DEBUG_TRACE(DEBUG::FileUtils, string_compose("exists_and_writable stat '%1': %2 \n", p, statbuf.st_mode));
if (!(statbuf.st_mode & S_IWUSR)) {
/* exists but is not thought to be writable - e.g. the filesystem may be
* mounted read-only, so even though file permissions permit access,
* the mount status does not. So test again using g_access():-
*/
if (g_access(p.c_str(), W_OK) != 0) {
DEBUG_TRACE(DEBUG::FileUtils, string_compose("exists_and_writable g_access '%1': !W_OK\n", p));
return false;
}
}
}
return true;
Admittedly I’m not a massive user of OneDrive so maybe incorporate into a nightly and ask @lennier or @gruust to test.
Yes, I have added 64bit compatibility when we did the 64bit windows release, years ago.
Getting output from that will require running a debug build of Ardour in a cmd.exe with the -DFileUtils parameter. I was likely overestimating the expertise of our Windows beta-testers 
@paul / @x42 - if the above change looks okay, let me know if you want me to commit it to Ardour git
The reason we test the directory is that we may need to create new files there. Testing only the session file itself doesn’t address whether or not we can do that.
I’m still interested what g_access(p.c_str(), W_OK) returns (when stat reports read-only).
On windows that uses _waccess under the hood.
Ah, I misread these changes. Yes, that would be valuable but needs the cleanup that Robin mentions and it would also be good just to output to std::cerr without DEBUG_TRACE
No surprise there. You effectively removed the use of g_stat() completely. Even if it fails or permissions are not S_IWUSR the function will return true; only relying on g_access on Unix systems now.
Huh?? What previously happened was that if g_stat() failed, the code was returning with FALSE and never bothering to test with g_access(). But if it had tested, g_access() would’ve succeeded (i.e. it would’ve returned 0). What I’ve done is to change this such that if g_stat() fails, the code will go on to test again with g_access() - and it’ll only return false if BOTH tests fail.
So you don’t rely on g_stat at all on Windows, you just pretend it succeeded even if it fails, and continue to check g_access() regardless. Maybe this can a workaround on Windows.
Have you verified that this works, and g_access() returns false on a read-only folder or read-only file-system (e.g. USB drive)?
–
PS. what’s worrying is that we use g_stat() in many places. Though in most cases jus for file modification time and/or file-size.
The basic problem is that g_stat() / stat() etc don’t seem to work properly with OneDrive folders. For non-OneDrive folders, g_stat() and g_access() both work as expected, although I haven’t tried folders on a USB drive (I could maybe try that tomorrow…)
Hi Robin. I ran some further tests this morning with a USB drive and the results for folders and files were identical to a normal drive (i.e. non-OneDrive). However, it reminded me of something that’s VERY important…
In Windows, folders don’t have a “read-only” flag. Yes you can right-click a folder and set it to be read-only but what actually happens is that Windows sets the read-only flag for all files within that folder. The folder itself always remains unchanged (in other words, folders are always deemed writable). In fact there’s a very good chance that when calling g_stat() it’ll likely give undefined results if you pass a folder - hence why g_access() reports folders as writable while g_stat() doesn’t. In fact even with POSIX, the g_stat() documentation suggests that it should only get used for files:-
So if Ardour needs to call ‘PBD::exists_and_writable()’ and pass a folder path, does this suggest that ‘PBD::exists_and_writable()’ should be sticking to g_access() and shouldn’t even call g_stat()?
In fact I just changed ‘PBD::exists_and_writable()’ to be this and it works successfully here for USB drives, normal drives and OneDrive:-
bool
exists_and_writable (const std::string & p)
{
if (g_access (p.c_str(), W_OK) != 0) {
DEBUG_TRACE (DEBUG::FileUtils, string_compose("exists_and_writable g_access '%1': !W_OK\n", p));
return false;
}
return true;
}
It works here for existing and non-existing .ardour files and it successfully detects if they’re writable or read-only. For a non-existent folder it correctly returns FALSE. And for an existing folder it always detects it as writable (i.e. correctly). So I wonder if those tests would work correctly for the non-Windows platforms?
@paul / @x42 - if you’re still not convinced, could we maybe release something like this to get some reactions…?
bool
exists_and_writable (const std::string & p)
{
#ifdef PLATFORM_WINDOWS
if (g_access (p.c_str(), W_OK) != 0) {
DEBUG_TRACE (DEBUG::FileUtils, string_compose("exists_and_writable g_access '%1': !W_OK\n", p));
return false;
}
return true;
#else
// <--- The existing code could go here
#endif
}
How about existing folders that are read-only? Either because the permission is unset, or it belongs to a different user, or is currently locked or because it’s a read-only (write protected) disk?
One we establish that those are correctly identified I’ll merge it.
So far it is safer to report a read-only session, rather than failing later when the user did some work and things fail actually writing new audio to the folder.
From testing here (and from my previous understanding) there’s no such thing as a write-protected folder on Windows. And the only disks I know that are write protected are CD’s / Blu-ray disks and the like. I doubt anyone would be expecting to edit Ardour sessions from either of those. Either way, the call (in Session::load_state() ) looks like this:-
_writable = exists_and_writable (xmlpath) && exists_and_writable(Glib::path_get_dirname(xmlpath));
So I’d expect the original “not writable” message to still get displayed. Unfortunately I don’t have a CD drive or Blu-ray drive any more so I can’t test that here - though maybe @lennier or @gruust could test it? (if it’s relevant)