macvim: 8.2.3455 -> 178 #260094

lilyball · 2023-10-09T22:47:48Z

Description of changes

This updates MacVim to the latest release. I've been running these changes locally for a while now, on both macOS Ventura and macOS Sonoma. It also fixes the ability to override macvim (previously it would override the macvim-configurable.nix file instead). The version is now listed as 178 as MacVim has switched to reporting its own version as its release number.

MacVim release notes:
Release 173 (Vim 9.0.0065)
Release 174 (Vim 9.0.472)
(Release 175 was pulled)
Release 176 (Vim 9.0.1276)
r177 (Vim 9.0.1677)
r178 (Vim 9.0.1897)

Things done

nixos-discourse · 2023-10-20T21:08:25Z

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/2786

lilyball · 2024-01-24T20:55:56Z

@millerjason I just requested review from you because you authored #239842 (which this obsoletes)

nixos-discourse · 2024-01-24T20:57:54Z

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3329

millerjason · 2024-01-31T06:05:21Z

@millerjason I just requested review from you because you authored #239842 (which this obsoletes)

Agreed and lgtm - thanks!

SuperSandro2000 · 2024-02-03T23:50:51Z

pkgs/applications/editors/vim/macvim.nix

+  # Python3 3.9 or above - the binary release builds against 3.11 so we will too
+  python3 = python311;


python3 defaults to that, do we really need to hardcode it here? Same for ruby_3_2.

At the time this PR was written, python3 evaluated to python310 instead. And ruby still evaluates to ruby_3_1. I'm open to changing this to just python3 as MacVim documents compatibility with "Python3 3.9 or above", but the ruby support is documented as "Ruby 3.2" so we should match that.

I've removed the python311 pin but left ruby_3_2.

SuperSandro2000 · 2024-02-03T23:51:04Z

