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

mlc: init at 3.9 #102670

Merged
merged 2 commits into from Nov 4, 2020
Merged

mlc: init at 3.9 #102670

merged 2 commits into from Nov 4, 2020

Conversation

basvandijk
Copy link
Member

Motivation for this change

https://software.intel.com/content/www/us/en/develop/articles/intelr-memory-latency-checker.html

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.


nativeBuildInputs = [ patchelf ];

phases = [ "unpackPhase" "installPhase" "patchPhase" ];
Copy link
Member

Choose a reason for hiding this comment

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

Generally you shouldn't overwrite phases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, done.

Comment on lines 19 to 20
cp mlc $out/bin/mlc
chmod +x $out/bin/mlc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cp mlc $out/bin/mlc
chmod +x $out/bin/mlc
install -Dm755 mlc $out/bin/

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I also removed the mkdir $out/bin since that's handled by install as well.

@@ -0,0 +1,34 @@
{ stdenv, fetchurl, patchelf, lib }:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{ stdenv, fetchurl, patchelf, lib }:
{ stdenv, fetchurl, patchelf }:

You can use with stdenv;.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 27 to 33
meta = {
homepage = "https://software.intel.com/content/www/us/en/develop/articles/intelr-memory-latency-checker.html";
description = "Intel Memory Latency Checker";
license = lib.licenses.unfree;
maintainers = with lib.maintainers; [ basvandijk ];
platforms = with lib.platforms; linux;
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
meta = {
homepage = "https://software.intel.com/content/www/us/en/develop/articles/intelr-memory-latency-checker.html";
description = "Intel Memory Latency Checker";
license = lib.licenses.unfree;
maintainers = with lib.maintainers; [ basvandijk ];
platforms = with lib.platforms; linux;
};
meta = with stdenv.lib; {
homepage = "https://software.intel.com/content/www/us/en/develop/articles/intelr-memory-latency-checker.html";
description = "Intel Memory Latency Checker";
license = licenses.unfree;
maintainers = maintainers; [ basvandijk ];
platforms = with platforms; linux;
};

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks.

@basvandijk basvandijk marked this pull request as ready for review November 4, 2020 09:26
@basvandijk
Copy link
Member Author

@GrahamcOfBorg build mlc

@basvandijk basvandijk merged commit a93c376 into NixOS:master Nov 4, 2020
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

2 participants