Use perl instead of sed

[FreddieAkeroyd]

Windows does not have sed so change to use perl that is already needed for building EPICS

See #62

[FreddieAkeroyd]

Have converted to draft as though it works on windows it doesn't on linux. The problem is that ' does not work on windows but using " means the perl variables $ get interpreted by the linux shell. Tried adding \ which makes it work on linux but then stops working on windows. Could get round this by putting the perl code in a separate file rather than using -e but maybe there is some cleverer quoting that can be done

[simon-ess]

It might be possible to do something like

ifeq ($(strip $(OS_CLASS)),WIN32)
 QUOTE="
else
 QUOTE='
endif


$(COMMON_DIR)/siteEnvVars.substitutions: $(EPICS_BASE)/configure/CONFIG_SITE_ENV
	@echo Expanding siteEnvVars.substitutions from CONFIG_SITE_ENV....
	@echo file iocEnvVar.template> $@
	@echo {>> $@
	@echo pattern>> $@
	@echo { ENVNAME, ENVVAR, ENVTYPE }>> $@
	@perl -n -e $(QUOTE)$$m = s/^EPICS_([A-Z_]*).*/{$${1}, EPICS_$${1}, epics}/; print $$_ if $$m;$(QUOTE) < $< >> $@
	@echo }>> $@

though I am not sure this is much better... 😄

[simon-ess]

See e.g. https://stackoverflow.com/questions/13631551/make-echo-includes-quotes-in-generated-file

[FreddieAkeroyd]

Well it makes it clear what we are trying to do at least, I am happy to go with that if you are - thanks

[FreddieAkeroyd]

Unless you think putting the s/ command into a separate file for perl to execute from Make is cleaner

[anjohnson]

Surely it would be simpler to use a stand-alone Perl script to do the whole job of generating the siteEnvVars.substitutions file? There'd be no need to mess around with the $(QUOTE) variable, and the Perl code would also be easier to read and modify. That might even fix the Base-3.14 build failure, but I haven't looked at that properly to be sure.

[simon-ess]

Oh no more perl scripts

😄

Anyhow, I think you are correct in this case that a standalone script is easier; this is a pretty simple parsing, but the solution is better.

[simon-ess]

Maybe add a shebang, usage, and copyright notice?

[anjohnson]

NB The Windows-2019 job failures here are because today is one of the days that GitHub is doing a brown-out of their windows-2019 CI runners, to remind everyone to upgrade. That also requires updating the Visual Studio version to 2022, see 2f0fbbe for the equivalent changes I applied to Base-7.0.

[FreddieAkeroyd]

did you want me to rebase the branch onto latest base-7.0 ?

[FreddieAkeroyd]

sorry - talking rubbish - this isn't a base PR. Will make changes to iocStats CI

[simon-ess]

I have updated the perl script to be in line with #74 and other changes, as well as rebased.

@FreddieAkeroyd if you could have a look at this and make sure this lines up with what you like, and I will merge it.

[simon-ess]

@nariox, if you could also take a look at this please.

[simon-ess]

@anjohnson Would you mind giving this a quick sanity look? I'm not as much a perl-sort of person, and I would appreciate a second set of eyes on this before merging it in.

[FreddieAkeroyd]

Thanks @simon-ess looks fine to me

when i build locally i get this message which doesn't seem to affect the build

make[3]: Entering directory 'C:/devel/iocStats/iocAdmin/Db/O.windows-x64-debug'
"c:/instrument/apps/epics-debug3/base/master/bin/windows-x64-debug/msi.exe" -D    -I. -I.. -I../O.Common -I../../../db -Ic:/instrument/apps/epics-debug3/base/master/db -o ../O.Common/iocAdminSoft.db -S../iocAdminSoft.substitutions  > iocAdminSoft.db.d
ERROR msi: Can't open file 'iocAdminScanMon.db'
input: '}
' at
make[3]: Leaving directory 'C:/devel/iocStats/iocAdmin/Db/O.windows-x64-debug'

I see explicit dependency rules in the Makefile for use during a build, but it looks like the automatic dependency generation that happens pre-build is having an issue. I don't think this is related to this particular ticket though

[anjohnson]

Hi Simon, this looks okay to me, the comments below are a few tweaks but they aren't essential.

[anjohnson]

Your regex does look to match the original given to sed, although it doesn't actually cover all of the env-params in the EPICS configure/CONFIG_{SITE_,}ENV files. It misses the three IOCSH_* variables, but maybe that's by design, I don't see that sites would need to see their values remotely.

Personally I might use this regex instead:

/^EPICS_([A-Z_0-9]+)\s*=.*\n/$1/

and maybe instead of substituting the match into $_ I'd just write the loop body as

    push @epics_vars, $1 if m/^EPICS_([A-Z_0-9]+)\s*=/;

although I haven't actually tested that.

I guess not capturing the leading EPICS_ and then adding it back later was done to speed up the text processing, although IMO that makes the code slightly more brittle in the event of later changes.

[simon-ess]

Ah yeah, I think I should update to include 0-9 as well. Done.

[anjohnson]

This could be written on one line instead of 3, but I doubt if there'd be any measurable performance difference:

print "{ $_, EPICS_$_, epics }\n" for @unique_vars;