Project

General

Profile

Task #4152

The Windows CI script is moving files around that CMake should be dealing with

Added by AnyOldName 3 10 days ago. Updated 5 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Code Maintenance
Target version:
Start date:
10/09/2017
% Done:

100%

Severity:
Normal

Description

At the end of the Windows CI script (/CI/before_script.msvc.sh), some resource files are copied from where CMake put them to the directories where the executables are actually put (the Debug and Release subdirectories of the build folder). In an ideal world, CMake would put them there. Someone could tweak things so this task is handled by CMake instead of the bash script.

Here's some incentive to do this potentially pointless-looking task:

For slightly convoluted reasons, if someone does this, it might actually allow me to tweak my workflow when I'm working on implementing shadows. Everyone likes shadows. I don't want to bury myself in CMake stuff if it's probably going to take longer than my time savings, but if someone else does it for me, then maybe the total delay before working shadows appear might be lower. I can't actually guarantee this, though, and I want to make that clear.

History

#1 Updated by Plutonic Overkill 10 days ago

One thing related to this that would be useful is to set it up so that as well as copying .dll files, it also copies debug information files, such as .exp or .pdb files. This would allow for using debug builds of external libraries such as OSG so that crashes related to these libraries can be debugged more easily.

#2 Updated by AnyOldName 3 10 days ago

That's sort of why I need this - my current workflow means that whenever I tweak the shaders, I have to rerun the CI script, and if I were to have OpenMW building against my own OSG builds, the CI script would revert it back to Ace's provided builds as it calls CMake with specific arguments to do this. If CMake handled copying the shaders, then Visual Studio would handle copying the shaders when it reran CMake, and everything would be fine.

I'm not 100% sure it would be helpful to just copy PDB files from Ace's builds, though, as they're not the hugest amount of use without the associated source code.

#3 Updated by Plutonic Overkill 9 days ago

Oh, I see what you mean now, I thought you were talking about copying .dll files. I'd be interested to know how you set up your own OSG builds to work with OpenMW, as I need to do this myself so that I can find out what's causing a bug with the terrain editing feature. Since Ace's builds don't contain any debugging information, I need to get OpenMW to use my own OSG builds with .pdb files. I was trying to work around this by packing the builds into .7z files and putting them where the build script normally expects to find them, but this would be very inconvenient if you wanted to modify the OSG source.

Also, it looks like the CMake files already do some copying. Copying resource files as well looks like it would be quite a simple fix, I can try to add it if I get time later today.

#4 Updated by Plutonic Overkill 9 days ago

Here is a commit to get CMake to copy resource files instead of the build script. However, I don't think this is the ideal option, since it has to run the entire CMake generation again every time you change a resource file. And keeping multiple copies of the same files around does not seem ideal. OpenMW (but seemingly not the CS or the other programs) has a --resources option to set the resources directory, couldn't the build be set up so that when debugging in VS, this option is used on the command line to set the actual resources directory?

#5 Updated by AnyOldName 3 9 days ago

CMake already copies the resources to the solution directory, just not to the Debug and release subdirectories of the solution directory. All the CI script was doing was this one last step.

I'm not even sure your commit would actually work the first time CMake was run, as I'm pretty sure the resource files are copied to the solution directory after the executables are added, so there wouldn't necessarily be any files to copy at the time you're doing the copy, and if there were, they'd be one version out of date.

Also, putting this functionality into OPENMW_ADD_EXECUTABLE is definitely silly as it means that it'll be done for every executable we produce. We only need the resources to be copied once per CMake run.

There're more things I have to say. I'm going to start using bullet points now.

  • The extra copying should only be done when using MSVC under Windows, not all the time.
  • If CMake is doing it, we don't need to echo. When I wrote that part of the script it was mostly so that users would see that it was the script and not CMake was the thing handling the resource files.
  • CMake is definitely the best thing to handle the resource files as VS automatically runs it every time a build is attempted.
  • Using the copy command you're using instead of something more CMakey probably means that either the command will be repeated every time CMake is run instead of just when it's needed or only the first time as CMake won't realise the build is dependent on those files and so won't realise it needs to refresh them when their source is changed.
  • You should probably just find the bits where CMake is already handling these files and wrap it in a macro or something that checks if we're generating for VS and if so, copies to a different directory.

#6 Updated by Plutonic Overkill 8 days ago

AFAIK, there is no way to run a command every CMake build without attaching it to a target, and since you can only attach a command to a single target, I had to put it in this macro so that it runs no matter which target is built. And this code is already enclosed in an if (MSVC) block so that it will only run if VS is being used.

CMake is definitely the best thing to handle the resource files as VS automatically runs it every time a build is attempted.

Is it really a good idea to require a rebuild every time a resource file is changed? I still don't get why these resource files need to be in the build directory at all. Using the --resources option as a command line parameter for OpenMW allows you to keep the resource files in the source directory so that no copying is required.

