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

nanovna-saver: init at 0.3.7 #101244

Merged
merged 2 commits into from Oct 22, 2020
Merged

nanovna-saver: init at 0.3.7 #101244

merged 2 commits into from Oct 22, 2020

Conversation

@zaninime
Copy link
Contributor

@zaninime zaninime commented Oct 21, 2020

Motivation for this change

Add package nanovna-saver. It depends on scipy 1.4, so I needed to add the dependency in pythonPackages. I'm not sure if that needed a separate PR.

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 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.
dontWrapGApps = true;
dontWrapQtApps = true;

postFixup = ''
wrapProgram $out/bin/NanoVNASaver \
"''${gappsWrapperArgs[@]}" \
"''${qtWrapperArgs[@]}"
Copy link
Member

@SuperSandro2000 SuperSandro2000 Oct 21, 2020

Can you add a comment why this is necessary? For me this is not logical.

Copy link
Contributor Author

@zaninime zaninime Oct 21, 2020

I actually thought the same. But without it, the executable doesn't start and fails with the dreaded qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in "". I'm not sure how else this can be solved.

Copy link
Member

@AndersonTorres AndersonTorres Oct 21, 2020

Looks fine to me. This is a very common pattern.

@AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Oct 21, 2020

@zaninime please create a commit for every different package/expression you deem needed.

In this particular case, a commit for scipy and another for nanovna-saver.

It helps in case of future updates and cherry-picks.

@zaninime
Copy link
Contributor Author

@zaninime zaninime commented Oct 22, 2020

@AndersonTorres @SuperSandro2000 anything else that should be done here? I think I addressed all the issues raised so far.

@AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Oct 22, 2020

@zaninime

Just being a bit pedantic: a clearer commit message.

pythonPackages.scipy_1_4: nanovna-saver dependency

@zaninime
Copy link
Contributor Author

@zaninime zaninime commented Oct 22, 2020

@AndersonTorres fixed, the previous one was inspired by the message left on scipy_1_3.

@AndersonTorres AndersonTorres merged commit 253fb76 into NixOS:master Oct 22, 2020
19 checks passed
@AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Oct 22, 2020

@zaninime Thanks!

scipy_1_4 = self.scipy.overridePythonAttrs (oldAttrs: rec {
version = "1.4.1";
src = oldAttrs.src.override {
inherit version;
sha256 = "0ndw7zyxd2dj37775mc75zm4fcyiipnqxclc45mkpxy8lvrvpqfy";
};
doCheck = false;
disabled = !isPy3k;
});
Copy link
Member

@dotlambda dotlambda Jun 22, 2021

Versioned attributes in python-packages.nix are not allowed! You have to use packageOverrides instead. I will have to remove nanovna-saver as well if this is not rectified promptly.

Copy link
Member

@dotlambda dotlambda Jun 23, 2021

fixed in #127910

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.

None yet

4 participants