Skip to content

Conversation

gwennlbh
Copy link
Contributor

@gwennlbh gwennlbh commented May 21, 2025

Description

Add a flake to build a binary using Nix

How I Tested

nix build .?submodules=1

Do not merge before

  • Check propagatedBuildInputs with ldd
  • Add a apps section to nix run
  • Add a way to boot a python shell/interpret a python file with result's site-packages in PYTHONPATH

see also open-dynamic-robot-initiative/master-board#173

I fulfilled the following requirements

  • All new code is formatted according to our style guide (for C++ run clang-format, for Python, run flake8 and fix all warnings).
  • All new functions/classes are documented and existing documentation is updated according to changes.
  • No commented code from testing/debugging is kept (unless there is a good reason to keep it).

@gwennlbh gwennlbh marked this pull request as draft May 21, 2025 08:54
@gwennlbh
Copy link
Contributor Author

i added a workflow file, seems like actions were not turned on for this repo -- is ci useful here?

@nim65s
Copy link
Contributor

nim65s commented May 21, 2025

because we did not have a package manager for master-board before I guess

gwennlbh added a commit to gwennlbh/gepetto-nix that referenced this pull request May 28, 2025
@gwennlbh gwennlbh marked this pull request as ready for review May 28, 2025 11:48
@gwennlbh
Copy link
Contributor Author

gwennlbh commented Jun 2, 2025

I ran the new CI locally with act and it passes

Copy link
Contributor

@nim65s nim65s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, github somehow lost part of my review :/

flake.nix Outdated
Comment on lines 63 to 67
shellHook = ''
controlSitePackages=`echo ${self'.packages.odri_control_interface}/lib/python*/site-packages | head -n 1`
masterboardSitePackages=`echo ${pkgs.odri_master_board_sdk}/lib/python*/site-packages | head -n 1`
export PYTHONPATH="$controlSitePackages:$masterboardSitePackages:$PYTHONPATH"
'';
Copy link
Contributor

@nim65s nim65s Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
shellHook = ''
controlSitePackages=`echo ${self'.packages.odri_control_interface}/lib/python*/site-packages | head -n 1`
masterboardSitePackages=`echo ${pkgs.odri_master_board_sdk}/lib/python*/site-packages | head -n 1`
export PYTHONPATH="$controlSitePackages:$masterboardSitePackages:$PYTHONPATH"
'';
PYTHONPATH = lib.concatMapStringsSep ":" (p: "${p}/${pkgs.python3.sitePackages}") [
self'.packages.odri_control_interface
pkgs.odri_master_board_sdk
]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or better: pkgs.python3.withPackages (_: [ self'.packages.odri_control_interface pkgs.odri_master_board_sdk ])

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or better: pkgs.python3.withPackages (_: [ self'.packages.odri_control_interface pkgs.odri_master_board_sdk ])

so as buildInputs ? i'm not sure about dropping into a python shell directly, because some python scripts require cli args

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in packages, so yes that is kind of buildInputs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm unfortunately doing that doesnt seem to add the packages to PYTHONPATH. maybe the packages have to be marked as python packages for this to work? i'm leaving it at PYTHONPATH = for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, yes, to tell nix that a derivation has a python module, we need to either define it with pkgs.python3Packages.callPackage instead of pkgs.callPackage (so in nixpkgs use pkgs/top-level/python-packages.nix instead of by-name, or in gepetto/nix put the package in py-pkgs instead of pkgs), or transform it into a module afterwards with pkgs.python3Packages.toPythonModule.

therefore,
here:

pkgs.python3.withPackages (p: [
  (p.toPythonModule pkgs.odri_master_board_sdk)
  (p.toPythonModule self'.packages.odri_control_interface)
])

and/or in gepetto/nix:

git mv {,py-}pkgs/odri-masterboard-sdk.nix
git mv {,py-}pkgs/odri-control-interface.nix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nim65s
Copy link
Contributor

nim65s commented Jun 3, 2025

Oh, yes, it is still here, but as outdated. I'll resolve those.

@gwennlbh gwennlbh force-pushed the nix branch 4 times, most recently from a6bc629 to 82653c8 Compare June 4, 2025 08:22
@gwennlbh gwennlbh requested a review from nim65s June 4, 2025 14:29
gwennlbh and others added 26 commits June 5, 2025 17:53
Signed-off-by: Gwenn Le Bihan <[email protected]>
This reverts commit 6706d9f.

Turns out that having #s in the file contents prevent -D from working...
thanks C++

> WARNING: Preprocessor definitions containing '#' may not be passed on the compiler command line because many compilers do not support it.
> odri-control-interface> CMake is dropping a preprocessor definition: CONFIG_TESTBENCH_YAML="robot:
> odri-control-interface>     interface: enp2s0

so we'll keep the files as is i guess.
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
github action does not support macos

Signed-off-by: Gwenn Le Bihan <[email protected]>
Co-authored-by: Guilhem Saurel <[email protected]>
Co-authored-by: Guilhem Saurel <[email protected]>
gwennlbh added 2 commits June 5, 2025 17:55
Signed-off-by: Gwenn Le Bihan <[email protected]>
Signed-off-by: Gwenn Le Bihan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants