Page MenuHome

Save_pre handlers are not triggered if file is read-only
Closed, ArchivedPublic

Description

System Information
Operating system: Windows 10 Pro
Graphics card: GeForce RTX 2080

Blender Version
Broken: 2.92.0, 02948a2cab44, master, 2021-02-24 16:25
Worked: I would not know if this used to work.

Short description of error
Hopefully, I am approaching something the wrong way here, but it seems that there might be a small oversight concerning the timing at which the save_pre handlers are processed. If a Blender file is read-only, the save_pre handlers will not be triggered because the Cannot save blend file, path 'C:\my\blender\file.blend' is not writable error is raised before that can happen.

As a result, a mechanism that unlocks a file before saving it cannot be implemented.
In my case, this is making it impossible to create an add-on that will integrate versioning systems such as Perforce or Git into Blender.

I suppose there are cases where you would not want to trigger the save_pre routine unless a file was actually writeable, but checking that should probably be the responsibility of whoever implements the handler.

Exact steps for others to reproduce the error

  • Open Blender.
  • Run the following Python script using the Text Editor:
import bpy
from bpy.app.handlers import persistent


@persistent
def save_pre_handler(*args, **kwargs):
    print("Doing something before saving.")
    

bpy.app.handlers.save_pre.append(save_pre_handler)
  • Open the system console (Window > Toggle System Console).
  • Save the file (File > Save).
  • You should see Doing something before saving. printed in the system console.
  • Make the file read-only (Right-click > Properties > Check Read-only checkbox).
  • Save the file again (File > Save).
  • Blender will raise an error and Doing something before saving. will not be printed in the system console.

Thanks in advance for your consideration.

Event Timeline

Falk David (filedescriptor) changed the task status from Needs Triage to Confirmed.Tue, May 25, 1:11 PM

Looked into this. The code calls the callback after checking the following:

  • The filepath is not empty
  • The filepath is not longer than FILE_MAX
  • The file does not exists and is not read-only
  • The file does not override a file in bmain->libraries.

I can see that this is a problem, because you might want to detect these conditions in the callback like you mentioned.

Since the code is working as intended, I will close this for now.
As it is right now, error handling is done before calling the callback handlers.

Since the code is working as intended this now falls under feature requests. Where is the right place to post these?

With more and more people working using VCS that tend to be designed around file locking for collaboration on binary files, addressing this feature would allow for nice integrations into Blender. I understand what @Campbell Barton (campbellbarton) is saying about providing a guaranteed pre and post save pair at all times as well as backward compatibility with previously written handlers, nevertheless, any sort of opinionated code like this comes with limitations such as the one I have described in this post. There is a case for a save_pre that does exactly what it says while documentation explains the potential safety checks that the Python developers should implement. That said, I would be equally happy with another handler that runs before the current safety checks. The current workaround of creating a custom save operator and overriding shortcuts is not very elegant.

As far as I know perforce is the only popular version control which relies on file locking.
IIRC unlocking a file is in is meant to be an explicit use action, so having to support this within every application that needs to save files feels like you're working against the design of the system you're using.

Having said that, we could have extra handlers to support this kind of thing since some use cases for them exist (it's not limited to unlocking files on save).

So for any action we could have:

  • {action}_init run before the action takes place (this is what you'd need).
  • {action}_pre run immediately before the action.
  • {action}_post run immediately after the action.
  • {action}_cancel runs if the action failed to run (could also be called error or fail).

This follows what we already have for the render handler.

I would accept a patch that implements these for save and load.

Thanks for looking into this further and very excited about the perspective of these new handlers.

To add to your comments, Git LFS is Git's solution to match Perforce's workflow benefits. The project is getting quite a lot of traction in the game industry and it does more than handling binaries well, with the addition of exclusive locks on files resulting in restricted writing permission on the local files.

Concerning the explicit unlocking of files, software like Unreal add additional dialogs when trying to save a file. Users will be prompted to "check out" the file before it's saved. Alternatively, it will inform them that a file cannot be saved because someone else has it checked out.

The new save_init handler should allow implementing these behaviors with the desired amount of prompting.

Git LFS is a 3rd party program, not part of a typical GIT workflow. Nevertheless it's an example that uses locking.

Using handlers still seems like an awkward solution. As far as I can see it won't work when using "Save As" since Python doesn't have access to the target path in that case. Support for this can be added but as is this doesn't seem like it's a complete solution for the problem you're looking to solve.

Is this likely to be a problem? Or is this functionality not needed when using save-as?

Save as a new file would not be an issue, but save as an existing file that is locked would be. I wish the handler was passed data such as previous_filename, new_filename as it would allow handling "save as "properly. That said, this happening more rarely, so one can probably live without it.

Note that Blender can save other kinds of files besides blend files (operators that export, saving images, saving a blend file may request saving modified images, Blender can also have external cache for physics and sculpting.

If it's a quick hack you doing your own system for convenience that's fine as a stop-gap, from a user perspective I think this is going to be unreliable.

For exports such as Alembic or FBX, because Blender does not retain export options from one session to the next, the best solution is still to create custom wrapper operators. In turn, those can take care of any "unlocking" before the file is written. It is my understanding that save handlers are not called on export and that there are currently no export handlers. That could be a nice addition for a deeper integration of VCS, but ultimately it's outside of the scope of this original task (see D6703).

What I am trying to get to is that there are clear benefits (not having to worry about export settings) for users to use a custom one-click export button instead of going through File > Export. On the other hand, having to use a custom save button instead of going through the traditional Ctrl + S or File > Save is seen as an annoyance. That's where handlers come into play in my opinion.

So I would not say that I am trying to quick hack this, more so trying to integrate unlocking where it makes the most sense for my users.
I'd love to have better support for "save as" of course. In that regard, is it out of the question to pass data to the handlers? I guess that would compromise backward compatibility with any handler ever written.