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

Conversation

zaninime
Copy link
Contributor

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.

Comment on lines +30 to +36
dontWrapGApps = true;
dontWrapQtApps = true;

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

@AndersonTorres
Copy link
Member

@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

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

@AndersonTorres
Copy link
Member

@zaninime

Just being a bit pedantic: a clearer commit message.

pythonPackages.scipy_1_4: nanovna-saver dependency

@zaninime
Copy link
Contributor Author

@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
@AndersonTorres
Copy link
Member

@zaninime Thanks!

Comment on lines +6392 to +6400
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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

fixed in #127910

@zaninime zaninime deleted the nanovna-saver branch July 30, 2022 16:37
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

4 participants