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

pythonPackages.ansible-lint: 5.0.2 -> 5.0.8 #122442

Merged
merged 5 commits into from May 14, 2021

Conversation

sengaya
Copy link
Contributor

@sengaya sengaya commented May 10, 2021

Motivation for this change

Various bug fixes and minor changes, see https://github.com/ansible-community/ansible-lint/releases/tag/v5.0.8

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.

@r-rmcgibbo
Copy link

r-rmcgibbo commented May 10, 2021

Result of nixpkgs-review pr 122442 at fce9671 run on x86_64-linux 1

2 packages failed to build:

Note that build failures may predate this PR, and could be nondeterministic or hardware dependent.
Please exercise your independent judgement. Does something look off? Please file an issue or reach out on IRC.


Result of nixpkgs-review pr 122442 at fce9671 run on aarch64-linux 1

2 packages failed to build:

Note that build failures may predate this PR, and could be nondeterministic or hardware dependent.
Please exercise your independent judgement. Does something look off? Please file an issue or reach out on IRC.

@mweinelt
Copy link
Member

@GrahamcOfBorg build ansible-lint

@mweinelt
Copy link
Member

mweinelt commented May 10, 2021

Executing pytestCheckPhase
patching script interpreter paths in src/ansiblelint/__main__.py
src/ansiblelint/__main__.py: interpreter directive changed from "#!/usr/bin/env python" to "/nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python"
ERROR: usage: __main__.py [options] [file_or_dir] [file_or_dir] [...]
__main__.py: error: unrecognized arguments: --no-success-flaky-report
  inifile: /build/ansible-lint-5.0.8/pytest.ini
  rootdir: /build/ansible-lint-5.0.8

builder for '/nix/store/64lccdvjdlg78iafybg4j6184isan3xa-python3.8-ansible-lint-5.0.8.drv' failed with exit code 4

Missing flaky in checkInputs: https://github.com/ansible-community/ansible-lint/blob/4de1b71cc3335672105474a7300b7e294df01983/pytest.ini#L10-L11

As you are on darwin the tests are disabled over there, maybe check if that is still necessary?

@mweinelt
Copy link
Member

mweinelt commented May 10, 2021

Also yamllint should be added to propagatedBuildInputs, it is optional, but also part of the test suite.

https://github.com/ansible-community/ansible-lint/blob/58f57e3d549da918842b70326704354f51b50bb3/src/ansiblelint/rules/YamllintRule.py#L16-L25

Also packaging dependeny is missing: https://github.com/ansible-community/ansible-lint/blob/v5.0.8/setup.cfg#L75

And even then I get two test failures:

______________________________ test_custom_kinds _______________________________
[gw1] linux -- Python 3.8.9 /nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python3.8

    def test_custom_kinds():
        """Check if user defined kinds are used."""
        result = run_ansible_lint('-vv', '--offline', 'examples/other/')
        assert result.returncode == 0
        # .yaml-too is not a recognized extension and unless is manually defined
        # in our .ansible-lint config, the test would not identify it as yaml file.
