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

wireshark: 4.2.6 -> 4.4.2 #344914

Merged
merged 4 commits into from
Dec 3, 2024
Merged

wireshark: 4.2.6 -> 4.4.2 #344914

merged 4 commits into from
Dec 3, 2024

Conversation

pabloaul
Copy link
Contributor

@pabloaul pabloaul commented Sep 27, 2024

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@teto
Copy link
Member

teto commented Sep 27, 2024

running a nixpkgs-review .python3.12-pyshark-0.6 takes > 1 hour to build and it is still building. It seems stuck in pytestCheckPhase (according to nix-output-monitor). I haven't investigated but seems like some test is stuck

@pabloaul
Copy link
Contributor Author

Had this issue aswell with the two stuck pytestCheckPhase. The processes for each were stuck sleeping, probably a race condition.
I am not sure if this is related to my changes though since the Github Actions went through fine

@bjornfor
Copy link
Contributor

I get the same hang (for 8+ hours) for nix-build -A python3Packages.pyshark on the base commit (ed96506) if I try to rebuild. However, on Hydra the build seems stable: https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.python312Packages.pyshark.x86_64-linux

@bjornfor
Copy link
Contributor

I get the same hang (for 8+ hours) for nix-build -A python3Packages.pyshark on the base commit (ed96506) if I try to rebuild.

Nope, that's wrong. (It's too early in the morning.) I can build pyshark just fine on the base branch, so the hang looks like a regression in this PR.

Copy link
Contributor

@bjornfor bjornfor left a comment

Choose a reason for hiding this comment

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

nix-build -A python3Packages.pyshark builds start to hang with this PR:

============================= test session starts ==============================
platform linux -- Python 3.12.5, pytest-8.3.2, pluggy-1.5.0
rootdir: /build/source
collected 73 items / 1 deselected / 72 selected                                

../tests/capture/test_capture.py .....                                   [  6%]
[...waited hours without more output...]

If we can't fix it, I think we should at least turn that hang into a quick fail before merging this PR.

@pabloaul
Copy link
Contributor Author

Okay, this must be patch related then... I'll take a closer look soon again at the changes from Wireshark

@pabloaul
Copy link
Contributor Author

pabloaul commented Oct 3, 2024

I was unable to track down the cause for the issue. If anyone more comfortable with the code and/or capture_sync.c could take a look, it would be much appreciated...

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 1, 2024
@fpletz fpletz removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 30, 2024
@fpletz fpletz changed the title wireshark: 4.2.6 -> 4.4.0 wireshark: 4.2.6 -> 4.4.2 Nov 30, 2024
@fpletz
Copy link
Member

fpletz commented Nov 30, 2024

Rebased, updated to new point release and picked a patch from pyshark master to fix the capture issue.

@pabloaul
Copy link
Contributor Author

fails some tests on a tool depending on pyshark: nix log of credslayer

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 344914


x86_64-linux

❌ 2 packages failed to build:
  • credslayer
  • credslayer.dist
✅ 26 packages built:
  • compactor
  • dbmonster
  • haka
  • hfinger
  • hfinger.dist
  • ostinato
  • python311Packages.dissect-cobaltstrike
  • python311Packages.dissect-cobaltstrike.dist
  • python311Packages.manuf
  • python311Packages.manuf.dist
  • python311Packages.pyshark
  • python311Packages.pyshark.dist
  • python312Packages.dissect-cobaltstrike
  • python312Packages.dissect-cobaltstrike.dist
  • python312Packages.manuf
  • python312Packages.manuf.dist
  • python312Packages.pyshark
  • python312Packages.pyshark.dist
  • qtwirediff
  • termshark
  • tshark (wireshark-cli)
  • tshark.dev (wireshark-cli.dev)
  • wifite2
  • wifite2.dist
  • wireshark (wireshark-qt)
  • wireshark.dev (wireshark-qt.dev)

@fpletz
Copy link
Member

fpletz commented Dec 2, 2024

I would lean towards marking credslayer or maybe even pyshark itself as broken since the latter is barely maintained. Not sure what the issue is here but it's probably not our issue. We shouldn't delay bumping wireshark.

@pabloaul
Copy link
Contributor Author

pabloaul commented Dec 2, 2024

I think it would be fine to mark credslayer as broken.
pyshark seemingly still sort of works for the other projects that depend on it so I think we can leave it for now

@fpletz fpletz dismissed bjornfor’s stale review December 3, 2024 10:04

pyshark builds, credslayer marked as broken

@fpletz fpletz merged commit 6d8ddd8 into NixOS:master Dec 3, 2024
20 of 21 checks passed
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.

5 participants