Add mantidworkbench as an entry point for the mantidworkbench Conda package #37750

thomashampson · 2024-08-06T08:37:12Z

Description of work

Add mantidworkbench as an entry point to the application. Users can now launch with either workbench or mantidworkbench from within a Conda environment.
Configure the Conda package entry points to use jemalloc on Linux. Previously, the workbench entry point did not include the same jemalloc setup as the mantidworkbench script that was available on Linux installs. Now on Linux both entry points will call the same launch script that will configure jemalloc to be used.
Here is a diagram to help illustrate the changes:

Summary of work

Added a new wrapper script called workbench_launch_wrapper that is now the entry point defined in the conda-build recipe. The new script will detect whether you are using Linux. If not, workbench is launched as before. If it is Linux, it will call the script that used to be named mantidworkbench that will configure jemalloc before launching the application.

Purpose of work

There have been cases where users have asked for a mantidworkbench entry point to avoid confusion.
It is already an entry point on Linux, but will perform additional steps before launching the application (e.g. setting up jemalloc)
Currently if you launch on Linux with the Conda entry point (workbench), it bypasses the jemalloc setup, and the application does not perform well.

Fixes #37538

To test:

On all OS:
Install the test package:
conda install jclarkestfc/label/entryPointTest::mantidworkbench
Test the following things:

Verify that mantid workbench launches with both mantidworkbench and workbench
Test that the error report functions with both entry points (e.g. use the SegFault algorithm)
Test that the error reporter doesn't pop up when passing the --single-process argument to either entry point
(Linux only) verify that both launch entry points configure jemalloc correctly by running os.environ['LD_PRELOAD'] in workbench's python console. It should output the location of libjemalloc.so.2 from inside your Conda environment.
(Linux only) Check that the --debug argument works when launching with either entry point.
On each OS, install the standalone packages and make sure they still work as expected:
https://builds.mantidproject.org/job/build_packages_from_branch/957/

Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review

Is the code of an acceptable quality?
Does the code conform to the coding standards?
Are the unit tests small and test the class in isolation?
If there is GUI work does it follow the GUI standards?
If there are changes in the release notes then do they describe the changes appropriately?
Do the release notes conform to the release notes guide?

Functional Tests

Do changes function as described? Add comments below that describe the tests performed?
Do the changes handle unexpected situations, e.g. bad input?
Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

peterfpeterson · 2024-08-08T15:57:03Z

buildconfig/CMake/LinuxPackageScripts.cmake

