Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if Godot executable is available to be written to before building #3926

Open
AaronRecord opened this issue Feb 6, 2022 · 6 comments · May be fixed by godotengine/godot#57767
Open

Comments

@AaronRecord
Copy link

AaronRecord commented Feb 6, 2022

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

When working on a feature, the workflow I usually go through looks something like this:

  1. Make changes
  2. Build
  3. Run built executable
  4. Make more changes/fixes as necessary, go back to step 3

A few too many times Sometimes I'll forget to close the open instance of Godot, and after waiting a few minutes for a rebuild, I'll end up with a message like this:
image

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Before starting to build, check if the resulting build file (e.g. bin\godot.windows.tools.64.exe) is available to be written to, and if it isn't, then don't build and print an error right away.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Not super familiar with scons but I'm guessing it'd just be something like
if os.access("bin\godot.windows.tools.64.exe", os.W_OK): ...
(https://stackoverflow.com/questions/3696080/check-if-i-can-write-a-file-to-a-directory-or-not-with-python,
https://docs.python.org/3/library/os.html#os.access)

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

Build-system is core

@AaronRecord AaronRecord changed the title Check if file is available Check if Godot executable is available to be written to before building Feb 6, 2022
@AaronRecord AaronRecord changed the title Check if Godot executable is available to be written to before building Check if Godot executable is available to be written to *before* building Feb 6, 2022
@AaronRecord AaronRecord changed the title Check if Godot executable is available to be written to *before* building Check if Godot executable is available to be written to before building Feb 6, 2022
@YuriSizov
Copy link
Contributor

Yeah, but if we change this, I will no longer be able to start the build, and then use the build time to close the running instance 😞

@AaronRecord
Copy link
Author

AaronRecord commented Feb 7, 2022

Yeah, but if we change this, I will no longer be able to start the build, and then use the build time to close the running instance 😞

Maybe it should print a warning at the start if the executable can't be written to? And maybe it can add a number to the end of the produced executable so it won't interfere with the existing one, like godot.windows.tools.64.1.exe. Then you can keep testing the old build while the new one is building without worrying about it getting cancelled.

@Xrayez
Copy link
Contributor

Xrayez commented Feb 8, 2022

This is always an issue on Windows (Windows user here).

Does it affect Linux/macOS?

There are some cases when you can build Godot while the editor is running (if you don't work on the editor itself). I've used to build Godot from within WSL (Windows Subsystem for Linux) and had no I/O access issues somehow. I really wonder what happens there.

And maybe it can add a number to the end of the produced executable so it won't interfere with the existing one,

Resolving this issue by appending an extra suffix is no-go because you'll be trashing all the object files and this will lead to full rebuild. Unless there's a way to reuse the cache, of course. But even then, it's better to have a fixed prefix for other purposes (like launching the debugger which requires an exact path to the executable).

But printing a warning could certainly be useful (without erroring out). Sometimes I'm fast enough to close the editor before the link operation starts. With the warning in place, I'll be able to close the editor myself almost every time (since on average it takes around 10-20 seconds to even parse SCons build scripts). That said, interrupting the build is easy with Ctrl + C once you know about this fact.

This suggests that perhaps SCons could automatically close the editor for you (optionally). Except for cases when you have unsaved scenes, so killing a process would lead to loss of data, but this is usually not an issue for development purposes.

@AaronRecord
Copy link
Author

AaronRecord commented Feb 8, 2022

@Xrayez good point about the number suffix. I don't know that if it's a good idea to automatically close the editor. I'm not familiar enough with how the build system works to say if this would work or not, but theoretically couldn't it build with the correct path and then before saving the finished build to a file, check if the file is accessible, and if it's not then add a number suffix onto the file name afterwards. Essentially, wait to add the suffix and print an error until after everything has built. Then the user could manually correct the file name as necessary, I know I'd personally rather have to do that than rebuild it from scratch.

@Xrayez
Copy link
Contributor

Xrayez commented Feb 8, 2022

I see, I thought that the suffix is modified before build.

Essentially, wait to add the suffix and print an error until after everything has built. Then the user could manually correct the file name as necessary

I think it's more intuitive to attempt to rebuild than to rename. Imagine another issue: the build finished successfully but then you launch the old executable when you forget to rename.

But I think that the fallback approach of appending the suffix will not interfere with anything.

Printing the warning would still be essential in either case.

@AaronRecord
Copy link
Author

I think it's more intuitive to attempt to rebuild than to rename. Imagine another issue: the build finished successfully but then you launch the old executable when you forget to rename.

Maybe it could be a setting? I think as long as the error message is visible enough it should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants