Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mantid doesn't work with latest setuptools on Windows #33118

Closed
DannyHindson opened this issue Dec 10, 2021 · 15 comments · Fixed by #35832
Closed

Mantid doesn't work with latest setuptools on Windows #33118

DannyHindson opened this issue Dec 10, 2021 · 15 comments · Fixed by #35832
Assignees
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS Maintenance Unassigned issues to be addressed in the next maintenance period. Technical Debt Marks a piece of work to address technical debt introduced to solve a problem quickly
Milestone

Comments

@DannyHindson
Copy link
Contributor

DannyHindson commented Dec 10, 2021

I recently updated the Python setuptools package on my Windows laptop by accident and I noticed that workbench and system tests stopped working. I looked into this a bit and came across some issues where support had been added to Mantid for setuptools v49.0.0 or later (eg #32068).

The problem on Windows seems to be that the python version is 3.8 which includes a new importlib.metadata module. This alters the behaviour of the shim (workbench-script.pyw) created by setuptools v49.0.0 or later:

#!C:\mantid\external\src\ThirdParty\lib\python3.8\pythonw.exe
# EASY-INSTALL-ENTRY-SCRIPT: 'MantidWorkbench','gui_scripts','workbench'
import re
import sys

# for compatibility with easy_install; see #2198
__requires__ = 'MantidWorkbench'

try:
    from importlib.metadata import distribution
except ImportError:
    try:
        from importlib_metadata import distribution
    except ImportError:
        from pkg_resources import load_entry_point


def importlib_load_entry_point(spec, group, name):
    dist_name, _, _ = spec.partition('==')
    matches = (
        entry_point
        for entry_point in distribution(dist_name).entry_points
        if entry_point.group == group and entry_point.name == name
    )
    return next(matches).load()


globals().setdefault('load_entry_point', importlib_load_entry_point)


if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
    sys.exit(load_entry_point('MantidWorkbench', 'gui_scripts', 'workbench')())

Ubuntu uses python 3.6 so the shim imports pkg_resources instead.

This difference matters because it appears to me that importlib doesn't recognise .egg-link files whereas pkg_resources does. The .egg-link file needs to be read in order to locate the MantidWorkbench package in a developer's source code directory at qt\applications\workbench

Mantid works OK on older versions of setuptools because the shim is different to the one above and always uses pkg_resources

Maybe this isn't A1 priority but I wondered about fixing it because it wasn't v obvious what was going wrong - I just had a dev environment which wouldn't work and failed with a "cannot find _kernel.." error*. Not sure what can be done to fix this though - perhaps there's a way of getting setuptools to create a custom .pyw file somehow. The standard .pyw file seems to be autogenerated when you use the gui_scripts entry point in the qt\applications\workbench\setup.py file.

@DannyHindson DannyHindson added the Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) label Dec 10, 2021
@DannyHindson DannyHindson added the Maintenance Unassigned issues to be addressed in the next maintenance period. label Jan 12, 2022
@DannyHindson
Copy link
Contributor Author

I think this is an issue only for developers where the workbench.exe\workbench-script.pyw and egg-link's are relied upon to access the MantidWorkbench package in the source directories from within build\bin

@DannyHindson
Copy link
Contributor Author

DannyHindson commented Feb 3, 2022

The sitecustomize.py mechanism previously set up to handle setuptools>=49.0.0 seems to clash on Windows with a pre-existing sitecustomize.py that lives in C:\mantid\external\src\ThirdParty\lib\python3.8\Lib\site-packages. I think Python only supports a single sitecustomize.py
I believe the aim of the sitecustomize.py in build\bin directory is to add the build\bin directory to the site directories which means the easy-install.pth file gets read (containing path to MantidWorkbench). As far as the shim (.pyw) file is concerned I think we can just remove the sitecustomize.py to fix the clash because the egg-link's do the work of pointing at the folder where the MantidWorkbench package lives.

@DannyHindson
Copy link
Contributor Author

DannyHindson commented Feb 4, 2022

So in summary two changes:

  1. change qt\applications\workbench\setup.py so it generates a shim (.pyw) file that correctly finds the MantidWorkbench package. This can be done by using the scripts keyword to put some hand-crafted code in the shim rather than the default
  2. stop generating the sitecustomize.py in build\bin

With these changes, the only bit that doesn't work is the MantidWorkbench.exe (which is Windows\OSX only) in a developer set up. I've only tested in on Windows but it's not finding the workbench.app.main path in the source directories. I guess this must have relied on the site.py mechanism with the older setup tools version. Maybe this should use the gui_scripts entry point as well?

@martyngigg
Copy link
Member

So in summary two changes:

1. change qt\applications\workbench\setup.py so it generates a shim (.pyw) file that correctly finds the MantidWorkbench package. This can be done by using the scripts keyword to put some hand-crafted code in the shim rather than the default

2. stop generating the sitecustomize.py in build\bin

With these changes, the only bit that doesn't work is the MantidWorkbench.exe (which is Windows\OSX only) in a developer set up. I've only tested in on Windows but it's not finding the workbench.app.main path in the source directories. I guess this must have relied on the site.py mechanism with the older setup tools version. Maybe this should use the gui_scripts entry point as well?

Ah I see now. Yes, I believe that Python just does an import sitecustomize so whatever it finds first will take over. Would another option be to keep the one in bin/<Config> and remove the one from Third party? We're generating bin anyway so can add the contents that is already in the other one to there? This will no longer be necessary once we do the Conda switch.

@DannyHindson
Copy link
Contributor Author

So in summary two changes:

1. change qt\applications\workbench\setup.py so it generates a shim (.pyw) file that correctly finds the MantidWorkbench package. This can be done by using the scripts keyword to put some hand-crafted code in the shim rather than the default

