epics-modules / motor

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

Changing Motor RELEASE file doesn't change local release #147

Closed keenanlang closed 4 years ago

keenanlang commented 4 years ago

The /modules makefile will only make RELEASE.$(EPICS_HOST_ARCH).local if the file doesn't exist. If you make the module, then later want to update a dependent module like ASYN to a newer version, changing the top-level configure/RELEASE file isn't enough, even with a make rebuild. You would need to 'make realclean' to delete the local RELEASE and then 'make'. This seems to be an unnecessary break from how all other modules work with the build system. A simple fix would be to add the line:

.PHONY: $(RELEASE_LOCAL)

to the modules/Makefile, so that the local RELEASE file is rebuilt when necessary.

anjohnson commented 4 years ago

Using .PHONY will actually cause it to be rebuilt every time the build gets into the Modules directory whether it needs to be changed or not. If you only want it to to be rebuilt whenever $(TOP)/configure/RELEASE changes, list that as a dependency in the build rule:

$(RELEASE_LOCAL): $(TOP)/configure/RELEASE
kmpeters commented 4 years ago

@anjohnson, adding $(TOP)/configure/RELEASE as a dependency of $(RELEASE_LOCAL) only helps if the RELEASE file is being used. If any of the RELEASE.local files are used instead, modules's RELEASE.$(EPICS_HOST_ARCH).local could be stale:

-include $(or $(MOTOR),$(TOP))/../RELEASE.local
-include $(or $(MOTOR),$(TOP))/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(or $(MOTOR),$(TOP))/configure/RELEASE.local

This makes the .PHONY approach more appealing...

anjohnson commented 4 years ago

@kmpeters True, I was responding literally to these words from @keenanlang

changing the top-level configure/RELEASE file isn't enough

BTW: Do any cdCommands or envParams files generated in the submodules contain the correct paths when a $(or $(MOTOR),$(TOP)) expression is needed to set paths? The convertRelease.pl script that generates those files doesn't currently have support for $(or ...) in its Release file parser, so I'm guessing that you may not be generating any of those files.

kmpeters commented 4 years ago

The cdCommands and envPaths files do have the correct paths in them, because the IOC's RELEASE file doesn't include motor's RELEASE file. The example IOCs use RELEASE.local files instead:

[thorlabsIOC]$ more configure/RELEASE 
# RELEASE - Location of external support modules

# Use motor/module's generated release file when buidling inside motor
-include $(TOP)/../../../RELEASE.$(EPICS_HOST_ARCH).local
# Use motorThorLabs's release file when building inside motorThorLabs, but outside motor
-include $(TOP)/../../configure/RELEASE.local
# Use thorlabsIOC's RELEASE.local when building outside motorThorLabs
-include $(TOP)/configure/RELEASE.local