Enable custom autosave configuration #1

henriquesimoes · 2024-01-18T14:12:40Z

The IOC loads autosave configuration automatically through the device.cmd, which makes it harder to have a personalized configuration. Moreover, using cd in the iocsh template makes the set_requirefile_path settings to be impossible to be used for local custom files, as the container volume mounted during execution is not predictable, which was the initial intention while writing, for instance, plugins.req.

Make this configuration possible to be overwritten by local custom files, either in the overall autosave configuration or in the specific requirement files used.

ericonr

For commit message, with "overseed" do you mean "override"?

ericonr · 2024-01-24T11:13:42Z

iocBoot/iocCamera/st.cmd

-cd /opt/ad-aravis-epics-ioc/iocBoot/iocCamera
+epicsEnvSet("IOC_CONFIG", "/opt/ad-aravis-epics-ioc/iocBoot/iocCamera")

-< envPaths
+< $(IOC_CONFIG)/envPaths


In order to use the variables that the build system already defines for us, I think you could do something like:

< /full/path/to/envPaths epicsEnvSet("IOC_CONFIG", "$(TOP)/iocBoot/$(IOC)")

It's unfortunate that we always need the full path to the binary and to envPaths...

Do you like this idea?

Do you like this idea?

I've thought about that option. I seems to me equally redundant in terms of the base path. Automatically handling the IOC name might be good, but I don't think we'll often change that. I turned out sticking with the one I committed because I thought it would be clearer to have only a constant (env var) defining the base path for the entire script (but the shebang). The way you suggest, the path for envPaths seems to define it before we reach IOC_CONFIG declaration.

Anyway, I currently don't think the rationale I followed is the most important, though. I'm more concerned about how envPath is defined when we install the IOC in another directory. What value will $(TOP) have in that case? Will it be the build or installation directory? In addition, is iocBoot installable?

I've though about that option. I seems to me equally redundant in terms of the base path.

It kind of is. I think my reasoning for it is that another st.cmd for a different IOC can copy that line without having to change anything but the top bits (shebang and envPaths, which kinda have to be that way?). But I don't know if that actually checks out 😅

The way you suggest, the path for envPaths seems to define it before we reach IOC_CONFIG declaration.

Indeed.

What value will $(TOP) have in that case? Will it be the build or installation directory? In addition, is iocBoot installable?

I have no idea 🤔 But as we discussed, source code doesn't seem to take up that much storage, right?

I think my reasoning for it is that another st.cmd for a different IOC can copy that line without having to change anything but the top bits.

I can quote your comment and add "(shebang and IOC_CONFIG, which kinda have to be that way?)" right after it, and it is still valid. Maybe it becomes a matter of which one is easier to identify where it should be changed. envPath seems less verbose in that sense, but maybe less close to the concept of path definition.

But as we discussed, source code doesn't seem to take up that much storage, right?

Yeah, it doesn't. But if that's really really easy to make independent of the installation path, that would be nice to reduce yet another point of breakage for custom paths. But coming to think about that, both of these definitions (envPath and shebang) are hard to overcome. Unless we make the IOC binary installed somewhere /usr/bin/env can find, and envPath is in a standard location. TBH, I can't see the downsides of it right away.

henriquesimoes · 2024-02-07T11:41:10Z

For commit message, with "overseed" do you mean "override"?

Yeah, it could be. I actually wanted to say "supersede" when I was writing, but I mistook it entirely. Thanks for the note.

User defined configuration files currently are supported for local directory (.) and `basler` subdirectory. However, other GenICam cameras can be used with this IOC. Therefore, make the `genicam` folder also a possibility to supersede the previous `basler` directory. The specification of the `basler` directory is kept for backward compatibility with existing deploys.

Defining the autosave in device.cmd makes the current template configuration not possible to be changed by users without conflict. This would render users to drop device.cmd entirely. Thus, move the call to autosave template in the st.cmd template. This also makes the template script more clear about the usage of autosave.

This makes possible to remove the hardcoded path used in the shebang, aiming at removing all the startup iocsh template script dependencies of the build directory. A post-install phase is used to create symbolic links instead of changing the INSTALL_LOCATION. This change is backward compatible, as the binary can still be found in the previous path.

Templates loaded with relative path requires that we `cd` to the template directory at the start of the iocsh script. This makes it harder to define autosave requirement files to be loaded based on the user's current directory, which is desired when it is mounted as a volume. Avoid changing the directory entirely by using absolute path for IOC configuration files based on `envPaths` definition. This makes it clearer which loaded files are templates and which are locally defined. Note that this configuration makes it the entire script independent of the build and install location of the IOC, since `envPaths` is placed in a standard directory. It has been chosen to be under `share/misc` as it is a single application architecture-independent configuration file [1]. [1]: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s11.html#usrsharemiscMiscellaneousArchitecture

henriquesimoes · 2024-03-25T20:20:36Z

Current implementation requires cnpem/epics-in-docker#55 to be released.

ericonr · 2024-03-28T17:28:33Z

Makefile

 iocBoot_DEPEND_DIRS += $(filter %App,$(DIRS))
+postinstall_DEPEND_DIRS += iocBoot $(wildcard *App)


Isn't it enough that we depend on iocBoot and it depends on all "App" directories? If not, wouldn't it be best to use the filter command from above, nonetheless?

Isn't it enough that we depend on iocBoot and it depends on all "App" directories?

Yes, it is enough, since iocBoot depends on App. I listed it explicitly because we depend directly on an artifact from the App directory.

It is okay to simplify this assuming that iocBoot dependencies will never change.

If not, wouldn't it be best to use the filter command from above, nonetheless?

Indeed. My mistake.

ericonr · 2024-03-28T17:39:41Z

postinstall/Makefile

+install:
+	ln -s $(shell realpath $(INSTALL_HOST_BIN))/Camera /usr/local/bin/


Could this use TOP instead of realpath?

Btw can you still run make clean with this simple makefile? Do the includes handle it?

Could this use TOP instead of realpath?

Yep. That seems better indeed.

Btw can you still run make clean with this simple makefile?

Hmm. I haven't analyzed it. I bet we would have broken links around. Probably it will require a clean target to unlink them.

Could this use TOP instead of realpath?

Yep. That seems better indeed.

Hmm. Actually, TOP and INSTALL_LOCATION are relative paths to the current folder when we reach this target (INSTALL_LOCATION becomes a copy of TOP as it is not defined in the CONFIG_SITE). Here are the available variables:

TOP = .. INSTALL_LOCATION = .. INSTALL_ABSOLUTE = /opt/ad-aravis-epics-ioc FINAL_LOCATION = /opt/ad-aravis-epics-ioc INSTALL_LOCATION_BIN = ../bin EPICS_HOST_ARCH = linux-x86_64 INSTALL_HOST_BIN = ../bin/linux-x86_64 IOCS_APPL_TOP = /opt/ad-aravis-epics-ioc

I thought it would be okay to use FINAL_LOCATION (or INSTALL_ABSOLUTE), which are absolute and undocumented, along with INSTALL_HOST_BIN, but it turns out it does not work well due to INSTALL_HOST_BIN being defined based on the relative TOP. That is, $(INSTALL_ABSOLUTE)/$(INSTALL_HOST_BIN)/Camera does not point to the correct binary path.

For me, either we use my original suggestion or $(FINAL_LOCATION)/bin/$(EPICS_HOST_ARCH)/Camera. I think using FINAL_LOCATION might be more appropriate here, as this is really the place the build system expects the binary to go (even though it does not finish the job on its own).

Using FINAL_LOCATION seems like a good choice.

ericonr · 2024-03-28T20:22:37Z

iocBoot/iocCamera/st.cmd

-cd /opt/ad-aravis-epics-ioc/iocBoot/iocCamera
-
-< envPaths
+< /usr/local/share/misc/envPaths


I'm wondering if camera-envPaths or envPathsCamera wouldn't be a better name... For namespacing reasons.

(I prefer the second one)

To namespace it, I'd rather move it to a dedicated folder under share. Maybe /usr/local/share/ad-aravis{,-epics-ioc}/envPaths. Other installed template scripts and configuration files could share the namespace this way.

