kicad: add srcs parameter to allow configuring kicad versions #100066

matthuszagh · 2020-10-09T02:04:36Z

Motivation for this change

Allows overriding kicad versions which are otherwise inaccessible.

Things done

Added srcs parameter that can be used to customize kicad or kicad-unstable versions, including libraries. For instance, you could do something like:

final: prev:

{
  kicad-unstable = (prev.kicad-unstable.override {
    srcs = {
      "kicad-unstable" = {
        kicadVersion = {
          version = "2020-10-08";
          src = {
            rev = "fd22fe8e374ce71d57e9f683ba996651aa69fa4e";
            sha256 = "sha256-F8qugru/jU3DgZSpQXQhRGNFSk0ybFRkpyWb7HAGBdc=";
          };
        };
        libVersion = {
          version = "2020-08-22";
          libSources = {
            i18n.rev = "cbbb1efd940094bf0c3168280698b2b059a8c509";
            i18n.sha256 = "1q4jakn6m8smnr2mg7jgb520nrb6fag9mdvlcpx3smp3qbxka818";
            symbols.rev = "9ca6a5348cdeb88e699582d4ed051ff7303b44d3";
            symbols.sha256 = "13w6pb34rhz96rnar25z7kiscy6q1fm8l39hq1bpb8g9yn86ssz4";
            templates.rev = "ae16953b81055855bcede4a33305413599d86a15";
            templates.sha256 = "1pkv90p3liy3bj4nklxsvpzh9m56p0k5ldr22armvgqfaqaadx9v";
            footprints.rev = "f94c2d5d619d16033f69a555b449f59604d97865";
            footprints.sha256 = "1g71sk77jvqaf9xvgq6dkyvd9pij2lb4n0bn0dqnwddhwam935db";
            packages3d.rev = "f699b0e3c13fe75618086913e39279c85da14cc7";
            packages3d.sha256 = "0m5rb5axa946v729z35ga84in76y4zpk32qzi0hwqx957zy72hs9";
          };
        };
      };
    };
}

I've also reformatted the existing code. Let me know if the reformatting is too invasive and I'll revert.

doronbehar · 2020-10-09T08:14:58Z

@matthuszagh this change is somewhat simple enough so it's easy for me to press the button, but I think it would be nice if it was possible to use a different fork of a library / kicad itself, not only a different revision.

P.S the eval errors have nothing to do with you.

matthuszagh · 2020-10-09T14:49:59Z

I didn't consider that but I agree with you. Let me dig into this a little more.

matthuszagh · 2020-10-10T18:29:46Z

@doronbehar this now allows overriding with a full fetchFromGitLab, or whatever you want. Versions can also be overriden with kicadVersion or libVersion. For example, this is how I'm using it:

final: prev:

{
  kicad-unstable = (prev.kicad-unstable.override {
    srcs = {
      kicadVersion = "2020-10-08";
      kicad = prev.fetchFromGitLab {
        group = "kicad";
        owner = "code";
        repo = "kicad";
        rev = "fd22fe8e374ce71d57e9f683ba996651aa69fa4e";
        sha256 = "sha256-F8qugru/jU3DgZSpQXQhRGNFSk0ybFRkpyWb7HAGBdc=";
      };
    };
  });
}

Let me know what you think.

doronbehar

Besides these, here are general comments:

It'd be easier to review this if the indentation settings will be in a separate commit.
More comments to how the logic is going on inside default.nix regarding srcs.
Since there's no way to query a package's override options, it'd be nice to put a big comment at the start of default.nix with all the possible ways one can override srcs.

pkgs/applications/science/electronics/kicad/libraries.nix

pkgs/applications/science/electronics/kicad/i18n.nix

pkgs/applications/science/electronics/kicad/default.nix

doronbehar · 2020-10-10T20:09:25Z

Oh and 1 more thing: I'm rather sure this change shouldn't trigger rebuilds - ofborg detects it this way and in the "eval" check of the PR, there's a link to the list of changed outputs. Also the PR's labels ofborg sets reflects that.

matthuszagh · 2020-10-10T20:19:57Z

@doronbehar thanks for the review. I've applied the changes. Let me know.

doronbehar

The review is a bit messy sorry, but the expression is getting better and better :). Some suggestions are better of in separate commits.

pkgs/applications/science/electronics/kicad/base.nix

pkgs/applications/science/electronics/kicad/default.nix

pkgs/applications/science/electronics/kicad/base.nix

pkgs/applications/science/electronics/kicad/default.nix

matthuszagh · 2020-10-10T23:25:12Z

@doronbehar I hope I got everything (I think I did), but sorry in advance if I messed something up (got into a little trouble with all the rebasing :)).

