search

epics-modules / xspress3

EPICS areaDetector xspress3 module
GNU Lesser General Public License v3.0
4 stars 18 forks source link

Question about configure/RELEASE #11

Closed jwlodek closed 4 years ago

jwlodek commented 4 years ago

Would it be possible to structure the configure/RELEASE files to be more in line with the epics-modules and synApps standard? What I mean by this is that most modules seem to have a RELEASE file with paths to all dependent modules, that are populated by make release. I feel like it would be beneficial to add this to maintain the standard.

MarkRivers commented 4 years ago

The current configure/RELEASE is modeled after all of the submodules in areaDetector, e.g. ADSimDetector, etc. It assumes that xspress3 is installed under areaDetector.

It could be converted to be like the dxp, dxpSITORO, quadEM, and dante modules in https://github.com/epics-modules. Those are the "standard" RELEASE files for modules that use areaDetector but are not in the areaDetector tree.

This is the RELEASE file for dxp, for example:

#RELEASE Location of external products
# Run "gnumake clean uninstall install" in the application
# top directory each time this file is changed.

TEMPLATE_TOP=$(EPICS_BASE)/templates/makeBaseApp/top

# If you don't want to install into $(TOP) then
# define INSTALL_LOCATION_APP here
#INSTALL_LOCATION_APP=<fullpathname>

SUPPORT=/corvette/home/epics/devel

#If using the sequencer, point SNCSEQ at its top directory:
SNCSEQ=$(SUPPORT)/seq-2-2-5

MCA=$(SUPPORT)/mca-7-8
DXP=$(SUPPORT)/dxp-5-0

# dxp requires areaDetector, and areaDetector/configure/RELEASE_PRODS.local already defines
# ASYN, CALC, etc.
AREA_DETECTOR=$(SUPPORT)/areaDetector-3-8
-include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

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

Note that because dxp (and xspress3) use areaDetector it is convenient to include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local. That ensures that whatever areaDetector was configured (optional plugins, etc.) with is also how xspress3 will be configured.

This makes this configure/RELEASE file much simpler, it only includes the things that RELEASE_PRODS.local does not already include, i.e. MCA and DXP in the case of dxp.

newville commented 4 years ago

@jwlodek following the areadetector conventions might make sense.
Hardwiring version numbers or locations of SUPPORT does not make sense.

MarkRivers commented 4 years ago

What @jwlodek is suggesting would not be hardcoding. Lines like

SUPPORT=XXXX ASYN=YYYY

in the file will be updated automatically when one runs "make release" at the top-level of the synApps/support directory.

But pulling in $(AREA_DETECTOR)/configure/RELEASE_PRODS.local ensures that any optional plugins used by areaDetector will automatically be used by xspress3, without having to independently manage them in xspress3.

newville commented 4 years ago

@MarkRivers @jwlodek Yep, following the areaDetector conventions for the names and layout of the configure files and adding 

$(AREA_DETECTOR)/configure/RELEASE_PRODS.local

to configureRELEASEso thatxspress3inherits the environment ofareaDetectormakes sense. It turns out,xspress3has no outside dependencies besides those it inherits fromareaDetector`, so nothing like that should be added.

Having specific values for environment variables in any of the config files that are wrong by default does not make sense. If they are to be overwritten by make release why are they even there?

MarkRivers commented 4 years ago

Having specific values for environment variables in any of the config files that are wrong by default does not make sense. If they are to be overwritten by make release why are they even there?

They are there to indicate what other modules that module requires. Because the actual paths are site-specific it does require that the distributed RELEASE files always need to be edited, either manually or by running "make release". This is the way all synApps modules work, including areaDetector.

However, I tried to make things easy for areaDetector, which has a top-level areaDetector directory, and over 30 submodules (ADSimDetector, etc.). Rather than having to edit the 30 submodule configure/RELEASE directories, they just include the top-level areaDetector configure/RELEASE files. But the top-level areaDetector/configure/RELEASE files do need to be edited, either manually or with "make release".

For modules like xspress3 that use areaDetector, I think pulling in those areaDetector files also makes the most sense, since they need to be set up correctly anyway.

But the whole idea is that RELEASE files are locally edited, so that if others want to do it differently they are free to do so.

jwlodek commented 4 years ago

Following up on this, what I would suggest is that in support of uniformity for epics-modules, the RELEASE file should be organized similarly to the one found in quadEM, as that is the format all the modules in this group share. I think having all modules be supported by the make release should be standard for any module included here, as it makes building them simpler for end users. If we wanted to use the areaDetector build format, perhaps xspress3 should be placed in the areaDetector group on github as opposed to epics-modules?

MarkRivers commented 4 years ago

the RELEASE file should be organized similarly to the one found in quadEM,

The way the xspress3 configure/RELEASE files are currently written it requires that the xspress3 module be placed under the top-level areaDetector, like ADSimDetector, etc. I can see that some sites might not want to do this, and might want it to be somewhere else, as quadEM, dxp, etc. are.

I have just fixed xspress3/configure/RELEASE and xspress3/configure/iocs/xspress3IOC/configure/RELEASE so that it works with "make release" and can be built either inside or outside of the areaDetector/ tree. I have tested building it both in the areaDetector/ tree and outside the tree.

MarkRivers commented 4 years ago

My changes are in the fix_release branch on Github. @jwlodek please test it, and if you are happy I will merge into master.

jwlodek commented 4 years ago

I just tested the new RELEASE file, and once I added XSPRESS3 to the synApps makefile:

https://github.com/EPICS-synApps/support/pull/17

It worked fine with make release. I seem to have run into an error with the build toward the end during the linking phase, is there something I could be missing?

/usr/bin/ld: /epics/test/support/xspress3/lib/linux-x86_64/libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: error: ld returned 1 exit status
/epics/test/base/configure/RULES_BUILD:206: recipe for target 'xspress3App' failed
MarkRivers commented 4 years ago

libimg_mod.a seems to be a binary library file that was provided by Quantum?

It builds fine for me on Centos 7 with gcc 4.8.5.

This is what "file" and "nm" tell me about that file.

corvette:xspress3Support/os/linux-x86_64>file libimg_mod.a
libimg_mod.a: current ar archive
corvette:xspress3Support/os/linux-x86_64>nm libimg_mod.a

img_mod_linux.o:
                 U __errno_location
000000000000010e T _os_datmod
00000000000003d8 T _os_link
0000000000000681 T _os_unlink
                 U close
000000000000078b T datamod_size
                 U fprintf
                 U free
                 U fstat
                 U ftruncate
                 U fwrite
                 U malloc
                 U memcpy
                 U mmap
00000000000006a0 T munlink
                 U munmap
0000000000000000 T new_mbuf
00000000000000b0 T search_mbuf
                 U shm_open
                 U stderr
                 U strcat
                 U strlen
0000000000000000 b top
                 U umask

img_mod.o:
                 U _os_datmod
                 U _os_link
0000000000000c1c T id_clear_mod
0000000000000db9 T id_copy_mod
0000000000000a95 T id_get_label
0000000000000afe T id_get_ptr
0000000000000000 T id_mkmod
0000000000000412 T id_mkmod3d
                 U memcpy
                 U munlink
                 U printf
                 U puts
                 U strcpy
                 U strlen
                 U strncpy

What do you see with those programs?

MarkRivers commented 4 years ago

Can you post the complete output of the link command that failed?

What version of Linux and gcc are you using?

jwlodek commented 4 years ago

The command output:

root@327a8e291869:/epics/src/support/xspress3/xspress3Support/os/linux-x86_64# file libimg_mod.a
libimg_mod.a: current ar archive
root@327a8e291869:/epics/src/support/xspress3/xspress3Support/os/linux-x86_64# nm libimg_mod.a

img_mod_linux.o:
                 U __errno_location
000000000000010e T _os_datmod
00000000000003d8 T _os_link
0000000000000681 T _os_unlink
                 U close
000000000000078b T datamod_size
                 U fprintf
                 U free
                 U fstat
                 U ftruncate
                 U fwrite
                 U malloc
                 U memcpy
                 U mmap
00000000000006a0 T munlink
                 U munmap
0000000000000000 T new_mbuf
00000000000000b0 T search_mbuf
                 U shm_open
                 U stderr
                 U strcat
                 U strlen
0000000000000000 b top
                 U umask

img_mod.o:
                 U _os_datmod
                 U _os_link
0000000000000c1c T id_clear_mod
0000000000000db9 T id_copy_mod
0000000000000a95 T id_get_label
0000000000000afe T id_get_ptr
0000000000000000 T id_mkmod
0000000000000412 T id_mkmod3d
                 U memcpy
                 U munlink
                 U printf
                 U puts
                 U strcpy
                 U strlen
                 U strncpy

Running in a fresh Ubuntu 18.04 docker container with gcc 7.4.0.

The full linking + error message:

make[5]: Entering directory '/epics/src/support/xspress3/iocs/xspress3IOC/xspress3App/src/O.linux-x86_64'
/usr/bin/g++ -o xspress3App -Wl,-Bstatic -L/epics/src/support/xspress3/iocs/xspress3IOC/lib/linux-x86_64 -L/epics/src/base/lib/linux-x86_64 -L/epics/src/support/areaDetector/ADCore/lib/linux-x86_64 -L/epics/src/support/areaDetector/ADSupport/lib/linux-x86_64 -L/epics/src/support/asyn/lib/linux-x86_64 -L/epics/src/support/autosave/lib/linux-x86_64 -L/epics/src/support/busy/lib/linux-x86_64 -L/epics/src/support/calc/lib/linux-x86_64 -L/epics/src/support/iocStats/lib/linux-x86_64 -L/epics/src/support/seq/lib/linux-x86_64 -L/epics/src/support/sscan/lib/linux-x86_64 -L/epics/src/support/xspress3/lib/linux-x86_64            -rdynamic -m64         xspress3App_registerRecordDeviceDriver.o xspress3AppMain.o    -lxspress3Epics -lxspress3 -limg_mod -lNDPlugin -lADBase -lqsrv -lntndArrayConverter -lnt -lpvDatabase -lpvAccessIOC -lpvAccessCA -lpvAccess -lpvData -lnetCDF -lMagick++ -lcoders -lMagick -ljbig -ljp2 -lbzlib -lpng -lwebp -llcms -lttf -lwmf -lfilters -ltiff -lxml2 -lNeXus -lhdf5 -lhdf5_hl -lbitshuffle -lblosc -lszip -lzlib -ljpeg -lasyn -lautosave -lbusy -lcalc -ldevIocStats -lsscan -lseq -lpv -ldbRecStd -ldbCore -lca -lCom -Wl,-Bdynamic -lboost_system -lboost_system -lX11 -lXext -lpthread   -lreadline -lm -lrt -ldl -lgcc
/epics/src/support/areaDetector/ADSupport/lib/linux-x86_64/libjp2.a(jas_stream.o): In function `jas_stream_tmpfile':
/epics/src/support/areaDetector/ADSupport/supportApp/GraphicsMagickSrc/jp2/src/libjasper/O.linux-x86_64/../../../../../../supportApp/GraphicsMagickSrc/jp2/src/libjasper/base/jas_stream.c:368: warning: the use of `tmpnam' is dangerous, better use `mkstemp'
/usr/bin/ld: /epics/src/support/xspress3/lib/linux-x86_64/libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: error: ld returned 1 exit status
/epics/src/base/configure/RULES_BUILD:206: recipe for target 'xspress3App' failed
make[5]: *** [xspress3App] Error 1
make[5]: Leaving directory '/epics/src/support/xspress3/iocs/xspress3IOC/xspress3App/src/O.linux-x86_64'
/epics/src/base/configure/RULES_ARCHS:58: recipe for target 'install.linux-x86_64' failed
make[4]: *** [install.linux-x86_64] Error 2
make[4]: Leaving directory '/epics/src/support/xspress3/iocs/xspress3IOC/xspress3App/src'
/epics/src/base/configure/RULES_DIRS:84: recipe for target 'src.install' failed
make[3]: *** [src.install] Error 2
make[3]: Leaving directory '/epics/src/support/xspress3/iocs/xspress3IOC/xspress3App'
/epics/src/base/configure/RULES_DIRS:84: recipe for target 'xspress3App.install' failed
make[2]: *** [xspress3App.install] Error 2
make[2]: Leaving directory '/epics/src/support/xspress3/iocs/xspress3IOC'
/epics/src/base/configure/RULES_DIRS:84: recipe for target 'xspress3IOC.install' failed
make[1]: *** [xspress3IOC.install] Error 2
make[1]: Leaving directory '/epics/src/support/xspress3/iocs'
/epics/src/base/configure/RULES_DIRS:84: recipe for target 'iocs.install' failed
make: *** [iocs.install] Error 2
MarkRivers commented 4 years ago

I just reproduced your problem on a non-virtual Ubuntu 18 system:

make -C O.linux-x86_64-ub18 -f ../Makefile TOP=../../.. \
    T_A=linux-x86_64-ub18 install
make[1]: Entering directory '/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/iocs/xspress3IOC/xspress3App/src/O.linux-x86_64-ub18'
/usr/bin/g++ -o xspress3App -Wl,-Bstatic -L/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/iocs/xspress3IOC/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/areaDetector-3-8/ADCore/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/areaDetector-3-8/ADSupport/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/asyn-4-37/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/autosave-5-10/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/busy-1-7-2/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/calc-3-7-3/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/devIocStats-3-1-16/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/seq-2-2-5/lib/linux-x86_64-ub18 -L/corvette/home/epics/devel/sscan-2-11-3/lib/linux-x86_64-ub18 -L/corvette/usr/local/epics-devel/base-7.0.3.1/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/iocs/xspress3IOC/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/areaDetector-3-8/ADCore/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/areaDetector-3-8/ADSupport/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/asyn-4-37/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/autosave-5-10/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/busy-1-7-2/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/calc-3-7-3/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/devIocStats-3-1-16/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/seq-2-2-5/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/home/epics/devel/sscan-2-11-3/lib/linux-x86_64-ub18 -Wl,-rpath,/corvette/usr/local/epics-devel/base-7.0.3.1/lib/linux-x86_64-ub18           -rdynamic -m64         xspress3App_registerRecordDeviceDriver.o xspress3AppMain.o    -lxspress3Epics -lxspress3 -limg_mod -lNDPlugin -lADBase -lqsrv -lntndArrayConverter -lnt -lpvDatabase -lpvAccessIOC -lpvAccessCA -lpvAccess -lpvData -lnetCDF -lMagick++ -lcoders -lMagick -ljbig -ljp2 -lbzlib -lpng -lwebp -llcms -lttf -lwmf -lfilters -ltiff -lxml2 -lNeXus -lhdf5 -lhdf5_hl -lbitshuffle -lblosc -lszip -lzlib -ljpeg -lasyn -lautosave -lbusy -lcalc -ldevIocStats -lsscan -lseq -lpv -ldbRecStd -ldbCore -lca -lCom -Wl,-Bdynamic -lX11 -lXext -lpthread   -lreadline -lm -lrt -ldl -lgcc
/corvette/home/epics/devel/areaDetector-3-8/ADSupport/lib/linux-x86_64-ub18/libjp2.a(jas_stream.o): In function `jas_stream_tmpfile':
/corvette/home/epics/devel/areaDetector-3-8/ADSupport/supportApp/GraphicsMagickSrc/jp2/src/libjasper/O.linux-x86_64-ub18/../../../../../../supportApp/GraphicsMagickSrc/jp2/src/libjasper/base/jas_stream.c:368: warning: the use of `tmpnam' is dangerous, better use `mkstemp'
/usr/bin/ld: /corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/lib/linux-x86_64-ub18/libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: error: ld returned 1 exit status
/corvette/usr/local/epics-devel/base-7.0.3.1/configure/RULES_BUILD:212: recipe for target 'xspress3App' failed
make[1]: *** [xspress3App] Error 1
make[1]: Leaving directory '/corvette/home/epics/devel/areaDetector-3-8/xspress3-2-2/iocs/xspress3IOC/xspress3App/src/O.linux-x86_64-ub18'
/corvette/usr/local/epics-devel/base-7.0.3.1/configure/RULES_ARCHS:58: recipe for target 'install.linux-x86_64-ub18' failed
make: *** [install.linux-x86_64-ub18] Error 2

