I want to shift it to etc/ where it should rightfully live
Created attachment 6323 [details] [review] Bug 7238 : Shifting SIPconfig.xml to the etc dir and the scripts to misc/bin
Hi, would it not be more interesting to have different directories in etc : as discussed here http://lists.koha-community.org/pipermail/koha-devel/2010-November/034735.html etc/services/SIP2/ And not directly in etc/ same for bin/
Yep, the packages do that on install, the Makefile.PL could do the same thing. The main thing is to first get them out of C4/ so that you don't bust your SIP server whenever you upgrade. But if you wanted to do a patch to move them into other dirs I certainly wouldn't object.
sip_run.sh still looks for the config file in the old location.
My strategy has always been to duplicate the SIPconfig.xml to a separate file, customize, then run SIP off that. Saves merge conflicts. But, yes, having this removed completely from C4/SIP is necessary. Getting these files to respect our Make options is vital; perhaps even adding a "do you want to set up SIP2 services?" set of questions to the installer would be good.
Created attachment 6998 [details] [review] Bug 7238 Followup - fix sip_run to require path to SIPconfig.xml This requires Chris C's patch to move the scripts to the new location outside of C4. To test: run sip_run.sh with no arguments - it should fail with a message to add the path and try again. Run sip_run.sh with a path to a valid SIPconfig.xml - it should start and run.
Created attachment 6999 [details] [review] Bug 7238 : Shifting SIPconfig.xml to the etc dir and the scripts to misc/bin Signed-off-by: Liz Rea <wizzyrea@gmail.com> The scripts run with the caveat that you must specify the path to SIPconfig.xml. The followup previously attached should take care of that issue.
QA question: Am I right if I say it changes the signature of sip_run ? before the patch: you could run without any arg, the SIPconfig.xml was taken from C4/Sip, but you could also pass a parameter with the config file after the patch: you must provide a parameter if i'm right, this patch is an ENH, and will be for 3.8 to avoid any regression for existing users during maintenante update. Otherwise, passed QA, but i'm waiting for the answer to push
Paul: that is correct. My opinion is that it can wait for 3.8.
Created attachment 7549 [details] [review] Bug 7238 - make packages deal with SIP config This moves the SIP config to the templates directory (on package building) in anticipation of making it configurable with package tools. Prevents it being installed into /etc.
Created attachment 7551 [details] [review] Bug 7238 - make packages deal with SIP config This moves the SIP config to the templates directory (on package building) in anticipation of making it configurable with package tools. Prevents it being installed into /etc.
(the update includes a change I forgot to add)
Created attachment 7602 [details] [review] Bug 7238 - make packages deal with SIP config This moves the SIP config to the templates directory (on package building) in anticipation of making it configurable with package tools. Prevents it being installed into /etc. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Nothing looks wrong to me on this follow-up. Will push it, however, I haven't tested it myself, trusting Robin & Jared, they're much more aware than I'm for this kind of packaging
There have been no further reports of problems so I am marking this bug resolved.