fmtlib: add pkgconfig #48960

colemickens · 2018-10-24T06:25:53Z

Motivation for this change

This enables fmt to be usable with meson/pkgconfig.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/) (Indirectly, via a package in an overlay that uses it.)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

Mic92 · 2018-10-24T08:06:56Z

pkgs/development/libraries/fmt/default.nix

+    cd /build/source
+    pc="fmt.pc"
+    substituteInPlace "$pc" --replace "@out@" "$out"
+    substituteInPlace "$pc" --replace "@version@" "${version}"


You can use substituteAll here which does the '@variable@' substitution out of the box.

Mic92 · 2018-10-24T08:07:47Z

pkgs/development/libraries/fmt/0001-add-fmt.pc.patch

@@ -0,0 +1,30 @@
+From 7ad56220fd23cfce7c1c3e3bc269fa88d34070eb Mon Sep 17 00:00:00 2001


This looks like you have send this also upstream? Can you include a link to the pull request if that's the case?

(I've now added it next to the patch line in default.nix, if that's okay.)

colemickens · 2018-10-24T08:39:05Z

I added a link to an upstream PR. I'm not sure it's good though, it has NixOS-isms in it still. I am not very experienced in this area to know how to best have a cmake project spit out a proper pkgconfig file?

Mic92 · 2018-10-24T10:02:15Z

@colemickens I made a suggestion in the pull request.

Synthetica9 · 2018-10-24T10:06:50Z

pkgs/development/libraries/fmt/default.nix

@@ -9,6 +9,10 @@ stdenv.mkDerivation rec {
    rev = "${version}";
    sha256 = "1cd8yq8va457iir1hlf17ksx11fx2hlb8i4jml8gj1875pizm0pk";
  };
