search

epics-modules / motor

APS BCDA synApps module: motor
https://epics-modules.github.io/motor/
20 stars 47 forks source link

*.req files fail to install #138

Closed kmpeters closed 5 years ago

kmpeters commented 5 years ago

The motorApp/Db/*.req files fail to install when building against both base 3.14 and 3.15. This doesn't break builds of motor or driver code, but it does break builds of example IOCs.

kmpeters commented 5 years ago

@keenanlang, was something missing from this commit b7eae96a769a286171e858254fe501b3da88faa0?

anjohnson commented 5 years ago

It probably needs this in the Makefile:

ifeq ($(BASE_3_14),YES)
  vpath %.req $(USR_VPATH) $(GENERIC_SRC_DIRS)
endif

Compare with the scalerApp/Db/Makefile

kmpeters commented 5 years ago

The Makefile does have those lines:

https://github.com/epics-modules/motor/blob/955b41a5a6ea9b24bb6364fb1179b267dcdec638/motorApp/Db/Makefile#L34

kmpeters commented 5 years ago

@keenanlang made some changes so that the req files could be installed with REQ += instead of DB +=:

https://github.com/epics-modules/motor/blob/955b41a5a6ea9b24bb6364fb1179b267dcdec638/motorApp/Db/Makefile#L22

I don't remember the reasons for that approach.

anjohnson commented 5 years ago

Are the autosave .req files that are included with synApps modules such as this only appropriate for sites using the full synApps collection of modules (and hence using the EPICS-synApps/configure module)? ASD/Controls will be using many synApps modules such as Motor for the APS Accelerator systems, but we'll be building them using Sumo so I'm not planning on using the configure or support modules at all. Your using the REQ variable for .req files instead of DB won't break our builds, but those files just won't get installed for us. That doesn't appear to be a problem though (it may even be an advantage for us as apparently we create our own .req files anyway) so I'm not asking for any changes, just rambling on and explaining my confusion above.

BTW I think those conditional vpath %.req statement could live inside the CONFIG_REQ file itself and not have to be spread around all the modules. By the time the cfg/CONFIG* files get pulled in GNUmake already knows the Base version, and I believe vpath directives can go anywhere in a Makefile. If I'm wrong about the latter and they need to be in the rules section you could put them into a cfg/RULES_REQ file instead.

kmpeters commented 5 years ago

Are the autosave .req files that are included with synApps modules such as this only appropriate for sites using the full synApps collection of modules (and hence using the EPICS-synApps/configure module)?

As far as I know, all the .req files in the modules that are included in synApps are appropriate for sites that use autosave. The use of modules from https://github.com/EPICS-synApps is not required. @keenanlang should correct me if I'm wrong.

anjohnson commented 5 years ago

The use of modules from https://github.com/EPICS-synApps is not required.

Sorry Kevin, but if you're expecting the REQ variable in a Makefile to cause .req files to be installed into $(INSTALL_DB) that means you are depending on the cfg/CONFIG_REQ file from the EPICS-synApps/configure module; Base has no make rules that use the REQ variable. Also the doAfterIocInit command in iocsh/allstop.iocsh will only work if the IOC that runs it was built and linked against the epics-modules/std module which implements that command.

I don't have a problem with either of those so as I said above I'm not complaining, just pointing out that you do have some hidden dependencies.

kmpeters commented 5 years ago

Sorry Kevin, but if you're expecting the REQ variable in a Makefile to cause .req files to be installed into $(INSTALL_DB) that means you are depending on the cfg/CONFIG_REQ file from the EPICS-synApps/configure module; Base has no make rules that use the REQ variable. Also the doAfterIocInit command in iocsh/allstop.iocsh will only work if the IOC that runs it was built and linked against the epics-modules/std module which implements that command.

That's good to know. That explains why the .req files get installed in our synApps builds, but not in motor's travis build or my local development builds.

keenanlang commented 5 years ago

I don't remember the reasons for that approach.

I did it because I didn't realize that DB+= was available, none of the modules used that capability and the EPICS build facility documentation doesn't seem to state that it will copy any files that aren't db/substitutions/template files.

Are the autosave .req files that are included with synApps modules such as this only appropriate for sites using the full synApps collection of modules (and hence using the EPICS-synApps/configure module)?

The req files themselves should only rely on autosave.

BTW I think those conditional vpath %.req statement could live inside the CONFIG_REQ file itself and not have to be spread around all the modules. By the time the cfg/CONFIG* files get pulled in GNUmake already knows the Base version, and I believe vpath directives can go anywhere in a Makefile. If I'm wrong about the latter and they need to be in the rules section you could put them into a cfg/RULES_REQ file instead.

This seems like the best system, this allows us to install all the reqs from all the modules within synApps without needing any changes to the modules themselves. I'll adjust the CONFIG_REQ file and revert changes to the individual modules.

kmpeters commented 5 years ago

Fixed by pull request #141

kmpeters commented 5 years ago

Also the doAfterIocInit command in iocsh/allstop.iocsh will only work if the IOC that runs it was built and linked against the epics-modules/std module which implements that command.

I'm not concerned about this at the moment, since the only users of allstop.iocsh are those with IOCs based on xxx, which depends on all of the synApps modules.

anjohnson commented 5 years ago

@keenanlang Sorry if you felt attacked by any of my previous remarks, they weren't intended as such. The documentation on the build system is very dated, Janet Anderson was working on rewriting the AppDevGuide chapter for Base-3.15 but she never finished and I've been too busy since she retired to be able to resurrect and complete her work (and then update it with the additional changes since). If you ever have questions or problems with the build system I'd be happy to help.

I asked a couple of the guys up here about the Motor .req files and Marty Smith said that the fields you save are obviously appropriate for beamline users, but for Accelerator applications we don't usually need to save as much stuff since many fewer fields ever get changed at run-time on the Accelerators. It seems there's a much closer connection between the specific application and the .req files (and maybe some of the opi files too) than with the code and database files.

I have imported a copy of the doAfterIocInit code into our iocStd module which all our IOCs will link against. I'd even be open to a suggestion that it be added to a future Base release since it's obviously a useful tool.

timmmooney commented 5 years ago

By the way, there's an intended way to override the .req files in synApps: autosave has a request-file path, which we populate with commands like this:

set_requestfile_path("$(TOP)/iocBoot/$(IOC)", "") set_requestfile_path("$(BUSY)", "busyApp/Db") etc.

If you make your own copy of, say, motor_settings.req and put it in a directory that comes before the motor modules directory, autosave will use your copy.

Tim Mooney (mooney@anl.gov) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab

From: Andrew Johnson notifications@github.com Sent: Tuesday, August 27, 2019 3:43 PM To: epics-modules/motor motor@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [epics-modules/motor] *.req files fail to install (#138)

@keenanlanghttps://github.com/keenanlang Sorry if you felt attacked by any of my previous remarks, they weren't intended as such. The documentation on the build system is very dated, Janet Anderson was working on rewriting the AppDevGuide chapter for Base-3.15 but she never finished and I've been too busy since she retired to be able to resurrect and complete her work (and then update it with the additional changes since). If you ever have questions or problems with the build system I'd be happy to help.

I asked a couple of the guys up here about the Motor .req files and Marty Smith said that the fields you save are obviously appropriate for beamline users, but for Accelerator applications we don't usually need to save as much stuff since many fewer fields ever get changed at run-time on the Accelerators. It seems there's a much closer connection between the specific application and the .req files (and maybe some of the opi files too) than with the code and database files.

I have imported a copy of the doAfterIocInit code into our iocStd module which all our IOCs will link against. I'd even be open to a suggestion that it be added to a future Base release since it's obviously a useful tool.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/motor/issues/138?email_source=notifications&email_token=ABT6PFZGA44E65UFUAFLC7DQGWGWZA5CNFSM4IPWY5K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5JBYLA#issuecomment-525474860, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABT6PF5PCYQJ7RUIW5HZMSTQGWGWZANCNFSM4IPWY5KQ.

kmpeters commented 5 years ago

@timmmooney, when the motor module was split into many driver modules, I decided that installing .req files in both the core motor module and the driver modules to a top-level db directory was a better solution than adding the Db directory of driver submodules to the autosave requestfile path. This worked for a while, but then some synApps-specific improvements were introduced that resulted in this issue being created, since they broke travis and my development builds.