Also let me know about those other two points.

doronbehar

Not many more to go. The commit messages look good overall. Only commit 239d16c should mention the rename of the *Support arguments to with*.

pkgs/applications/science/electronics/kicad/base.nix

pkgs/applications/science/electronics/kicad/default.nix

pkgs/applications/science/electronics/kicad/base.nix

pkgs/applications/science/electronics/kicad/default.nix

matthuszagh · 2020-10-11T13:21:53Z

I've implemented the changes. Let me know if there's anything else.

pkgs/applications/science/electronics/kicad/base.nix

doronbehar

Thanks for your cooperation. Only issues I see are in the assertions commit. It's almost perfect.

pkgs/applications/science/electronics/kicad/base.nix

pkgs/applications/science/electronics/kicad/default.nix

pkgs/applications/science/electronics/kicad/base.nix

matthuszagh · 2020-10-12T17:56:19Z

I've made the changes, let me know what you think @doronbehar.

doronbehar

Also, to be nice to everyone else, we should add release notes to rl-2103.xml (ideally in seperate commit from this one).

The commit message of this "rename options commit" should mention also the fact that the defaults are not needed in base.nix, although it doesn't affect users directly.

1 last round I promise!

pkgs/applications/science/electronics/kicad/default.nix

doronbehar · 2020-10-12T18:25:05Z

I know it ruins the git commit --fixup= workflow, but could you please mention in that commit the fact that base.nix doesn't need the defaults to be specified in it?

Also, we'll definitely need a commit that mentions these api changes in rl-2103.xml.

Also: Use assertions instead of silently ignoring arguments that don't cooperate (occ+oce) / won't compile (aarch64 + oce). base.nix no longer provides default argument values since these are provided by default.nix.

This also exposes the full src and version parameters for each derivation, allowing them to overrideable by srcs.

matthuszagh · 2020-10-12T18:33:45Z

Changes in.

doronbehar

Thanks for your contribution.

evils

sorry for such a late review
i missed it because i confused its mails for #85456
and it being merged just 3 days after being opened did not leave a lot of time to discover that mistake

evils · 2021-01-24T04:28:24Z

pkgs/applications/science/electronics/kicad/base.nix

-
-  # oce on aarch64 fails a test
-  withOCE = oceSupport && !stdenv.isAarch64;
-  withOCC = (withOCCT && !withOCE) || (oceSupport && stdenv.isAarch64);


it took me quite a long time figuring this out
can you prove that what you used instead is equivalent?

evils · 2021-01-24T04:37:39Z

pkgs/applications/science/electronics/kicad/default.nix

 }:

-assert ngspiceSupport -> libngspice != null;
+# The `srcs` parameter can be used to override the kicad source code