It appears to me that the pre-built libraries that Quantum is providing work on Centos 7 but not on Ubuntu 18.

Can they either provide the source code for these libraries, or new pre-built libraries that work on Ubuntu 18?

newville commented 4 years ago

@jwlodek Yes, libimg_mod.a is the vendor library provided as a binary from Quantum.
As far as I am aware, they only support Redhat/Centos 7, which uses gcc 4.8.
Did they say that they support building on ubuntu 18 and gcc 7.4?

newville commented 4 years ago

@MarkRivers please remove the specific and wrong environmental variables from configure/RELEASE. They do not belong there.

MarkRivers commented 4 years ago

@newville please look at every other repository in epics-modules. They all have configure/RELEASE files with entries like SUPPORT=/corvette/home/epics/devel or some other value that is specific to a particular site. It is well documented that RELEASE files in EPICS require local editing, either manual or automated as in the synApps “make release” mechanism.

The EPICS base Perl script for generating an example application generates such a RELEASE file as well.

MarkRivers commented 4 years ago

As far as I am aware, they only support Redhat/Centos 7, which uses gcc 4.8.

Where do they document that?

MarkRivers commented 4 years ago

Thanks to help from @mdavidsaver I was able to build on Ubuntu 18. It requires the linker flag -no-pie, which prevents building a position-independent executable.

This is a work-around, the better solution would be for Quantum Detectors to build libimg_mod.a with the -fPIC flag.

Unfortunately the -no-pie flag is not recognized on older gcc (e.g. 4.8) so I have commented out that line in iocs/xspress3IOC/xspress3App/src/Makefile by default. @jwlodek you will need to uncomment it to build on Ubuntu 18.

I have pushed the changes to the fix_release branch.

newville commented 4 years ago

I'm not sure if they document what systems they do not support. They distribute their calibration tools, epics modules, and tango support codes as Centos7 x86_64 rpms at https://www.quantumdetectors.com/rpm/el7/.

They provided us with a server with Centos6 on the initial purchase. In the fall of 2017, I upgraded that machine to Centos7 (at their direction) to get their latest drivers. All the Xspress3 minis I've seen come with a computer running Centos7.

I don't think they're doing anything too fancy with the fiber interface or with access to shared memory device they use, but I can believe that jumping many generations of gcc and glibc might not "just work" and that they would be reluctant to support tools they don't test and distribute.

So, maybe it would work to link their dll with versions of glibc, and maybe that will run. I don't know where you would do if something didn't work.

My goal (for many years) is to get them to support and distribute this epics support code as the default build. At present, they are still distributing epics support code from DLS and based on AD 1.9 which is outdated and missing important features that we actually need from the detector. I don't care if it has to run on the OS they choose or even if it works like other synApps modules. To me, the important question is: Is this code that QD could distribute and support?

For sure, having hardwired DLS configuration and support code was hard for us to understand when we converted it from AD1.9 to AD2. The configuration files should not appear to be APS specific.

mdmoo1978 commented 4 years ago

I converted this to AD2 more than four years ago or more, I don’t think it is APS specific at all, but quantum didn’t want to use it. The changes were really structural, except that they were also changing the code in addition to my conversion.

On Dec 7, 2019, at 6:08 AM, Matt Newville notifications@github.com wrote:

 I'm not sure if they document what systems they do not support. They distribute their calibration tools, epics modules, and tango support codes as Centos7 x86_64 rpms at https://www.quantumdetectors.com/rpm/el7/.

