Fix #1907, Cmake modifiable table tool path parameter

[paulober]

Cmake option to change the path of the ef2cfetbl executable file via the TABLETOOL_EXE environment variable. This also enables the user to specify other tools as table tool if needed.

Describe the contribution

• Fixes Relax CMake directory expectations for elf2cfetbl #1907

Testing performed

1. Github actions

Expected behavior changes

• Nothing only if TABLETOOL_EXE environment variable is set. Then the cmake would use the content as path to an executable used for transforming lib into tbl.
  IMPORTANT!
  If TABLETOOL_EXE isn't set the build process does not change. But if it is set the program won't add the default ef2cfetbl tool
  as a dependency in cmake and the with TABLETOOL_EXE specified executable will be run with the parameters like for the
  ef2cfetbl utility.

Additional context
See above.

Contributor Info - All information REQUIRED for consideration of pull request
@PavLL

[astrogeco]

Hey @PavLL thanks for the contribution, we're still debating what approach we want to take to solve #1907. In the meantime one small improvement would be to use CMake cache variables instead of environment ones.

[paulober]

Ok, thanks for the quick info + tip.

[paulober]

Ok, now you can run your Mission Build with -DTABLETOOL_EXECUTABLE=/path/to/my/custom/tabletool, and then it will set it as CMake cache variable which then will be used in Arch Build to parse it (if set, else the default) thought -DTBLTOOL to the Generate Table (like it is at the moment).

[dzbaker]

@jphickey Would you be able to review this PR?

[jphickey]

I'm not seeing how the value gets passed from the top-level mission build into the arch build where it is used.

BUT - I'm not entirely convinced that this is the right way to go in the first place. Ideally, the intent with the issue #1907 was to make the table build method more modular. With this change, only the path to the EXE file for the tool can be modified, but all other parts of the build process are kept the same. This means that whatever alternate tool is provided, must have exactly the same functionality as elf2cfetbl, including all its quirks. So at that point I don't know why someone would need to override it, it has to do what it is expected to do at that particular step.

I'd rather see a change that only captures the table build metadata here (include paths, compile defs, source files, etc) and defers the actual binary generation to a user-defined script that runs at the mission level. This way, a user can control the whole process, and actually do something different (as opposed to parsing an ELF file which has always been troublesome/problematic).

[jphickey]

Also - as this has been stale for over a year now, I haven't heard users clamoring for something like this - if it is not a pressing issue I wouldn't risk possible unintended side effects by adding a feature that isn't high priority or unlikely to be used.

In many ways there are already too many options/special features/overrides in the build as it is, and many aren't tested on a regular basis so might be broken.

[jphickey]

Finally I'll also note that through the use of arch_build_custom.cmake in the defs directory, a user can already override the entire add_cfe_tables function and provide their own implementation for this. (just redefine the function to do something different).

While that might be a bit heavy handed, it does allow a user to completely override the table build process and requires no change at all to the existing scripts.

[dzbaker]

It may be best to simply close this one for now. @paulober @jphickey, would you agree?

[jphickey]

It may be best to simply close this one for now. @paulober @jphickey, would you agree?

Yes, I would agree.

Recommendation for a user that needs something different would be to copy the "add_cfe_tables" function in its entirety into your arch_build_custom.cmake file and then modify it to do what you want.

[paulober]

It may be best to simply close this one for now. @paulober @jphickey, would you agree?

I agree with that too. The overwrite option via arch_build_custom.cmake sounds much clearer and easier.