@@ -100,10 +100,11 @@ set(PYTHON_ARGS " -Wdefault::DeprecationWarning -Werror:::mantid -Werror:::manti
 set(PYTHON_EXEC_LOCAL "\${CONDA_PREFIX}/bin/python")
 set(PREAMBLE "${CONDA_PREAMBLE_TEXT}")
 set(LOCAL_PYPATH "${CMAKE_CURRENT_BINARY_DIR}/bin/")
+# The python command to start workbench
+set(MANTIDWORKBENCH_EXEC "-c \"from workbench.app.main import main; main()\"")


If you are feeling a little crazy, you can rename workbench.app.main to workbench.app.__main__ and people then can run python -m workbench.app to get workbench started.

For simpler things, the little import-y bit here is autogenerated by setuptools once you define the entrypoint.

We appear to be most of the way there https://github.com/mantidproject/mantid/blob/main/qt/applications/workbench/workbench/__main__.py

peterfpeterson · 2024-08-08T16:02:00Z

qt/applications/workbench/workbench/app/workbench_launch_wrapper.py

+from workbench.app.main import main
+
+
+def launch():


I think this is missing the bits for return codes. The final lines should be like this. Things that return None will exit with a zero return code as you would expect.

peterfpeterson · 2024-08-08T16:15:41Z

Can the new wrapper get named mantidworkbench_whatever instead? workbench in the PATH is pretty ambigious.

github-actions · 2024-09-04T14:06:24Z

👋 Hi, @thomashampson,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

sf1919 · 2024-09-11T13:55:22Z

I'm going to change the milestone on this as it's unlikely we can get this sorted before end of tomorrow.

peterfpeterson · 2024-11-19T16:27:35Z

qt/applications/workbench/workbench/app/mantidworkbench_launch_wrapper.py

+from workbench.app.main import main
+
+
+def launch():


This becomes much easier to add tests to if the sys.argv is an argument to the function

Suggested change

def launch():

def launch(args):

peterfpeterson · 2024-11-19T16:27:58Z

qt/applications/workbench/workbench/app/mantidworkbench_launch_wrapper.py

+
+
+if __name__ == "__main__":
+    sys.exit(launch())


then this becomes

Suggested change

sys.exit(launch())

sys.exit(launch(sys.argv))

peterfpeterson · 2024-11-19T16:28:44Z

qt/applications/workbench/workbench/app/mantidworkbench_launch_wrapper.py

+        command = ["launch_mantidworkbench"] + sys.argv[1:]
+        subprocess.run(command)
+    else:
+        main()


shouldn't main() get the command line arguments?

thomashampson · 2024-12-06T13:31:41Z

I've updated the diagram in the description to reflect the recent changes.

thomashampson · 2024-12-06T13:42:40Z

I've verified on Windows that:

both entry points work from a Conda install
the standalone install launches fine

thomashampson · 2024-12-06T13:51:01Z

I've verified that os.environ['LD_PRELOAD'] outputs the expected jemalloc location on Linux conda {workbench, mantidworkbench} and standalone package.

thomashampson · 2024-12-06T17:18:18Z

For some reason the standalone package isn't launching on MacOS.

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Thomas Hampson <[email protected]>

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

…inux Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

for more information, see https://pre-commit.ci

Renamed from workbench_launcher_jemalloc to launch_mantidworkbench

Co-authored-by: Pete Peterson <[email protected]>

jclarkeSTFC · 2024-12-10T09:14:36Z

https://builds.mantidproject.org/job/build_packages_from_branch/956/

macOS standalone after rebase

jhaigh0

Whoops didn't mean to approve yet

jhaigh0

(fix the mac stand alone)

jclarkeSTFC · 2024-12-10T14:03:14Z

https://builds.mantidproject.org/job/build_packages_from_branch/959/

This one's going to be a winner

Edit: updated to second attempt at winning

Then someone can pass in arguments directly instead of using sys.argv. If no arguments are given then sys.argv gets used by default. Co-authored-by: Thomas Hampson <[email protected]>

thomashampson marked this pull request as ready for review August 6, 2024 12:53

jhaigh0 added the ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS label Aug 6, 2024

thomashampson added this to the Release 6.11 milestone Aug 8, 2024

peterfpeterson reviewed Aug 8, 2024

View reviewed changes

thomashampson marked this pull request as draft August 13, 2024 11:05

github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Sep 4, 2024

sf1919 modified the milestones: Release 6.11, Release 6.12 Sep 11, 2024

jclarkeSTFC force-pushed the 37538_mantidworkbench_entry_point branch from 44d22a5 to c262ecd Compare October 29, 2024 10:10

jclarkeSTFC removed the Has Conflicts Used by the bot to label pull requests that have conflicts label Oct 29, 2024

jclarkeSTFC marked this pull request as ready for review November 13, 2024 09:53

sf1919 requested a review from peterfpeterson November 18, 2024 09:19

peterfpeterson reviewed Nov 19, 2024

View reviewed changes

jclarkeSTFC force-pushed the 37538_mantidworkbench_entry_point branch from 88fdeea to 01e69ff Compare November 20, 2024 13:37

jhaigh0 and others added 8 commits December 10, 2024 09:12

Add new wrapper script to unify launch commands

d7b832b

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Thomas Hampson <[email protected]>

Add arg passing to workbench launch wrapper script

6b0ae0f

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Thomas Hampson <[email protected]>

Add release notes

beeb375

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Thomas Hampson <[email protected]>

Update launch script name in linux standalone packaging script

130c1f1

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

Change python exec command to avoid infinite loop when launching on L…

ad7d131

…inux Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

Fix name of mantid launch script in standalone package script

1536ade

Co-authored-by: James Clarke <[email protected]> Co-authored-by: Jonathan Haigh <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

33b8d9f

for more information, see https://pre-commit.ci

Move release notes to 6.12 folder

d7d62b5

jclarkeSTFC and others added 4 commits December 10, 2024 09:12

Add return code handling

f278012

Rename workbench_launch_wrapper to mantidworkbench_launch_wrapper

5dff77b

Rename standalone launcher script

a5117a4

Renamed from workbench_launcher_jemalloc to launch_mantidworkbench

Pass command-line arguments through to main

e34ccfb

Co-authored-by: Pete Peterson <[email protected]>

jclarkeSTFC force-pushed the 37538_mantidworkbench_entry_point branch from 01e69ff to e34ccfb Compare December 10, 2024 09:13

jhaigh0 approved these changes Dec 10, 2024

View reviewed changes

jhaigh0 reviewed Dec 10, 2024

View reviewed changes

jhaigh0 requested changes Dec 10, 2024

View reviewed changes

Add optional argument to workbench main() and launch()

6492632

Then someone can pass in arguments directly instead of using sys.argv. If no arguments are given then sys.argv gets used by default. Co-authored-by: Thomas Hampson <[email protected]>

jclarkeSTFC force-pushed the 37538_mantidworkbench_entry_point branch from 748c60f to 6492632 Compare December 11, 2024 09:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mantidworkbench as an entry point for the mantidworkbench Conda package #37750

Add mantidworkbench as an entry point for the mantidworkbench Conda package #37750

thomashampson commented Aug 6, 2024 •

edited by jhaigh0

Loading

peterfpeterson Aug 8, 2024 •

edited

Loading

peterfpeterson Aug 8, 2024

peterfpeterson Aug 8, 2024 •

edited

Loading

peterfpeterson commented Aug 8, 2024

github-actions bot commented Sep 4, 2024

sf1919 commented Sep 11, 2024

peterfpeterson Nov 19, 2024

peterfpeterson Nov 19, 2024

peterfpeterson Nov 19, 2024

thomashampson commented Dec 6, 2024

thomashampson commented Dec 6, 2024 •

edited

Loading

thomashampson commented Dec 6, 2024

thomashampson commented Dec 6, 2024

jclarkeSTFC commented Dec 10, 2024

jhaigh0 left a comment

jhaigh0 left a comment

jclarkeSTFC commented Dec 10, 2024 •

edited

Loading

Add mantidworkbench as an entry point for the mantidworkbench Conda package #37750

Are you sure you want to change the base?

Add mantidworkbench as an entry point for the mantidworkbench Conda package #37750

Conversation

thomashampson commented Aug 6, 2024 • edited by jhaigh0 Loading

Description of work

Summary of work

Purpose of work

To test:

Reviewer

Code Review

Functional Tests

Gatekeeper

peterfpeterson Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

peterfpeterson Aug 8, 2024

Choose a reason for hiding this comment

peterfpeterson Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

peterfpeterson commented Aug 8, 2024

github-actions bot commented Sep 4, 2024

sf1919 commented Sep 11, 2024

peterfpeterson Nov 19, 2024

Choose a reason for hiding this comment

peterfpeterson Nov 19, 2024

Choose a reason for hiding this comment

peterfpeterson Nov 19, 2024

Choose a reason for hiding this comment

thomashampson commented Dec 6, 2024

thomashampson commented Dec 6, 2024 • edited Loading

thomashampson commented Dec 6, 2024

thomashampson commented Dec 6, 2024

jclarkeSTFC commented Dec 10, 2024

jhaigh0 left a comment

Choose a reason for hiding this comment

jhaigh0 left a comment

Choose a reason for hiding this comment

jclarkeSTFC commented Dec 10, 2024 • edited Loading

thomashampson commented Aug 6, 2024 •

edited by jhaigh0

Loading

peterfpeterson Aug 8, 2024 •

edited

Loading

peterfpeterson Aug 8, 2024 •

edited

Loading

thomashampson commented Dec 6, 2024 •

edited

Loading

jclarkeSTFC commented Dec 10, 2024 •

edited

Loading