They provided us with a server with Centos6 on the initial purchase. In the fall of 2017, I upgraded that machine to Centos7 (at their direction) to get their latest drivers. All the Xspress3 minis I've seen come with a computer running Centos7.

I don't think they're doing anything too fancy with the fiber interface or with access to shared memory device they use, but I can believe that jumping many generations of gcc and glibc might not "just work" and that they would be reluctant to support tools they don't test and distribute.

So, maybe it would work to link their dll with versions of glibc, and maybe that will run. I don't know where you would do if something didn't work.

My goal (for many years) is to get them to support and distribute this epics support code as the default build. At present, they are still distributing epics support code from DLS and based on AD 1.9 which is outdated and missing important features that we actually need from the detector. I don't care if it has to run on the OS they choose or even if it works like other synApps modules. To me, the important question is: Is this code that QD could distribute and support?

For sure, having hardwired DLS configuration and support code was hard for us to understand when we converted it from AD1.9 to AD2. The configuration files should not appear to be APS specific.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

mdavidsaver commented 4 years ago

imo -fPIC builds are not a big ask. Compiler support goes back to (at least) gcc 2.95 circa 2001, which predates the first x86_64 (aka amd64) processors by two years.

mdavidsaver commented 4 years ago

Unfortunately the -no-pie flag is not recognized on older gcc (e.g. 4.8) so I have commented out that line in iocs/xspress3IOC/xspress3App/src/Makefile by default

@anjohnson Another good example of why being able to probe the toolchain would be useful. As with check_cxx_compiler_flag()

MarkRivers commented 4 years ago

@jwlodek have you been able to test the new version uncomenting the line for -no-pie? I tested that the IOC application can start and run, but I did not test communication with the Xspress3 because ours is in use.

Liam from Quantum says Ubuntu is not supported, but it looks like it may work anyway,

jwlodek commented 4 years ago

@MarkRivers Unfortunately I won't be able to test the connection at the moment as I don't yet have access to the device, but with the added -no-pie flag the build finishes successfully. However, it seems the most recent commit on the fix_release branch has introduced an issue:

jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git branch
* (HEAD detached at b064eac)
  fix_release
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ make -sj
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git checkout fix_release
M   configure/RELEASE
M   iocs/xspress3IOC/xspress3App/src/Makefile
Previous HEAD position was b064eac Add commented out USR_LDFLAGS option to avoid link errors on newer gcc, e.g. gcc 7.5
Switched to branch 'fix_release'
Your branch is up to date with 'origin/fix_release'.
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ make -sj
configure/CONFIG:17: /configure/CONFIG: No such file or directory
configure/RULES_TOP:2: /configure/RULES_TOP: No such file or directory
make[2]: *** No rule to make target '/configure/RULES_TOP'.  Stop.
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'xspress3IOC.install' failed
make[1]: *** [xspress3IOC.install] Error 2
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'iocs.install' failed
make: *** [iocs.install] Error 2
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$

I had to roll back on commit and then it compiled OK.

MarkRivers commented 4 years ago

The fix_release branch builds fine for me on Ubuntu 18.

TahoeU18:/corvette/home/epics/devel/xspress3> git checkout fix_release
M       iocs/xspress3IOC/xspress3App/src/Makefile
Already on 'fix_release'
Your branch is up to date with 'origin/fix_release'.
TahoeU18:/corvette/home/epics/devel/xspress3> make -sj

My only edit to fix_release is this:

TahoeU18:/corvette/home/epics/devel/xspress3> git diff
diff --git a/iocs/xspress3IOC/xspress3App/src/Makefile b/iocs/xspress3IOC/xspress3App/src/Makefile
index d954ead..7a0e794 100644
--- a/iocs/xspress3IOC/xspress3App/src/Makefile
+++ b/iocs/xspress3IOC/xspress3App/src/Makefile
@@ -12,7 +12,7 @@ PROD_IOC_Linux  += $(PROD_NAME)
 # Uncomment the USR_LDFLAGS line to avoid the following error which happens with newer versions of gcc
 #    libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
 #    /usr/bin/ld: final link failed: Nonrepresentable section on output
-#USR_LDFLAGS_Linux += -no-pie
+USR_LDFLAGS_Linux += -no-pie

 DBD += $(PROD_NAME).dbd

I see that you have also modified configure/RELEASE. I am using the unmodified version of configure/RELEASE

##RELEASE Location of external products
# Run "gnumake clean uninstall install" in the application
# top directory each time this file is changed.

TEMPLATE_TOP=$(EPICS_BASE)/templates/makeBaseApp/top

# If you don't want to install into $(TOP) then
# define INSTALL_LOCATION_APP here
#INSTALL_LOCATION_APP=<fullpathname>

SUPPORT=/corvette/home/epics/devel

# This module
XSPRESS3=$(SUPPORT)/xspress3-2-2

# XSPRESS3 requires areaDetector, and areaDetector/configure already defines
# ASYN, CALC, etc.
AREA_DETECTOR=$(SUPPORT)/areaDetector-3-8
-include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

-include $(TOP)/../RELEASE.local
-include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(TOP)/configure/RELEASE.local
jwlodek commented 4 years ago

The only modifications made to configure/RELEASE were those done by make release:

##RELEASE Location of external products
# Run "gnumake clean uninstall install" in the application
# top directory each time this file is changed.

TEMPLATE_TOP=$(EPICS_BASE)/templates/makeBaseApp/top

# If you don't want to install into $(TOP) then
# define INSTALL_LOCATION_APP here
#INSTALL_LOCATION_APP=<fullpathname>

SUPPORT=/epics/test/support

# This module
XSPRESS3=$(SUPPORT)/xspress3

# XSPRESS3 requires areaDetector, and areaDetector/configure already defines
# ASYN, CALC, etc.
AREA_DETECTOR=$(SUPPORT)/areaDetector
-include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

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

Taking a look at the most recent commit, I see that there were additions to the iocs/xspress3IOC/configure/RELEASE file, with paths to areaDetector and support. I modified these to the correct ones, but I still get the same error when trying to run make:

make[2]: Entering directory '/epics/test/support/xspress3/iocs/xspress3IOC'
configure/CONFIG:17: /configure/CONFIG: No such file or directory
configure/RULES_TOP:2: /configure/RULES_TOP: No such file or directory
make[2]: *** No rule to make target '/configure/RULES_TOP'.  Stop.
make[2]: Leaving directory '/epics/test/support/xspress3/iocs/xspress3IOC'
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'xspress3IOC.install' failed
make[1]: *** [xspress3IOC.install] Error 2
make[1]: Leaving directory '/epics/test/support/xspress3/iocs'
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'iocs.install' failed
make: *** [iocs.install] Error 2
MarkRivers commented 4 years ago

Please send your iocs/xspress3IOC/configure/RELEASE file.

It is building fine for me with the version of that file on Github. It looks to me like your error indicates a missing or incorrect definition.

jwlodek commented 4 years ago

I removed my local copy cloned a fresh one, and I was able to reproduce the issue:

jwlodek@HP-Z6-G4-Workstation:/epics/test/support$ rm -rf xspress3/
jwlodek@HP-Z6-G4-Workstation:/epics/test/support$ git clone --single-branch --branch=fix_release https://github.com/epics-modules/xspress3
Cloning into 'xspress3'...
remote: Enumerating objects: 75, done.
remote: Counting objects: 100% (75/75), done.
remote: Compressing objects: 100% (52/52), done.
remote: Total 4716 (delta 26), reused 51 (delta 22), pack-reused 4641
Receiving objects: 100% (4716/4716), 13.93 MiB | 23.81 MiB/s, done.
Resolving deltas: 100% (3206/3206), done.
jwlodek@HP-Z6-G4-Workstation:/epics/test/support$ make release

I then changed the paths in iocs/xspress3IOC/configure/RELEASE. The only changes are the module paths, and the -no-pie flag:

#RELEASE Location of external products
# Run "gnumake clean uninstall install" in the application
# top directory each time this file is changed.

#SUPPORT=/corvette/home/epics/devel
SUPPORT=/epics/test

# This module
XSPRESS3=$(TOP)/../..

# XSPRESS3 requires areaDetector, and areaDetector/configure already defines
# ASYN, CALC, etc.
AREA_DETECTOR=$(SUPPORT)/areaDetector
-include $(XSPRESS3)/../RELEASE.$(EPICS_HOST_ARCH).local
-include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

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

Here is the diff after all the changes made:

jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git diff
diff --git a/configure/RELEASE b/configure/RELEASE
index 0b70a2e..0c0fd31 100644
--- a/configure/RELEASE
+++ b/configure/RELEASE
@@ -8,14 +8,14 @@ TEMPLATE_TOP=$(EPICS_BASE)/templates/makeBaseApp/top
 # define INSTALL_LOCATION_APP here
 #INSTALL_LOCATION_APP=<fullpathname>

-SUPPORT=/corvette/home/epics/devel
+SUPPORT=/epics/test/support

 # This module
-XSPRESS3=$(SUPPORT)/xspress3-2-2
+XSPRESS3=$(SUPPORT)/xspress3

 # XSPRESS3 requires areaDetector, and areaDetector/configure already defines
 # ASYN, CALC, etc.
-AREA_DETECTOR=$(SUPPORT)/areaDetector-3-8
+AREA_DETECTOR=$(SUPPORT)/areaDetector
 -include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
 -include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

diff --git a/iocs/xspress3IOC/configure/RELEASE b/iocs/xspress3IOC/configure/RELEASE
index 7ebb9cc..b310000 100644
--- a/iocs/xspress3IOC/configure/RELEASE
+++ b/iocs/xspress3IOC/configure/RELEASE
@@ -2,14 +2,15 @@
 # Run "gnumake clean uninstall install" in the application
 # top directory each time this file is changed.

-SUPPORT=/corvette/home/epics/devel
+#SUPPORT=/corvette/home/epics/devel
+SUPPORT=/epics/test

 # This module
 XSPRESS3=$(TOP)/../..

 # XSPRESS3 requires areaDetector, and areaDetector/configure already defines
 # ASYN, CALC, etc.
-AREA_DETECTOR=$(SUPPORT)/areaDetector-3-8
+AREA_DETECTOR=$(SUPPORT)/areaDetector
 -include $(XSPRESS3)/../RELEASE.$(EPICS_HOST_ARCH).local
 -include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

diff --git a/iocs/xspress3IOC/xspress3App/src/Makefile b/iocs/xspress3IOC/xspress3App/src/Makefile
index d954ead..7a0e794 100644
--- a/iocs/xspress3IOC/xspress3App/src/Makefile
+++ b/iocs/xspress3IOC/xspress3App/src/Makefile
@@ -12,7 +12,7 @@ PROD_IOC_Linux  += $(PROD_NAME)
 # Uncomment the USR_LDFLAGS line to avoid the following error which happens with newer versions of gcc
 #    libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
 #    /usr/bin/ld: final link failed: Nonrepresentable section on output 
-#USR_LDFLAGS_Linux += -no-pie
+USR_LDFLAGS_Linux += -no-pie

 DBD += $(PROD_NAME).dbd

And then running make:

jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ make -sj
configure/CONFIG:17: /configure/CONFIG: No such file or directory
configure/RULES_TOP:2: /configure/RULES_TOP: No such file or directory
make[2]: *** No rule to make target '/configure/RULES_TOP'.  Stop.
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'xspress3IOC.install' failed
make[1]: *** [xspress3IOC.install] Error 2
/epics/test/base/configure/RULES_DIRS:84: recipe for target 'iocs.install' failed
make: *** [iocs.install] Error 2

If I roll back to the previous commit and try again:

jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git stash
Saved working directory and index state WIP on fix_release: 55c793a Fix to build on Windows in same tree as Linux
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git checkout b064eacab3904c85dc649498fd2aa0f40dd9b654 && cd ..
jwlodek@HP-Z6-G4-Workstation:/epics/test/support$ make release && cd xspress3
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ git diff
diff --git a/configure/RELEASE b/configure/RELEASE
index 0b70a2e..0c0fd31 100644
--- a/configure/RELEASE
+++ b/configure/RELEASE
@@ -8,14 +8,14 @@ TEMPLATE_TOP=$(EPICS_BASE)/templates/makeBaseApp/top
 # define INSTALL_LOCATION_APP here
 #INSTALL_LOCATION_APP=<fullpathname>

-SUPPORT=/corvette/home/epics/devel
+SUPPORT=/epics/test/support

 # This module
-XSPRESS3=$(SUPPORT)/xspress3-2-2
+XSPRESS3=$(SUPPORT)/xspress3

 # XSPRESS3 requires areaDetector, and areaDetector/configure already defines
 # ASYN, CALC, etc.
-AREA_DETECTOR=$(SUPPORT)/areaDetector-3-8
+AREA_DETECTOR=$(SUPPORT)/areaDetector
 -include $(TOP)/../RELEASE.$(EPICS_HOST_ARCH).local
 -include $(AREA_DETECTOR)/configure/RELEASE_PRODS.local

diff --git a/iocs/xspress3IOC/xspress3App/src/Makefile b/iocs/xspress3IOC/xspress3App/src/Makefile
index d954ead..7a0e794 100644
--- a/iocs/xspress3IOC/xspress3App/src/Makefile
+++ b/iocs/xspress3IOC/xspress3App/src/Makefile
@@ -12,7 +12,7 @@ PROD_IOC_Linux  += $(PROD_NAME)
 # Uncomment the USR_LDFLAGS line to avoid the following error which happens with newer versions of gcc
 #    libimg_mod.a(img_mod_linux.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
 #    /usr/bin/ld: final link failed: Nonrepresentable section on output 
-#USR_LDFLAGS_Linux += -no-pie
+USR_LDFLAGS_Linux += -no-pie

 DBD += $(PROD_NAME).dbd

jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$ make -sj
/epics/test/support/areaDetector/ADSupport/lib/linux-x86_64/libjp2.a(jas_stream.o): In function `jas_stream_tmpfile':
/epics/test/support/areaDetector/ADSupport/supportApp/GraphicsMagickSrc/jp2/src/libjasper/O.linux-x86_64/../../../../../../supportApp/GraphicsMagickSrc/jp2/src/libjasper/base/jas_stream.c:368: warning: the use of `tmpnam' is dangerous, better use `mkstemp'
jwlodek@HP-Z6-G4-Workstation:/epics/test/support/xspress3$

and it compiles no problem.

MarkRivers commented 4 years ago

Look carefully. You have defined SUPPORT differently in the 2 RELEASE files. In the first it is /epics/test/support in the second it is /epics/test

MarkRivers commented 4 years ago

@jwlodek have you been able to build the fix_release branch after fixing the definition of SUPPORT in iocs/xspress3IOC/configure/RELEASE?

jwlodek commented 4 years ago