>       assert "Examining examples/other/some.yaml-too of type yaml" in result.stderr
E       AssertionError: assert 'Examining examples/other/some.yaml-too of type yaml' in 'INFO     Added ANSIBLE_LIBRARY=:plugins/modules:./.cache/modules\nINFO     Added ANSIBLE_COLLECTIONS_PATHS=:./.cache/...amllint config.\nINFO     Discovering files to lint: git ls-files -z\nDEBUG    Examining examples/other of type role\n'
E        +  where 'INFO     Added ANSIBLE_LIBRARY=:plugins/modules:./.cache/modules\nINFO     Added ANSIBLE_COLLECTIONS_PATHS=:./.cache/...amllint config.\nINFO     Discovering files to lint: git ls-files -z\nDEBUG    Examining examples/other of type role\n' = CompletedProcess(args=['/nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python3.8', '-m', 'ansiblelint',...mllint config.\nINFO     Discovering files to lint: git ls-files -z\nDEBUG    Examining examples/other of type role\n').stderr

result     = CompletedProcess(args=['/nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python3.8', '-m', 'ansiblelint',...mllint config.\nINFO     Discovering files to lint: git ls-files -z\nDEBUG    Examining examples/other of type role\n')

test/TestExamples.py:66: AssertionError
_____________________________ test_rule_no_handler _____________________________
[gw10] linux -- Python 3.8.9 /nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python3.8

    def test_rule_no_handler() -> None:
        """Verify rule."""
        collection = RulesCollection()
        collection.register(UseHandlerRatherThanWhenChangedRule())

        lintable = Lintable('examples/playbooks/rule-no-handler.yml')
        results = Runner(lintable, rules=collection).run()

>       assert len(results) == 3
E       assert 1 == 3
E         +1
E         -3

collection = internal-error: Unexpected internal error
  This error can be caused by internal bugs but also by custom rules. Instea...ove that task to handlers.
parser-error: AnsibleParserError
  Ansible parser fails, this usually indicate invalid file.
lintable   = examples/playbooks/rule-no-handler.yml (playbook)
results    = [[internal-error] (the role 'a-role' was not found in /build/ansible-lint-5.0.8/examples/playbooks/roles:/build/tmp.xb...blem.

The offending line appears to be:

  roles:
    - {role: a-role}  # 1st from within included role
      ^ here
]

@sengaya
Copy link
Contributor Author

sengaya commented May 10, 2021

@mweinelt thanks, I didn't take into account that tests are disabled on darwin. I'll double check if that is still needed and in doubt will test on Linux.

- Add missing test dependencies
- Enable tests on darwin again
- Adapt list of disabled tests
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

Test are working now, here are a few improvement ideas, if you're interested.

- Enabled and fixed multiple previously disabled tests
- Removed unneeded dependencies stdenv and git
- Sorted lists
- Parallelized the test suite
@sengaya
Copy link
Contributor Author

sengaya commented May 11, 2021

Many thanks for the suggestions @mweinelt. I also looked closer into the failing tests and could resolve all but two that requires network. I think I found two upstream bugs, but for now I fixed them with postPatch.

--replace 'os.path.join(root, name)' 'os.path.normpath(os.path.join(root, name))'
# fixes test_custom_kinds
substituteInPlace src/ansiblelint/file_utils.py \
--replace "if name.endswith('.yaml') or name.endswith('.yml')" ""
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 open up issues upstream and reference them here? I think that is all that is left to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll do that

@mweinelt mweinelt added this to the 21.05 milestone May 12, 2021
@mweinelt
Copy link
Member

FYI: I'm currently updating ansible to 2.10 over in #122670, if you are interested in trying that out! 😉

@sengaya
Copy link
Contributor Author

sengaya commented May 13, 2021

@mweinelt Thanks again for the feedback. I noticed one thing since a couple of versions and wonder if you might have an idea (sorry, a bit off-topic now):
During the build it says:

Created wheel for ansible-lint: filename=ansible_lint-0.0.0-py3-none-any.whl

which somehow results in

$ ansible-lint --version
ansible-lint 0.0.0 using ansible 2.9.12

Not really a big deal but somehow annoying. I tried to dig into it a bit but I did not understand where the 0.0.0 is coming from, especially as in other places the correct 5.0.8 is used.

@mweinelt mweinelt merged commit 7cd876f into NixOS:master May 14, 2021
@mweinelt
Copy link
Member

Whoops. Sorry, I only looked at the latest change and pressed merge. Looking into your question now.

@mweinelt
Copy link
Member

The reason why the wheel is created and why it has no proper version is that the nativeBuildInputs are missing setuptools-scm.

https://github.com/ansible-community/ansible-lint/blob/v5.0.8/pyproject.toml#L1-L8

Also for some reason I missed python in buildInputs, that looks unnecessary.

diff --git a/pkgs/development/python-modules/ansible-lint/default.nix b/pkgs/development/python-modules/ansible-lint/default.nix
index c6d66bc5315..d0df1119baf 100644
--- a/pkgs/development/python-modules/ansible-lint/default.nix
+++ b/pkgs/development/python-modules/ansible-lint/default.nix
@@ -2,7 +2,7 @@
 , buildPythonPackage
 , isPy27
 , fetchPypi
-, python
+, setuptools-scm
 , ansible
 , enrich
 , flaky
@@ -27,7 +27,9 @@ buildPythonPackage rec {
     sha256 = "sha256-tnuWKEB66bwVuwu3H3mHG99ZP+/msGhMDMRL5fyQgD8=";
   };
 
-  buildInputs = [ python ];
+  nativeBuildInputs = [
+    setuptools-scm
+  ];
 
   propagatedBuildInputs = [
     ansible

which results in this:

❯ ./result/bin/ansible-lint --version
ansible-lint 5.0.8 using ansible 2.9.21

@sengaya
Copy link
Contributor Author

sengaya commented May 14, 2021

Thanks, I'll apply that on the next update.

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

3 participants