iocStats is unaware of new EPICS_BASE.<arch> files in synApps

[kmpeters]

iocStats is unaware of the new EPICS_BASE. files in the synApps configure directory that allow one to build for Windows and Linux using the same source tree:

  https://github.com/EPICS-synApps/configure

[anjohnson]

Why should iocStats be aware of the way that synapps modules are configured this week? iocStats was not developed as a synapps module, and probably nobody outside of the APS BCDA group (not even the core EPICS developers) knows anything about how you are modifying the synapps build system. There are other conventions that the EPICS community has been using recently (e.g. RELEASE.local files), and nobody has told us what's wrong with them yet. Having a file to contain just the location of EPICS Base seems somewhat wasteful to me at first glance.

Perhaps the synApps developers should discuss this kind of design choice with the rest of the community in advance, so we could all understand what problems you're trying to solve and help pick the best way to solve them for everyone. The synapps collection provides a very valuable set of modules, but your build-system development processes have been rather insular in the past, and I would like to see them opening up. The core-talk mailing list would be an appropriate place to discuss this kind of thing.

(Sorry Kevin, nothing personal...)

[kmpeters]

FWIW, most of the repositories in epics-modules already contain this change.

Windows and Linux aren't able to use the same directory paths. To use a single source tree for Windows and Linux builds, the paths in the RELEASE file would need to be corrected before each build. areaDetector solved this problem by introducing files with $(EPICS_HOST_ARCH) extensions. I recently made a similar change to the synApps configure directory.

If there is a better approach to allowing Windows and Linux builds to share a source tree, I'd love to know what it is.

[anjohnson]

The EPICS build system reads several RELEASE files in an application's configure directory to permit simultaneous multi-architecture builds. From Base/configure/CONFIG:

-include $(CONFIG)/RELEASE
-include $(CONFIG)/RELEASE.$(EPICS_HOST_ARCH)
-include $(CONFIG)/RELEASE.$(EPICS_HOST_ARCH).Common
ifdef T_A
  -include $(CONFIG)/RELEASE.Common.$(T_A)
  -include $(CONFIG)/RELEASE.$(EPICS_HOST_ARCH).$(T_A)
endif

The same set of files are parsed by the Perl scripts that also read the application RELEASE files. You can use $(EPICS_HOST_ARCH) but not the $(T_A) macro in your RELEASE files because the latter isn't set by the Perl parser. Thus a RELEASE.windows-x64 or RELEASE.windows-x64.Common file can override the paths in the RELEASE file for windows-x64 builds, etc.

The next versions of Base have the following change to their makeBaseApp template configure/RELEASE files, which demonstrates the approach the core developers have been using recently:

=== modified file 'src/makeBaseApp/top/configure/RELEASE'
--- src/makeBaseApp/top/configure/RELEASE       2016-05-31 16:18:29 +0000
+++ src/makeBaseApp/top/configure/RELEASE       2016-11-04 22:18:14 +0000
@@ -35,3 +35,9 @@
 # Set RULES here if you want to use build rules from somewhere
 # other than EPICS_BASE:
 #RULES = $(MODULES)/build-rules
+
+# These allow developers to override the RELEASE variable settings
+# without having to modify the configure/RELEASE file itself.
+-include $(TOP)/../RELEASE.local
+-include $(TOP)/configure/RELEASE.local
+

I would encourage synapps to provide a RELEASE.local file in the parent directory, which can itself contain the line

-include $(TOP)/../configure/EPICS_BASE.$(EPICS_HOST_ARCH)

to take advantage of the convention which the V4 modules have been using for several years now. However this will unfortunately not work with the pvaSrv or exampleCPP modules which contain embedded tops...

[ralphlange]

Consistency and conformity are so out of fashion nowadays.

[kmpeters]

@anjohnson, I'm a little confused. If the RELEASE.local file is added to the synApps support dir as you suggest, we would still need to modify every module to include that file, since only new apps generated by the new version of makeBaseApp will include that file by default, right?

So we shouldn't do this in the RELEASE file of synApps modules:

-include $(TOP)/../configure/EPICS_BASE.$(EPICS_HOST_ARCH)

but this is ok?

-include $(TOP)/../RELEASE.local

And if that is ok, why isn't this ok?

-include $(TOP)/../configure/RELEASE.local

It seems strange to me to put a RELEASE.local file outside of a configure directory, when the convention is that the configure directory is where the RELEASE files reside.

[MarkRivers]

Why should iocStats be aware of the way that synapps modules are configured this week?

asyn has used the convention that Kevin is proposing for iocStats since R4-17 in 2011, so it is hardly "this week". That is when I added it to all synApps modules as well. It was developed to avoid the need to have many RELEASE.$(EPICS_HOST_ARCH) files (win32-x86, win32-x86-static, windows-x64, etc.) in file trees that are common to Linux and Windows. synApps has a very convenient "make release" Perl script to update all of the configure/RELEASE files, and that is how we consistently update release numbers. But the definitions of SUPPORT and EPICS base are done through the --include mechanism.

I am quite sure that this use significantly predates the use of RELEASE.local.

I'm fine with switching to RELEASE.local, and in fact that is how we have been doing it in areaDetector. But there are a lot of changes that need to be made, so we need to make sure we chose the best mechanism.

[MarkRivers]

On a related topic, there is something I either don't understand or don't like about the build system in bundleCPP.

At the top-level of bundleCPP I have created the following 2 files:

RELEASE.local

EPICS_BASE=/usr/local/epics-devel/base-3.14.12.5
-include $(TOP)/../RELEASE.local.$(EPICS_HOST_ARCH)

RELEASE.local.windows-x64

