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

python310Packages.yaramod: init at 3.12.2 #211504

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

msm-code
Copy link
Contributor

@msm-code msm-code commented Jan 19, 2023

Description of changes

YARA is a language used for creating signatures, especially for malware.
Yaramod is a library created by avast, for parsing and working with yara rules (signatures).
The main project page is https://github.com/avast/yaramod

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@fabaff
Copy link
Member

fabaff commented Jan 21, 2023

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

1 package failed to build:
  • python311Packages.yaramod
1 package built:
  • python310Packages.yaramod
error: builder for '/nix/store/x06rihw9jcirx0w8qvawaqybamqjnfqw-python3.11-yaramod-3.12.2.drv' failed with exit code 1;
       last 10 log lines:
       >   File "/nix/store/i6fyrqgmmix4ysq5gpi277sz675kj5gz-python3.11-setuptools-65.6.3/lib/python3.11/site-packages/setuptools/dist.py", line 1208, in run_command
       >     super().run_command(command)
       >   File "/nix/store/i6fyrqgmmix4ysq5gpi277sz675kj5gz-python3.11-setuptools-65.6.3/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
       >     cmd_obj.run()
       >   File "setup.py", line 109, in run
       >     subprocess.check_call(build_cmd)
       >   File "/nix/store/hi1kv51l7rba36vqr3gf6wcpg57aiy3y-python3-3.11.1/lib/python3.11/subprocess.py", line 413, in check_call
       >     raise CalledProcessError(retcode, cmd)
       > subprocess.CalledProcessError: Command '['cmake', '--build', '.', '--', '-j12']' returned non-zero exit status 2.
       > /nix/store/chilfhdcsnmwjl7igrw26j1lrc0zar35-stdenv-linux/setup: line 1570: pop_var_context: head of shell_variables not a function context

#211343

@fabaff
Copy link
Member

fabaff commented Jan 21, 2023

Depends on #211703

@msm-code
Copy link
Contributor Author

Sorry for obvious mistakes - I've fixed the description and changelog locally but apparently didn't push my ammended commit (or forgot --force).

Don't know how I missed the build fail for 3.11 - thanks!

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Please squash.

@msm-code
Copy link
Contributor Author

Thanks for valuable comments. Squashed and pushed the results.

In the current state of this PR, the build fails. I'm in the process of reworking it to build without the vendored dependencies.

@msm-code
Copy link
Contributor Author

I've rewritten the derivation to remove the vendored deps, and re-downloaded them myself.

I've tried to use versions already in nixpkgs, but the code seems to depend on the specific vendored versions (or in one case even patches them). For this reason this modification feels fragile to me (what if the project updates the vendored sources). But if that's the preferred solution, I'm OK with it.

I've tested the code with nixpkgs-review and in a nix-shell. I've also squashed my commit. Asked @dotlambda for re-review.

@msm-code msm-code requested review from dotlambda and removed request for FRidh and jonringer January 22, 2023 00:19
@msm-code msm-code force-pushed the master branch 2 times, most recently from f3cfcee to f706538 Compare January 22, 2023 01:13
@msm-code
Copy link
Contributor Author

Hi @dotlambda, what do you think about the current version?

I'll add once again that my preference would be to use vendored dependencies (to reduce the maintanance overhead when they are updated in the project - they are pinned to specific version and the project doesn't work with the version we have in nixpkgs).

Nevertheless, I've removed vendored deps as suggested. Let me know if you see anything else that can be improved.

Comment on lines 18 to 29
pybind11 = fetchFromGitHub {
owner = "pybind";
repo = "pybind11";
rev = "refs/tags/v2.9.2";
hash = "sha256-O3bkexUBa+gfiJEM6KSR8y/iVqHqlCFmz/9EghxdIpw=";
};
json = fetchFromGitHub {
owner = "nlohmann";
repo = "json";
rev = "refs/tags/v3.9.1";
hash = "sha256-THordDPdH2qwk6lFTgeFmkl7iDuA/7YH71PTUe6vJCs=";
};
Copy link
Member

Choose a reason for hiding this comment

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

Why are we not using nixpkgs versions of json and pybind11?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, not sure about pybind11. I tested it earlier and it was broken with nixpkgs vesion of pybind11. Now I've changed it to the nixpkgs version, and it passes all my tests (and the included pytest tests). So probably I was confused/tested a wrong version.

For json I didn't know how to get the "single header include" for it, but found an example in another package from nixpkgs, so I replaced that.

Co-authored-by: Robert Schütz <[email protected]>
Co-authored-by: Fabian Affolter <[email protected]>
@msm-code
Copy link
Contributor Author

msm-code commented Feb 7, 2023

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

2 packages built:
  • python310Packages.yaramod
  • python311Packages.yaramod

@SuperSandro2000 SuperSandro2000 changed the title python38Packages.yaramod: init at 3.12.2 python310Packages.yaramod: init at 3.12.2 Feb 22, 2023
@SuperSandro2000
Copy link
Member

@ofborg build python310Packages.yaramod

@SuperSandro2000 SuperSandro2000 merged commit 988cc95 into NixOS:master Feb 22, 2023
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.

4 participants