p4: 2021.2.2201121 -> 2022.1.2305383, build from source and remove unfree binaries #190105

impl · 2022-09-07T08:33:07Z

Description of changes

The actual p4 command is open-source software released under the 2-clause BSD license, so we can build it here (for pretty much every architecture we support!) and include it in the cache.

This change removes the server-side commands from this package, but they are now available as part of a separate p4d package (#190100) instead. (The server package remains unfree, and to make users' lives easier its PR should probably be merged before this one.)

As an added bonus, we can also include the libraries and headers for the C/C++ API, which will allow us to package any software that uses Perforce as a library in the future.

Please note that I am employed by Perforce but I am not currently supporting these packages in an official capacity.

Things done

corngood · 2022-09-07T12:58:01Z

pkgs/applications/version-management/p4/default.nix

+  CCFLAGS =
+    # Appears to be missing from upstream distribution, but is in fact
+    # open-source redistributable software.
+    lib.optionals stdenv.isAarch64 [ "-I${./aarch64/zlib/include}" ];


could we pull this from somewhere instead of keeping it in nixpkgs?

corngood

Thanks!

AndersonTorres · 2022-09-11T03:49:37Z

It means that it is possible to catch jam directly from the original repo using this P4?

impl · 2022-09-11T04:02:21Z

Hmm, I suppose in theory you could implement a fetchp4 fetcher and use it to acquire Jam, but it would introduce a dependency cycle like fetchurl<->curl (i.e., it would need a bootstrapping phase of some sort anyway) because p4 needs Jam to build itself, right?

AndersonTorres · 2022-09-12T01:20:00Z

Well, that was unexpected.
I was talking about p4->git clone Jam to a Github repository, as a form of backup.

impl · 2022-09-12T03:25:41Z

Oh! Ha. Sorry for the misunderstanding! Yes, that's certainly possible, and there's actually even a git p4 command that can bridge the two.

Edit: If you have the p4 command installed, then you can do:

$ P4PORT=public.perforce.com:1666 P4USER=guest git p4 clone //guest/perforce_software/jam/src/...@all --destination jam

nixos-discourse · 2022-10-07T19:57:13Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/631

SuperSandro2000 · 2022-10-08T00:18:59Z

pkgs/applications/version-management/p4/default.nix

+    ++ lib.optionals stdenv.isLinux [ "-sOSVER=26" ]
+    ++ lib.optionals stdenv.isDarwin [ "-sOSVER=1013" ];
+
+  outputs = [ "bin" "dev" "out" ];


Suggested change

outputs = [ "bin" "dev" "out" ];

outputs = [ "out" "bin" "dev" ];

otherwise leftover files end up in bin

SuperSandro2000 · 2022-10-08T00:20:00Z

pkgs/applications/version-management/p4/default.nix

+  CROSS_COMPILE = stdenv.cc.targetPrefix;
+  CCFLAGS =
+    # The file contrib/optimizations/slide_hash_neon.h is missing from the
+    # upstream distribution. It comes from the Android/Chromium sources.
+    lib.optionals stdenv.isAarch64 [ "-I${androidZlibContrib}" ];
+  "C++FLAGS" =
+    # Avoid a compilation error that only occurs for 4-byte longs.
+    lib.optionals stdenv.isi686 [ "-Wno-narrowing" ]
+    # See the "Header dependency changes" section of
+    # https://www.gnu.org/software/gcc/gcc-11/porting_to.html for more information
+    # on why we need to include these.
+    ++ lib.optionals
+      (stdenv.cc.isClang || (stdenv.cc.isGNU && lib.versionAtLeast stdenv.cc.cc.version "11.0.0"))
+      [ "-include" "limits" "-include" "thread" ];
+  MALLOC_OVERRIDE = "no";
+  SSLINCDIR = "${lib.getDev opensslStatic}/include";
+  SSLLIBDIR = "${lib.getLib opensslStatic}/lib";
+  MACOSX_SDK = lib.optionalString stdenv.isDarwin "${emptyDirectory}";
+  "LIBC++DIR" = lib.optionalString stdenv.isDarwin "${libcxxUnified}/lib";


Those should be exportet in some prePhase like preBuild

These are all actually flags to jam, so I'm just going to move them into there. The way I have them split out right now doesn't really make any sense I don't think.

I moved all of them except CCFLAGS and C++FLAGS which are Jam's standard equivalent to the autotools/make convention of CFLAGS and CXXFLAGS, respectively, so they felt like they could stay at the top level. Since they're built from a list of conditionals it's also harder to put them inside a preBuild-style script that maintains readability.

SuperSandro2000 · 2022-10-08T00:20:17Z

pkgs/applications/version-management/p4/default.nix

+  buildInputs = lib.optionals stdenv.isDarwin [ CoreServices Foundation Security ];
+  nativeBuildInputs = [ jam ];


Suggested change

buildInputs = lib.optionals stdenv.isDarwin [ CoreServices Foundation Security ];

nativeBuildInputs = [ jam ];

nativeBuildInputs = [ jam ];

buildInputs = lib.optionals stdenv.isDarwin [ CoreServices Foundation Security ];

SuperSandro2000 · 2022-10-08T00:20:42Z

pkgs/applications/version-management/p4/default.nix

-    # actually  https://cdist2.perforce.com/perforce/r21.2/bin.linux26x86_64/helix-core-server.tgz but upstream deletes releases
-    url = "https://web.archive.org/web/20211118024943/https://cdist2.perforce.com/perforce/r21.2/bin.linux26x86_64/helix-core-server.tgz";
-    sha256 = "sha256-SrfI2ZD7KDyttCd8+fo8g4UZKljYYO/SbzqrS9tAcC8=";
+    # Upstream replaces minor versions, so use cached URL.


Suggested change

# Upstream replaces minor versions, so use cached URL.

# Upstream replaces minor versions, so use cached URL.

Suggested change

# Upstream replaces minor versions, so use cached URL.

# Upstream replaces minor versions, so use archived URL.

SuperSandro2000 · 2022-10-08T00:21:28Z

pkgs/applications/version-management/p4/default.nix

+    paths = [ libcxx libcxxabi ];
+  };
+
+  version = "2022.1.2305383";