EPICS_BASE=H:\epics-devel\base-3.14.12.5

I would think this is all I should need to do to build for default, e.g. Linux and windows-x64. I have defined the location of EPICS_BASE for both architectures. However, when I type "make" at the top-level of pvCommon with either Linux or Windows I get the following error:

H:\epics-devel\epicsV4\bundleCPP>make Makefile:51: *** EPICS_BASE is not set or doesn't exist. Stop.

Why is it complaining that I have not defined EPICS_BASE? The submodules like pvDatabaseCPP all have lines like this in their configure/RELEASE -include $(TOP)/../RELEASE.local

I have created $(TOP)/../RELEASE.local and defined EPICS_BASE there.

How can I run make at the top-level of bundleCPP without having to define EPICS_BASE on the command line?

[ralphlange]

On Fri, Dec 2, 2016 at 12:03 AM, Mark Rivers wrote:

asyn has used the convention that Kevin is proposing for iocStats since R4-17 in 2011, so it is hardly "this week". [...] I am quite sure that this use significantly predates the use of RELEASE.local.

EPICS Base (V4) is using RELEASE.local files since December 2011 (SCM commit). I think we can safely call it a draw.

The original intention was to avoid the common situation with multiple remote developers working on a project that everyone's configure/RELEASE file is always in modified state, cluttering the output of SCM "diff" commands. You can easily ignore *.local in your SCM setup, and the common configure/RELEASE never has to be modified. This same applies to configure/CONFIG_SITE.

It was later extended to include *.local files in the directory above the modules' TOP directories, because that allows to configure a whole group of applications by changing a single file, ensuring consistency in a very simple and efficient way. (I.e. without the need to have an additional script that goes into all modules and changes all the RELEASE files.)

[MarkRivers]

I like the fact that the RELEASE.local files don't show up as modified in SCM. I switched to using that in areaDetector, with Marty's help in 2013. What I didn't like was having the EXAMPLE_RELEASE files in the top-level of areaDetector because there are lots of them cluttering the top-level directory, and because RELEASE files traditionally belong in configure/ directories. So I moved them into areaDetector/configure. This is what synApps also does, it has a top-level configure directory. So it would be nice if the next version of base had makeBaseApp template that extended this:

-include $(TOP)/../RELEASE.local
-include $(TOP)/configure/RELEASE.local

to this:

+-include $(TOP)/../RELEASE.local
+-include $(TOP)/../configure/RELEASE.local
+-include $(TOP)/configure/RELEASE.local

Then we can keep all *RELEASE* files in configure/ directories if we want to.

[anjohnson]

@kmpeters I didn't design the V4 convention (I would probably have used a ../configure directory as you suggest) and as Ralph describes it wasn't trying to solve the same problem as the synapps one, but I think it's sufficient for both needs.

I'm not keen on the idea of using one file just for setting the path to Base. It seems wasteful — each additional file that GNUmake and our convertRelease.pl script have to read in take a non-zero period of time to open and parse, and when building multiple modules that time gets multiplied by the number of source Makefiles that GNUmake has to process (once per directory in each module, including all of the O.arch directories).

Looking at the configure/RELEASE file from Asyn I see why synapps is doing this (it's more obvious there because that module also has pointers to other modules, unlike the iocStats one). I believe it's to keep the order of entries in the RELEASE file(s) correct. However it isn't actually necessary; the order of the definitions is important for the convertRelease.pl script, but when a value gets redefined it keeps its original place in the order where it was first set, just with the new value. Redefining macros in the RELEASE.<arch> files thus allows you to change paths without worrying about affecting the order.

You also don't have to define a RELEASE macro before you use it (unless you use := for assignments that rely on its value before it gets defined), so your RELEASE.<host> file(s) could define an arch-specific path macro to the synapps installation say which has already been used (but not necessarily defined) to set support module paths earlier in the RELEASE file(s). I think this means that you don't need the separate EPICS_BASE.<arch> file at all.

@MarkRivers The order of including RELEASE.local files matters, to allow overrides. I agree that the module-specific one should come last, but I'm not so sure which of the other two should come first if we do include both of them.

There is another issue that I'm slightly concerned about: The top of the tree isn't always $(TOP)/.. sometimes it's at $(TOP)/../.. or maybe even higher. Some of the V4 modules (e.g. pvaSrv) contain embedded tops, and their configure/RELEASE files are already doing

-include $(TOP)/../../configure/RELEASE.local
-include $(TOP)/../configure/RELEASE.local

so the RELEASE.local file in the parent directory cannot use paths relative to $(TOP), it has to use an absolute path for any includes it does.

I believe this explains your earlier issue about adding a RELEASE.local file to bundleCPP. The bundleCPP Makefile generates arch-specific RELEASE.<arch>.Common files when you configure it anyway, so you do have to give it the EPICS_BASE argument once for each architecture, but that shouldn't affect any other architectures that have already been built. The one exception to that is pvaPy; currently you can only build that for one architecture at at a time because its configuration script only generates the generic-named RELEASE.local and CONFIG_SITE.local files.

[MarkRivers]

I'd be happy to replace synApps/support/configure/EPICS_BASE.$(EPICS_HOST_ARCH) and SUPPORT.$(EPICS_HOST_ARCH) with a single synApps/support/configure/RELEASE.local.$(EPICS_HOST_ARCH) that defined both EPICS_BASE and SUPPORT. I just didn't realize that was possible when I did it the current way.

[anjohnson]

Before making any more changes I suggest that a small group should define the problem(s), then test and write up the best solution and present it to the community for discussion. Maybe we should move this topic to tech-talk and ask for any other interested parties?

[MarkRivers]

Moving the discussion to tech-talk seems like a good idea.