2. stop generating the sitecustomize.py in build\bin

With these changes, the only bit that doesn't work is the MantidWorkbench.exe (which is Windows\OSX only) in a developer set up. I've only tested in on Windows but it's not finding the workbench.app.main path in the source directories. I guess this must have relied on the site.py mechanism with the older setup tools version. Maybe this should use the gui_scripts entry point as well?

Ah I see now. Yes, I believe that Python just does an import sitecustomize so whatever it finds first will take over. Would another option be to keep the one in bin/<Config> and remove the one from Third party? We're generating bin anyway so can add the contents that is already in the other one to there? This will no longer be necessary once we do the Conda switch.

Unfortunately a sitecustomize.py located in bin/<Config> only gets read if running from PyCharm using the Run, Debug menu option ie when it's run using a "python pydevd.py ..." command. If you run workbench using all other methods (eg from outside of PyCharm or you use the Run, Release option in PyCharm) the sitecustomize.py file in bin/<Config> gets ignored. This is a problem regardless of whether you also have a sitecustomize.py in external\src\ThirdParty.

I think this is because the path of the workbench startup script supplied to python (ie bin/<Config>) only gets added to the system path after all the site.py stuff gets done during python's start up. Using the pydevd utility seems to change the order in which paths get added to the system path.

So my earlier comments about there being a clash when you have 2 sitecustomize.py files need clarifying a bit. This is only a problem when starting up workbench using the PyCharm Run, Debug menu.

One way around this is to rely on the .egg-link files to guide the workbench start up exe\script to the python packages in the source code directories (eg qt\applications\workbench) when a developer is running workbench from a build directory. I've made a change to mantidworkbench.cpp (the unit that wraps the error handler around the workbench start up) so that it uses the egg info to find the "entry point" and this seems to work. Maybe I'll create a PR and you can see what I've done? Or if I'm not making any of this clear we could have a chat tomorrow

@martyngigg
Copy link
Member

Yeah happy to review any pull request. It sounds like something that is heading in a promising direction.

@DannyHindson
Copy link
Contributor Author

I've just noticed there's been some quite recent discussion about this problem:

https://stackoverflow.com/questions/70809447/oddities-when-pip-installing-a-local-package-with-prefix-and-e-potential-bug

python/importlib_metadata#364

This discussion suggests the .egg-link files are being deprecated so maybe my solution isn't going to be very long lasting even if it works for now. These links instead suggest relying on the sitecustomize.py as you suggested. I'll see if I can work out a way of getting the bin/<Config>/sitecustomize.py found reliably.

@stale
Copy link

stale bot commented Aug 10, 2022

This issue has been automatically marked as stale because it has not had activity in 6 months. It will be closed in 7 days if no further activity occurs.
Allowing issues to close as stale helps us filter out issues which can wait for future development time. All issues closed by stale bot act like normal issues; they can be searched for, commented on or reopened at any point.
If you'd like a closed stale issue to be considered, feel free to either re-open the issue directly or contact a developer.
To extend the lifetime of an issue please comment below, it helps us see that this is still affecting you and you want it fixed in the near-future. Extending the lifetime of an issue may cause the development team to prioritise it over other issues, which may be closed as stale instead.

@stale stale bot added the Stale This label is automatically applied to issues that are automatically closed by the stale bot label Aug 10, 2022
@DannyHindson
Copy link
Contributor Author

I think this is still an issue. For example this change was put through in earlier in this (6.5) release cycle to pin setuptools to 47.0.0:
#34134

@stale
Copy link

stale bot commented Aug 31, 2022

This issue has been closed automatically. If this still affects you please re-open this issue with a comment or contact us so we can look into resolving it.

@stale stale bot closed this as completed Aug 31, 2022
@martyngigg
Copy link
Member

This is still an issue that should be addressed.

@martyngigg martyngigg reopened this Aug 31, 2022
@martyngigg martyngigg added Technical Debt Marks a piece of work to address technical debt introduced to solve a problem quickly and removed Stale This label is automatically applied to issues that are automatically closed by the stale bot labels Aug 31, 2022
@stale
Copy link

stale bot commented Mar 6, 2023

This issue has been automatically marked as stale because it has not had activity in 6 months. It will be closed in 7 days if no further activity occurs.
Allowing issues to close as stale helps us filter out issues which can wait for future development time. All issues closed by stale bot act like normal issues; they can be searched for, commented on or reopened at any point.
If you'd like a closed stale issue to be considered, feel free to either re-open the issue directly or contact a developer.
To extend the lifetime of an issue please comment below, it helps us see that this is still affecting you and you want it fixed in the near-future. Extending the lifetime of an issue may cause the development team to prioritise it over other issues, which may be closed as stale instead.

@stale stale bot added the Stale This label is automatically applied to issues that are automatically closed by the stale bot label Mar 6, 2023
@stale
Copy link

stale bot commented Mar 14, 2023

This issue has been closed automatically. If this still affects you please re-open this issue with a comment or contact us so we can look into resolving it.

@stale stale bot closed this as completed Mar 14, 2023
@DannyHindson
Copy link
Contributor Author

Think this is still relevant

@DannyHindson DannyHindson reopened this Mar 14, 2023
@stale stale bot removed the Stale This label is automatically applied to issues that are automatically closed by the stale bot label Mar 14, 2023
@thomashampson thomashampson added the ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS label Jun 9, 2023
@thomashampson
Copy link
Contributor

This is a blocker for moving to python 3.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS Maintenance Unassigned issues to be addressed in the next maintenance period. Technical Debt Marks a piece of work to address technical debt introduced to solve a problem quickly
Projects
No open projects
Status: Done
4 participants