Cavern and Aven should throw a more-helpful error when directory is not writeable

[Eric C. Landgraf]

I spent more time than I'd like to admit trying to figure out why a walls dataset with no errors was erroring out with error: Couldn’t open file “<FILENAME>.3d” What was not clear is that it could not open it for WRITING, rather than for reading.

Also in unexpected behaviours: if the 3d file does exist but is not writeable, aven will successfully open the *old* file after failing to write one.

[Olly Betts]

Should be fairly easy to improve the message (we even already have a message Failed to open output file “%s”).

Also in unexpected behaviours: if the 3d file does exist but is not writeable, aven will successfully open the *old* file after failing to write one.

I can't seem to reproduce this (using current git main: 37909a2ab8629d256de1384e6fe4e19320c8b2b9):

$ mkdir unwritable
$ cd unwritable
$ cp ../tests/walls.srv .
$ echo BAD3D > walls.3d
$ chmod 444 walls.3d
$ ../src/aven walls.srv

The log window ends with:

../src/cavern: error: Couldn’t open file “walls.3d”

and has "Save Log" and "Reprocess" buttons.

Loading the 3d file directly with ../src/aven walls.3d pops up Bad 3d image file "walls.3d".

Trying to open walls.srv with File->Open crashes when built with sanitisers and valgrind shows some use of uninitialised values when handling paint events for aven's log window. I suppose it's conceivable that could manifest differently for you.

[Olly Betts]

Looking at improving the message, I noticed extend fails to check if it opened the output .3d file, and exits with status 0 so aven would open an existing extended .3d file in that case.

[Olly Betts]

Replying to Olly Betts:

Should be fairly easy to improve the message (we even already have a message Failed to open output file “%s”).

Fixed by b7a28bb3fdf24f2f75b51f843887a4ef3811f4ab (this is actually a regression introduced in 1.2.35).

Replying to Olly Betts:

I noticed extend fails to check if it opened the output .3d file, and exits with status 0 so aven would open an existing extended .3d file in that case.

Fixed by 7194bbcb65f4d5a655b55c7375765416970d0e7f. I checked that all other calls to open a 3d file for reading or writing report an error upon failure.

[Olly Betts]

In chat you said:

might have been that it could still write the extant file but couldn't write a new file. need to check some unix permissions obscura

Perhaps your situation is this

$ mkdir unwritable
$ cd unwritable
$ cp ../tests/walls.srv .
$ echo BAD3D > walls.3d
$ touch walls.err
$ chmod 555 .
$ ../src/aven walls.srv

which leads to aven processing the file, pausing because there are warnings, and upon "OK" it shows the newly processed data (if it actually showed the existing .3d file it would complain it would throw up an error dialog instead).

[Eric C. Landgraf]

that's exactly the case. which again, somewhat unexpected behaviour, but it's a pretty far-out edge case so maybe not critical to do anything about.

[Olly Betts]

that's exactly the case. which again, somewhat unexpected behaviour, but it's a pretty far-out edge case so maybe not critical to do anything about.

I don't really think there's anything wrong with this now. It might initially seem surprising that a writable file in a non-writable directory can be written, but even that doesn't seem unreasonable when you think about it. It's also standard behaviour of the operating system, and not something specific to Survex.

If the output files exist and are writable then Survex itself successfully processes the file, updates the output files, and loads the processed data, exactly as if the directory was writable. If the output files don't already exist or aren't writable, it fails with a sensible error.

We could write output to a temporary file and then rename. That has some pros and cons: pro: it can't leave the output partly written on a crash; con: it can leave a temporary file behind on a crash (on Unix-like platforms you can unlink the temporary file and only link it when it's fully written to have a very small window for this; AFAIK nothing similar is possible on Microsoft Windows). With this approach cavern would fail when the directory wasn't writable, but that doesn't really seem like a pro or a con. Overall I don't think it's worth changing the approach here, therefore closing.