Ibek j20 changes #3

gilesknap · 2023-09-18T20:26:53Z

This is a major refactor to tidy up ioc compilation in epics-containers

See related changes in the first test ioc here epics-containers/ioc-adsimdetector#1

gilesknap · 2023-09-18T20:31:46Z

@coretl its getting to the point where I am happy with it.

I think we need a 'add block to file' function which would make the changes here https://github.com/epics-containers/ibek-support/blob/ibek-j20-changes/ADSupport/install.sh#L26-L62 in an idempotent fashion.

coretl

Much neater.

We definitely need support.py for those things that are best written in python, but I think we should go down one route:

Public interface is bash functions only that may call into support.py (but we hide this fact away)
Public interface is python functions only in support.py and we ditch the bash

If we do 1 we would have things like add_module_to_release instead of support add-module-to-release, but functions would grow.

If we do 2 we would have things like ${SUPPORT} git-clone-tag ${NAME} ${VERSION} --org=https://github.com/areaDetector instead of git_clone_tag ${NAME} ${VERSION} https://github.com/areaDetector, but there would be no bash functions to read.

I don't know which is better. @GDYendell thoughts?

coretl · 2023-09-19T09:00:50Z

iocStats/dbd

Do we like the separate files for lib and dbd?

coretl · 2023-09-19T09:01:52Z

iocStats/install.sh

+git_clone_tag ${NAME} ${VERSION}
+
+support add-module-to-release ${NAME}
+


Or should we have

add_ioc_libs devIocStats add_ioc_dbds devIocStats.dbd

I MUCH prefer this. Forgot that I wanted to review this bit before doing the PR.

This is now implemented

GDYendell · 2023-09-19T09:47:53Z

Public interface is python functions only in support.py and we ditch the bash

Including ditching the <support module>/install.sh scripts?

gilesknap · 2023-09-19T09:49:26Z

I like 1. and that is what we currently have, although this one is a bit of a stretch to say it is bash 🙂 # Call functions in support.py function support { # Make RELEASE files to point to the correct paths by adding RELEASE.local python ${GLOBALS_DIR}/support.py ${@} }

coretl · 2023-09-19T10:01:20Z

Public interface is python functions only in support.py and we ditch the bash

Including ditching the <support module>/install.sh scripts?

I meant ditch the bash functions... install.sh would remain

gilesknap · 2023-09-19T10:22:19Z

@GDYendell @coretl We have decided to ditch functions.sh and go pure python.

Custom steps inside of install.sh instances would use bash commands if appropriate but can also call python,

gilesknap · 2023-09-20T15:16:47Z

OK @coretl @GDYendell the yet another refactor is complete.

There is now no functions.sh and all functions have been moved into ibek.

See the related PRs at:

epics-containers/ibek#107
To see how the support functions have been integrated into ibek

epics-containers/ioc-adsimdetector#1
To see what a Dockerfile for a generic IOC looks like now

In this repo you will want to look at the install.sh files for each support module. In particular I realized that the same CONFIG_SITE changes were required in ADCore and ADSimDetector. These changes are really at the generic IOC level rather than at the support module level. Therefore I have provided a mechanism for supplying parameters from the IOC (see https://github.com/epics-containers/ioc-adsimdetector/blob/ibek-j20-changes/generic_config_AD/config_site)

gilesknap · 2023-09-20T17:06:18Z

Note I still need to add tests for the ibek support features.

gilesknap · 2023-09-21T12:09:37Z

@coretl @GDYendell

After the latest changes we have a pretty neat Dockerfile

The most basic install.sh is tidy.

The whole build of ADSimDetector generic IOC container takes under 4 mins. so I'm voting for keeping ADCore build in the individual Generic IOCs. This includes all the optional ADCore features except GraphicsMagic but no use of ADSupport.

The final bit to do is tidy up the apt installs. Here is ADCore install.h as it stands.

I'm wondering if there is any benefit in having ibek functions that just call back out to bash. But having said that I thought that:-

I could have ibek support apt-install take a list that includes http:// urls and knows to wget them first.
We need to declare what apt installs are required in the runtime stage and and ibek command for that could collect the list from each support module and then perform the install in the final stage.

Therefore I argue that I might as well add:

ibek support apt-install [list of pakcages / http addtesses]
ibek support apt-install-runtime [list of pakcages / http addtesses]

coretl · 2023-09-21T12:35:15Z

After the latest changes we have a pretty neat Dockerfile

Excellent, is ibek-support/_global/global.sh still the source of global patches, or is that moved to ibek now?

The most basic install.sh is tidy.

That looks very neat

The whole build of ADSimDetector generic IOC container takes under 4 mins. so I'm voting for keeping ADCore build in the individual Generic IOCs. This includes all the optional ADCore features except GraphicsMagic but no use of ADSupport.

That makes sense

The final bit to do is tidy up the apt installs. Here is ADCore install.h as it stands.

Yes, and we don't do composition of the runtime stage apt installs at the moment...

I'm wondering if there is any benefit in having ibek functions that just call back out to bash. But having said that I thought that:-

* I could have `ibek support apt-install` take a list that includes `http:// urls` and knows to wget them first.

* We need to declare what apt installs are required in the runtime stage and and ibek command for that could collect the list from each support module and then perform the install in the final stage.

Therefore I argue that I might as well add:

* ibek support apt-install [list of pakcages / http addtesses]

* ibek support apt-install-runtime [list of pakcages / http addtesses]

That sounds reasonable, but there are likely to be some that are installed at dev and runtime (like the hdf ones), so how about:

ibek support apt-install [list of package names / addresses] install now and put in the runtime install list
ibek support apt-install --only dev [list of package names / addresses] install now
ibek support apt-install --only run [list of package names / addresses] put in the runtime install list

gilesknap · 2023-09-21T16:21:21Z

Right - had a bit of a hiccup with a trivial problem that required rebuild of all container stages. Because it was an interaction between the dev and run stages. So wasted too much time on that:

    if only is AptWhen.run or AptWhen.both:
        add_list_to_file(RUNTIME_DEBS, debs)

Is NOT the same as:-

    if (only is AptWhen.run) or (only is AptWhen.both):
        add_list_to_file(RUNTIME_DEBS, debs)

DOH! maybe I need some sleep.

Anyway @coretl @GDYendell I feel ready to merge.

Let me know what you think. Here is what ADCore install.sh now looks like.
And here is the Dockerfile for ioc_adsimdetctor

I kinda like it.

Thanks for iterating on this with me, Both. Its been very useful (my first attempt at an install.sh was crap - but useful for starting the discussion)

Regarding global.sh - its not currently doing anything and I will look at it again when I get to RTEMS. I think its nice to have a place where you can change global patching without rebuilding ibek so maybe ibek should call a script ... no sure.

GDYendell · 2023-09-22T08:31:25Z

I think this looks really good now!

I am a bit confused about ibek support apt-install --runtime vs ibek support apt-install --only=run. The latter configures the packages to be installed and also installs it into the current running container and the former pulls packages from the file to install, is that right? Are these mutually exclusive flags or might they be used together? Is the intention that ioc container Dockerfile might run RUN ibek support apt-install ... with some explicit packages, rather than just reading the runtime packages file?

gilesknap · 2023-09-22T09:30:11Z

I think this looks really good now!

I am a bit confused about ibek support apt-install --runtime vs ibek support apt-install --only=run. The latter configures the packages to be installed and also installs it into the current running container and the former pulls packages from the file to install, is that right? Are these mutually exclusive flags or might they be used together? Is the intention that ioc container Dockerfile might run RUN ibek support apt-install ... with some explicit packages, rather than just reading the runtime packages file?

They are not mutually exclusive but you would normally only use --runtime in the runtime section.

ibek support apt-install --runtime mypackage1 mypackage2

would install all the things that had previously been declared with a --run (or --both - the default) plus the two mypackage?

`ibek support apt-install --run --runtime xxx

would install all the packages declared, plus xxx plus it would add xxx to the runtime list (not something you want to do really but its sort of consistent right? ??? )

gilesknap and others added 6 commits September 15, 2023 16:20

first pass j20 refactor

8563880

autosave iocStats asyn BUILDING OK

d6a01e5

autosave iocStats asyn BUILDING OK

23567ae

make RELEASE edits pure regex

38a6d05

improvements for ADSupport

a350904

factor out add_macro in support.py

97060b0

gilesknap marked this pull request as draft September 18, 2023 20:28

gilesknap requested a review from coretl September 18, 2023 20:29

ADSimDetector compilation completed

4027afd

coretl requested a review from GDYendell September 19, 2023 08:01

coretl reviewed Sep 19, 2023

View reviewed changes

gilesknap added 5 commits September 19, 2023 15:28

update install.sh to use ibek support

cc23921

ioc now builds using ibek support functions

11d4498

make AD config_site changes consistent

d519f9f

add-libs and add-dbds

7bec9b8

libs and dbds added: working adsimdetector

3c441a4

fix ADSupport install.sh

2fda0a7

gilesknap marked this pull request as ready for review September 20, 2023 17:05

gilesknap added 3 commits September 21, 2023 07:13

remove redundant files

e302359

move all apt installs into install.sh

73ea4f3

fix hdf installer script lines

3718ebc

GDYendell approved these changes Sep 21, 2023

View reviewed changes

update apt-install model

872fefa

gilesknap force-pushed the ibek-j20-changes branch from bad5377 to 872fefa Compare September 21, 2023 15:49

add additional runtime libs

bf28c08

update README

fb26749

gilesknap force-pushed the ibek-j20-changes branch from 5b900e2 to fb26749 Compare September 22, 2023 06:39

update README

bea6c03

add github CI for testing support files

a58654c

gilesknap force-pushed the ibek-j20-changes branch 12 times, most recently from e5f3910 to 00c961a Compare September 22, 2023 10:32

add runtime test capability

ec6a07f

gilesknap force-pushed the ibek-j20-changes branch from 00c961a to ec6a07f Compare September 22, 2023 10:35

gilesknap added 3 commits September 22, 2023 11:54

remove tests dependency on supermodule

e30dddc

update to epics-base 23.9.3 and ibek 1.0.0

fd32409

fix CI

6d885bd

gilesknap merged commit 41f79af into main Sep 22, 2023
2 checks passed

gilesknap deleted the ibek-j20-changes branch December 15, 2023 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ibek j20 changes #3

Ibek j20 changes #3

gilesknap commented Sep 18, 2023 •

edited

Loading

gilesknap commented Sep 18, 2023 •

edited

Loading

coretl left a comment •

edited

Loading

coretl Sep 19, 2023

coretl Sep 19, 2023

gilesknap Sep 19, 2023

gilesknap Sep 20, 2023

GDYendell commented Sep 19, 2023 •

edited

Loading

gilesknap commented Sep 19, 2023 via email •

edited

Loading

coretl commented Sep 19, 2023

gilesknap commented Sep 19, 2023

gilesknap commented Sep 20, 2023

gilesknap commented Sep 20, 2023

gilesknap commented Sep 21, 2023

coretl commented Sep 21, 2023 •

edited

Loading

gilesknap commented Sep 21, 2023

GDYendell commented Sep 22, 2023

gilesknap commented Sep 22, 2023

		git_clone_tag ${NAME} ${VERSION}

		support add-module-to-release ${NAME}

Ibek j20 changes #3

Ibek j20 changes #3

Conversation

gilesknap commented Sep 18, 2023 • edited Loading

gilesknap commented Sep 18, 2023 • edited Loading

coretl left a comment • edited Loading

Choose a reason for hiding this comment

coretl Sep 19, 2023

Choose a reason for hiding this comment

coretl Sep 19, 2023

Choose a reason for hiding this comment

gilesknap Sep 19, 2023

Choose a reason for hiding this comment

gilesknap Sep 20, 2023

Choose a reason for hiding this comment

GDYendell commented Sep 19, 2023 • edited Loading

gilesknap commented Sep 19, 2023 via email • edited Loading

coretl commented Sep 19, 2023

gilesknap commented Sep 19, 2023

gilesknap commented Sep 20, 2023

gilesknap commented Sep 20, 2023

gilesknap commented Sep 21, 2023

coretl commented Sep 21, 2023 • edited Loading

gilesknap commented Sep 21, 2023

GDYendell commented Sep 22, 2023

gilesknap commented Sep 22, 2023

gilesknap commented Sep 18, 2023 •

edited

Loading

gilesknap commented Sep 18, 2023 •

edited

Loading

coretl left a comment •

edited

Loading

GDYendell commented Sep 19, 2023 •

edited

Loading

gilesknap commented Sep 19, 2023 via email •

edited

Loading

coretl commented Sep 21, 2023 •

edited

Loading