fmpy: init at 0.3.23 #397658

tmplt · 2025-04-10T11:56:57Z

Continuing the work from #131741, FMpy, a library for working with the FMI standard is packaged.

Things done

Add a 👍 reaction to pull requests you find important.

doronbehar

A few more comments on the commits of the PR: Always make sure the PR has the following commits:

maintainers.tmplt: fix email and name
rpclib: init at 2.3.0
python312Packages.fmpy: init at <fill this>

I'd let go of the sundials commit, and put all the overriding details inside the fmpy expression. Note how I use python312Packages.fmpy and not python3Packages.fmpy - the later is not an attribute directly built by Hydra, and hence the PR does not trigger an ofborg build of it automatically.

pkgs/top-level/all-packages.nix

pkgs/development/libraries/rpclib/default.nix

pkgs/development/libraries/sundials/default.nix

pkgs/development/python-modules/fmpy/default.nix

tmplt · 2025-04-10T14:23:43Z

Thanks for the review, @doronbehar. I'll handle your comments when all the kinks are ironed out.

tmplt · 2025-04-10T14:39:35Z

Things to fix:

fmpy simulate fail on dlopen
Webapp impl not compat with current dash API: [webapp] app.run_server has been replaced by app.run CATIA-Systems/FMPy#756
Pyside6 GUI is not generated during build; manual uic call required?
Handle build_remoting.py
cleanup pyproject.toml artifact override
Adhere to commit message format

tmplt · 2025-04-23T11:37:41Z

dlopen error due to FMPy extracting FMU under /tmp which is mounted noexec on my system. Setting TMPDIR=~/tmp resolves this. I don't think this is a nixpkgs issue?

tmplt · 2025-04-23T13:30:35Z

I'd let go of the sundials commit, and put all the overriding details inside the fmpy expression.

Makes sense. Done.

Release notes via https://github.com/CATIA-Systems/FMPy/releases/tag/v0.3.23 Co-authored-by: Vladimir Korolev <[email protected]>

tmplt · 2025-04-29T11:58:09Z

Could not get the remoting feature to build so I gated it behind a package option, enableRemoting ? false.
The error is

nix-repl> :b (outputs.legacyPackages.x86_64-linux.python3Packages.fmpy.overrideAttrs(prev: { meta.broken = false; })).override({ enableRemoting = true; })
[...]
       > In file included from /nix/store/71xyq87gpb2qrwn314p0sf2n002lgd91-glibc-2.40-66-dev/include/fcntl.h:341,
       >                  from /build/source/native/remoting/client_tcp.cpp:12:
       > In function ‘int open(const char*, int, ...)’,
       >     inlined from ‘void* fmi2Instantiate(fmi2String, fmi2Type, fmi2String, fmi2String, const fmi2CallbackFunctions*, fmi2Boolean, fmi2Boolean)’ at /build/source/native/remoting/client_tcp.cpp:217:28:
       > /nix/store/71xyq87gpb2qrwn314p0sf2n002lgd91-glibc-2.40-66-dev/include/bits/fcntl2.h:52:31: error: call to ‘__open_missing_mode’ declared with attribute error: open with O_CREAT or O_TMPFILE in second argument needs 3 arguments
       >    52 |           __open_missing_mode ();
       >       |           ~~~~~~~~~~~~~~~~~~~~^~

Unsure what's going on here, or whether this is an upstream issue.

doronbehar

Lots of comments! Indeed a complex package you are trying to package!

doronbehar · 2025-04-29T12:45:33Z

pkgs/by-name/rp/rpclib/package.nix

+stdenv.mkDerivation rec {
+  pname = "rpclib";
+  version = "2.3.0";
+
+  src = fetchFromGitHub {
+    owner = "rpclib";
+    repo = "rpclib";
+    rev = "v${version}";
+    sha256 = "0dlbkl47zd2fkxwbn93w51wmvfr8ssp4zribn5wi4cpiky44a4g9";


Suggested change

stdenv.mkDerivation rec {

pname = "rpclib";

version = "2.3.0";

src = fetchFromGitHub {

owner = "rpclib";

repo = "rpclib";

rev = "v${version}";

sha256 = "0dlbkl47zd2fkxwbn93w51wmvfr8ssp4zribn5wi4cpiky44a4g9";

stdenv.mkDerivation (finalAttrs: {

pname = "rpclib";

version = "2.3.0";

src = fetchFromGitHub {

owner = "rpclib";

repo = "rpclib";

rev = "v${finalAttrs.version}";

sha256 = "0dlbkl47zd2fkxwbn93w51wmvfr8ssp4zribn5wi4cpiky44a4g9";

doronbehar · 2025-04-29T12:45:44Z

pkgs/by-name/rp/rpclib/package.nix

+    license = with lib.licenses; [ mit ];
+    maintainers = with lib.maintainers; [ tmplt ];
+  };
+}


Suggested change

}

})

doronbehar · 2025-04-29T12:46:34Z

pkgs/by-name/rp/rpclib/package.nix

+  meta = {
+    description = "RPC library for C++, providing both a client and server implementation";
+    homepage = "https://github.com/rpclib/rpclib/";
+    license = with lib.licenses; [ mit ];


This will probably not change too often to justify opening a with *;. So I'd suggest:

Suggested change

license = with lib.licenses; [ mit ];

license = lib.licenses.mit;

doronbehar · 2025-04-29T12:48:12Z

pkgs/development/python-modules/fmpy/default.nix

+    homepage = "https://github.com/CATIA-Systems/FMPy";
+    license = with lib.licenses; [ bsd2 ];
+    maintainers = with lib.maintainers; [ tmplt ];
+    platforms = with lib.platforms; linux ++ darwin ++ windows;


This is pretty much the same as lib.platforms.all;.

doronbehar · 2025-04-29T12:48:30Z

pkgs/development/python-modules/fmpy/default.nix

+    broken = enableRemoting;
+    description = "Simulate Functional Mockup Units (FMUs) in Python";
+    homepage = "https://github.com/CATIA-Systems/FMPy";
+    license = with lib.licenses; [ bsd2 ];


Same here:

Suggested change

license = with lib.licenses; [ bsd2 ];

license = lib.licenses.bsd2;

doronbehar · 2025-04-29T13:15:57Z

pkgs/development/python-modules/fmpy/default.nix

+      substituteInPlace src/fmpy/gui/__init__.py \
+          --replace-fail "@QT6_LIBEXEC_DIR@" "${qt6.qtbase}/libexec/"


To make it a bit clearer that the derivation is eventually built using Nixpkgs Python setup hooks, maybe it'd be a bit more correct to put this in a postConfigure.

doronbehar · 2025-04-29T13:17:26Z

pkgs/development/python-modules/fmpy/default.nix

+    '';
+
+  # Don't run upstream build scripts as they are too specialized.
+  # cvode is already built, so we only need to build native binaries.


Another worth noting thing to comment about could be:

Suggested change

# cvode is already built, so we only need to build native binaries.

# cvode is already built, so we only need to build native binaries.

# We run these cmake builds and then run the standard Nixpkgs Python

# setup-hooks.

doronbehar · 2025-04-29T13:17:57Z

pkgs/development/python-modules/fmpy/default.nix

+    + lib.optionalString (enableRemoting && stdenv.isLinux) ''
+      # The reproduction of build_remoting.py
+      # NB: does not build due do to incorrect open(8) call


Put that comment near broken = true;.

Suggested change

+ lib.optionalString (enableRemoting && stdenv.isLinux) ''

# The reproduction of build_remoting.py

# NB: does not build due do to incorrect open(8) call

+ lib.optionalString (enableRemoting && stdenv.isLinux) ''

doronbehar · 2025-04-29T13:22:45Z

pkgs/development/python-modules/fmpy/default.nix

+    # Remove artifacts that we do not build
+    + lib.optionalString stdenv.isLinux ''
+      sed --regexp-extended --in-place '/(.dll|.dylib|.exe)/d' pyproject.toml
+    ''
+    + lib.optionalString stdenv.isDarwin ''
+      sed --regexp-extended --in-place '/(.dll|.so|linux|.exe)/d' pyproject.toml
+    ''
+    + lib.optionalString (!enableRemoting) ''
+      sed --regexp-extended --in-place '/(client_tcp|server_tcp)/d' pyproject.toml
+    '';


Upstream should be nudged to not require these artifacts depending on the platform... Have you made an effort in that regard? Even if we'd decide to keep removing these lines ourselves, maybe it'd recommend at least for the darwin v.s linux. something like:

# Remove the other system's extension. '' sed --regexp-extended --in-place '/\.(dll|${ "linux" = "dylib"; "darwin" = "so"; }.${stdenv.hostPlatform.system}|exe)/d' pyproject.toml ''

NOTE: Technically, a bit more sophisticated Nix function should be used here, since exe is always removed although Windows is a supported platform.

In general, I'm very surprised upstream writes all of these extensions, and that you have to remove them because you want to build to a specific platform... I'd try to discuss this with upstream to make it easier for us and for others to build and distribute their software...

doronbehar · 2025-04-29T13:32:20Z

pkgs/development/python-modules/fmpy/default.nix

+          --subst-var-by cvode ${cvode}
+
+      # Sundials are provided outside of this package
+      sed --regexp-extended --in-place '/(sundials_)/d' pyproject.toml


Not to mention they hard-code these sundials paths... Why do they do it?

tmplt · 2025-04-29T13:55:59Z

Good comments, @doronbehar, thanks! It may be a week or two before I can handle these.

github-actions bot added 6.topic: python 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Apr 10, 2025

This comment was marked as resolved.

Sign in to view

github-actions bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 10, 2025

tmplt force-pushed the fmpy branch from 6985ab8 to 6c1733e Compare April 10, 2025 13:49

This comment was marked as outdated.

Sign in to view

tmplt force-pushed the fmpy branch from 6c1733e to d33b66a Compare April 10, 2025 13:56

This comment was marked as outdated.

Sign in to view

tmplt force-pushed the fmpy branch from d33b66a to 9b1b5b8 Compare April 10, 2025 14:06

doronbehar reviewed Apr 10, 2025

View reviewed changes

tmplt force-pushed the fmpy branch from 54acc91 to be2c94d Compare April 23, 2025 09:13

github-actions bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 23, 2025

maintainers.tmplt: fix email and name

678debd

tmplt force-pushed the fmpy branch from f23148a to c38f003 Compare April 23, 2025 10:25

rpclib: init at 2.3.0

9690096

tmplt force-pushed the fmpy branch from c38f003 to ca85df1 Compare April 23, 2025 10:29

github-actions bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Apr 23, 2025

tmplt force-pushed the fmpy branch from ca85df1 to 8f9dcda Compare April 23, 2025 12:18

tmplt force-pushed the fmpy branch from 8f9dcda to 1071efa Compare April 28, 2025 14:34

tmplt changed the title ~~fmpy: init at 0.3.22~~ fmpy: init at 0.3.23 Apr 28, 2025

tmplt force-pushed the fmpy branch 3 times, most recently from 329dd38 to 6c06b7c Compare April 29, 2025 10:37

python312Packages.fmpy: init at 0.3.23

81e4568

Release notes via https://github.com/CATIA-Systems/FMPy/releases/tag/v0.3.23 Co-authored-by: Vladimir Korolev <[email protected]>

tmplt force-pushed the fmpy branch from 6c06b7c to 81e4568 Compare April 29, 2025 11:46

tmplt marked this pull request as ready for review April 29, 2025 11:46

nix-owners bot requested a review from natsukium April 29, 2025 11:48

tmplt requested a review from doronbehar April 29, 2025 11:58

doronbehar reviewed Apr 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fmpy: init at 0.3.23 #397658

fmpy: init at 0.3.23 #397658

tmplt commented Apr 10, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

doronbehar left a comment

tmplt commented Apr 10, 2025

tmplt commented Apr 10, 2025 •

edited

Loading

tmplt commented Apr 23, 2025

tmplt commented Apr 23, 2025

tmplt commented Apr 29, 2025

doronbehar left a comment

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

doronbehar Apr 29, 2025

tmplt commented Apr 29, 2025

	license = with lib.licenses; [ mit ];
	license = lib.licenses.mit;

	license = with lib.licenses; [ bsd2 ];
	license = lib.licenses.bsd2;

		substituteInPlace src/fmpy/gui/__init__.py \
		--replace-fail "@QT6_LIBEXEC_DIR@" "${qt6.qtbase}/libexec/"

fmpy: init at 0.3.23 #397658

Are you sure you want to change the base?

fmpy: init at 0.3.23 #397658

Conversation

tmplt commented Apr 10, 2025 • edited Loading

Things done

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

doronbehar left a comment

Choose a reason for hiding this comment

tmplt commented Apr 10, 2025

tmplt commented Apr 10, 2025 • edited Loading

tmplt commented Apr 23, 2025

tmplt commented Apr 23, 2025

tmplt commented Apr 29, 2025

doronbehar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmplt commented Apr 29, 2025

tmplt commented Apr 10, 2025 •

edited

Loading

tmplt commented Apr 10, 2025 •

edited

Loading