Build a catalogue in CI

[schmitts]

No description provided.

[brenthuisman]

We cannot bump the Python version due to EBrains Lab, which is stuck at 3.6. Once the Spackified version is available, and we can easily load up more recent Pythons, we can revisit this bump.

[bcumming]

Can we verify that Python 3.6 is a hard requirement of EBRAINS?

[brenthuisman]

That's the most recent version available in the default and only image. New images (even though I doubt this would help) or new distribution methods in the current image (Spack?) are not yet available.

[schmitts]

Are we going to wait this out or fix the failing script to work with Python 3.6?

[brenthuisman]

We can merge as soon as the script bumps back to 3.6 and the call to subprocess is changed (I believe that was the only actual problem here). Do you want to do it or shall I take a look?

[schmitts]

I think @thorstenhater already had a look and proposed a solution, cf. https://mcnest.slack.com/archives/C0176SS8Z6F/p1629447749076700

subprocess.run(["ls", "-l", "/dev/null"], stdout=subprocess.PIPE)

But I did not test.

[thorstenhater]

Look almost good, but this looks like it will not print the build progress if run with -v?

[schmitts]

@brenthuisman can you have another look?

[thorstenhater]

Hey @schmitts thanks for updating this, but it's not completely correct yet. Could you take another look?

[thorstenhater]

This pattern captures the output streams, but it does not do anything with them.

In [1]: import subprocess as sp

In [2]: sp.run('ls', shell=True, check=True, stdout=sp.PIPE, stderr=sp.PIPE)
Out[2]: CompletedProcess(args='ls', returncode=0, stdout=b'...', stderr=b'...')

In [3]: p = sp.run('ls', shell=True, check=True, stdout=sp.PIPE, stderr=sp.PIPE)

In [4]: p.stdout
Out[4]: b'...'

In [5]: print(p.stdout.decode('utf8'))
...
...

Assigning to p and decoding/printing the pipes contents is what you actually want to do here, I think.

[brenthuisman]

Actually, I needed to swap the if block around: by default things are already printed. Not setting verbose should simply hide them.

[thorstenhater]

ok, that seems weird to me. I'll take a look.

[thorstenhater]

For bonus credits: add a try block around the whole affair, as check=True will throw.
But that can be cleaned-up later as well, the whole script could benefit from an overhaul.

[brenthuisman]

That's however a change from the original script, which was not the point of this fix (which was to remove a flag because it was not in Python 3.6).

Shall we have actual changes in another PR?

[thorstenhater]

That's completely fine, as it was just something I noticed in passing.

[Helveg]

I have proposed a fix for this in #1679. Ignore it here and I'll merge when this in.

[schmitts]

@brenthuisman can you recheck?

[thorstenhater]

Looks good, tiny clean-ups needed. Also please open an issue for making this script better.

[thorstenhater]

👍 don't forget that issue ;)

[brenthuisman]

In the process of testing the make installed build-catalogue script and libraries, I encountered a Macos related issue]. For any non-default prefix, you need to update certain env vars, AFAIK, in order to have stuff installed to a custom CMake prefix be discoverable. This appears not to work the same as on Linux, or at least, the steps I'd run on Linux don't seem to work on the Macos machine Github so kindly provides us:

• Add customprefox/bin to PATH, but this did not appear to be set correctly (in fact, PATH is empty on Macos)
• Add customprefix/lib to LD_LIBRARY_PATH. However, this env var is not passed to child processes.

Since this is up to the user in case of installs to non-standard prefixes anyway, I'll let CMake install to the default prefix and count on that to work. If it doesn't, we have another problem anyway.

If we want to test to a custom prefix, I need a Macos user to help me out :)

[thorstenhater]

@brenthuisman I think the standard way would be to export VAR=VAL.

One final note here: I think it could be useful not only to build the catalogue, but also to execute a tiny test against it, eg
python3 -c 'import arbor as A; print([n for n in A.load_catalogue("mycat.cat").keys()])'

[brenthuisman]

So, as per Macos post and report above, env vars seem to be shielded in the interest of security. I'm sure there's another way to do this on Macos, but I couldn't find it.

[thorstenhater]

We can do it in CI though. See the env part at the top.

[brenthuisman]

Right...