Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kicad: link through /lib and /share from kicad.base #88923

Closed
wants to merge 2 commits into from

Conversation

evils
Copy link
Member

@evils evils commented May 26, 2020

Motivation for this change

https://discourse.nixos.org/t/7317
and #40835

Things done
  • add a kicad.py attribute which is toPythonModule kicad.base (this yields the same as kicad.base, does it add context?)

    • change python.pkgs.kicad to point to that, add python.pkgs.kicad-unstable
  • link through to kicad.base's /lib

    • exposes the python bindings and some .so files
  • link through to kicad.base's /share

    • exposes some .desktop files, MIME info
      • and a whole bunch more stuff i'm not sure need to be
  • bump kicad-unstable to the latest version

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)

  • Built on platform(s)

    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"

  • Tested execution of all binary files (usually in ./result/bin/)

  • Determined the impact on package closure size (by running nix path-info -S before and after)

  • Ensured that relevant documentation is up to date

  • Fits CONTRIBUTING.md.

@evils evils requested a review from FRidh May 26, 2020 08:19

in
stdenv.mkDerivation rec {

passthru.libraries = callPackages ./libraries.nix versionConfig.libVersion;
# a link to this exists as python3.pkgs.[kicad|kicad-unstable]
passthru.py = python.pkgs.toPythonModule base;
Copy link
Member

Choose a reason for hiding this comment

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

please keep the toPythonModule only in python-packages.nix.

Copy link
Member Author

Choose a reason for hiding this comment

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

this makes it so kicad-small.py works

Copy link
Member

Choose a reason for hiding this comment

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

Python modules need to be inside python-packages.nix, not outside of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

a link to the module exists there
independently defining it there would result in not only duplicating the same snippet at least 4 times
but risk breaking stuff if a pname changes (for example you couldn't do an ad-hoc (kicad.override {pname ="kicad-testing"; change=otherStuff;};).py )

Copy link
Member Author

@evils evils May 26, 2020

Choose a reason for hiding this comment

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

removing the kicad.py attribute would fix those concerns i suppose
but that would feel like a very scattered implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

can you tell me if there's anything added by toPythonModule?
kicad.base == kicad.src == kicad.py == python3.pkgs.kicad

i think with the bindings exposed directly (by linking /lib (and /share?))
having kicad.py doesn't make much sense
and python.pkgs.kicad makes sense as an end-point for kicad.base to avoid pulling in the kicad libraries if you just want the bindings

Copy link
Member Author

Choose a reason for hiding this comment

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

resolved with the new commit? (will be squashed before undrafting)

Copy link
Member

Choose a reason for hiding this comment

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

lookup the definition of toPythonModule. It adds a passthru that sets relevant attributes for it to be usable by python.buildEnv/python.withPackages.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/make-a-derivation-for-a-kicad-plugin/7317/14

@FRidh
Copy link
Member

FRidh commented May 26, 2020

Before making further changes I suggest introducing tests for the kicad bindings, so one can see what functions and what does not.

@trepetti
Copy link
Contributor

Will linking through /share also fix the missing desktop icons associated with the Kicad applications, or would we also need to add desktopItem = makeDesktopItem { ... }; to address that?

@evils
Copy link
Member Author

evils commented Jul 24, 2020

@trepetti i have no idea how those work, i suggest you try it out

side note: with #93578 scripting support was disabled because wxPython broke
TBD but presumably this will have some effect on our ability to enable kicad plugins

pass through a kicad.py
and link to it with python.pkgs.kicad
to allow access to the bindings without pulling in the 3d models etc

linking /lib and /share should allow the same while keep kicad usable
it also exposes .desktop files and mime info
@eduardosm
Copy link
Contributor

Would it be possible to make a subset of this PR that links share to fix #106295?

@evils
Copy link
Member Author

evils commented Dec 23, 2020

@eduardosm did you (or anyone else with a desktop environment) manage to try this PR and confirm linking through /share fixes #106295?

@eduardosm
Copy link
Contributor

I can confirm that this PR fixes #106295

@stale
Copy link

stale bot commented Jun 22, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 22, 2021
@evils evils closed this Jun 24, 2021
@r0hit05
Copy link
Contributor

r0hit05 commented Aug 16, 2023

#249464

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants