Issues/0111 ctlrender pickup module_name function (#111)

[ThomasWilshaw]

• fix module main function finding on Windows

• add improved error handling when correct function name cannot be found

Fixes #111

[michaeldsmith]

Can you try adding a test for this so it could be run with Ctest ?

I think you just need to add something like this to \CTL\unittest\ctlrender\CMakeLists.txt

add_test(NAME "ctlrender-ctl-function-name-is-filename" COMMAND ctlrender -force -ctl functionname_is_filename.ctl bars_photoshop.exr out.exr)

and then add the CTL file functionname_is_filename.ctl to the folder \CTL\unittest\ctlrender\

[michaeldsmith]

@ThomasWilshaw you could also try allowing changes on your pull request so I could try to collaborate with you on the code and tests. I think you would need to follow these instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

[ThomasWilshaw]

Allow Edits by Maintainers is already checked on the PR so you should be good to do any edits you need.

Thanks, I wasn't sure how best to add this test so I'll give that a try.

[michaeldsmith]

@ThomasWilshaw I just tried running Ctest with your func_name branch, I think the current ctlrender tests may actually already be testing the function-name-is-filename feature that you fixed for windows, since test.sh uses the -ctl unity.ctl argument and unity.ctl contains the following:

void unity
    (output varying half rOut,
     output varying half gOut,
     output varying half bOut,
     input varying half rIn,
     input varying half gIn,
     input varying half bIn)
{
  rOut=rIn;
  gOut=gIn;
  bOut=bIn;
}

[michaeldsmith]

It seems your fix works for when calling ctlrender from windows command prompt and windows powershell, like this

ctlrender.exe -force -ctl ..\unittest\ctlrender\functionname_is_filename.ctl ..\unittest\ctlrender\bars_photoshop.exr out.exr

And this makes sense since both windows command prompt and windows powershell use normal windows paths like C:\temp with a backslash character \

But it doesn't seem to work with git bash in windows or wsl that use paths with forward slash character / like this:

Mike@blade13-2020 MINGW64 /c/temp/ThomasWilshawCTL/CTL/build (func_name)
$ ./ctlrender/Debug/ctlrender.exe -force -ctl ../unittest/ctlrender/functionname_is_filename.ctl ../unittest/ctlrender/bars_photoshop.exr out.exr
exception thrown (oops...): CTL file must contain either a main or <module_name> (../unittest/ctlrender/functionname_is_filename) function

[michaeldsmith]

Does it make sense to check for both \ and / and use which ever is found ?

char *backslash = strrchr(name, '\\');
char *forwardslash = strrchr(name, '/');
slash = backslash != NULL ? backslash : forwardslash;

[ThomasWilshaw]

That seems reasonable, I can't imagine a path having both forward and back slashes. Is it worth adding a test that deliberately fails as well? I.e. to make sure we properly handle an inccorect function name?

[michaeldsmith]

If it were possible, this would pick the largest value (closest to the end of the string).

char *backslash = strrchr(name, '\\');
char *forwardslash = strrchr(name, '/');
slash = backslash > forwardslash ? backslash : forwardslash;

[michaeldsmith]

yeah, i think we should test a CTL file that doesn't include either the filename-as-functionname or main() and it should fail. I've never used add_test() for a test that should fail.

[michaeldsmith]

The add_test() documentation says the WILL_FAIL property should be set for a test that should fail

https://cmake.org/cmake/help/latest/command/add_test.html

[michaeldsmith]

looks good - thanks for the contribution!