On Sun, 30 Jun 2019 02:39:52 +0000 John Soo wrote: > Hi all, > > I've done linting, and indentation checks. There are still some > issues with licenses that I've marked and I don't know where to put > the changes that I've included in llvm.scm. Also I discovered some > non-determinism in the build of freecad just now. I thought it would > be good to share my work, though. It's been long enough. > > - John Hi John, this is not a easy "guix import" package. Thanks for looking into this. Here are some remarks from my side: Commit-log: You have a summary line, but missing this: * gnu/package/...scm (freecad): New variable. This holds for all packages. See the manual (https://guix.gnu.org/manual/en/guix.html#Submitting-Patches) and other commits as reference. You might like to work with magit and yasnippets to automate this. Add a copyright line for yourself to all files you touched. For the dependencies, you often used commits from the git/hg repository instead of a released version. Was there specific reason for that? If so, please add a comment on why you need this specific version. For some (all), I found out that a proper release was only created after you submitted/edited your patch. Maybe you can give it a try with the released versions? I wanted to give it a try for myself but failed: I could not build qtwebkit locally, and no substitutes are available. I don't want to hold back my comments any longer, so I postponed trying it out. Also, I wanted to compare with the AppPack provided by FreeCAD, but I wasn't able to extract it: I did not want to do it on my machine directly, because you use the AppPack executable to extract it! And it did not work in a Guix-VM, probably due to some "standard" library paths being expected. We should fix this package and then suggest them using Guix or at least "guix pack" for really relocatable BLOBs :-) coin3D: There are two binaries in the source repository: ./cfg/csubst.exe ./cfg/wrapmsvc.exe Also the .hg directory is included. Could you please remove them with "snippets"? You can search for "snippet" to find examples for that. I did it like that: + (modules '((guix build utils))) + (snippet + '(begin + (delete-file-recursively ".hg") + (for-each delete-file + '("cfg/csubst.exe" + "cfg/wrapmsvc.exe")) + #t)))) soqt: Same snippets apply here too. soqt: There is a 1.6.0-package available since some hours. Would you like to use this? Also, they say it is superseeded by Quarter. Would it be possible/better to use this one? Is there anything known from FreeCAD side? llvm-toolchain-6: I haven't looked into that yet. pyside2/shiboken: There are two Qt-bindings for Python: PyQt and PySide2. As we have PyQt already in gnu/packages/qt.scm, I think it would make sense to move Pyside2 and its binding generator Shiboken into that package instead of python-xyz.scm. PySide2/Qt version: We currently have 5.11.3 in Guix. 5.12 is the current LTS available and 5.13 is also out since June. The PySide homepage (https://wiki.qt.io/Qt_for_Python) says: The module was released mid June 2018 as a Technical Preview (supporting Qt 5.11), and it was officially released without the Technical Preview tag, in December 2018 for Qt 5.12. What does it mean if we use it still with 5.11? Or do we need to update to 5.12 first? I wonder if we need the dependency on qtwebkit or if we can get rid of this, but I haven't investigated yet. medfile: Is there a specific reason you chose 3.x, instead of the available 4.0.0? If you, could you add a comment of why? License is GPLv3+ libarea: Also licensed under GPL v3 (files under "pocket" directory). freecad: Can you use the releasted sources instead of the git-version? Björn