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

nicstat: init at 0-unstable-2018-05-09 #324417

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

okvik
Copy link
Contributor

@okvik okvik commented Jul 3, 2024

Project description

nicstat is a tool similar to iostat, but for network interfaces.
Originally written by Brendan Gregg for Solaris. Ported to Linux by Tim Cook.

Closes #324060

  • 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.

pkgs/by-name/ni/nicstat/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nicstat/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nicstat/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nicstat/package.nix Outdated Show resolved Hide resolved
Copy link
Member

@juliusrickert juliusrickert left a comment

Choose a reason for hiding this comment

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

I successfully tested this on x86_64-linux.

Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

Is this really needed? The repository seems to be archived and the last commit is from 6 years ago. No offense but it sounds like a maintenance burden and a magnet for attracting CVEs.

EDIT: and this does not compile on aarch64-darwin so I don't know the point for setting unix in meta.platforms.

johnrtitor@darwin01 ~ % nix build "github:nixos/nixpkgs/refs/pull/324417/merge#nicstat" --print-out-paths
error: builder for '/nix/store/4yaxj1f6c1wlg3qpk6kw2nz9yc711hzl-nicstat-0-unstable-2018-05-09.drv' failed with exit code 1;
       last 10 log lines:
       > nicstat.c:2804:3: error: call to undeclared function 'update_stats'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
       >                 update_stats(net_dev);
       >                 ^
       > nicstat.c:2857:6: warning: add explicit braces to avoid dangling else [-Wdangling-else]
       >                         } else {
       >                           ^
       > nicstat.c:2891:4: error: call to undeclared function 'sleep_for'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
       >                         sleep_for(pause_m, &now);
       >                         ^
       > 1 warning and 17 errors generated.
       For full logs, run 'nix log /nix/store/4yaxj1f6c1wlg3qpk6kw2nz9yc711hzl-nicstat-0-unstable-2018-05-09.drv'.
nix build "github:nixos/nixpkgs/refs/pull/324417/merge#nicstat"   2.22s user 12.50s system 39% cpu 37.506 total
1 johnrtitor@darwin01 ~ %                                                                                                 

@juliusrickert
Copy link
Member

Is this really needed? The repository seems to be archived and the last commit is from 6 years ago. No offense but it sounds like a maintenance burden and a magnet for attracting CVEs.

Attracting CVEs where? Affecting Nix's build infrastructure?

If you could point me to alternatives that provide similar functionality, I'd be very grateful. :)
To me, the software is just "done". (And it's not hugely complex.) Therefore, it doesn't need to be updated regularly.

EDIT: and this does not compile on aarch64-darwin so I don't know the point for setting unix in meta.platforms.

@AndersonTorres, could you please clarify this? :)

@AndersonTorres
Copy link
Member

AndersonTorres commented Jul 13, 2024

Attracting CVEs where? Affecting Nix's build infrastructure?

Theey meant that it would imply we are providing outdated and potentially insecure software, not that insecure software is being used to maintain Nixpkgs infra.

If you could point me to alternatives that provide similar functionality, I'd be very grateful. :) To me, the software is just "done". (And it's not hugely complex.) Therefore, it doesn't need to be updated regularly.

I tend to agree.

EDIT: and this does not compile on aarch64-darwin so I don't know the point for setting unix in meta.platforms.

@AndersonTorres, could you please clarify this? :)

Theey meant to say theey tried to build it on Darwin and it failed.

Then, please add broken = stdenv.isDarwin; to meta set.

@juliusrickert
Copy link
Member

EDIT: and this does not compile on aarch64-darwin so I don't know the point for setting unix in meta.platforms.

@AndersonTorres, could you please clarify this? :)

Theey meant to say theey tried to build it on Darwin and it failed.

It only supports Linux and Solaris. Not sure whether we track Solaris support for packages somewhere.

Then, please add broken = stdenv.isDarwin; to meta set.

Will do.

@JohnRTitor
Copy link
Contributor

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

1 package built:
  • nicstat

Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

LGTM

@JohnRTitor JohnRTitor merged commit d068bb7 into NixOS:master Jul 16, 2024
24 of 26 checks passed
@okvik okvik deleted the init-nicstat branch August 23, 2024 17:37
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.

Package request: nicstat
5 participants