Why is that moved here? It is unused.

It's used further down to determine the path of the p4api directory, so I can call mkDerivation without using rec. If using rec is OK with you I can of course just put it directly in there.

Using rec is totally fine.

The actual p4 command is open-source software released under the 2-clause BSD license, so we can build it here (for pretty much every architecture we support!) and include it in the cache. This change removes the server-side commands from this package, but they are now available as part of a separate p4d package instead. (The server package remains unfree.) As an added bonus, we can also include the libraries and headers for the C/C++ API, which will allow us to package any software that uses Perforce as a library in the future.

impl · 2022-10-09T22:52:33Z

@SuperSandro2000 Thank you for the detailed review! I think I addressed everything, but please let me know if there are any other changes you'd like to see.

pkgs/applications/version-management/p4/default.nix

github-actions bot added 6.topic: nixos 8.has: changelog 8.has: documentation labels Sep 7, 2022

impl mentioned this pull request Sep 7, 2022

p4d: init at 2022.1.2305383 #190100

Merged

13 tasks

ofborg bot added the 8.has: package (new) label Sep 7, 2022

ofborg bot requested a review from corngood September 7, 2022 08:43

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Sep 7, 2022

corngood reviewed Sep 7, 2022

View reviewed changes

impl force-pushed the free-p4 branch from 344512f to 797125a Compare September 8, 2022 05:34

impl marked this pull request as ready for review September 8, 2022 05:34

impl force-pushed the free-p4 branch from 797125a to becc78d Compare September 8, 2022 06:45

ofborg bot requested a review from corngood September 8, 2022 06:54

corngood approved these changes Sep 8, 2022

View reviewed changes

bobby285271 added 12.approvals: 1 12.approved-by: package-maintainer labels Sep 8, 2022

impl mentioned this pull request Sep 10, 2022

{jam,ftjam}: set target-specific macros correctly when cross compiling #190248

Merged

13 tasks

SuperSandro2000 reviewed Oct 8, 2022

View reviewed changes

impl force-pushed the free-p4 branch 2 times, most recently from 97064c5 to 8a56bcb Compare October 8, 2022 06:23

ofborg bot requested a review from corngood October 8, 2022 06:34

impl added 2 commits October 9, 2022 15:47

p4d: add corngood to maintainers

453192a

impl force-pushed the free-p4 branch from 8a56bcb to 453192a Compare October 9, 2022 22:48

AndersonTorres reviewed Oct 10, 2022

View reviewed changes

pkgs/applications/version-management/p4/default.nix Show resolved Hide resolved

AndersonTorres merged commit 77c986e into NixOS:master Oct 11, 2022

impl deleted the free-p4 branch October 11, 2022 03:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

p4: 2021.2.2201121 -> 2022.1.2305383, build from source and remove unfree binaries #190105

p4: 2021.2.2201121 -> 2022.1.2305383, build from source and remove unfree binaries #190105

impl commented Sep 7, 2022

corngood Sep 7, 2022

corngood left a comment

AndersonTorres commented Sep 11, 2022

impl commented Sep 11, 2022

AndersonTorres commented Sep 12, 2022

impl commented Sep 12, 2022 •

edited

nixos-discourse commented Oct 7, 2022

SuperSandro2000 Oct 8, 2022

SuperSandro2000 Oct 8, 2022

impl Oct 8, 2022

impl Oct 9, 2022

SuperSandro2000 Oct 8, 2022

SuperSandro2000 Oct 8, 2022

SuperSandro2000 Oct 8, 2022

impl Oct 8, 2022

SuperSandro2000 Oct 9, 2022

impl commented Oct 9, 2022

	outputs = [ "bin" "dev" "out" ];
	outputs = [ "out" "bin" "dev" ];

		buildInputs = lib.optionals stdenv.isDarwin [ CoreServices Foundation Security ];
		nativeBuildInputs = [ jam ];

	# Upstream replaces minor versions, so use cached URL.
	# Upstream replaces minor versions, so use cached URL.

p4: 2021.2.2201121 -> 2022.1.2305383, build from source and remove unfree binaries #190105

p4: 2021.2.2201121 -> 2022.1.2305383, build from source and remove unfree binaries #190105

Conversation

impl commented Sep 7, 2022

Description of changes

Things done

Choose a reason for hiding this comment

corngood left a comment

Choose a reason for hiding this comment

AndersonTorres commented Sep 11, 2022

impl commented Sep 11, 2022

AndersonTorres commented Sep 12, 2022

impl commented Sep 12, 2022 • edited

nixos-discourse commented Oct 7, 2022

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

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

Choose a reason for hiding this comment

impl commented Oct 9, 2022

impl commented Sep 12, 2022 •

edited