Using the copy command you're using instead of something more CMakey probably means that either the command will be repeated every time CMake is run instead of just when it's needed or only the first time as CMake won't realise the build is dependent on those files and so won't realise it needs to refresh them when their source is changed.

The command runs every time CMake runs as long as the source files have been modified, otherwise VS doesn't build anything so nothing gets copied. I don't think there is a way around this without resorting to some other approach.

#7 Updated by AnyOldName 3 7 days ago

Using the --resources option is a terrible idea. A few of the multitude of reasons why are below:

  • It only works for openmw.exe, not other things that might use the shaders.
  • It won't work if openmw.exe is called by openmw-launcher.exe, and changing the default working directory was done mostly so that you could expect openmw.exe to work when started through the launcher in Visual Studio.
  • You can't set the debugging arguments for a program via CMake unless you manually provide a default .vcxproj.user file, which is bad practice.
  • A programmer needs to know that this is what's going on when they're changing the arguments they call openmw.exe with.
  • A programmer suddenly can't actually investigate the behaviour of the --resources option if a bug is discovered.
  • Stuff that morally should be part of the build system is now infecting the build output.

In other news:

I've re-read your commit, and it isn't as bad as I thought it initially was. I'd not noticed that it was a post-build event, and I hadn't noticed it was in the IF (MSVC) block either. However, I still have some issues with it.

Are you certain that using cmake -E copy marks the project as dependent on the file, and will trigger a 'rebuild' if you attempt a debugging session when you change it? The resources are copied the first step of the way via configure_file, and this means that when I try and launch a debugging session from Visual Studio, even if openmw.exe itself doesn't need rebuilding, the resources are copied the first half of the way. This is because the openmw project is dependent on the ZERO_CHECK project, which, in turn, is dependent on the shader files. I'm not sure cmake -E copy adds the source files as a dependency of ZERO_CHECK.

You don't need to put the copy as a post-build event, as ZERO_CHECK should put it as a pre-debugging and pre-build event. You also don't need to explicitly do it for each project as they're all dependent on ZERO_CHECK anyway.

The good thing about this discussion is that in working out why I didn't like your commit, I think I've figured out a couple of better ideas:

It may be sufficient (and cleanest) to adjust the value of OpenMW_BINARY_DIR to contain <CONFIG>, and then fix everything that this breaks. I'm not sure, though, that this is actually possible, as I don't think everything supports generator strings, which <CONFIG> is. The appeal of this idea is that OpenMW_BINARY_DIR starts pointing at the same directory as the OpenMW binaries.

Alternatively, we can create a new macro wrapping configure_file specifically for copying resource files to where they're needed. This would check we're generating for VS, and if so, copy files to all the locations they're needed, and if not, it could do exactly what we already do. This could probably be combined with some tidying up, too. Currently, some of the resources are managed by the top-level CMake file, and others by ones in their source folders, which seems messy to me.

Despite making this issue report in an effort to get someone else to handle this for me, I now think I've actually done the bit I was trying to avoid, and now just have to type some stuff, so I'll do that.

#8 Updated by scrawl . 7 days ago

Is it really a good idea to require a rebuild every time a resource file is changed? I still don't get why these resource files need to be in the build directory at all.

The resource files are copied to the build folder to separate the build (and everything needed to run the build) from the source tree. I can then check out a different revision on the source tree, and still run the build I made earlier. If the build was dependent on a matching source revision still being checked out, that would probably lead to hard-to-debug errors and much confusion.

If, as developer, one is currently testing changes to resource files, it might be convenient to create a symlink of build/resources/ to resources/ so the one in build automatically updates from the source. Then it should not be needed to hit build every time a modification is made.

#9 Updated by AnyOldName 3 7 days ago

The PR I just submitted (https://github.com/OpenMW/openmw/pull/1498) doesn't require a rebuild every time the resources are changed. The resources are copied under the same circumstances as a change in source code would trigger a rebuild:

If you run an existing build outside of VS, it'll continue being the same build with the same resources as before.

If you attempt to debug an existing build in VS without changing the source or resources, nothing gets copied.
If you attempt to debug an existing build in VS changing the resources but not the source, the resources are copied, but no rebuild occurs.
If you attempt to debug an existing build in VS changing the source but not the resources, the resources are not copied, but a rebuild occurs. This maybe conflicts with what Scrawl just said was best, but it's how Visual Studio works, so there's nothing OpenMW can do about it.
If you attempt to debug an existing build in VS with both the source and resources changed, a rebuild is done and the resources are copied.

When debugging a build, it always reflects what's in the source tree, both with regards to source and resources, as this is what Visual Studio does. The PR makes it consistent for source and resources.

#10 Updated by Alexei Dobrohotov 7 days ago

  • Status changed from New to Resolved
  • Assignee set to AnyOldName 3
  • Target version set to openmw-0.43
  • % Done changed from 0 to 100

#11 Updated by Alexei Dobrohotov 5 days ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF