search

slac-epics / epics-base

Other
0 stars 3 forks source link

Clean up absolute paths in Makefiles #11

Open JJL772 opened 6 months ago

JJL772 commented 6 months ago

Currently there's a lot of /afs/slac/blah in the configure Makefiles, mainly to point EPICS at the correct cross compilers.

These absolute paths should be replaced with $(PACKAGE_SITE_TOP) or a similar environment variable to improve portability.

That said, current environment scripts don't have any envvars pointed at /afs/slac/package, PACKAGE_TOP and PACKAGE_SITE_TOP all point to /afs/slac/g/lcls/package. So, this change is only going to be relevant for S3DF, where we have a single package directory referred to by PACKAGE_SITE_TOP.

Alternatively, we could introduce a new PACKAGE_SITE-like variable in the new S3DF setup scripts, perhaps EPICS_PACKAGE_TOP, which will default to /afs/slac/package if it does not exist. This would ensure builds don't break on old machines.

JJL772 commented 5 months ago

This should be applied in the future, after we migrate to S3DF. Currently no way to refer to /afs/slac/package via envvars. PACKAGE_TOP and PACKAGE_SITE_TOP point to /afs/slac/g/lcls/package


diff --git a/configure/os/CONFIG.Common.RTEMS b/configure/os/CONFIG.Common.RTEMS
index 9c601e7ce..d217f4177 100644
--- a/configure/os/CONFIG.Common.RTEMS
+++ b/configure/os/CONFIG.Common.RTEMS
@@ -30,7 +30,7 @@
 -include $(CONFIG)/os/CONFIG_SITE.Common.RTEMS

 ifeq ($(T_A),RTEMS-mvme5500)
-RTEMS_SITE_TOP=/afs/slac/package/rtems/4.6.2/
+RTEMS_SITE_TOP=$(PACKAGE_SITE_TOP)/rtems/4.6.2/
 RTEMS_BASE=$(RTEMS_SITE_TOP)/host/i386_linux2x/
 else
 #RTEMS_MAKEFILE_PATH=/afs/slac/package/rtems/4.7/target/ssrlApps/config
@@ -49,7 +49,7 @@ else
 # since checkRelease is pretty dumb...
 #
 #RTEMS_SITE_TOP=/afs/slac/package/rtems/4.9.4/
-RTEMS_SITE_TOP=/afs/slac/package/rtems/4.10.2/
+RTEMS_SITE_TOP=$(PACKAGE_SITE_TOP)/rtems/4.10.2/
 RTEMS_BASE=$(RTEMS_SITE_TOP)/host/amd64_linux26/

 #RTEMS_PATCHLVL=rtems_p0
diff --git a/configure/os/CONFIG_SITE.Common.linuxRT-arm_zynq b/configure/os/CONFIG_SITE.Common.linuxRT-arm_zynq
index 777df15df..30f42b732 100644
--- a/configure/os/CONFIG_SITE.Common.linuxRT-arm_zynq
+++ b/configure/os/CONFIG_SITE.Common.linuxRT-arm_zynq
@@ -1,6 +1,6 @@
 # Location of linuxRT GNU ToolChain
 #LINUX_RT_HOME=/nfs/slac/g/lcls/build/linuxRT
-LINUX_RT_HOME=/afs/slac/package/linuxRT
+LINUX_RT_HOME=$(PACKAGE_SITE_TOP)/linuxRT
 #LINUX_RT_HOME=/afs/slac/g/lcls/package/linuxRT

 #LINUXRT_BUILDROOT_VERSION=buildroot-2015.02
@@ -35,7 +35,7 @@ COMMANDLINE_LIBRARY = READLINE
 # =========================================================================
 # Valgrind support
 # =========================================================================
-VALGRIND_TOP=/afs/slac/g/lcls/package/valgrind/3.10.1
+VALGRIND_TOP=$(PACKAGE_SITE_TOP)/valgrind/3.10.1
 VALGRIND_T_A=$(VALGRIND_TOP)/$(T_A)
 ifneq ($(wildcard $(VALGRIND_T_A)),)
 VALGRIND_INCLUDE=$(VALGRIND_T_A)/include
diff --git a/configure/os/CONFIG_SITE.Common.linuxRT-i686 b/configure/os/CONFIG_SITE.Common.linuxRT-i686
index 777df15df..30f42b732 100644
--- a/configure/os/CONFIG_SITE.Common.linuxRT-i686
+++ b/configure/os/CONFIG_SITE.Common.linuxRT-i686
@@ -1,6 +1,6 @@
 # Location of linuxRT GNU ToolChain
 #LINUX_RT_HOME=/nfs/slac/g/lcls/build/linuxRT
-LINUX_RT_HOME=/afs/slac/package/linuxRT
+LINUX_RT_HOME=$(PACKAGE_SITE_TOP)/linuxRT
 #LINUX_RT_HOME=/afs/slac/g/lcls/package/linuxRT

 #LINUXRT_BUILDROOT_VERSION=buildroot-2015.02
@@ -35,7 +35,7 @@ COMMANDLINE_LIBRARY = READLINE
 # =========================================================================
 # Valgrind support
 # =========================================================================
-VALGRIND_TOP=/afs/slac/g/lcls/package/valgrind/3.10.1
+VALGRIND_TOP=$(PACKAGE_SITE_TOP)/valgrind/3.10.1
 VALGRIND_T_A=$(VALGRIND_TOP)/$(T_A)
 ifneq ($(wildcard $(VALGRIND_T_A)),)
 VALGRIND_INCLUDE=$(VALGRIND_T_A)/include
diff --git a/configure/os/CONFIG_SITE.Common.linuxRT-x86_64 b/configure/os/CONFIG_SITE.Common.linuxRT-x86_64
index 777df15df..30f42b732 100644
--- a/configure/os/CONFIG_SITE.Common.linuxRT-x86_64
+++ b/configure/os/CONFIG_SITE.Common.linuxRT-x86_64
@@ -1,6 +1,6 @@
 # Location of linuxRT GNU ToolChain
 #LINUX_RT_HOME=/nfs/slac/g/lcls/build/linuxRT
-LINUX_RT_HOME=/afs/slac/package/linuxRT
+LINUX_RT_HOME=$(PACKAGE_SITE_TOP)/linuxRT
 #LINUX_RT_HOME=/afs/slac/g/lcls/package/linuxRT

 #LINUXRT_BUILDROOT_VERSION=buildroot-2015.02
@@ -35,7 +35,7 @@ COMMANDLINE_LIBRARY = READLINE
 # =========================================================================
 # Valgrind support
 # =========================================================================
-VALGRIND_TOP=/afs/slac/g/lcls/package/valgrind/3.10.1
+VALGRIND_TOP=$(PACKAGE_SITE_TOP)/valgrind/3.10.1
 VALGRIND_T_A=$(VALGRIND_TOP)/$(T_A)
 ifneq ($(wildcard $(VALGRIND_T_A)),)
 VALGRIND_INCLUDE=$(VALGRIND_T_A)/include
ernestow commented 5 months ago

We don't want use environment variables in EPICS configuration files. The technique recommended is to set variables in the configure files and use the method of included config files in other config files. So, hard-coded paths are okay when specifying the location of external packages such as our toolchains, valgrind, ...

JJL772 commented 5 months ago

Alternative solution proposed in #16

Instead of environment variables, they're just make variables that can be overridden by CONFIG_SITE.local

ernestow commented 5 months ago

I like this solution much better as it is in line with what EPICS maintainers proposed in the workflow.

From: Jeremy L. @.> Sent: Friday, March 15, 2024 2:34 PM To: slac-epics/epics-base @.> Cc: Williams Jr., Ernest L. @.>; Comment @.> Subject: Re: [slac-epics/epics-base] Clean up absolute paths in Makefiles (Issue #11)

Alternative solution proposed in #16https://github.com/slac-epics/epics-base/pull/16

Instead of environment variables, they're just make variables that can be overridden by CONFIG_SITE.local

— Reply to this email directly, view it on GitHubhttps://github.com/slac-epics/epics-base/issues/11#issuecomment-2000475534, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB44BJDMH2DEAICXD7IBS43YYNSPRAVCNFSM6AAAAABENX3ACGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBQGQ3TKNJTGQ. You are receiving this because you commented.Message ID: @.***>