pkgs/applications/editors/vim/macvim.nix


  src = fetchFromGitHub {
    owner = "macvim-dev";
    repo = "macvim";
-    rev = "snapshot-172";
-    sha256 = "sha256-LLLQ/V1vyKTuSXzHW3SOlOejZD5AV16NthEdMoTnfko=";
+    rev = "release-178";


This should be derived from version

Are you proposing that this should use finalAttrs and programmatically change r### into release-###, so someone overriding the version to r179 will automatically update the rev too? Or do you just not like that the 178 is duplicated? I'm partial to the idea that changing the version changes the rev too, but that requires defining what happens if someone overrides the version to something that doesn't match r###.

Are you proposing that this should use finalAttrs and programmatically change r### into release-###, so someone overriding the version to r179 will automatically update the rev too?

First yes but then I remembered that we are stripping prefixes like r or v from versions, so we just need to substitute it here.

Suggested change

rev = "release-178";

rev = "release-${version}";

hraban · 2024-03-26T01:59:24Z

Can confirm this fixes the perl configuration issue on aarch64-darwin. It fails later in the build process but I assume that's unrelated to this PR?

macvim-r178> xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

LeSuisse · 2024-04-02T10:20:56Z

Includes the fix for CVE-2023-41036.

lilyball · 2024-04-17T20:25:16Z

Can confirm this fixes the perl configuration issue on aarch64-darwin. It fails later in the build process but I assume that's unrelated to this PR?
macvim-r178> xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

Yeah, that error is saying you need to have Xcode.app installed rather than using the CommandLineTools installation. The latter provides compiler support but does not provide everything that Xcode does, and MacVim is an Xcode project.

This error is the unfortunate consequence of the fact that MacVim is an impure derivation, relying on the user to have installed Xcode.app.

Previously, attemping to override macvim would override the `macvim-configurable.nix` definition instead of overriding the macvim package.

lilyball · 2024-06-04T03:57:25Z

Rebased to fix merge conflict. I also removed the hardcoding of python311 in the process.

SuperSandro2000 · 2024-06-25T22:32:55Z

pkgs/applications/editors/vim/macvim.nix

@@ -29,13 +37,13 @@ in
 stdenv.mkDerivation {
  pname = "macvim";

-  version = "8.2.3455";
+  version = "r178";


Suggested change

version = "r178";

version = "178";

SuperSandro2000 · 2024-06-25T22:33:52Z

pkgs/applications/editors/vim/macvim.nix


  src = fetchFromGitHub {
    owner = "macvim-dev";
    repo = "macvim";
-    rev = "snapshot-172";
-    sha256 = "sha256-LLLQ/V1vyKTuSXzHW3SOlOejZD5AV16NthEdMoTnfko=";
+    rev = "release-178";


Are you proposing that this should use finalAttrs and programmatically change r### into release-###, so someone overriding the version to r179 will automatically update the rev too?

First yes but then I remembered that we are stripping prefixes like r or v from versions, so we just need to substitute it here.

Suggested change

rev = "release-178";

rev = "release-${version}";

MacVim now uses its own release number as its primary version, but this is equivalent to 9.0.1897.

lilyball · 2024-06-27T00:45:17Z

I've applied the suggestion to set version = "178" and to use that version in the definition of rev.

lilyinstarlight

The diff looks good to me and is at least a clear improvement to status-quo for this package

Thank you!

(I technically stepped down from nixpkgs maintainership yesterday, but I'll send this as my last contribution to nixpkgs since it appears ready and I was already reviewing it yesterday before stepping down)

github-actions bot added the 6.topic: vim label Oct 9, 2023

This was referenced Oct 9, 2023

macvim: disable broken interpreters #239842

Closed

macvim fails to install on MacOS Ventura #201197

Closed

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 10, 2023

lilyball requested a review from millerjason January 24, 2024 20:55

millerjason approved these changes Jan 31, 2024

View reviewed changes

SuperSandro2000 reviewed Feb 3, 2024

View reviewed changes

wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 8, 2024

LeSuisse added the 1.severity: security Issues which raise a security issue, or PRs that fix one label Apr 2, 2024

wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 22, 2024

macvim: fix overrides

7eb3f27

Previously, attemping to override macvim would override the `macvim-configurable.nix` definition instead of overriding the macvim package.

lilyball force-pushed the update-macvim branch from 9e0b9d3 to 683d2a3 Compare June 4, 2024 03:57

lilyball removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 4, 2024

SuperSandro2000 reviewed Jun 25, 2024

View reviewed changes

wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 26, 2024

macvim: 8.2.3455 -> 178

5c5dd69

MacVim now uses its own release number as its primary version, but this is equivalent to 9.0.1897.

lilyball force-pushed the update-macvim branch from 683d2a3 to 5c5dd69 Compare June 27, 2024 00:44

lilyball changed the title ~~macvim: 8.2.3455 -> r178~~ macvim: 8.2.3455 -> 178 Jun 27, 2024

lilyinstarlight approved these changes Jul 2, 2024

View reviewed changes

lilyinstarlight merged commit 87ceeab into NixOS:master Jul 2, 2024
23 checks passed

lilyball deleted the update-macvim branch July 2, 2024 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

macvim: 8.2.3455 -> 178 #260094

macvim: 8.2.3455 -> 178 #260094

lilyball commented Oct 9, 2023 •

edited

Loading

nixos-discourse commented Oct 20, 2023

lilyball commented Jan 24, 2024

nixos-discourse commented Jan 24, 2024

millerjason commented Jan 31, 2024

SuperSandro2000 Feb 3, 2024

lilyball Mar 2, 2024

lilyball Jun 4, 2024

SuperSandro2000 Feb 3, 2024

lilyball Mar 2, 2024

SuperSandro2000 Jun 25, 2024

lilyball Jun 27, 2024

hraban commented Mar 26, 2024

LeSuisse commented Apr 2, 2024 •

edited

Loading

lilyball commented Apr 17, 2024

lilyball commented Jun 4, 2024

SuperSandro2000 Jun 25, 2024

SuperSandro2000 Jun 25, 2024

lilyball commented Jun 27, 2024

lilyinstarlight left a comment

		# Python3 3.9 or above - the binary release builds against 3.11 so we will too
		python3 = python311;

macvim: 8.2.3455 -> 178 #260094

macvim: 8.2.3455 -> 178 #260094

Conversation

lilyball commented Oct 9, 2023 • edited Loading

Description of changes

Things done

nixos-discourse commented Oct 20, 2023

lilyball commented Jan 24, 2024

nixos-discourse commented Jan 24, 2024

millerjason commented Jan 31, 2024

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

hraban commented Mar 26, 2024

LeSuisse commented Apr 2, 2024 • edited Loading

lilyball commented Apr 17, 2024

lilyball commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilyball commented Jun 27, 2024

lilyinstarlight left a comment

Choose a reason for hiding this comment

lilyball commented Oct 9, 2023 •

edited

Loading

LeSuisse commented Apr 2, 2024 •

edited

Loading