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: cleanup, fix and update #74259

Merged
merged 10 commits into from Jan 3, 2020
Merged

kicad: cleanup, fix and update #74259

merged 10 commits into from Jan 3, 2020

Conversation

@evils
Copy link
Contributor

@evils evils commented Nov 26, 2019

Motivation for this change

fixes #72248
fixes #49090

includes / closes #72813
affected by #73145, #39493

Things done
  • 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 nix-review --run "nix-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)
    • +101466360 (whatever units those are...) (comparing old kicad-with-packages3d to new kicad)
    • +105110176 (old kicad vs new kicad-small)
    • +5342432736 (old kicad vs new kicad, now includes the 3D packages)
    • +9955620808 (fixed kicad-unstable)
      • of which 3517424376 because debug = true;
  • Ensured that relevant documentation is up to date
    • i couldn't find any...
  • Fits CONTRIBUTING.md.
  • start from #72813

  • separate the building in base.nix

    • separate derivation, reused for kicad-small
    • source switched to gitlab as development has moved there
  • separate the libraries' definitions in libraries.nix

  • separate derivations, reused for kicad-small

    • i18n isn't a library but here, gets linked into each base derivation
      • currently the only one moved to gitlab
  • a bunch of wrapping in default.nix

    • moved all the revs, hashes and dependency specifics here
    • accept a pname to select which package to build [ kicad | kicad-small | kicad-unstable ]
    • pass the library paths as env vars in the wrapper
  • make kicad have the 3D packages by default

    • add alias for kicad-with-packages3d
    • and remove that from all-packages.nix
  • add kicad-small to preserve the original kicad behaviour of not including the 3D packages (5gb)

  • fix kicad-unstable

    • use wxGTK31
      • brings high DPI support?
      • the python shell in pcbnew fails to open...
Status
  • perfect default.nix
    • kicad from nix on debian

      • the 3D viewer doesn't work, missing the viewport
        • could be because of my old debian machine
        • probably unrelated, but the only remaining output is fontconfig errors
      • python error no module named 'math' goes away in a nix-shell --pure, how replicate this behaviour in an installation?
    • figure out what meta.platforms to set (kicad is cross-platform)

    • aarch64 builds by using opencascade-occt

      • functionality tested on x86_64
      • functionality tested on aarch64
    • file chooser not getting the icon for *.pro

      • i seem to recall it worked at some point, but can no longer find it
    • 5.1.x fails 1 test (eeschema)

      • probably not nix's fault, kicad's master branch tests right
    • kicad-small (without 3D models), should it be?

    • unstable

      • needs a way to override version, and rev and sha256 for each of the separated libraries
        • passing in a pname and using that to switch the sources and some other stuff
nice-to-haves
  • separate the built kicad from the wrapping (to make wrapper debugging quicker)
    • i18n can't be fully separated
    • move all the all-packages.nix stuff to the wrapper (default.nix)
reminders to self, before undrafting, do:
  • switch all sources to gitlab if they've completed the move by then
  • summarize the changes
  • the fix for #72248 is temporary, check the upstream MR
  • condense commits, fully history available here
    • add myself to maintainers list
Notify maintainers

cc @berce
and @Kiwi who was added (on their request) as a maintainer

@evils evils force-pushed the evils:kicad-unstable branch 2 times, most recently from aa3b108 to 0bb9895 Nov 28, 2019
@evils evils force-pushed the evils:kicad-unstable branch from 0bb9895 to 066f9bc Dec 1, 2019
@evils evils force-pushed the evils:kicad-unstable branch from 066f9bc to 00f6970 Dec 4, 2019
@gebner
Copy link
Member

@gebner gebner commented Dec 4, 2019

Does this PR address the issue with kicad not finding the new libraries after updating?

I.e., right now kicad stores the current library path in its configuration:

$ grep nix/store -r ~/.config/kicad/
/home/gebner/.config/kicad/kicad_common:KICAD_SYMBOL_DIR=/nix/store/9rjjsly6vf5qawihvgvpf1vcx8dg899k-kicad-5.1.2/share/kicad/library
/home/gebner/.config/kicad/kicad_common:KICAD_TEMPLATE_DIR=/nix/store/9rjjsly6vf5qawihvgvpf1vcx8dg899k-kicad-5.1.2/share/kicad/template
/home/gebner/.config/kicad/kicad_common:KISYS3DMOD=/nix/store/9rjjsly6vf5qawihvgvpf1vcx8dg899k-kicad-5.1.2/share/kicad/modules/packages3d/
/home/gebner/.config/kicad/kicad_common:KISYSMOD=/nix/store/9rjjsly6vf5qawihvgvpf1vcx8dg899k-kicad-5.1.2/share/kicad/modules

Even if you update kicad, it still looks for footprints, etc. in the old path.

@evils
Copy link
Contributor Author

@evils evils commented Dec 4, 2019

with this PR i add those paths to the wrapper
from a quick test,
it looks like it uses those in favour of the .config ones
it even looks like it doesn't save them, which is probably preferred in this case

@evils evils force-pushed the evils:kicad-unstable branch 5 times, most recently from 20eee28 to 88e245a Dec 5, 2019
@evils evils force-pushed the evils:kicad-unstable branch 11 times, most recently from 008099c to 2f5beb1 Dec 14, 2019
@evils evils force-pushed the evils:kicad-unstable branch 3 times, most recently from 490b538 to 7f58be1 Dec 23, 2019
@ofborg ofborg bot requested a review from Kiwi Dec 23, 2019
@evils evils force-pushed the evils:kicad-unstable branch from 7f58be1 to 259c56d Dec 23, 2019
@evils evils changed the title Kicad cleanup, fix and update kicad: cleanup, fix and update Dec 23, 2019
@evils evils force-pushed the evils:kicad-unstable branch 3 times, most recently from 7ed5e76 to 9761522 Dec 24, 2019
@evils evils marked this pull request as ready for review Dec 30, 2019
@evils evils force-pushed the evils:kicad-unstable branch from 9761522 to 0b4b1df Dec 30, 2019
matthuszagh and others added 10 commits Nov 5, 2019
This is needed for python scripting support.
use wxGTK31
  brings hiDPI support
  no python shell...
make unstable use kicad-libraries
  still using a link in $out..., not sure that's a bad thing
  this allows setting that path in makeWrapperArgs
    can't use $out there

kicad-with-packages3d -> kicad and kicad-small

default to OCCT, OCE is outdated
  enforce OCCT on aarch64, where OCE is broken
  withOCE flag allows using OCE on non-aarch64
switch source to gitlab as that's the new upstream source

use wrapper variables for everything but i18n
  add sym and fp templates to template path

update meta to reflect kicad's own language
  set license to AGPLv3, according to the source's LICENSE.README

reduce diff between default and unstable in preparation of merging

on debian gets "no module named 'math'"
  pcbnew 3d viewer is unusable (at least on my T410)
use latest libraries for unstable
move all revs and hashes to default.nix

thanks TQ for getting me through this blockage
minor simplification, fix base version

split i18n and get from gitlab

correct wrapper pythonpath, cleanup build output

update & fake git describe

correct base.nix to fit contributing.md
@evils evils force-pushed the evils:kicad-unstable branch from 0b4b1df to e4786f3 Jan 2, 2020
@gebner
Copy link
Member

@gebner gebner commented Jan 3, 2020

Works for me. I can confirm that the issue with the stale paths is resolved now. @Evils-Devils Is this ready to merge? (I think the remaining todos should be done in follow-up PRs.)

@evils
Copy link
Contributor Author

@evils evils commented Jan 3, 2020

@gebner as this is a rather large overhaul, i wouldn't mind a bit more confirmation that i didn't violate some convention (i can't find any other package using the approach i ended up with).

I do believe it's ready to merge (though i'm about to force push the latest unstable hashes, after i confirm it they work)

As i don't know how to fix the remaining things i'd like done, should i open an issue for those?

@gebner
Copy link
Member

@gebner gebner commented Jan 3, 2020

This PR has now been open for a few months. If somebody had a strong opinion, they would have commented by now. Since this fixes a lot of issues with kicad, I'm going to merge it.

Feel free to open issues for the remaining todo items if you want.

@gebner gebner merged commit 3604ae8 into NixOS:master Jan 3, 2020
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
kicad, kicad-unstable on aarch64-linux Success
Details
kicad, kicad-unstable on x86_64-linux Success
Details
@gebner
Copy link
Member

@gebner gebner commented Jan 3, 2020

though i'm about to force push the latest unstable hashes, after i confirm it they work

Sorry, completely missed this part. Please make a new PR for the updated unstable hashes.

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

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.