Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved module dependencies #36

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bhill-slac
Copy link
Contributor

Replaced original SLAC specific iocReleaseCreateDb.py w/ improved python module dependency generation that handles more variations on RELEASE files included following nested includes.
This version should work for other sites as well.
To test, add this line in your IOC *App/Db/Makefile:
IOCRELEASE_DB += iocRelease.db

@anjohnson
Copy link
Member

Are these changes all SLAC-specific? Certainly that long list of modules in pkgNameToMacroNames.py is, and I do have a problem with that.

My build of iocStats-3.1.16 has installed the iocReleaseCreateDb.py script but it doesn't look like it has ever actually been run. In that case I don't really care about it, although I would prefer that it not be installed at all. This PR does stop it being installed, but the change in configure/RULES_BUILD to call it from the source tree won't work if this module is ever built with INSTALL_LOCATION set in the configure/CONFIG_SITE file. It would be better to install it only when IOCADMIN is defined, and the rule should run it from the bin/$hostarch directory.

The switch to the python2 interpreter will not work on some architectures/distributions that EPICS supports, for example MacOS doesn't have a binary with that name:

kato$ which python2
kato$ which python
/usr/bin/python
kato$ python --version
Python 2.7.10

TBH since I suspect nobody outside of SLAC can properly review or test these changes I think the rest of us would prefer that all the SLAC-specific files be moved into a separate SLAC-only module. Non-SLAC maintainers can't do much work on this module as it exists right now since we can't tell if we've broken your parts.

@bhill-slac
Copy link
Contributor Author

I would assume no other sites are currently using iocReleaseCreateDb.py as the prior version only read $TOP/RELEASE_SITE and $TOP/configure/RELEASE and only handled SLAC style ASYN_MODULE_VERSION release versions.

As part of an effort to merge the accelerator version of iocAdmin w/ the photon version (where we weren't using iocReleaseCreateDb.py), I made the python script more robust and able to work in both SLAC environments.

I think this version will likely work for most other sites, but have no feedback yet as to whether or not there is any interest from other sites. If you add the IOC_RELEASE_DB = iocRelease.db line to your IOC it should produce iocRelease.db, a collection of stringin PVs named $(IOC):RELEASE00 to $(IOC):RELEASE29. For each, the DESC is the module name and VAL is the release version.

Re pkgNamesToMacroNames.py, I've removed SLAC specific comments. Any sites are welcome to contribute macro names to the list. It's just used for cosmetic purposes so the DESC field can be populated with the actual module name instead of a macro like ADCORE.

Re installation, if we want it to work from the IOCADMIN INSTALL_LOCATION we need to install it when the iocStats module is built, as we don't know yet which IOC's will define IOCADMIN and IOC_RELEASE_DB. I also made the python2 default to python.

Re splitting out SLAC specific files to other modules, it appears that other sites are using iocAdmin/Db. I have no objection to moving iocAdmin/srcDisplay/*.edl to op/edl along w/ Mark's other UI files. The srcRestore directory just has some old channelWatcher config files that may not even be used by any SLAC IOCs. SLAC isn't deploying the master branch anyway, so there's some flexibility re how we handle this.

As @steffallison has retired, Ernest Williams suggested that @kukhee and/or myself might get granted write access to this github repo.

@anjohnson
Copy link
Member

Hi Bruce, to answer your last point, @kukhee should be able to write to this repository as he is a member of the @epics-modules/iocstats-developers team and I believe has been since he was nominated as its maintainer by Ernest quite some time ago. I'd be happy to add you to that team too, as you're the one doing work on the module.

I haven't had a chance to look at your latest changes myself, and I probably won't now until after ICALEPCS.

@bhill-slac
Copy link
Contributor Author

FYI - I'm retiring from SLAC and don't have any strong opinions re whether or not this pull request gets accepted, so I'll leave that up to Simon and Kukhee to work out. Cheers, Bruce

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants