Skip to content

lastpass-cli: fix build failure with cmake 4#452443

Merged
smancill merged 1 commit intoNixOS:masterfrom
drawbu:clement/push-yyoqzuqrvmwp
Oct 17, 2025
Merged

lastpass-cli: fix build failure with cmake 4#452443
smancill merged 1 commit intoNixOS:masterfrom
drawbu:clement/push-yyoqzuqrvmwp

Conversation

@drawbu
Copy link
Member

@drawbu drawbu commented Oct 16, 2025 •
edited
Loading

Fix lastpass-cli build failure on newer cmake 4.

Applied patch from PR upstream.

curl -L https://patch-diff.githubusercontent.com/raw/lastpass/lastpass-cli/pull/716.patch > pkgs/by-name/la/lastpass-cli/716-bump-cmake-minimum-version.patch

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 16, 2025
@drawbu
Copy link
Member Author

drawbu commented Oct 16, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 452443
Commit: 466d082c10f5c39e89a58ef464039bf7da23c740

x86_64-linux

✅ 1 package built:
  • lastpass-cli

@nix-owners nix-owners bot requested a review from vinylen October 16, 2025 01:40
@nixpkgs-ci nixpkgs-ci bot added 9.needs: reviewer This PR currently has no reviewers requested and needs attention. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Oct 16, 2025
vinylen
vinylen approved these changes Oct 16, 2025
View reviewed changes
Copy link
Member

@vinylen vinylen left a comment

Choose a reason for hiding this comment

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

Thanks!

@vinylen
Copy link
Member

vinylen commented Oct 16, 2025

@NixOS/nixpkgs-merge-bot merge

@nixpkgs-merge-bot
Copy link
Contributor

nixpkgs-merge-bot bot commented Oct 16, 2025

@vinylen merge not permitted (#305350):
R-Ryantm Maintainer merge: pr author is not r-ryantm
CommitterPR: pr author is not committer

@nixpkgs-ci nixpkgs-ci bot 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 Oct 16, 2025
@Sigmanificient Sigmanificient mentioned this pull request Oct 16, 2025
Open
@drawbu drawbu force-pushed the clement/push-yyoqzuqrvmwp branch from 466d082 to a012871 Compare October 16, 2025 20:14
@nixpkgs-ci nixpkgs-ci bot removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 16, 2025
@drawbu
Copy link
Member Author

drawbu commented Oct 16, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 452443
Commit: a01287160bdf417dbec55449f9e4037a86d71340

x86_64-linux

✅ 2 packages built:
  • lastpass-cli
  • tests.pkgs-lib

@Sigmanificient
Copy link
Member

Sigmanificient commented Oct 16, 2025 •
edited
Loading

@drawbu Have you consider vendoring this pr?

@drawbu drawbu force-pushed the clement/push-yyoqzuqrvmwp branch from a012871 to 232d467 Compare October 16, 2025 22:10
@drawbu
Copy link
Member Author

drawbu commented Oct 16, 2025

Fair point @Sigmanificient! I haven't seen it, actually! Just modified the pr to integrate this fix instead of the substituteInPlace.

@drawbu drawbu force-pushed the clement/push-yyoqzuqrvmwp branch from 232d467 to 20b66a0 Compare October 16, 2025 22:13
@drawbu
lastpass-cli: fix build failure with cmake 4
02f2321 
- CMake 4 is no longer retro compatible with versions < 3.5
@drawbu drawbu force-pushed the clement/push-yyoqzuqrvmwp branch from 20b66a0 to 02f2321 Compare October 16, 2025 22:24
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Oct 16, 2025
smancill
smancill previously requested changes Oct 17, 2025
View reviewed changes
Copy link
Contributor

@smancill smancill left a comment

Choose a reason for hiding this comment

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

No need to add the patch to the tree.

Use fetchpatch to get it directly from:

https://github.com/lastpass/lastpass-cli/commit/31a4ad5f735933ff8e96403103d5b4f61faee945.patch

@drawbu
Copy link
Member Author

drawbu commented Oct 17, 2025

No need to add the patch to the tree.

Use fetchpatch to get it directly from:


https://github.com/lastpass/lastpass-cli/commit/31a4ad5f735933ff8e96403103d5b4f61faee945.patch

From what I understood it was actually preferred to vendor the patches to avoid GitHub garbage collecting the commit.

But I can change it if you prefer!

@Sigmanificient
Copy link
Member

Sigmanificient commented Oct 17, 2025 •
edited
Loading

@smancill When a pr is made to upstream, the patch should be venderored until resolution, see:

@smancill smancill dismissed their stale review October 17, 2025 12:37

Patch should be vendored then.

@smancill
Copy link
Contributor

smancill commented Oct 17, 2025

ok, I didn't realize it wasn't merged into upstream repo.

I am merging this then.

@smancill smancill added this pull request to the merge queue Oct 17, 2025
Merged via the queue into NixOS:master with commit 6dadef6 Oct 17, 2025
27 checks passed
@drawbu drawbu deleted the clement/push-yyoqzuqrvmwp branch October 17, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sigmanificient Sigmanificient Sigmanificient approved these changes

@smancill smancill smancill left review comments

+1 more reviewer

@vinylen vinylen vinylen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 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.

4 participants

@drawbu @vinylen @Sigmanificient @smancill