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

commonDriverMakefiles for all modules #49

Open
keenanlang opened this issue May 5, 2021 · 11 comments
Open

commonDriverMakefiles for all modules #49

keenanlang opened this issue May 5, 2021 · 11 comments

Comments

@keenanlang
Copy link
Member

I think it would be a good idea to pull out a lot of the makefile code in xxx and split it out into files like ADCore's commonDriverMakefile. This would clean up xxx's makefile quite a bit and would make non-standard support that is added to the IOC more clear. Could also help with keeping the makefile up-to-date with newly added support (like new motor modules requiring a dbd and library includes).

If we go this way, it may be a good idea to decide on a different naming convention than just having them all be "commonDriverMakefile", probably something with the module's name included. Also, we might think about the best place for these files to live. While App/ is a reasonable enough spot, if there's a usecase that may require more than one of these types of makefiles, then we may want to create a separate folder for them.

@anjohnson
Copy link
Member

Do you guys know about how the EPICS build system automatically pulls in files from the cfg/ install directories of all support modules that are listed in the RELEASE file? I actually want to get rid of Mark's commonDriverMakefile includes and have this kind of thing happen automatically using that mechanism, with the Makefile setting a variable if necessary to request some particular set of things to be configured. I don't want to describe the whole cfg/ mechanism here right now but I can help you put that together.

@MarkRivers
Copy link
Member

MarkRivers commented May 5, 2021

@anjohnson has a PR in areaDetector to simplify the build system using the cfg/ mechanism.
areaDetector/areaDetector#72

I like this approach.

It is still a draft PR (nudge, nudge 😉)

@keenanlang
Copy link
Member Author

A cfg/ based solution seems perfect, though for most of our modules, something to the extent of the PR in areaDetector is overkill. Playing around with it, I think something like the following might be good as a baseline:

INCLUDE_<MODULE>

$(DBD_NAME)_DBD += <MODULE>Support.dbd
$(PROD_NAME)_LIBS := <MODULE> $($(PROD_NAME)_LIBS)
    
-include $(MODULE)/cfg/INCLUDE_<MODULE>.$(EPICS_HOST_ARCH).Common
-include $(MODULE)/cfg/INCLUDE_<MODULE>.Common.$(T_A)
-include $(MODULE)/cfg/INCLUDE_<MODULE>.$(EPICS_HOST_ARCH).$(T_A)

configure/Makefile

CFG += $(wildcard INCLUDE_<MODULE>*)

@anjohnson
Copy link
Member

@keenanlang I'd prefer that applications don't need to add include lines to their Makefiles, since there's a mechanism to do that already in the Base build system (although maybe not in 3.14 releases if that matters to you). I wasn't thinking about the stuff in my PR for this so much as setting variables in a cfg/CONFIG_XXX_MODULE file (which gets included by the Makefile's include $(TOP)/configure/CONFIG statement) and doing other stuff in a cfg/RULES_XXX_MODULE file. One slightly more complex example which does that is Michael Davidsaver's pvxs module, which might give you some ideas. There is no way to provide arch-specific CONFIG* or RULES* files though, so you have to put conditionals in them as needed.

The aim should be to minimize what has to be put in the application to be able to use the features provided by the modules it links to. One idea I would like @MarkRivers to consider would be to create a cfg/CONFIG_ASYN_MODULE file which sets variables such as ASYN_IOC_LIBS and ASYN_IOC_SYS_LIBS that include the libraries that must be linked against when building with a specific installation of Asyn and which vary depending on how that installation has been configured (e.g. does an IOC need to link with libtirpc, or libgpib, or libusb-1.0 etc.). Doing this would simplify the downstream modules so they don't need to know the rules that Asyn knows or to be configured the same way it is. Currently building StreamDevice on RHEL-8 is a (minor) problem for exactly that reason.

@keenanlang
Copy link
Member Author

keenanlang commented May 17, 2021

Testing some things out, dbd files seem to be a bit of tricky case as I can't think of a great way to differentiate between support and ioc dbd files. For downstream modules, you'd want to automatically add your module's support dbd's into any ioc dbd files that downstream module is creating, but not to any of that module's support dbd files. Doing something like you suggest with asyn would work, just have a set of variables set by a config file, then in a makefile you can just do
<PROD>_DBD += <MODULE>_IOC_DBDS
or whatever to include the right files. Though, that doesn't simplify downstream makefiles as much as other options.

Alternately, if there was a specified macro for the dbd name, the modules could reference that macro to only add their dbd includes to the dbd file that the downstream module specifies. XXX already specifies a PROD_NAME and DBD_NAME macro for commonDriverMakefile (and the rest of the xxx makefile) to use. RULES__MODULE can just have:
$(DBD_NAME)_DBD += <MODULE>Support.dbd
$(PROD_NAME)_LIBS := <MODULE> $($(PROD_NAME)_LIBS)

or CONFIG__MODULE could define a function that takes in the name of the DBD file and the PROD name and the downstream module runs through all the modules defined in RELEASE and calls the ADD_MODULE function for each of those modules. The latter is a bit more complex, but does make what is happening a bit more clear rather than relying on specific macros to be defined.

Finally, if there is a way to determine the name of just the DBD files that are being constructed rather than being copied, that might be a reasonable enough proxy for determining what is a support dbd file and what is a full dbd file for an IOC. I'd have to look at if any modules are constructing their support dbd files as well as if the build system in base has a way to grab information about which dbd files are being constructed. If that all works, then downstream modules wouldn't have to do anything more than:

PROD_IOC = <name>
DBD += <name>.dbd
<name>_DBD += base.dbd

and then all the necessary parts of including dependencies are done automatically by the build system.

@keenanlang
Copy link
Member Author

keenanlang commented Sep 25, 2021

Ultimately thought that the easiest way for module developers / end users would be to have a file like CONFIG_NAME_MODULE for each module which would append to the macros: PROD_LIB_DEPENDS and PROD_DBD_DEPENDS (and PROD_SYS_LIB_DEPENDS or others, if necessary). A dependent IOC would then just have to have a single line each for dbd/libs/sys_libs/etc, or ignore things entirely if they don't use the synApps framework.

This runs into the problem that dbd files need to be added in dependency order, but CONFIG_ files are processed in the order that the modules appear in the RELEASE file.

What I could do to solve this issue is to have a CONFIG file like:

# auto-compute location of this file.
# Necessary for dependency resolution
_MODULE := $(dir $(lastword $(MAKEFILE_LIST)))

# we're appending so must be idempotent
ifeq (,$(_MODULE_CONF_INCLUDED))
_MODULE_CONF_INCLUDED := YES

ifdef T_A

_MODULE_DEPENDENT_MODULES := $(shell $(CONVERTRELEASE) -T $(_MODULE)/.. releaseTops)
_MODULE_DEPENDENT_MODULES := $(filter-out EPICS_BASE SUPPORT, $(_MODULE_DEPENDENT_MODULES))

$(foreach mod, $(_MODULE_DEPENDENT_MODULES), $(eval -include $($(mod))/cfg/CONFIG_$(mod)_MODULE ))



PROD_DBD_DEPENDS += MODULESupport.dbd
PROD_LIB_DEPENDS := MODULE $(PROD_LIB_DEPENDS)

endif # T_A

endif # _MODULE_CONF_INCLUDED

This would find and try to include the config files of any dependent modules. Because of the check, if the config file is already included, you aren't causing duplication. But if a module is included before any modules it is dependent on, the config files will be fully included and the DBD files will end up in the proper order.

This isn't the loveliest solution, so if there's a more elegant way, that would be great.

@anjohnson
Copy link
Member

Cross-reference to epics-modules/asyn#141 for @keenanlang's benefit.

I'm not personally very keen on going overboard with the automation/magic. As discussed in the above issue I would prefer that when building an IOC which links to a support module, the IOC's Makefile should explicitly add variables defined in the module's cfg/CONFIG_* file(s) to its appropriate Makefile variables.

The part I haven't thought about is the simplest portable way to create the target-specific CFG file. The template expansion capabilities defined in base/configure/RULES_EXPAND would work, but they weren't very well documented before EPICS 7.0.4 when I enhanced them and added comments to that file. Base-3.14.x did provide a working rule though.

@keenanlang
Copy link
Member Author

I'm not personally very keen on going overboard with the automation/magic. As discussed in the above issue I would prefer that when building an IOC which links to a support module, the IOC's Makefile should explicitly add variables defined in the module's cfg/CONFIG_* file(s) to its appropriate Makefile variables.

This would be the case, the CONFIG_* files would only be creating makefile variables which are optional. I don't want anything to potentially affect people using our modules outside of the synApps environment. The only difference between what you are suggesting in the asyn issue and what I have here is that I build up a single set of makefile variables, where you are suggesting sets of variables for every module under the idea that:

I think the IOC's Makefile should have to explicitly mention every module that it wants to be linked against.

Putting a module's top-level directory in your RELEASE file is already something that will automagically pull in extensions to your makefile through CONFIG and RULES files that can change how your makefile works. It seems that the RELEASE file is already treated like a list of modules that you are going to link against. A RELEASE definition is enough to add files you never explicitly included into your makefile, but it's a step too far to use it to build up a list within a variable?

@anjohnson
Copy link
Member

Consider the case where I have a single IOC application area where I'm building several different but related IOCs — they share most of their modules and databases, but some IOCs need module X, while others need module Y. This kind of thing is fairly common in the APS Accelerator systems. The same RELEASE file is used to build both since they're in the same application. I can configure exactly the modules I want to link into each IOC binary, whereas I don't think that's possible with your approach. Have I misunderstood what you're suggesting?

@keenanlang
Copy link
Member Author

I can configure exactly the modules I want to link into each IOC binary, whereas I don't think that's possible with your approach. Have I misunderstood what you're suggesting?

You would still be able to ignore the variables and just write a makefile like normal. It doesn't particularly help in that instance, but that isn't the instance that it is optimizing for because it isn't really the use-case that we run into. We do have some installations on beamlines with multiple IOCs within the same directory structure, but the support is built and shared among all the IOCs.

What BCDA normally wants to do is that we want to build in as much support as is available into most IOCs that we deal with. This has resulted in a large makefile for XXX and one that needs to be constantly updated as synApps modules are changed and makes it difficult to see what is common to synApps and what is custom to the IOC. When we update an IOC from one version of synApps to another, in order to properly update the makefile you have to do a diff between your IOC's makefile and the older version of the XXX module's makefile, then apply those differences to the new XXX makefile.

This and similar issues with startup scripts have led our group to have two primary work patterns in regards to updating IOCs. Either you copy the current version of XXX and then go through and add in everything custom to the IOC, potentially missing things in the process. Or alternately, you copy the old IOC and try to add new support from the new synApps, potentially missing things in the process.

I've worked to try to clean up the startup scripts to separate those concerns, so that once a startup script is updated to use common.iocsh and to call iocsh files from synApps modules, then an iocBoot directory can pretty much just be used continuously across future synApps versions and changes that modules find to be necessary are properly included without needing changes in the IOC scripts. I'd like something similar for the makefile so that updating an IOC to a new version of synApps should end up being roughly as simple as updating SUPPORT in the RELEASE file.

@keenanlang
Copy link
Member Author

Concept module setup for automatic dependencies at https://github.com/epics-modules/autosave/tree/build-dependencies

A CONFIG_MODULE file is generated using file templating and the module-owner providing a module name. That automatic file then include files with the given module name or that have the module name, an underscore, and then anything afterwards. This allows for modularized systems like areaDetector or motor, or just modules that split out their content into different libraries like MCA.

This makes it very easy to include into epics modules, copy over the template file and new makefile, and then edit CONFIG_SITE to say "MODULE=name". Then, for many modules, it would just be a two line file defining dbd and lib names, with a "CFG += filename" in a makefile. In the concept, I've put that with the src directory as I think that makes the most sense.

This uses the same arch formatting for including files as CONFIG_SITE does. So, alongside the file '@module@' you also have, in the following order:

@MODULE@.$(EPICS_HOST_ARCH)
@MODULE@.$(EPICS_HOST_ARCH).Common
@MODULE@.Common.$(T_A)
@MODULE@.$(EPICS_HOST_ARCH).$(T_A)

@module@ being the defined module name that is replaced in file templating.

All these files would be responsible for declaring two primary macros: @module@_IOC_LIBS and @module@_IOC_DBDS. These names were chosen to try to match up with the EPICS_BASE_IOC_LIBS macro.

When building an IOC, one would then be able to define their makefile using those macros:

my_DBD += base.dbd
my_DBD += $(AUTOSAVE_IOC_DBDS)

my_LIBS := $(AUTOSAVE_IOC_LIBS) $(EPICS_BASE_IOC_LIBS)

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

No branches or pull requests

3 participants