Bug 236507 - [NEW PORT] science/py-Mcstas-Tools: Python based set of tools for the McStas neutron simulation package
Summary: [NEW PORT] science/py-Mcstas-Tools: Python based set of tools for the McStas ...
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Kai Knoblich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-13 12:20 UTC by Erik B Knudsen
Modified: 2019-08-27 10:08 UTC (History)
1 user (show)

See Also:


Attachments
svn diff for the new port py-McStas-Tools (4.28 KB, patch)
2019-03-13 12:20 UTC, Erik B Knudsen
erkn: maintainer-approval+
Details | Diff
svn diff for the new port py-McStas-Tools (2.54 KB, patch)
2019-03-14 08:41 UTC, Erik B Knudsen
erkn: maintainer-approval+
Details | Diff
svn diff of the new port py-Mcstas-Tools (9.75 KB, patch)
2019-04-11 08:01 UTC, Erik B Knudsen
erkn: maintainer-approval+
Details | Diff
py-mcstas-tools-revised.patch (10.89 KB, patch)
2019-08-19 09:19 UTC, Kai Knoblich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Erik B Knudsen 2019-03-13 12:20:53 UTC
Created attachment 202841 [details]
svn diff for the new port py-McStas-Tools

This port contains a python/qt set of tools for the McStas neutron simulation package. McStas itself is in science/mcstas.

This port is intended the upstream replacement for p5-Mcstas-Tools, but they are expected to coexist for some time (appr. 1yr).

Notes:
- Upstream has split the package in several source-files hence the extra logic in do-install etc.
- Scripts with other back-ends than pyqtgraph exist upstream but are not yet in this port.
Comment 1 Tobias Kortkamp freebsd_committer 2019-03-14 07:15:05 UTC
Comment on attachment 202841 [details]
svn diff for the new port py-McStas-Tools

Wrong patch?  The patch seems to be for p5-Mcstas-Tools.  I do not see anything 
Python related here.
Comment 2 Erik B Knudsen 2019-03-14 08:41:43 UTC
Created attachment 202851 [details]
svn diff for the new port py-McStas-Tools

This is now the correct patch with the python stuff in it.
Comment 3 Erik B Knudsen 2019-03-14 08:43:59 UTC
(In reply to Tobias Kortkamp from comment #1)
Yes indeed - it was the wrong patch. I have now submitted the right one.
Stupid mistake. I am deeply embarrassed. Thanks for noticing.
Comment 4 Tobias Kortkamp freebsd_committer 2019-04-08 17:08:18 UTC
Comment on attachment 202851 [details]
svn diff for the new port py-McStas-Tools

The port looks mostly ok, but the patch is still incomplete and is
missing at least pkg-descr, distinfo.

+USES=		cmake python:3.6+ pyqt:5

There is little reason to have USES=pyqt:5 without USE_PYQT.  What
PyQt components are needed at runtime?

+CMAKE_ARGS=	-Denable_mcstas=1

CMAKE_ON=	enable_mcstas

+	${RLN} -s  ${STAGEDIR}${PREFIX}/mcstas/${PORTVERSION}/bin/${e} ${STAGEDIR}${PREFIX}/bin/${e}

${RLN} uses `install -l rs` under the hood not ln(1), so `-s` is
meaningless.  At the moment install(1) ignores it but better not
add it.
Comment 5 Erik B Knudsen 2019-04-11 07:59:58 UTC
(In reply to Tobias Kortkamp from comment #4)
Thank you for your advice, corrections, and suggestions -  and most of all for your time. I am still a ports amateur.

I have now created yet another diff which 
- includes pkg-descr and distinfo.
- where the Makefile does indeed set USE_PYQT to the things needed
- does not have the -s in the ${RLN}-line

But which retains the "CMAKE_ARGS= -Denable_mcstas=1" line as CMAKE_ON does not work due to an upstream issue. I have reported this upstream, wo it is epxeted to change for the next release.

Thanks again,
Erik
Comment 6 Erik B Knudsen 2019-04-11 08:01:20 UTC
Created attachment 203578 [details]
svn diff of the new port py-Mcstas-Tools
Comment 7 Kai Knoblich freebsd_committer 2019-08-19 09:19:06 UTC
Created attachment 206691 [details]
py-mcstas-tools-revised.patch

Hi Erik,

thank you for your work and sorry for the delay. I did some extensive QA that is reflected in the new attachment. The updated patch based on your submission does following: 

- Renamed the port from py-Mcstas-Tools to py-mcstas-tools because Python ports have usually lowercase names. But there are some exceptions and I'll add that point in the review that I'll open soon a possible.

- Sets PKGNAMEPREFIX to ${PYTHON_PKGNAMEPREFIX} instead of "py-"

- Points to a binary file "mcpl2ssw" instead the "examples" directory for the port science/mcstas-comps. Otherwise the dependency of that port isn't registered.

- Patches the shell script wrappers for the Python executables by replacing "python3" with ${PYTHON_CMD} and adding the "-f" argument to readlink. Otherwise the shell scripts aren't much of use.

I've only one question/note at the moment:

- Is the "mcdoc" component really required? If I start the program it quits with the info that "mcdoc.html" can't be found. I'm not able to find that HTML file in the source tarballs.

I can change the diff accordingly if the "mcdoc" component isn't required. Otherwise, if you're fine my additions I'll open a review as soon as possible to let other member of the Python team to look at it ("Four or more eyes sees more than two").
Comment 8 Erik B Knudsen 2019-08-22 11:02:51 UTC
(In reply to Kai Knoblich from comment #7)
Hi Kai,

Thanks for the update and the edits.

The mcdoc component _is_ required, albeit it would appear that something goes wrong here. I unfortunately missed that buggy behaviour. It is supposed to autogenerate html-documentation (mcdoc.html) from *.comp-files in the distribution, but obviously something goes wrong here. I'll take a closer look at that. 

Othere than that the other edits are reasonable - I guess some things stems from me being new-ish to the inner workings of port management.

cheers
Erik
Comment 9 Kai Knoblich freebsd_committer 2019-08-27 09:57:39 UTC
(In reply to Erik B Knudsen from comment #8)

Hi Erik,

thank you for your feedback and the clarification about the "mcdoc" component. 

Should I open the review on FreeBSD's Phabricator anyway to let other Python members look at it or still wait a bit longer until you've found the issue with "mcdoc"?
Comment 10 Erik B Knudsen 2019-08-27 10:08:39 UTC
(In reply to Kai Knoblich from comment #9)
Please postpone putting it to review until I've had a look. I suspect that some post-install config option does not get propagated the correct way.