McCode icon indicating copy to clipboard operation
McCode copied to clipboard

MCPL and NCrystal need .bat Python wrappers on Windows

Open willend opened this issue 1 year ago • 17 comments

When built via legacy "mkdist" mechanism, mcpl and ncrystal get wrappers ala

@python @CPACK_NSIS_INSTALL_ROOT@\\bin\\mcpl-config %* or @python @CPACK_NSIS_INSTALL_ROOT@\\bin\\pymcpltool %*

  • to ensure that the McStas-related Python is used... We need something similar when building via "standard" CMake. (the .py. commands alone don't work automatically / as expected on Windows...)

willend avatar Aug 30 '23 14:08 willend

@tkittel once we get around to look at this, there are snippets to start from in my 3rdparty packages

willend avatar Sep 01 '23 12:09 willend

@willend, thanks - let us discuss next week - my gut reaction is certainly to try to avoid any sort of custom .bat stuff at all cost in the NCrystal/mcpl packages themselves. I guess the issues are actually related to the fact that a pure CMake based install of NCrystal/mcpl does not actually put the python modules in the correct location in the first place even on linux (which is related to the fact the the "correct place" depends on the python version number). Perhaps that can be remedied, but it is a bit of a not-in-the-short-term thing. Perhaps :-)

tkittel avatar Sep 01 '23 17:09 tkittel

So I wonder if you might for instance use:

-DNCrystal_PYPATH="$(python -c 'import site; print(site.getsitepackages()[0])')"

Or something like that. It is untested of course, and I didn't even check the quotes and escaping in the above command. ]

tkittel avatar Sep 01 '23 17:09 tkittel

If it works, we could make it a more convenient cmake option for NCrystal (and then one can use -DPYTHON_EXECUTABLE=... to select which python). Anyway, just thinking aloud.

tkittel avatar Sep 01 '23 17:09 tkittel

The main issue is that on "Windows proper"

  • The "system-wide" Python interpreter is something defined via the "registration database"
  • There is no such thing as #!/usr/bin/env python
  • "Something" needs to define that a given interpreter should be called on a given Python script

For a python script to become "executable" one needs to e.g. set PATHEXT=%PATHEXT%;.py ... which then uses the registry database for looking up Python to "handle the file" / along the lines of a mime-type on a "real" operating system ;-)

The "only" full-control solution I found myself so far is to "wrap in a batch", i.e.

@REM script.bat: c:\the\path\to\python.exe c:\some\other\path\to\python\script.py

and in fact this is the "glue" that makes e.g. mcpl-config function from McStas on Windoohze (proper).

But of course nothing prevents that we e.g. maintain a small set of these batches on the McStas side courtesy of the people wanting to use MCPL/NCrystal from McStas in a native Windoohze env?

(In conclusion: Yes, Windows is still not so nice and not very POSIX or Unix like....)

willend avatar Sep 07 '23 13:09 willend

(And for sake of being sure, I enquired with Piotr who is quite Windows knowledgable - the situation is very much what I describe above... :-) )

willend avatar Sep 07 '23 13:09 willend

If we need it, we need it :-)

I have a theory though, that migrating all the ncrystal commands to live in regular NCrystal python modules (e.g. the ncrystal_xxx command would become a main() function in NCrystal._cli_xxx.py), and declaring those functions as commands in the pyproject.toml file would make it work automatically (i.e. pip or conda would take care of any .bat wrapping, or modifying the shebang or whatever). So I think I would like to try that first since that would result in an identical and standard solution on all platforms. In theory... it might fail spectacularly when I finally get around to trying it of course :-)

One underlying question though: Does NCrystal actually work at all on windows? (I didn't fix anything systematically on my side so I am surprised).

tkittel avatar Sep 07 '23 15:09 tkittel

From https://packaging.python.org/en/latest/specifications/entry-points/:

Distributions can specify console_scripts entry points, each referring to a function. When pip (or another console_scripts aware installer) installs the distribution, it will create a command-line wrapper for each entry point.

Which is the theory.

tkittel avatar Sep 07 '23 15:09 tkittel

So I just checked. If on linux I do "pip install ncrystal" I get:

(dgcodecondaaug4) (tkittel@tkittel-XPS-13-9380 .system)> head $(which ncrystal-config)
#!/usr/bin/env python3

Which I think is really just the unedited ncrystal-config file, likely because we are somehow not being completely proper and doing what I aluded to above and define the command via an entry point to a python module+function. If on the other hand I look at one of the commands that I am installing with the entry point declaration (in another unrelated project) into the same virtual environment then I get:

(dgcodecondaaug4) (tkittel@tkittel-XPS-13-9380 .system)> head $(which dgbuild2)
#!/home/tkittel/scr/miniforge_install/miniforge_installed/envs/dgcodecondaaug4/bin/python3.11
# -*- coding: utf-8 -*-
import re
import sys
from ess_dgbuild_internals._cli_dgbuild import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

Which is not only an autogenerated script around the function in question, the shebang also has a very hardwired path corresponding to the virtual environment. I of course imagine that on windows they would instead auto-generate something which works on windows :-)

If my theory is correct, then I should take a few days and move all the meat of the ncrystal cmdline scripts into modules + add the entry points in question. The nice thing is that I can do that on linux and without knowing too much about windows :-)

If you want to test, then try to do "conda install ase" or "pip install ase" on windows, and see what is inside the resulting ase command. On my machine it has the same shebang as the dgbuild2 command above.

tkittel avatar Sep 07 '23 15:09 tkittel

Testing with ASE via conda seems to bring an .exe in fact - which is also OK I guess... Let's investigate further...

Screenshot 2023-09-08 at 09 57 52

willend avatar Sep 08 '23 07:09 willend

That is actually also what I observed in some tutorial where some guy was setting up a simple python project and did "pip install" on windows. That .exe might in fact really a python file underneath, can you open it and see?

tkittel avatar Sep 08 '23 08:09 tkittel

Or maybe the ase.exe is invoking the ase-script file lying next to it.

tkittel avatar Sep 08 '23 08:09 tkittel

I would think it is a minimal .exe generated via py2exe http://www.py2exe.org or similar? Screenshot 2023-09-08 at 10 18 29

willend avatar Sep 08 '23 08:09 willend

Yes, and then presumably the ase-script is the actual python script.

It all looks good to me, the pip people did the necessary hacks :-)

So it is just a matter of when I can make a new NCrystal release. Unfortunately I have broken everything in my dev code right now and am busy reassembling the pieces, so it might not be immediately. By when will it start to become a problem for you (if it is not already)?

tkittel avatar Sep 08 '23 08:09 tkittel

Btw., the ncrystal installation in question - is it done manually via CMake? In that case there won't be any pip/conda magic and we might in any case need .bat wrappers...

tkittel avatar Sep 08 '23 08:09 tkittel

It does look promising indeed! :-)

It only becomes a "problem" the moment we transfer fully to the "new" way of building distributing, i.e. getting mcpl and ncrystal via conda.

My legacy "hack" scripts do local builds of both packages via CMake and include my batch-glue. :) (In fact only mcpl is functional - ncrystal misses some string/traits symbols as is...)

So no real hurry I think - as I was anyway planning on doing one last release via the legacy method - which will minimum buy us some months. ;-)

willend avatar Sep 08 '23 08:09 willend

Perfect, I should hopefully get around to this in "a few weeks".

tkittel avatar Sep 08 '23 09:09 tkittel