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.

Sign up for GitHub

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

Jump to bottom

pcm: init at 201710 #36928

Merged
merged 2 commits into from Mar 15, 2018
Merged

pcm: init at 201710 #36928

merged 2 commits into from Mar 15, 2018

Conversation

roosemberth
Copy link
Contributor

Signed-off-by: Roosembert Palacios roosembert.palacios@epfl.ch

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

nlewo
nlewo requested changes Mar 14, 2018
View reviewed changes
pkgs/os-specific/linux/pcm/default.nix Outdated
sha256 = "02rq8739zwwbfrhagvcgf6qpmnswxl9b0qsld26rg6zp91v2npbj";
};

nativeBuildInputs = [ ];
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this please?

pkgs/os-specific/linux/pcm/default.nix Outdated
};

nativeBuildInputs = [ ];
buildInputs = [ ];
Copy link
Member

Choose a reason for hiding this comment

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

Same here

pkgs/os-specific/linux/pcm/default.nix Outdated
nativeBuildInputs = [ ];
buildInputs = [ ];

buildPhase = ''
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the default behavior of the build phase. So you could also remove this.

pkgs/os-specific/linux/pcm/default.nix Outdated
description = "Processor counter monitor";
homepage = https://www.intel.com/software/pcm;
license = licenses.bsd3;
platforms = 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.

Would you like to become the maintainer of this package? In this case, could you add yourself in the maintainers list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@GrahamcOfBorg GrahamcOfBorg added 6.topic: xfce The Xfce Desktop Environment 8.has: documentation labels Mar 14, 2018
@roosemberth
Copy link
Contributor Author

Why is there a xfce label in this PR?

@nlewo
Copy link
Member

nlewo commented Mar 14, 2018

@roosemberth you pushed commits that are not yours:) I don't know exactly what you did. But it sounds like a wrong rebasing.
I think this explain why this PR has a xfce label.

Add name to maintainers' list
e955391 
Signed-off-by: Roosembert Palacios <roosembert.palacios@epfl.ch>
@roosemberth
Copy link
Contributor Author

My bad, properly rebased.

@GrahamcOfBorg GrahamcOfBorg removed 6.topic: xfce The Xfce Desktop Environment 8.has: documentation labels Mar 14, 2018
@nlewo
Copy link
Member

nlewo commented Mar 14, 2018

@GrahamcOfBorg build pcm

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: pcm

Partial log (click to expand)

shrinking /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin/pcm-sensor.x
shrinking /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin/pcm.x
shrinking /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin/pcm-power.x
shrinking /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin/pcm-iio.x
shrinking /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin/pcm-memory.x
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710/bin 
patching script interpreter paths in /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710
checking for references to /tmp/nix-build-pcm-201710.drv-0 in /nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710...
/nix/store/cqsi9jbsins0s9g9w88h8796g5xdwpv7-pcm-201710

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: pcm

Partial log (click to expand)

In file included from cpucounters.cpp:32:0:
cpucounters.h: In member function 'uint64 PCM::getTickCountRDTSCP(uint64)':
cpucounters.h:1542:62: error: unknown register name '%rdx' in 'asm'
            "=r" (high), "=r" (low) :: "%rax", "%rcx", "%rdx");
                                                              ^
cpucounters.h:1542:62: error: unknown register name '%rcx' in 'asm'
cpucounters.h:1542:62: error: unknown register name '%rax' in 'asm'
make: *** [Makefile:56: cpucounters.o] Error 1
builder for '/nix/store/kb9yb4r4xpkbr6xdzps15xiq8q49nj5a-pcm-201710.drv' failed with exit code 2
�[31;1merror:�[0m build of '/nix/store/kb9yb4r4xpkbr6xdzps15xiq8q49nj5a-pcm-201710.drv' failed

nlewo
nlewo reviewed Mar 14, 2018
View reviewed changes
pkgs/os-specific/linux/pcm/default.nix Outdated
@@ -0,0 +1,26 @@
{ stdenv, callPackage, fetchFromGitHub }:
Copy link
Member

Choose a reason for hiding this comment

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

callPackage argument is useless.

@nlewo
Copy link
Member

nlewo commented Mar 14, 2018

Could you please either fix the build for aarch64 or restrict this package to x86 only?

@roosemberth
Copy link
Contributor Author

roosemberth commented Mar 14, 2018 •
edited

FYI, it's expected it won't build in other architectures than x86_64, it's an intel cpu tool

EDIT: Ninja'd, how can I restrict to x86?

pcm: init at 201710
3328709 
Signed-off-by: Roosembert Palacios <roosembert.palacios@epfl.ch>
@roosemberth
Copy link
Contributor Author

Fixed: restrict arch and remove useless argument

@nlewo
Copy link
Member

nlewo commented Mar 15, 2018 •
edited

ahh, i'm feeling a bit stupid regarding aarch64 :)
Thanks for your contrib!

@nlewo nlewo merged commit a3205ea into NixOS:master Mar 15, 2018
@roosemberth roosemberth deleted the pcm branch March 15, 2018 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlewo nlewo nlewo requested changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@roosemberth @nlewo @GrahamcOfBorg