python3Packages.py-cpuinfo: add sysctl to runtime dependencies

[bobrik]

Motivation for this change

There is a test a new version that caught the issue.

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.

[bobrik]

Please consider merging #115221 at the same time.

[mweinelt]

Please consider merging #115221 at the same time.

Please consider merging them into them same branch, so we don't have to.

[mweinelt]

Also if this is a fix for the py-cpuinfo bump (0a0fb23) that is currently in staging-next, please retarget your change to staging-next.

[bobrik]

This PR fixes x86_64-darwin, the other one fixes aarch64-darwin. I put them in different PRs because they fix two different issues. I can put both commits in this PR if you still think it's better that way.

[bobrik]

Please retarget your change to staging-next.

What's the right way to do it? I can't use /rebase here and if I change the target branch, Github will try to pull all of the staging that isn't already in staging-next along with my commit.

[github-actions]

Failed to rebase

[mweinelt]

/rebase staging-next

[github-actions]

Failed to rebase

[mweinelt]

There is a conflict, so you have to do it by hand.

Please retarget your change to staging-next.

What's the right way to do it? I can't use /rebase here and if I change the target branch, Github will try to pull all of the staging that isn't already in staging-next along with my commit.

Rebase onto staging-next and git push --force-with-lease. At the same time the push happens, try to change the target branch of this pull request, so we don't unnecessarily notify too many people.

[github-actions]

Failed to rebase

[bobrik]

Rebased.

[mweinelt]

Curious, why do they need sysctl as a buildtime dependency?

[bobrik]

Tests depend on it.

[mweinelt]

That is odd, since you already added it to propagatedBuildInputs, which are available during the checkPhase. I'm not sure if sysctl even needs to be propagated. It would probably even be sufficient as buildInputs.

[bobrik]

Don't you need sysctl in propagatedBuildInputs if you want to use py-cpuinfo somewhere?

I've tried removing sysctl from nativeBuildInputs and the tests fail. It works with checkInputs so I amended to use that.

[mweinelt]

Uh yeah, I was mixing this up with something else. Having it in propagatedBuildInputs should be sufficient. It should be available during the checkPhase then.

[bobrik]

I must be holding it wrong, but it doesn't work with just propagatedBuildInputs on aarch64-darwin for me.

With this:

  # On Darwin sysctl is used to read CPU information.
  propagatedBuildInputs = lib.optionals stdenv.isDarwin [ sysctl ];

  preCheck = ''
    type -f sysctl
    exit 1
  '';

I see:

pytestCheckPhase
Executing pytestCheckPhase
/nix/store/l4ha6r368qyba7klmzgiyhbrxpcyqdc4-stdenv-darwin/setup: line 88: type: sysctl: not found

What am I missing?

[mweinelt]

Maybe we need to patch the path to sysctl in the python code.

[bobrik]

Updated with a somewhat ugly substituteInPlace, let me know if you have something better in mind.

[SuperSandro2000]

Rebase onto staging-next and git push --force-with-lease. At the same time the push happens, try to change the target branch of this pull request, so we don't unnecessarily notify too many people.

Are you searching for git rebase --onto=master @~1 ?

[mweinelt]

Rebase onto staging-next and git push --force-with-lease. At the same time the push happens, try to change the target branch of this pull request, so we don't unnecessarily notify too many people.

Are you searching for git rebase --onto=master @~1 ?

Likely, yeah.

[SuperSandro2000]

When propagating checkInputs shouldn't require sysctl but I tested it and the test fails. Maybe we need to hardcode the path or wrap python to set the path right?

[siraben]

Result of nixpkgs-review pr 115220 run on x86_64-darwin 1

1 package built:

• python3Packages.py-cpuinfo

[siraben]

[nix-shell:~/.../nixpkgs-review/pr-115220-1]$ python
Python 3.8.8 (default, Feb 21 2021, 06:37:46) 
[Clang 7.1.0 (tags/RELEASE_710/final)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from cpuinfo import get_cpu_info
>>> 
>>> for key, value in get_cpu_info().items():
...         print("{0}: {1}".format(key, value))
... 
python_version: 3.8.8.final.0 (64 bit)
cpuinfo_version: [7, 0, 0]
cpuinfo_version_string: 7.0.0
arch: X86_64
bits: 64
count: 4
arch_string_raw: x86_64
vendor_id_raw: GenuineIntel
brand_raw: Intel(R) Core(TM) i5-4288U CPU @ 2.60GHz
hz_advertised_friendly: 2.6000 GHz
hz_actual_friendly: 2.6000 GHz
hz_advertised: [2600000000, 0]
hz_actual: [2600000000, 0]
l2_cache_size: 262144
stepping: 1
model: 69
family: 6
flags: ['1gbpage', 'abm', 'acpi', 'aes', 'apic', 'avx', 'avx1.0', 'avx2', 'bmi1', 'bmi2', 'clflush', 'clfsh', 'cmov', 'cx16', 'cx8', 'de', 'ds', 'ds_cpl', 'dscpl', 'dtes64', 'dts', 'em64t', 'erms', 'est', 'f16c', 'fma', 'fpu', 'fpu_csds', 'fxsr', 'ht', 'htt', 'ibrs', 'invpcid', 'l1df', 'lahf', 'lahf_lm', 'lzcnt', 'mca', 'mce', 'mdclear', 'mmx', 'mon', 'monitor', 'movbe', 'msr', 'mtrr', 'osxsave', 'pae', 'pat', 'pbe', 'pcid', 'pclmulqdq', 'pdcm', 'pge', 'pni', 'popcnt', 'pse', 'pse36', 'rdrand', 'rdrnd', 'rdtscp', 'rdwrfsgs', 'seglim64', 'sep', 'smep', 'ss', 'ssbd', 'sse', 'sse2', 'sse3', 'sse4.1', 'sse4.2', 'sse4_1', 'sse4_2', 'ssse3', 'stibp', 'syscall', 'tm', 'tm2', 'tpr', 'tsc', 'tsc_thread_offset', 'tscdeadline', 'tsci', 'tsctmr', 'vme', 'vmx', 'x2apic', 'xd', 'xsave', 'xtpr']
l2_cache_line_size: 256
l2_cache_associativity: 6

[SuperSandro2000]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 115220 run on x86_64-darwin 1

3 packages built:

• python37Packages.py-cpuinfo
• python38Packages.py-cpuinfo
• python39Packages.py-cpuinfo