@MarkRivers Looks like that was in fact the issue, no idea how I could have missed it twice. Regarding the paths in that iocs/xspress3IOC/configure/RELEASE file, would there be a way for them to also get updated with make release? Thanks again.

MarkRivers commented 4 years ago

Regarding the paths in that iocs/xspress3IOC/configure/RELEASE file, would there be a way for them to also get updated with make release?

I was thinking the same thing. I will take a look and talk to Tim Mooney about it.

newville commented 4 years ago

@jwlodek @MarkRivers Just to be clear, and coming back to the origin of this conversation: This module is definitely designed to be built as an areaDetector, putting the top-level xspress3 directory as a subdirectory of areaDetector.

Structuring it this way allows the configuration files in xspress3/configure and xspress3/iocs/xspress3IOC/configure to be very generic, needing no explicit setting of SUPPORT or pointing to other modules outside of areaDetector. It has been this way since @mdmoo1978 first ported this module to AD2, and it has worked well for us for many years. The configuration files are very generic. This directory structure is why configure/RELEASE in current master has 

 -include $(TOP)/../configure/RELEASE_LIBS_INCLUDE

I guess it could also have

-include $(TOP)/../configure/RELEASE_PRODS_INCLUDE

though I do not think this is actually necessary.

I'm completely lost about why one would want to run this on a system other than the one supplied and supported by the vendor. The haardware needs to be a fiber link from the Xspress3 / Xspress3 mini box to the computer running this IOC. The vendor supplies that computer, with an OS they support. Wiping the disk and installing a different OS is not supported by the vendor. Why should the epics interface support that?

So, I think that the right answer might be that we do not want this to work like other epics modules. We want it to work like an areaDetector, and then it does not need any changes to the configuration files.

jwlodek commented 4 years ago

At NSLS2 we wish to run xspress3 on different machines than those supplied by the vendor (ubuntu, debian). Also, we are working on a tool that given a configuration builds the entire EPICS stack on a CI server, which is why I would like the module to behave like other epics-modules. I have no objection to it keeping the areaDetector build system, but then I would suggest that it be moved to the areaDetector github page, and then perhaps other modules that depend on areaDetector should follow suit. Basically, my argument is that there should be consistency in how these get handled, to avoid confusion when building.

mdavidsaver commented 4 years ago

FYI. https://github.com/epics-base/ci-scripts

@ralphlange

newville commented 4 years ago

@jwlodek

At NSLS2 we wish to run xspress3 on different machines than those supplied by the vendor (ubuntu, debian).

OK. I wish to run it on whatever machine the vendor says it should use.

Also, we are working on a tool that given a configuration builds the entire EPICS stack on a CI server, which is why I would like the module to behave like other epics-modules. I have no objection to it keeping the areaDetector build system, but then I would suggest that it be moved to the areaDetector github page, and then perhaps other modules that depend on areaDetector should follow suit. Basically, my argument is that there should be consistency in how these get handled, to avoid confusion when building.

Is the confusion based on the location and name of this repository? Like, if this code were within areaDetector would that make a difference?

This code has a long and convoluted history with many people from DLS and APS involved over the years. There has been a high correlation with working on this code and "just about to leave the community". You may notice that the QuantumDetectors version of the xspress3 code (ie, what they ship) at https://github.com/quantumdetectors/xspress3-epics is 52 commits ahead, 187 commits behind epics-modules:master, while the DLS xspress3 code at https://github.com/dls-controls/xspress3 is 86 commits ahead, 160 commits behind epics-modules:master. This dates back about five years now, when the code was ported from AD1.9 to AD2 and greatly cleaned up. I am much more concerned about that difference than I am about the name or location of this module.

I guess we could consider moving this code. I don't think that would help the already dim prospects of sorting out the differences with DLS and QD. But, it sort of sound like you would like this code to move from epics-modules because it does not meet your expectations for what should be in epics-modules. Is there consistency among the other modules in epics-modules? Is there a documented consistency that the repos in epics-modules are expected to take?

jwlodek commented 4 years ago

Most all of the modules in epics-modules (including the ones that use areaDetector such as quadEM) have their configure/RELEASE file written with paths to dependencies, see here. Then, make release is run from the top of the support directory which updates these paths relative to a central location, and then you can simply run make in each module's folder to build it. This allows modules to be placed in any location and still be build-able (as in xspress3 can be placed outside of areaDetector).

The benefit of this is perhaps limited, but since all the modules are written this way, there is consistency. Placing xspress3 in the areaDetector folder during building is fine as well, but as someone who has not used xspress3 prior to this, because it was part of epics-modules I was expecting it to work like the other ones. If it was part of the areaDetector organization on github instead, I would know right away that it should be placed in the areaDetector directory in order to build it.

MarkRivers commented 4 years ago

I will write an explanation of the history, present state, and possible future of handling RELEASE files in areaDetector and related modules (dxp, dxpSITORO, quadEM, Xspress3, etc) in a day or two. I’m on travel now.

oksanagit commented 4 years ago

I think this is resolved and should work with make release.

newville commented 4 years ago

@oksanagit yes, I agree, this is resolved. Will close.