Skip to content

Commit

Permalink
Merge pull request #311015 from annaleeleaves/ip2unix-fix
Browse files Browse the repository at this point in the history 
ip2unix: upstream PR for out of range access
  • Loading branch information
@wegank
wegank committed May 12, 2024
2 parents 6046e88 + 4a74c4c commit 315e42d
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion pkgs/tools/networking/ip2unix/default.nix
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ lib, stdenv, fetchFromGitHub, meson, ninja, pkg-config, yaml-cpp, systemd
{ lib, stdenv, fetchFromGitHub, fetchpatch, meson, ninja, pkg-config, yaml-cpp, systemd
, python3Packages, asciidoc, libxslt, docbook_xml_dtd_45, docbook_xsl
, libxml2, docbook5
}:
Expand All @@ -14,6 +14,15 @@ stdenv.mkDerivation rec {
hash = "sha256-+p5wQbX35LAjZ4vIE4AhI4M6gQ7gVviqf9jJDAr9xg8";
};

patches = [
# https://github.com/nixcloud/ip2unix/pull/35
# fix out of range string_view access
(fetchpatch {
url = "https://github.com/nixcloud/ip2unix/commit/050ddf76b4b925f27e255fbb820b0700407ceb2b.patch";
hash = "sha256-5vaLmZmwuiMGV4KnVhuDSnXG1a390aBU51TShwpaMLs=";
})
];

nativeBuildInputs = [
meson ninja pkg-config asciidoc libxslt.bin docbook_xml_dtd_45 docbook_xsl
libxml2.bin docbook5 python3Packages.pytest python3Packages.pytest-timeout
Expand Down

5 comments on commit 315e42d

@aszlig
Copy link
Member

@aszlig aszlig commented on 315e42d May 12, 2024

Choose a reason for hiding this comment

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

@wegank: While I'm fine with this change, this was merged ten hours after the PR was issued. Don't you think this is a bit too early, given that the upstream pull request hasn't been merged and no maintainer has acked?

@wegank
Copy link
Member Author

@wegank wegank commented on 315e42d May 12, 2024

Choose a reason for hiding this comment

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

The PR mentioned the ZHF issue I created, and I tend to act quickly to prevent open PRs from piling up. I'm very sorry if my quick merge offended you.

@aszlig
Copy link
Member

@aszlig aszlig commented on 315e42d May 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The quick merge itself isn't the problem but rather generally simply merging a fix that hasn't even been accepted upstream.

Oh, and just to clarify: It's perfectly fine in this case, so no need to revert or anything.

@wegank
Copy link
Member Author

@wegank wegank commented on 315e42d May 13, 2024

Choose a reason for hiding this comment

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

Oh, but patches from open PRs are sort of acceptable in Nixpkgs, since we don't usually assume that the upstream is active, nor do we want to keep patches in the tree. Perhaps it would be better to open an RFC for that...

@7c6f434c
Copy link
Member

Choose a reason for hiding this comment

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

I'd say that every single facet of this merge would be OK with slightly different context, and as it happened in practice it was faster than in the «ideally consistent world otherwise close to ours».

Please sign in to comment.