Use `epicscorelibs` for python modules

Tom-Willemsen commented 3 months ago

Where?

As a developer I would like to build all of our epics python wrappers against a single consistent set of EPICS libraries in a supported way (e.g. a way that can be installed via pip without depending on external bat scripts).

This goes part-way towards https://github.com/ISISComputingGroup/IBEX/issues/8381 , and also helps with bluesky tickets as we are currently unable to install any module which depends on p4p in genie_python.

How?

Use epicscorelibs as the central implementation

Acceptance criteria

[ ] epicscorelibs used as a single consistent implementation of the EPICS base libraries for our python-epics modules.
- Should be used by CaChannel, pcaspy, p4p, (aioca not currently in our build but will be used by bluesky)
[ ] This can all be done via standard python packaging mechanisms e.g. pip, not custom batch scripts.

PRs

It may make sense to delay merging this change until just after the release, this change shouldn't change any functionality but as it changes underlying libraries for all of our python-epics wrappers probably best to merge just after a release rather than just before.

rerpha commented 3 months ago

i;m happy to review these now, but to delay merging until after a release if you think it's risky - perhaps a task is good enough?

rerpha commented 3 months ago

Moving to impeded - see tasks board.

FreddieAkeroyd commented 2 months ago

This builds epics base in a different way to usual, it doesn’t call “make” but has a list of files in “setup.py.d” and extracta some things from config files before calling the python extension build system. However I note on windows this leads to different compiler options e.g.:

cl /c /nologo /O2 /W3 /GL /DNDEBUG /MD -DEPICS_BUILD_DLL -DEPICS_CALL_DLL -DNOMINMAX -DUSE_TYPED_RSET -DBUILDING_libCom_API -DEPICS_COMMANDLINE_LIBRARY=EPICS_COMMANDLINE_LIBRARY_EPICS -Imodules\libcom\src\as -Imodules\libcom\src\env -Imodules\libcom\src\osi -Imodules\libcom\src\osi\os\win32 -Imodules\libcom\src\osi\os\default -Imodules\libcom\src\osi\compiler\msvc -I. -Ibuild\temp.win-amd64-cpython-311\. -Ibuild\lib.win-amd64-cpython-311\epicscorelibs\include -Ibuild\lib.win-amd64-cpython-311\. -Ibuild\.  /Tcmodules\libcom\src\osi\os\win32\osdevent.c /Fobuild\temp.win-amd64-cpython-311\Release\modules\libcom\src\osi\os\win32\osdevent.obj

compared to a make based build of epics base

cl -nologo -FC -D__STDC__=0 -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DBUILDING_libCom_API   -Ox -Oy- -Z7   -W3        -MD -DEPICS_BUILD_DLL -DEPICS_CALL_DLL    -I. -I../O.Common -I. -I../osi/compiler/msvc -I../osi/compiler/default -I. -I../osi/os/WIN32 -I../osi/os/default -I.. -I../as -I../bucketLib -I../calc -I../cvtFast -I../cppStd -I../cxxTemplates -I../dbmf -I../ellLib -I../env -I../error -I../fdmgr -I../flex -I../freeList -I../gpHash -I../iocsh -I../log -I../macLib -I../misc -I../osi -I../pool -I../ring -I../taskwd -I../timer -I../yacc -I../yacc -I../yajl -I../../../../include/compiler/msvc -I../../../../include/os/WIN32 -I../../../../include         -c ../osi/os/WIN32/osdEvent.c

so “epicscorelibs” is compiled using /O2 rather than /Ox, from the compiler help page "The additional options applied by /O1 and /O2 can cause pointers to strings or to functions to share a target address, which can affect debugging and strict language conformance." EPICS base has chosen to use /Ox over /O2 and this is the compiler flags that will have been most tested – this may or may not be an issue. Also epicscorelibs is not passing /Oy- and /Z7 that will affect debugging, /Oy- is from standard epics base but we add /Z7 in our base and install the symbol files. The include file list is also a bit different, and __STDC__=0 is missing, again may or may not be an issue.

So I am concerned about a difference in compiler options that make this build potentially different and also lacking in debug symbols and any local changes we have made. Ideally there would be an option to either package up an existing epics base build built in the usual way, or trigger such a standard build. I can drop Michael Davidsaver a note and see what he thinks may be possible.

Tom-Willemsen commented 2 months ago

so “epicscorelibs” is compiled using /O2 rather than /Ox, from the compiler help page "The additional options applied by /O1 and /O2 can cause pointers to strings or to functions to share a target address, which can affect debugging and strict language conformance." EPICS base has chosen to use /Ox over /O2 and this is the compiler flags that will have been most tested – this may or may not be an issue. Also epicscorelibs is not passing /Oy- and /Z7 that will affect debugging, /Oy- is from standard epics base but we add /Z7 in our base and install the symbol files. The include file list is also a bit different, and STDC=0 is missing, again may or may not be an issue.

We can probably do a PR onto epicscorelibs to get the standard flags like /Ox etc set if needed. The flags are here I think.

On linux epicscorelibs adds debug flags if it's a debug python, so I'd guess an equivalent change for windows is also likely to be accepted upstream (but I don't think the debugging options should necessarily block this being merged)

FreddieAkeroyd commented 2 months ago

We can do a PR to put flags in sync, but my question for upstream is whether there is an alternative that avoids such a manual keeping in sync process that to me is error prone. I just picked one makefile, but it is possible to define per file or per module flags, so i don't know how you guarantee you have built something correctly is you don't run its build system. As well as debug flags the resulting PDB files need to be installed via symstore but I think that is already part of our genie python build for other things. If epicscorelib could just package an exisiting EPICS build, or initiate a standard EPICS build, to me sounds potentially less error prone.

Tom-Willemsen commented 2 months ago

ok - that's a much bigger structural change to epicscorelibs you're suggesting I think - could we have a bit of a discussion either over teams, or in person when we're both on site?

Tom-Willemsen commented 2 months ago

Putting this back into review now that release/deploy has been done.

ISISComputingGroup / IBEX