Do not write file in `io.writefile` if contents hasn't changed

[p0358]

What does this PR do?

io.writefile utility function always writes the file it's been given, even if the contents didn't change.

This is problematic for build systems that check file timestamps in order to determine whether a particular file and whatever depends on it should be rebuilt.

In my particular case, there was a premake build step ran before every build to generate primary version file. But to do so, premake first had to run to parse all workspaces, and scripts in there would generate some files within project on their own (transforming .h.in to .h etc.). This unfortunately caused parts of these dependent projects to rebuild every single time, despite no changes in them.

How does this PR change Premake's behavior?

No breaking changes. Files will still be written if different or not existing yet. It will help build systems to rebuild less files when not needed.

Besides, there is no reason that I see to write a file and update its timestamp on drive if the contents hasn't changed, thus I didn't provide an option to forcefully write it out. If someone really wants that, they can use io.open manually...

Anything else we should know?

This is consistent with main logic for generating project files: files won't get written to drive if their contents hasn't changed. So this PR should make this behavior more consistent for if someone uses io.writefile function somewhere. Plus it makes for cleaner code than if someone was to check for this manually.

I was considering using a+b file mode, then seek to beginning to read the file, in order to avoid creating two file handles in the process. Unfortunately I'm not aware of a way for Lua to truncate the file then to write out the new contents. So the current way I think is better, simpler, more readable...

Did you check all the boxes?

• [X] Focus on a single fix or feature; remove any unrelated formatting or code changes
• [ ] Add unit tests showing fix or feature works; all tests pass
• [ ] Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
• [X] Follow our coding conventions
• [X] Minimize the number of commits
• [ ] Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

[p0358]

Only after writing out the PR and looking at docs, I've noticed that apparently os.writefile_ifnotequal function exists: https://github.com/premake/premake-core/blob/7b309649ce08cb40f4947d6952fb554c94f8cff6/website/docs/os.writefile_ifnotequal.md

Therefore I ask maintainers what should be done about it, it seems to be a bit out of place to have that separately in os, where there's no other similar funcs there, plus the order of arguments in this func is different...

I see that os.writefile_ifnotequal is implemented in native code, however it doesn't avoid the issue of double opening+closing of file handles either.

[nickclark2016]

Only after writing out the PR and looking at docs, I've noticed that apparently os.writefile_ifnotequal function exists: https://github.com/premake/premake-core/blob/7b309649ce08cb40f4947d6952fb554c94f8cff6/website/docs/os.writefile_ifnotequal.md

Therefore I ask maintainers what should be done about it, it seems to be a bit out of place to have that separately in os, where there's no other similar funcs there, plus the order of arguments in this func is different...

I could see an argument being made to have a function in I/O that calls this native function. The I/O lib is just an extension of the Lua IO namespace. There are a few file related functions in the OS namespace. OS is just functions that we expose from the OS, such as checking file equality, touching files, etc.

I see that os.writefile_ifnotequal is implemented in native code, however it doesn't avoid the issue of double opening+closing of file handles either.

If you think this is a bug, I would say change this PR to fix that and expose it in the I/O namespace, rather than modifying the io.writefile function (as the behavior to always write to the file may be relied upon by users).

[p0358]

If you think this is a bug

It could fall into that category, given it could technically cause race condition between the two openes and closes of handle (during which some other process could change the contents). In any case I think it's currently not ideal and not really better in any way than pure-Lua solution. So I definitely think the two options are to either improve this or remove it altogether.

rather than modifying the io.writefile function (as the behavior to always write to the file may be relied upon by users).

While I see the point, at the same time I'm not sure if I can imagine reasonable scenarios in which some workflow would be broken by that... (The change should be clearly documented though if this was made, to resolve any confusion someone could possibly have)

I would say change this PR to fix that and expose it in the I/O namespace

So to have io.writefile_ifnotequal as new function basically just calling os.writefile_ifnotequal but with reversed argument order to match up io.writefile?

I'm not 100% sure here though, having just a single io.writefile and making it write only if contents are different would be consistent with how projects are generated already, which is why I still think it'd be reasonable to make that the default (possibly by redirecting it to os.writefile_ifnotequal after making it reuse a single file handle?)

[nickclark2016]

If you think this is a bug

It could fall into that category, given it could technically cause race condition between the two openes and closes of handle (during which some other process could change the contents). In any case I think it's currently not ideal and not really better in any way than pure-Lua solution. So I definitely think the two options are to either improve this or remove it altogether.

The native function would be faster than the Lua solution, so that is a downside. We want file IO to be relatively fast. Removal isn't an option, as it would break existing code.

rather than modifying the io.writefile function (as the behavior to always write to the file may be relied upon by users).

While I see the point, at the same time I'm not sure if I can imagine reasonable scenarios in which some workflow would be broken by that... (The change should be clearly documented though if this was made, to resolve any confusion someone could possibly have)

Hyrum's Law comes into play here. While we make no guarantees in our API about file modification if the contents are equivalent, there are likely people relying on this behavior (just given the number of users of Premake).

I would say change this PR to fix that and expose it in the I/O namespace

So to have io.writefile_ifnotequal as new function basically just calling os.writefile_ifnotequal but with reversed argument order to match up io.writefile?

I'm not 100% sure here though, having just a single io.writefile and making it write only if contents are different would be consistent with how projects are generated already, which is why I still think it'd be reasonable to make that the default (possibly by redirecting it to os.writefile_ifnotequal after making it reuse a single file handle?)

Again, I'm not convinced that the correct approach at this time is to change underlying behavior of a well established function. I could be convinced to add another, optional parameter that allows you to to an equality check before writing, instead of adding a separate function.

[samsinsane]

Again, I'm not convinced that the correct approach at this time is to change underlying behavior of a well established function. I could be convinced to add another, optional parameter that allows you to to an equality check before writing, instead of adding a separate function.

Sorry, pretty late to the party, but I mostly agree with this. However, the native code for os.writefile_ifnotequal should be moved to io.writefile_ifnotequal flipping the inputs, then os.writefile_ifnotequal should be a deprecated function that calls io.writefile_ifnotequal with the inputs flipped. Then at some future date we can remove os.writefile_ifnotequal. It's also worth remembering not everyone is writing out a tiny file with io.writefile, some people write out much larger files or overwrite much larger files, and doing contents == io.readfile(filename) would be significantly slower for these users.