But then I fear we are losing the benefit of defining a (copy-and-paste) standard for different IOCs. Do we want to namespace to support different IOCs in the same container?

But then I fear we are losing the benefit of defining a (copy-and-paste) standard for different IOCs.

I don't think things are strictly copy and paste now, since the binary has a specific name...

Do we want to namespace to support different IOCs in the same container?

For most cases, I don't think so. But I feel like using a file in the same path for every container, which has different contents based on the container, can be a bit confusing; if the path is namespaced, then it's clear it's specific to this application.

Otherwise, for consistency, there should be a dynamic link to the Camera binary with a generic name (and we should document such a standard). I might be in favor of it if we go the whole way, but we can discuss more in person.

ericonr · 2024-03-28T20:26:03Z

postinstall/Makefile

+	mkdir -p /usr/local/share/misc
+	ln -s $(shell realpath $(TOP))/iocBoot/iocCamera/envPaths /usr/local/share/misc/


We could accept a PREFIX argument here, I think. Just a tad cleaner :)

Could you also add a comment to the commit message that envPaths is arch-independent because it only lists the root directory for each module, not any internal build directory?

We could accept a PREFIX argument here, I think. Just a tad cleaner :)

Seems nice. Would PREFIX ?= /usr/local do a good job here or is there a better way to parameterize this?

Could you also add a comment to the commit message that envPaths is arch-independent because it only lists the root directory for each module, not any internal build directory?

Sure. Will do.

PREFIX ?= /usr/local

Good enough. If you want to go further you can define bindir and sharedir based on PREFIX so even those specific directories can be overridden, but I think that's unnecessary.

henriquesimoes added the bug Something isn't working label Jan 18, 2024

henriquesimoes requested a review from ericonr January 18, 2024 14:12

ericonr reviewed Jan 24, 2024

View reviewed changes

henriquesimoes force-pushed the autosave branch from a3094a2 to 509f0b2 Compare February 20, 2024 12:48

henriquesimoes added 2 commits February 20, 2024 09:48

henriquesimoes force-pushed the autosave branch from 509f0b2 to 11156e4 Compare February 20, 2024 12:49

henriquesimoes added 2 commits March 25, 2024 16:25

henriquesimoes force-pushed the autosave branch from 11156e4 to 30f8ffc Compare March 25, 2024 19:34

henriquesimoes marked this pull request as draft March 25, 2024 19:39

ericonr suggested changes Mar 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable custom autosave configuration #1

Enable custom autosave configuration #1

henriquesimoes commented Jan 18, 2024

ericonr left a comment

ericonr Jan 24, 2024

henriquesimoes Feb 7, 2024 •

edited

Loading

ericonr Feb 7, 2024

henriquesimoes Feb 7, 2024 •

edited

Loading

henriquesimoes commented Feb 7, 2024

henriquesimoes commented Mar 25, 2024

ericonr Mar 28, 2024

henriquesimoes Apr 1, 2024

ericonr Mar 28, 2024

ericonr Mar 28, 2024

henriquesimoes Apr 1, 2024

henriquesimoes Apr 2, 2024

ericonr Apr 26, 2024

ericonr Mar 28, 2024

henriquesimoes Apr 1, 2024

ericonr Apr 26, 2024

ericonr Mar 28, 2024

henriquesimoes Apr 1, 2024

ericonr Apr 26, 2024

		iocBoot_DEPEND_DIRS += $(filter %App,$(DIRS))
		postinstall_DEPEND_DIRS += iocBoot $(wildcard *App)

		install:
		ln -s $(shell realpath $(INSTALL_HOST_BIN))/Camera /usr/local/bin/

		mkdir -p /usr/local/share/misc
		ln -s $(shell realpath $(TOP))/iocBoot/iocCamera/envPaths /usr/local/share/misc/

Enable custom autosave configuration #1

Are you sure you want to change the base?

Enable custom autosave configuration #1

Conversation

henriquesimoes commented Jan 18, 2024

ericonr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henriquesimoes Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henriquesimoes Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

henriquesimoes commented Feb 7, 2024

henriquesimoes commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henriquesimoes Feb 7, 2024 •

edited

Loading

henriquesimoes Feb 7, 2024 •

edited

Loading