Use directories for engines and add EGTB support

[protonspring]

This patch creates the engines in their separate directories and simplifies setup_engine. With this patch, engines can use different EGTB tables depending on which are added to the branches being tested.

This only allows a developer to test EGTB tables. If one proves to be beneficial, then we can discuss permanently adding it to fishtest. This is a different issue not addressed here.

I know very little Python and barely understand the code, but I think this does what I would want it to do.

Hopefully this is close. Please don't crucify me.

[tomtor]

This only allows a developer to test EGTB tables. If one proves to be beneficial, then we can discuss permanently adding it to fishtest. This is a different issue not addressed here.

If this is merged then it would be a permanent part of fishtest, because normally we don't add features for a limited time. I am OK with adding such a feature (not much code added), but @vondele opinion is important here.

Regarding the code, @ppigazzini and @vondele are most familiar with the worker file tree layout, so it would be nice if they review this PR.

[noobpwnftw]

I think it doesn't have to be just for 'egtb', we can use a more generic way of supplying 'additional files for the engine'.
Additionally, we could use soft links instead of making a hard copy every time, which I believe is supported on all worker platforms now.

[ppigazzini]

@protonspring updater.py changes missing, check my review.

[ppigazzini]

Leftover of the original code: if engine_dir exist we don't call setup_engine()

[ppigazzini]

'custom_make.txt' is written in working_dir

[ppigazzini]

destination needs + EXE_SUFFIX

[ppigazzini]

This should work better:

    if os.path.exists(egtb_dir_src):
      shutil.move(egtb_dir_src, engine_dir)

[ppigazzini]

@vondele I think that is your responsibility to decide if/how use additional files for Stockfish :)

[protonspring]

I'd be glad to make those changes if this may be committed. I'll wait for an indicator from big boss man.

[vondele]

I haven't fully reviewed the patch, if we want to be generic we could use 'data' instead of 'egtb' for the directory that is symlinked/copied.

My concern with this is that this is a tool, but we have not discussed on the policy on how to use it. It really is attractive to use a resource like fishtest to try interesting experiments, and I like creative experiments (I'd have some of my own as well that would benefit from such a resource ;-)).

However some questions we need to answer would be:

• Will we allow arbitrary data to be read (we can review code somewhat, but with arbitrary (binary) data that becomes hard). We're not malicious, but are we exercising sufficient care ?
• Will we allow any quantity of data (e.g. test KRBPvKR) ? Could this result in github denying access (@noobpwnftw had this issue already with the books IIRC, but possibly solved it as well)
• Will we allow things other than egtb (e.g. opening books, files used for 'self-learning') ?
• Only read or also write access, possibly persistent ?
• If any of those things show Elo gain, will it imply we add those to SF ?

Feel free to give partial answers.

[protonspring]

• I like the data dir so this can be used for other things.

• Size limits would be wise, else someone could add GB's to their repository and force all workers to download the data.

• Stuff that shows elo gain should be added if sensible. This wouldn't require a change to fishtest as the added data would just be in the respective code branches.

[protonspring]

Nothing is happening here. Should I close this?

[vondele]

I still don't like the idea of adding EGTB (or any other non-source file blob) to patches on fishtest. The cleanups themselves, I haven't judged.

[protonspring]

NP. I'll close this.