+  patches = [
+    # https://github.com/fmtlib/fmt/pull/916


Might use something like the following then:

patches = [ (fetchPatch { name = "0001-add-fmt"; url = "https://github.com/fmtlib/fmt/pull/916.patch"; sha256 = "xxxxxxxxxxxxxxxxxxxxxxxxx"; }) ];

Instead of the pull request please create a dedicated branch in this case, so the patch never changes, if upstreams wants you to change the pull request.

Could do https://github.com/fmtlib/fmt/pull/916/commits/0dbf735fe3a54b78b2e35122baa5a9c0e7216445.patch in that case

Ah github even ensures those are not deleted: https://stackoverflow.com/questions/15261880/does-github-garbage-collect-dangling-commits-referenced-in-pull-requests

colemickens · 2018-10-24T13:22:45Z

Thanks for the review and suggestions @Synthetica9 @Mic92.

I've made the suggestion change upstream, tested it with this PR and waybar which uses fmt, and can confirm this all works. Also, this has the nice benefit of being self-documenting via the patch/PR url.

For additional verification, and/or in case it's of interest, I have this and some related things in an overlay (built for nixos-unstable), which is also how I'm testing this: https://github.com/colemickens/nix-overlay-sway (fmt/default.nix has the same fetchpatch)

colemickens · 2018-10-24T13:24:24Z

$ cd /etc/nix-overlay-sway
$ cat $(nix-build build.nix | grep fmt)/share/pkgconfig/fmt.pc
prefix=/nix/store/6vdylp1wysqkk2szkwxgwafgkckvnj6z-fmt-5.2.1
exec_prefix=/nix/store/6vdylp1wysqkk2szkwxgwafgkckvnj6z-fmt-5.2.1
libdir=/nix/store/6vdylp1wysqkk2szkwxgwafgkckvnj6z-fmt-5.2.1/lib
includedir=/nix/store/6vdylp1wysqkk2szkwxgwafgkckvnj6z-fmt-5.2.1/include

Name: fmt
Description: A modern formatting library
Version: 5.2.1
Libs: -L${libdir} -lfmt
Cflags: -I${includedir}

Synthetica9 · 2018-10-24T13:28:50Z

pkgs/development/libraries/fmt/default.nix

@@ -9,6 +9,12 @@ stdenv.mkDerivation rec {
    rev = "${version}";
    sha256 = "1cd8yq8va457iir1hlf17ksx11fx2hlb8i4jml8gj1875pizm0pk";
  };
+  patches = [
+    pkgs.fetchpatch {


You need parentheses here, otherwise it is interpreted as being two elements of an array instead of a call.

Synthetica9 · 2018-10-24T13:29:43Z

pkgs/development/libraries/fmt/default.nix

@@ -9,6 +9,12 @@ stdenv.mkDerivation rec {
    rev = "${version}";
    sha256 = "1cd8yq8va457iir1hlf17ksx11fx2hlb8i4jml8gj1875pizm0pk";
  };
+  patches = [
+    pkgs.fetchpatch {


pkgs.fetchpatch is also not in scope, so you need to add it to your arguments (I'd reccomend removing the pkgs, and adding fetchpatch as an argument.)

Mic92 · 2018-10-24T15:09:01Z

pkgs/development/libraries/fmt/default.nix

@@ -9,6 +9,12 @@ stdenv.mkDerivation rec {
    rev = "${version}";
    sha256 = "1cd8yq8va457iir1hlf17ksx11fx2hlb8i4jml8gj1875pizm0pk";
  };
+  patches = [
+    pkgs.fetchpatch {
+      url = "https://github.com/fmtlib/fmt/pull/916/commits/57ae5189351665715c98b3b6ca8595b30d83033f.patch";


Now that the patch is merged you can also pull from master.

colemickens · 2018-10-24T21:08:43Z

Thanks again to all. I took a stab at it. It's pointing at the latest commit from fmtlib/fmt master with my pkgconfig change included. I left the version string alone, though, please advise if I should change it.

What's the general nixpkgs guidance on something like with changes like this (aka, pinning to a non-tagged/released revision)?

For example, in this case, especially with such a speedy upstream merge, I don't mind carrying this override in my fork/overlay; it feels almost hasty to do this here, now, especially since I won't be sending a PR for the dependent package yet due to there being no release.

Mic92 · 2018-10-25T16:52:30Z

So basically you are waiting for the next release?

colemickens · 2018-10-26T00:22:47Z

@Mic92 I went ahead and already updated this PR to pull from fmt's master, I just don't know if it's prudent or appropriate for nixpkgs. I'm looking for some encouragement one way or the other.

Patch is adding pkgconfig support

c0bw3b · 2018-11-19T21:54:25Z

@GrahamcOfBorg build fmt

GrahamcOfBorg · 2018-11-19T21:56:20Z

Success on aarch64-linux (full log)

Attempted: fmt

Partial log (click to expand)

strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/xrpnlaqqzmyl8kkchl6yfigrm2n5z021-fmt-5.2.1/lib
patching script interpreter paths in /nix/store/xrpnlaqqzmyl8kkchl6yfigrm2n5z021-fmt-5.2.1
checking for references to /build in /nix/store/xrpnlaqqzmyl8kkchl6yfigrm2n5z021-fmt-5.2.1...
shrinking RPATHs of ELF executables and libraries in /nix/store/b61mcp9pbzyv5q6n87851hdprzqgx5fp-fmt-5.2.1-dev
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/b61mcp9pbzyv5q6n87851hdprzqgx5fp-fmt-5.2.1-dev/lib
patching script interpreter paths in /nix/store/b61mcp9pbzyv5q6n87851hdprzqgx5fp-fmt-5.2.1-dev
checking for references to /build in /nix/store/b61mcp9pbzyv5q6n87851hdprzqgx5fp-fmt-5.2.1-dev...
/nix/store/xrpnlaqqzmyl8kkchl6yfigrm2n5z021-fmt-5.2.1

GrahamcOfBorg · 2018-11-19T21:56:40Z

Success on x86_64-linux (full log)

Attempted: fmt

Partial log (click to expand)

strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/kd8ji5p3s643fb520khp7nl5j5z1gfp0-fmt-5.2.1/lib
patching script interpreter paths in /nix/store/kd8ji5p3s643fb520khp7nl5j5z1gfp0-fmt-5.2.1
checking for references to /build in /nix/store/kd8ji5p3s643fb520khp7nl5j5z1gfp0-fmt-5.2.1...
shrinking RPATHs of ELF executables and libraries in /nix/store/183q3v20c0g44x30z3zwr474py98iy6g-fmt-5.2.1-dev
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/183q3v20c0g44x30z3zwr474py98iy6g-fmt-5.2.1-dev/lib
patching script interpreter paths in /nix/store/183q3v20c0g44x30z3zwr474py98iy6g-fmt-5.2.1-dev
checking for references to /build in /nix/store/183q3v20c0g44x30z3zwr474py98iy6g-fmt-5.2.1-dev...
/nix/store/kd8ji5p3s643fb520khp7nl5j5z1gfp0-fmt-5.2.1

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 24, 2018

Mic92 reviewed Oct 24, 2018

View reviewed changes

colemickens force-pushed the pr-fmt branch from fe90053 to 53e4ee9 Compare October 24, 2018 08:36

Synthetica9 reviewed Oct 24, 2018

View reviewed changes

colemickens force-pushed the pr-fmt branch 2 times, most recently from 53a9fad to 8b44b12 Compare October 24, 2018 13:10

fmtlib: add pkgconfig

a244172

colemickens force-pushed the pr-fmt branch from 8b44b12 to a244172 Compare October 24, 2018 13:19

colemickens mentioned this pull request Oct 24, 2018

add fmt.pc for pkgconfig fmtlib/fmt#916

Merged

Synthetica9 requested changes Oct 24, 2018

View reviewed changes

Mic92 reviewed Oct 24, 2018

View reviewed changes

colemickens mentioned this pull request Nov 10, 2018

remove fmt package nix-community/nixpkgs-wayland#28

Closed

c0bw3b added 2 commits November 19, 2018 22:05

fmt: fix patch fetching

65e103c

Patch is adding pkgconfig support

Merge branch 'master' into pr-fmt

69c8464

GrahamcOfBorg added 2.status: merge conflict and removed 2.status: merge conflict labels Nov 19, 2018

c0bw3b merged commit 719feb8 into NixOS:master Nov 19, 2018

colemickens deleted the pr-fmt branch January 30, 2020 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fmtlib: add pkgconfig #48960

fmtlib: add pkgconfig #48960

colemickens commented Oct 24, 2018

Mic92 Oct 24, 2018

Mic92 Oct 24, 2018

colemickens Oct 24, 2018

colemickens commented Oct 24, 2018

Mic92 commented Oct 24, 2018

Synthetica9 Oct 24, 2018

Mic92 Oct 24, 2018

Synthetica9 Oct 24, 2018

Mic92 Oct 24, 2018

colemickens commented Oct 24, 2018

colemickens commented Oct 24, 2018

Synthetica9 Oct 24, 2018

Synthetica9 Oct 24, 2018

Mic92 Oct 24, 2018

colemickens commented Oct 24, 2018

Mic92 commented Oct 25, 2018

colemickens commented Oct 26, 2018

c0bw3b commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

		@@ -0,0 +1,30 @@
		From 7ad56220fd23cfce7c1c3e3bc269fa88d34070eb Mon Sep 17 00:00:00 2001

fmtlib: add pkgconfig #48960

fmtlib: add pkgconfig #48960

Conversation

colemickens commented Oct 24, 2018

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemickens commented Oct 24, 2018

Mic92 commented Oct 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemickens commented Oct 24, 2018

colemickens commented Oct 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemickens commented Oct 24, 2018

Mic92 commented Oct 25, 2018

colemickens commented Oct 26, 2018

c0bw3b commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018