← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles into lp:widelands


1. Regarding the autosave stuff:

This wasn't anything this branch tried to solve. It only solved the the conflicts that arose from trying to modify files where the map fs pointed to, resulting in either some crash or error (if the file was currently locked) or resulting in corrupt savefiles.

Those other file conflicts (like trying to modify a savefile that is locked because you have it open with 7zip, or having a file write-protected or some such) are solved with the latest branch: https://code.launchpad.net/~widelands-dev/widelands/robust-saving

As for trying to specifically test "is_writable" instead of "file_exists" (or other potentially problematic file states) beforehand, I don't think it's worth it. For one thing, we might have to deal with the specifics of the various access control models on the different platforms. (Just looking at this stuff makes me nauseaous.) There is also the more general problem with any filesystem stuff that you can never be sure that the file state you just checked is still the same when you try your file operation next. Sure, it's very likely, but there can always be race conditions, and some other process might just have snatched the file away between your state check and your attempt to operate on a file.
So aside from a few minor exist checks I favour a simple, practical approach: just try the file operation, but catch and handle any errors. The error codes give a hint about what the actual problem was, but usually that doesn't matter much anyway: either it worked and we continue or it didn't and we abort.
Anyway, the latest branch https://code.launchpad.net/~widelands-dev/widelands/robust-saving does exactly that and handles file errors in all the map/game saving routines and cleanup routines. I still need to check in what other places we don't catch file errors but should. (Definitely when deleting files in the load/save menu, maybe synchstream creation, probably some other places.) That's going to be part of future branches though.

2. Those travis build errors

I hadn't actually seen this before. (I'm still a noob on Launchpad and don't know my way around very well, but I am trying to catch up. If there are any important features I might not but should know about, feel free to point something out.)

Anyway, those errors seem like minor nitpicks. I'll fix them.
Btw, are those builds made automatically here for any new branches? Or do I have to initiate them manually? (I haven't checked out the online building possibilities yet. Still on my todo list.)

Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles into lp:widelands.