Opened 12 years ago

Closed 12 years ago

Last modified 11 months ago

#8735 closed patch

Refactor engines handling in configure script

Reported by: jvprat Owned by: sev-
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

After seeing the added "Engines Skipped" listing, it made me think about the current hardcoded handling of engines in the configure script.

The attached patch refactors it so all the information about an engine is compacted in a small array of variables and then this information is used wherever it's needed in a generalized way.

This could also be used to dynamically generate the engine information in engines/engines.mk and base/plugins.cpp, but I remember Fingolfin said some ports don't use the configure script... Maybe the engine information could be extracted outside the configure script to make it even more general? If there's interest on this I could try to work on it.

Again, this eases the maintenance of out-of-trunk engines, having just to mantain the engine information array.

This approach uses evaluation of variables with dynamic names. I've tried it with bash and busybox's ash, but I don't know if I'm using some shell specifics that could cause troubles in other shells and reduce portability.

Ticket imported from: #1822349. Ticket imported from: patches/840.

Attachments (2)

configure-engines.patch (13.2 KB ) - added by jvprat 12 years ago.
configure-engines2.patch (13.0 KB ) - added by jvprat 12 years ago.

Download all attachments as: .zip

Change History (9)

by jvprat, 12 years ago

Attachment: configure-engines.patch added

by jvprat, 12 years ago

Attachment: configure-engines2.patch added

comment:1 by jvprat, 12 years ago

I'm attaching an updated patch after the Igor addition and the previous changes in engine listing style. It also fixes some syntax errors in other shells.

I'm mantaining the style of the engines listing as it was before rev. 29315 because it's what I had done in the previous patch and also because I think it looks cleaner. I can adapt it to the "new" style if you think it's worth.

Regarding the portability issues, Eugene has mentioned it didn't work for him in bash. I've tried some other shells and it worked for them all, so I suppose it's a configuration issue. Eugene: what's your output for "echo $-"?
File Added: configure-engines2.patch

comment:2 by fingolfin, 12 years ago

That patch seems to work fine over here: Mac OS X 10.4, GNU bash, version 3.2.9(1)-release (powerpc-apple-darwin8.10.0)

comment:3 by fingolfin, 12 years ago

Owner: set to sev-

comment:4 by fingolfin, 12 years ago

Patch looks reasonable to me, we only need to decide how we want the engine enabled/disabled output. I don't mind much which way we do it... Eugene, what do you think?

comment:5 by sev-, 12 years ago

Status: newclosed

comment:6 by sev-, 12 years ago

Well, it is OK as it is now. I just wanted to have skipped engines reported. So I commit this patch as is. It works well on FreeBSD now. Thanks, Jordi.

Committed to SVN.

comment:7 by digitall, 11 months ago

Component: --Other--
Note: See TracTickets for help on using tickets.