checkov: init at 1.0.674 #107028

anhdle14 · 2020-12-16T13:44:20Z

Motivation for this change

Things done

symphorien · 2020-12-16T17:18:42Z

Is there a reason to not put the python dependencies in pkgs/development/python-modules and pkgs/top-level/python-packages.nix ?

anhdle14 · 2020-12-16T22:11:23Z

The 4 I put here is the only version that worked with the build. I tried to not override, but the build will fail with the versions in python-packages.nix.

symphorien · 2020-12-17T19:29:14Z

I don't know what the policy is about duplicating python packages in full vs overriding, cc @jonringer ?

deep_merge and bc-python-hcl2 are not in nixpkgs, so they should go with other python modules.

In any case, you should try to run tests. If you leave doCheck = false, please also leave a comment explaining why it's not possible to run the tests.

jonringer · 2020-12-17T19:42:08Z

The more generic python packages should be added to python-modules.nix.

If specific version is necessary, then I would prefer to do an override specific to the application as so:

let
  py = python.override {
    packageOverrides = self: super: {
      aws-sam-translator = super.aws-sam-translator.overridePythonAttrs (oldAttrs: rec {
        version = "1.25.0";
        src = oldAttrs.src.override {
          inherit version;
          sha256 = "08756yl5lpqgrpr80f2b6bdcgygr37l6q1yygklcg9hz4yfpccav";
        };
      });

      flask = super.flask.overridePythonAttrs (oldAttrs: rec {
        version = "1.0.2";
        src = oldAttrs.src.override {
          inherit version;
          sha256 = "0j6f4a9rpfh25k1gp7azqhnni4mb4fgy50jammgjgddw1l3w0w92";
        };
      });

      cookiecutter = super.cookiecutter.overridePythonAttrs (oldAttrs: rec {
        version = "1.6.0";
        src = oldAttrs.src.override {
          inherit version;
          sha256 = "0glsvaz8igi2wy1hsnhm9fkn6560vdvdixzvkq6dn20z3hpaa5hk";
        };
      });
    };
  };

in

with py.pkgs;

buildPythonApplication rec {
   ....

Overall stance on pinning python packages:

The preference to handling pinning is to relax version bounds in the "install_requires" field. (could be in setup.py, pyproject.toml, requirements or others). In most cases, packages are still compatible with small API changes which may warrant a major version bump. We use test suites to verify that the package still works correctly.

If the package is still incompatible with the latest major version, then the most proper way to handle this is make an issue with the upstream package to adopt the latest major version. Or if upstream is not very responsive, you are free patch the source to make it compatible.

In very few circumstances, two versions of the same package are allowed to exist if the packages are extremely difficult to package. Some examples of this are tensorflow, which has huge ecosystems built around it and is hard to package. Another is django, which has 2 actively developed versions, and large ecosystems built around each.

One exception to this is applications, due to the way buildPythonApplication and toPythonApplication functions work, the related derivations will not bleed dependencies between packages. If the package doesn't need to be imported by other python modules, then this package would be a good candidate to convert into application. You can look at https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/admin/awscli/default.nix as an example of using an overlay within a python application.

Info on buildPythonApplication can be found here.
Info on toPythonApplication can be found here.

anhdle14 · 2020-12-23T05:15:55Z

@jonringer I updated based on guideline. Only dpath I kept within checkov/default.nix because dpath 2.0 will crash. Should I write dpath/1.5.nix fo it?

pkgs/development/python-modules/bc-python-hcl2/default.nix

SuperSandro2000 · 2020-12-25T23:22:07Z

pkgs/development/python-modules/deep_merge/default.nix

+    sha256 = "tUQV+Qk0xC4zQRTihky01OczWzStOW41rYYQyWBlpH4=";
+  };
+
+  propagatedBuildInputs = [


Isn't this just required for the tests?

pkgs/development/python-modules/bc-python-hcl2/default.nix

SuperSandro2000 · 2020-12-25T23:22:40Z

pkgs/development/python-modules/deep_merge/default.nix

+    nose
+  ];
+
+  doCheck = false;


Please add a pythonImportsCheck.

When added pythonImportsCheck it is always failing:

deep_merge.tests.test_merge.test_default_behaviors ... ok deep_merge.tests.test_merge.test_big_merge ... FAIL deep_merge.tests.test_merge.test_no_references Ensure that read-only parameters aren't being modified. ... FAIL ====================================================================== FAIL: deep_merge.tests.test_merge.test_big_merge ---------------------------------------------------------------------- Traceback (most recent call last): File "/nix/store/g80x8dysb6hnhrcjrpav7dmsb8jnwnyr-python3.8-nose-1.3.7/lib/python3.8/site-packages/nose/case.py", line 197, in runTest self.test(*self.arg) File "/private/tmp/nix-build-python3.8-deep_merge-0.0.4.drv-0/deep_merge-0.0.4/deep_merge/tests/test_merge.py", line 60, in test_big_merge assert merge(a, b, c) == expected AssertionError ====================================================================== FAIL: deep_merge.tests.test_merge.test_no_references Ensure that read-only parameters aren't being modified. ---------------------------------------------------------------------- Traceback (most recent call last): File "/nix/store/g80x8dysb6hnhrcjrpav7dmsb8jnwnyr-python3.8-nose-1.3.7/lib/python3.8/site-packages/nose/case.py", line 197, in runTest self.test(*self.arg) File "/private/tmp/nix-build-python3.8-deep_merge-0.0.4.drv-0/deep_merge-0.0.4/deep_merge/tests/test_merge.py", line 73, in test_no_references assert output == {"key1": {"key1a": "2"}, "key2": [3, 4, 4, 5]} AssertionError ---------------------------------------------------------------------- Ran 3 tests in 0.013s FAILED (failures=2) Test failed: <unittest.runner.TextTestResult run=3 errors=0 failures=2> error: Test failed: <unittest.runner.TextTestResult run=3 errors=0 failures=2> error: --- Error ------------------------------------------------------------------------------------------------------------------------ nix-build builder for '/nix/store/41dyki3vl4qp53vd6kl92lg1yrn2w797-python3.8-deep_merge-0.0.4.drv' failed with exit code 1; last 10 log lines: File "/private/tmp/nix-build-python3.8-deep_merge-0.0.4.drv-0/deep_merge-0.0.4/deep_merge/tests/test_merge.py", line 73, in test_no_references assert output == {"key1": {"key1a": "2"}, "key2": [3, 4, 4, 5]} AssertionError ---------------------------------------------------------------------- Ran 3 tests in 0.013s FAILED (failures=2) Test failed: <unittest.runner.TextTestResult run=3 errors=0 failures=2> error: Test failed: <unittest.runner.TextTestResult run=3 errors=0 failures=2>

Hum, that does not look like a spurious failure. Can you open an issue upstream to ensure that it's not nixpkgs-specific?

That is a 3 years old repo... But I will try.

pkgs/top-level/all-packages.nix

pkgs/development/tools/analysis/checkov/default.nix

SuperSandro2000 · 2020-12-26T11:20:35Z

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

7 packages built:

checkov
python37Packages.bc-python-hcl2
python37Packages.deep_merge
python38Packages.bc-python-hcl2
python38Packages.deep_merge
python39Packages.bc-python-hcl2
python39Packages.deep_merge

SuperSandro2000 · 2020-12-26T11:24:13Z

Result of nixpkgs-review pr 107028 run on x86_64-linux 1

7 packages built:

checkov
python37Packages.bc-python-hcl2
python37Packages.deep_merge
python38Packages.bc-python-hcl2
python38Packages.deep_merge
python39Packages.bc-python-hcl2
python39Packages.deep_merge

anhdle14 · 2020-12-30T11:29:36Z

@symphorien @SuperSandro2000 I will fix all the commits one the changes are all agreed.

I want to ask on another part when moving propagatedBuildInputs to buildInputs.

This is the problem of moving lark-parser 0.7.8 from propagatedBuildInputs to buildInputs in bc-python-hcl2/default.nix.

