Skip to content

upgrade perforce packages: p4, p4v#110168

Merged
Artturin merged 1 commit intoNixOS:masterfrom
corngood:p4
May 5, 2022
Merged

upgrade perforce packages: p4, p4v#110168
Artturin merged 1 commit intoNixOS:masterfrom
corngood:p4

Conversation

@corngood
Copy link
Copy Markdown
Contributor

@corngood corngood commented Jan 20, 2021 •
edited
Loading

Motivation for this change

This still doesn't have a permanent url for the specific minor version, and will break whenever they update. :(

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jan 20, 2021
@SuperSandro2000
Copy link
Copy Markdown
Member

SuperSandro2000 commented Mar 21, 2021

Download link is no longer available.

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Mar 23, 2021

Yeah, looks like they mysteriously recalled 20.2:

https://www.perforce.com/support/software-release-index#tab-panel-6208

Release 2020.2 *Temporarily Unavailable for Download

@SuperSandro2000
Copy link
Copy Markdown
Member

SuperSandro2000 commented Apr 5, 2021

What now? Update to a newer version? Update the URL? Can this PR be closed?

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Apr 5, 2021

Note: The 20.2 release of Helix Core Suite has been temporarily removed as we perform some Root Cause Analysis on customer reported issues.

Once they post a new version, I'll update this PR.

@SuperSandro2000 SuperSandro2000 added the 2.status: wait-for-upstream Waiting for upstream fix (or their other action). label Apr 5, 2021
@SuperSandro2000 SuperSandro2000 marked this pull request as draft April 5, 2021 17:33
@corngood corngood changed the title p4: 2020.1.2007551 -> 2020.2.2057778 upgrade perforce packages: p4, p4v Sep 10, 2021
@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Sep 10, 2021

I switched both of these to archive.org, as suggested in #131292.

@corngood corngood marked this pull request as ready for review September 10, 2021 00:41
@ofborg ofborg bot requested review from nathyong and nioncode September 10, 2021 00:47
@ofborg ofborg bot removed the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Sep 10, 2021
nioncode
nioncode approved these changes Sep 10, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@nioncode nioncode left a comment

Choose a reason for hiding this comment

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

Looks good to me!

SuperSandro2000
SuperSandro2000 reviewed Sep 11, 2021
View reviewed changes
@corngood corngood force-pushed the p4 branch 2 times, most recently from f5425a3 to 6e3af11 Compare November 18, 2021 03:02
@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Nov 18, 2021

Updated again, and applied your suggestions @SuperSandro2000.

@ofborg ofborg bot requested a review from nioncode November 18, 2021 03:07
SuperSandro2000
SuperSandro2000 reviewed Nov 24, 2021
View reviewed changes
pkgs/applications/version-management/p4v/default.nix Outdated
Comment on lines 22 to 26
Copy link
Copy Markdown
Member

@SuperSandro2000 SuperSandro2000 Nov 24, 2021

Choose a reason for hiding this comment

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

I don't feel comfortable merging this with this patch. I don't know if it breaks crypto or not.

Copy link
Copy Markdown
Contributor Author

@corngood corngood Nov 25, 2021

Choose a reason for hiding this comment

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

We could try to build a modified Qt instead. p4v is still a binary blob either way.

Copy link
Copy Markdown
Contributor Author

@corngood corngood Dec 2, 2021

Choose a reason for hiding this comment

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

I tested latest with this hack disabled, and it still fails. The error you get is:

Connect to server failed; check $P4PORT.
Programming error: CRYPTO_set_mem_functions(): Could not set OpenSSL allocation functions.

@nathyong
Copy link
Copy Markdown
Contributor

nathyong commented Nov 25, 2021

Trying to test this now, but I don't seem to be able to download the sources from archive.org.

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Nov 25, 2021

@nathyong What's the problem exactly? I found archive.org extremely slow, and p4v is like 130m. Any other ideas for mirrors are welcome.

@alex-ameen alex-ameen mentioned this pull request Dec 2, 2021
Closed
13 tasks
@nathyong
Copy link
Copy Markdown
Contributor

nathyong commented Dec 12, 2021

I wasn't able to get any bytes from archive.org in the past. Seems fine (if a bit slow) now though.

@SuperSandro2000
Copy link
Copy Markdown
Member

SuperSandro2000 commented Dec 16, 2021

If the download link is not changing over time and is not gone again we shouldn't use web.archive.org.

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Dec 16, 2021

The download link does change, unfortunately. Perforce only provides a link to the minor version, which always points to the latest patch release.

Also, I had a good look at using the shipped qt libs in case it allowed the crypto hack to be removed, but things got hacky very quickly and I wasn't able (so far) to fix a crash in qt web engine. It was feeling like it would be a worse outcome.

@corngood corngood mentioned this pull request Dec 28, 2021
Merged
13 tasks
@ofborg ofborg bot added the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Jan 26, 2022
@Artturin
Copy link
Copy Markdown
Member

Artturin commented May 1, 2022

@ofborg build p4v

@Artturin Artturin added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels May 4, 2022
@Artturin
Copy link
Copy Markdown
Member

Artturin commented May 5, 2022

files that should not be in bin are in bin

[nix-shell:~/.cache/nixpkgs-review/pr-110168]$ ls ./results/p4v/bin
p4admin  p4admin.bin  p4merge  p4merge.bin  p4v  p4v.bin  p4vc  qt.conf  QtWebEngineProcess

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented May 5, 2022

@Artturin yeah, that's not ideal, but it's unchanged in this upgrade and it's how they're distributed upstream.

@Artturin
Copy link
Copy Markdown
Member

Artturin commented May 5, 2022

did not test

@Artturin Artturin merged commit 1808791 into NixOS:master May 5, 2022
This was referenced Sep 6, 2022
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@nathyong nathyong Awaiting requested review from nathyong

@nioncode nioncode Awaiting requested review from nioncode

Assignees

No one assigned

Labels

2.status: wait-for-upstream Waiting for upstream fix (or their other action). 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@corngood @SuperSandro2000 @nathyong @Artturin @nioncode