having never used overlays, i did not write this package with them in mind
my original intent was for the versions.nix being the single reference for all version info
i don't entirely get your changes, that's ok (but because of that i'd like you to be added to meta.maintainers)

but would it not be simpler to just provide kicad with a different versions.nix (with which i mean the nix content of that file) (or for example a way to merge updates to it so one does not have to provide a full replacement)?

right now i'm assuming that's not possible
or that i am missing a benefit to this more complicated approach

evils · 2021-01-24T04:45:23Z

pkgs/applications/science/electronics/kicad/libraries.nix

 let
  mkLib = name:
    stdenv.mkDerivation {
      pname = "kicad-${name}";
-      # Use the revision instead of `version` (which is an ISO 8601 date)
-      # to prevent duplicating the library when just the date changed
-      version = "${builtins.substring 0 10 libSources.${name}.rev}";


this was a rev substring for a very good reason
with it being the date (for kicad-unstable), the 3D models get downloaded and "built" again when only the date, but nothing else, changed
since this is a ~6gb download, i strongly believe it is worth avoiding if at all possible (which it is)

(i suspect this PR causes this to now be the date, but i can't actually tell...)

evils · 2021-01-24T05:01:22Z

pkgs/applications/science/electronics/kicad/default.nix


-  versions =  import ./versions.nix;
-  versionConfig = versions.${baseName};
+  libSrcFetch = name: fetchFromGitLab {


one of the goals of my rewrite was to simplify default.nix to be as much as possible just the wrapper
code related to building the components that get tied together here should be in base.nix or libraries.nix respectively

evils · 2021-01-24T05:02:43Z

pkgs/applications/science/electronics/kicad/libraries.nix

@@ -42,30 +28,4 @@ in
  templates = mkLib "templates";
  footprints = mkLib "footprints";
  packages3d = mkLib "packages3d";
-
-  # i18n is a special case, not actually a library


i think this description of i18n being optional, separate and only found via a relative path should have been retained

matthuszagh requested a review from evils October 9, 2020 02:04

matthuszagh mentioned this pull request Oct 9, 2020

Ability to override kicad-unstable version #100045

Closed

matthuszagh force-pushed the kicad-srcs branch from 332d1ea to 0465a38 Compare October 10, 2020 14:56

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 10, 2020

matthuszagh force-pushed the kicad-srcs branch from 0465a38 to c63ff9d Compare October 10, 2020 17:26

ofborg bot requested review from berce and Kiwi October 10, 2020 17:36

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 10, 2020

matthuszagh force-pushed the kicad-srcs branch from c63ff9d to 475cc05 Compare October 10, 2020 18:26

doronbehar reviewed Oct 10, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/libraries.nix Outdated Show resolved Hide resolved

pkgs/applications/science/electronics/kicad/i18n.nix Outdated Show resolved Hide resolved

pkgs/applications/science/electronics/kicad/default.nix Outdated Show resolved Hide resolved

kicad: reformat with nixpkgs-fmt

b3e2b52

matthuszagh force-pushed the kicad-srcs branch from 475cc05 to 8be221d Compare October 10, 2020 20:17

doronbehar reviewed Oct 10, 2020

View reviewed changes

matthuszagh added 2 commits October 10, 2020 15:15

kicad: clarify wrapGAppsHook comment

e2fea84

kicad: remove unnecessary with lib; statements

85d5195

matthuszagh force-pushed the kicad-srcs branch from 8be221d to edb81b7 Compare October 10, 2020 23:23

doronbehar reviewed Oct 11, 2020

View reviewed changes

matthuszagh force-pushed the kicad-srcs branch from edb81b7 to 173a122 Compare October 11, 2020 13:19

doronbehar reviewed Oct 11, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/base.nix Show resolved Hide resolved

doronbehar reviewed Oct 11, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/base.nix Outdated Show resolved Hide resolved

pkgs/applications/science/electronics/kicad/base.nix Outdated Show resolved Hide resolved

pkgs/applications/science/electronics/kicad/base.nix Outdated Show resolved Hide resolved

matthuszagh force-pushed the kicad-srcs branch from 173a122 to 72caaed Compare October 12, 2020 17:35

doronbehar reviewed Oct 12, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/default.nix Outdated Show resolved Hide resolved

doronbehar reviewed Oct 12, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/base.nix Show resolved Hide resolved

matthuszagh force-pushed the kicad-srcs branch from 72caaed to 4c2306b Compare October 12, 2020 17:55

doronbehar requested changes Oct 12, 2020

View reviewed changes

pkgs/applications/science/electronics/kicad/default.nix Outdated Show resolved Hide resolved

matthuszagh force-pushed the kicad-srcs branch from 4c2306b to aa9478e Compare October 12, 2020 18:21

matthuszagh added 3 commits October 12, 2020 11:32

kicad: Rename <feature>Support arguments to with<Feature>

9d13164

Also: Use assertions instead of silently ignoring arguments that don't cooperate (occ+oce) / won't compile (aarch64 + oce). base.nix no longer provides default argument values since these are provided by default.nix.

kicad: add srcs parameter to allow configuring kicad versions

f2cb2c4

This also exposes the full src and version parameters for each derivation, allowing them to overrideable by srcs.

add 21.03 release notes for renamed kicad options

b90776c

matthuszagh force-pushed the kicad-srcs branch from aa9478e to b90776c Compare October 12, 2020 18:33

ofborg bot added 6.topic: nixos 8.has: changelog 8.has: documentation labels Oct 12, 2020

doronbehar approved these changes Oct 12, 2020

View reviewed changes

doronbehar merged commit 58b8ae4 into NixOS:master Oct 12, 2020

matthuszagh deleted the kicad-srcs branch October 12, 2020 19:59

matthuszagh mentioned this pull request Oct 12, 2020

kicad: add option to override footprint, symbol and 3d model locations #85456

Merged

10 tasks

evils reviewed Jan 24, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kicad: add srcs parameter to allow configuring kicad versions #100066

kicad: add srcs parameter to allow configuring kicad versions #100066

matthuszagh commented Oct 9, 2020 •

edited

doronbehar commented Oct 9, 2020

matthuszagh commented Oct 9, 2020

matthuszagh commented Oct 10, 2020 •

edited

doronbehar left a comment

doronbehar commented Oct 10, 2020

matthuszagh commented Oct 10, 2020

doronbehar left a comment

matthuszagh commented Oct 10, 2020

doronbehar left a comment

matthuszagh commented Oct 11, 2020

doronbehar left a comment

matthuszagh commented Oct 12, 2020

doronbehar left a comment

doronbehar commented Oct 12, 2020

matthuszagh commented Oct 12, 2020

doronbehar left a comment

evils left a comment

evils Jan 24, 2021

evils Jan 24, 2021

evils Jan 24, 2021

evils Jan 24, 2021 •

edited

evils Jan 24, 2021

kicad: add srcs parameter to allow configuring kicad versions #100066

kicad: add srcs parameter to allow configuring kicad versions #100066

Conversation

matthuszagh commented Oct 9, 2020 • edited

Motivation for this change

Things done

doronbehar commented Oct 9, 2020

matthuszagh commented Oct 9, 2020

matthuszagh commented Oct 10, 2020 • edited

doronbehar left a comment

Choose a reason for hiding this comment

doronbehar commented Oct 10, 2020

matthuszagh commented Oct 10, 2020

doronbehar left a comment

Choose a reason for hiding this comment

matthuszagh commented Oct 10, 2020

doronbehar left a comment

Choose a reason for hiding this comment

matthuszagh commented Oct 11, 2020

doronbehar left a comment

Choose a reason for hiding this comment

matthuszagh commented Oct 12, 2020

doronbehar left a comment

Choose a reason for hiding this comment

doronbehar commented Oct 12, 2020

matthuszagh commented Oct 12, 2020

doronbehar left a comment

Choose a reason for hiding this comment

evils left a comment

Choose a reason for hiding this comment

evils Jan 24, 2021

Choose a reason for hiding this comment

evils Jan 24, 2021

Choose a reason for hiding this comment

evils Jan 24, 2021

Choose a reason for hiding this comment

evils Jan 24, 2021 • edited

Choose a reason for hiding this comment

evils Jan 24, 2021

Choose a reason for hiding this comment

matthuszagh commented Oct 9, 2020 •

edited

matthuszagh commented Oct 10, 2020 •

edited

evils Jan 24, 2021 •

edited