Both checkov and bc-python-hcl2 share the lark-parser requirements and they have to be the same version to build successfully.
After testing, I think if switch to buildInputs I may need to patch it twice (checkov/default.nix, and bc-python-hcl2/default.nix). So I think it is best to include a lark-parser pinned version at 0.7.8 or just use propagatedBuildInputs.

I need your comments on the findings. Thank you!

pkgs/development/python-modules/bc-python-hcl2/default.nix

pkgs/development/python-modules/deep_merge/default.nix

symphorien · 2020-12-31T12:48:42Z

leaving runtime python dependencies in propagatedBuildInputs is fine.

Co-authored-by: Guillaume Girol <symphorien@users.noreply.github.com>

pkgs/development/tools/analysis/checkov/default.nix

maintainers: add anhdle14

b5fd71f

ofborg bot added 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 16, 2020

anhdle14 requested review from FRidh and jonringer as code owners December 23, 2020 05:11

ofborg bot added the 6.topic: python label Dec 23, 2020

ofborg bot removed 10.rebuild-darwin: 1 10.rebuild-linux: 1 labels Dec 23, 2020

SuperSandro2000 reviewed Dec 25, 2020

View reviewed changes

symphorien reviewed Dec 31, 2020

View reviewed changes

pkgs/development/python-modules/bc-python-hcl2/default.nix Outdated Show resolved Hide resolved

symphorien reviewed Dec 31, 2020

View reviewed changes

pkgs/development/python-modules/deep_merge/default.nix Outdated Show resolved Hide resolved

anhdle14 and others added 2 commits January 3, 2021 19:37

bc-python-hcl2: init at 0.3.11

90b522e

Co-authored-by: Guillaume Girol <symphorien@users.noreply.github.com>

deep_merge: init at 0.0.4

7dc9da6

Co-authored-by: Guillaume Girol <symphorien@users.noreply.github.com>

symphorien reviewed Jan 3, 2021

View reviewed changes

pkgs/development/tools/analysis/checkov/default.nix Outdated Show resolved Hide resolved

checkov: init at 1.0.674

a0a4feb

symphorien merged commit afd7b55 into NixOS:master Jan 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

checkov: init at 1.0.674 #107028

checkov: init at 1.0.674 #107028

anhdle14 commented Dec 16, 2020

symphorien commented Dec 16, 2020

anhdle14 commented Dec 16, 2020 •

edited

symphorien commented Dec 17, 2020

jonringer commented Dec 17, 2020

anhdle14 commented Dec 23, 2020

SuperSandro2000 Dec 25, 2020

SuperSandro2000 Dec 25, 2020

anhdle14 Dec 28, 2020

symphorien Dec 28, 2020

anhdle14 Dec 30, 2020

SuperSandro2000 commented Dec 26, 2020

SuperSandro2000 commented Dec 26, 2020

anhdle14 commented Dec 30, 2020

symphorien commented Dec 31, 2020

checkov: init at 1.0.674 #107028

checkov: init at 1.0.674 #107028

Conversation

anhdle14 commented Dec 16, 2020

Motivation for this change

Things done

symphorien commented Dec 16, 2020

anhdle14 commented Dec 16, 2020 • edited

symphorien commented Dec 17, 2020

jonringer commented Dec 17, 2020

anhdle14 commented Dec 23, 2020

SuperSandro2000 Dec 25, 2020

Choose a reason for hiding this comment

SuperSandro2000 Dec 25, 2020

Choose a reason for hiding this comment

anhdle14 Dec 28, 2020

Choose a reason for hiding this comment

symphorien Dec 28, 2020

Choose a reason for hiding this comment

anhdle14 Dec 30, 2020

Choose a reason for hiding this comment

SuperSandro2000 commented Dec 26, 2020

SuperSandro2000 commented Dec 26, 2020

anhdle14 commented Dec 30, 2020

symphorien commented Dec 31, 2020

anhdle14 commented Dec 16, 2020 •

edited