macvim: 179 -> 181 by lilyball · Pull Request #454837 · NixOS/nixpkgs

lilyball · 2025-10-23T09:51:24Z

This fixes the various ways in which the MacVim package was broken, and then updates it to r181.

One of the changes here is Python support has been disabled by default, with an enablePython configuration option added to reenable it. Enabling Python also requires overriding the python3 input to fix the breakage (e.g. by disabling static libpython and symlinking in libpython${pythonVersion}.dylib into the correct location).

This also adds me back to the maintainer list and marks myself as maintainer of this package.

Changelogs:

Fixes #399964.

Things done

Add a 👍 reaction to pull requests you find important.

Xcode 26.0 sets `*_DEPLOYMENT_TARGET` env vars for all platforms in shell script build phases, which breaks invocations of clang from those phases, as they target the wrong platform.

We don't provide `eview` so we need to get rid of that manpage, and there's also a bad `man/man1/mvim.1` symlink that goes nowhere.

Pass `-headerpad_max_install_names` again. This is something we used to do, and then the flag got removed at some point. For some reason we need to provide it both to configure and to Xcode.

We need to resign the binary after running `install_name_tool` or it will have an invalid code signature.

MacVim currently fails to link against nixpkgs python because Xcode's clang doesn't understand the LLVM bitcode in the `libpython${pythonVersion}.a` static library. I'm not sure what this static library is for when `lib/libpython${pythonVersion}.dylib` exists, but I don't know of a way to tell MacVim to ignore it. Linking against python works if we rebuild python to skip the static library but that doesn't seem like a proper solution, so until a permanent solution is found, just disable python by default. Also mark macvim as no longer broken.

Updates macvim to r181, including bumping the Ruby version to 3.4 since that's what MacVim r181 is built against. There's still 3 CVEs in the `knownVulnerabilities` list that are newer than this release, but this is the latest MacVim release available right now.

lilyball · 2025-10-23T09:51:54Z

I'm tempted to re-add myself as maintainer for this package again, but I'd have to add myself back to the maintainer list first.

lilyball · 2025-10-23T10:01:35Z

I went ahead and added commits to put myself back into the maintainer list.

SuperSandro2000

otherwise LGTM

SuperSandro2000 · 2025-10-23T11:48:42Z

pkgs/applications/editors/vim/macvim.nix

+      --replace-warn " -L${stdenv.cc.libc}/lib" "" \
+      --replace-warn " -L${darwin.libunwind}/lib" "" \
+      --replace-warn " -L${libiconv}/lib" ""


Suggested change

--replace-warn " -L${stdenv.cc.libc}/lib" "" \

--replace-warn " -L${darwin.libunwind}/lib" "" \

--replace-warn " -L${libiconv}/lib" ""

--replace-fail " -L${stdenv.cc.libc}/lib" "" \

--replace-fail " -L${darwin.libunwind}/lib" "" \

--replace-fail " -L${libiconv}/lib" ""

we probably want to fail to build if anything about this changes

I basically don’t think these should be an issue at all these days. We do build our own libiconv, but I doubt it hurts anything – stdenv already propagates it, so it’s almost surely on the linker path even if we substitute this away. stdenv.cc.libc and darwin.libunwind are stub packages and we link against the system versions of those libraries using the stubs in the SDK. I can’t imagine these can’t just be removed without issue.

I just tweaked them to be --replace-warn since I noticed there was a warning about using bare --replace, but I didn't feel like checking if the replacements were actually still finding anything to replace (since this is a rather old hack and stuff has changed since then). I can try deleting them outright and seeing what happens, though if I'm going to end up actually switching over to the nixpkgs toolchain for building this (in a future PR) then this will all get replaced anyway.

SuperSandro2000 · 2025-10-23T11:49:29Z

pkgs/applications/editors/vim/macvim.nix

+  # Note: The shell script build phase in question uses /bin/zsh.
+  + ''
+    substituteInPlace src/MacVim/MacVim.xcodeproj/project.pbxproj \
+      --replace-fail 'make \' $'for x in ''${(k)parameters}; do if [[ $x = *_DEPLOYMENT_TARGET ]]; then [[ $x = MACOSX_DEPLOYMENT_TARGET ]] || unset $x; fi; done\nmake \\'


At some point I would recommend doing a patch instead 😅

SuperSandro2000 · 2025-10-23T11:49:39Z

pkgs/applications/editors/vim/macvim.nix

+  ++ optionals enablePython [
    "--enable-python3interp=dynamic"
+  ]
+  ++ [
    "--enable-perlinterp=dynamic"
    "--enable-rubyinterp=dynamic"
    "--enable-tclinterp=yes"
    "--without-local-dir"
    "--with-luajit"
    "--with-lua-prefix=${luajit}"
+  ]
+  ++ optionals enablePython [


Maybe reorder things to only have one condition

SuperSandro2000 · 2025-10-23T11:49:50Z

pkgs/applications/editors/vim/macvim.nix

  src = fetchFromGitHub {
    owner = "macvim-dev";
    repo = "macvim";
    rev = "release-${finalAttrs.version}";


Suggested change

rev = "release-${finalAttrs.version}";

tag = "release-${finalAttrs.version}";

emilazy

FWIW, I doubt that most of the contortions in this build are necessary these days, now that we have actual SDKs and modern Clang. I expect this can be made into a relatively normal package with at most making things like ibtool(1) available – which could be obtained either impurely or from our darwin.xcode package. That would also likely make it a lot less fragile with regards to the toolchain churn that has affected it a lot.

I won’t block on this, but I would strongly suggest trying it to increase future maintainability – the mixing of toolchains here is inherently fragile, and should be completely unnecessary. Our SDKs contain the same headers and stubs that the native toolchain links against, and all the frameworks are available. All we are missing is the proprietary tools for things like Interface Builder files and Metal – but those don’t depend on the compiler toolchain, so can be pulled into a build with fewer hacks than are used here currently.

emilazy · 2025-10-23T16:59:22Z

pkgs/applications/editors/vim/macvim.nix

+      --replace-warn " -L${stdenv.cc.libc}/lib" "" \
+      --replace-warn " -L${darwin.libunwind}/lib" "" \
+      --replace-warn " -L${libiconv}/lib" ""


I basically don’t think these should be an issue at all these days. We do build our own libiconv, but I doubt it hurts anything – stdenv already propagates it, so it’s almost surely on the linker path even if we substitute this away. stdenv.cc.libc and darwin.libunwind are stub packages and we link against the system versions of those libraries using the stubs in the SDK. I can’t imagine these can’t just be removed without issue.

emilazy · 2025-10-23T17:00:28Z

pkgs/applications/editors/vim/macvim.nix

-    python3
-  ];
+  ]
+  ++ optional enablePython python3;


Using the Nixpkgs toolchain would presumably fix this.

emilazy · 2025-10-23T17:01:26Z

pkgs/applications/editors/vim/macvim.nix

+  + optionalString stdenv.isAarch64 ''
+    # Resign the binary and set the linker-signed flag.
+    rcodesign sign --code-signature-flags linker-signed $exe


Is there a reason not to use autoSignDarwinBinariesHook here?

The answer is that I didn't know about that hook. That said, we only need to resign one binary, and it looks like that hook is going to check every single file to see if it needs a signature, so that's going to end up doing a lot more work for the same effect.

Fair enough. I’m surprised if install_name_tool is breaking the signature, as ours doesn’t, but maybe this is something weird with the Xcode toolchain being used here.

lilyball · 2025-10-23T19:27:21Z

I really need to look more into the nixpkgs toolchain changes. This PR came about by just trying to get the darn thing working again, but if we can actually swap over to the nixpkgs toolchain and still expose ibtool and have things work, that would be a lot better. At the moment I think my priority is "unbreak this package", but I'll experiment with the toolchain stuff as soon as I find the time. I already attempted to switch this over to xcbuild but there's some frustrating blockers there.

emilazy

This seems fine as an incremental improvement. I didn’t test the build, because it scares me.

xcbuild being inadequate could indeed be a problem. Swift Build includes a FOSS implementation of the Xcode build system which should solve this, but it’s waiting on @reckenrode to finish his Swift 6.2 work for 26.05 before it’ll be available for us. I am not sure if our SDK can be used with the Xcode xcodebuild(1); I think in theory it should be possible, but IIRC Randy had to adjust the SDK to work with Swift Build, so there may be annoying complications. Still, it’s probably worth trying, or even just patching around xcbuild issues for now; I doubt the contortions for that could be worse than the ones this currently has to use. Otherwise, Swift Build should do the trick.

FWIW, it looks like the r181.2 prerelease isn’t affected by any of the remaining vulnerabilities, so we could probably bump to that.

emilazy · 2025-10-23T22:08:12Z

pkgs/applications/editors/vim/macvim.nix

+  + optionalString stdenv.isAarch64 ''
+    # Resign the binary and set the linker-signed flag.
+    rcodesign sign --code-signature-flags linker-signed $exe


Fair enough. I’m surprised if install_name_tool is breaking the signature, as ours doesn’t, but maybe this is something weird with the Xcode toolchain being used here.

lilyball added 6 commits October 23, 2025 02:42

macvim: work around Xcode 26.0 bug

636104f

Xcode 26.0 sets `*_DEPLOYMENT_TARGET` env vars for all platforms in shell script build phases, which breaks invocations of clang from those phases, as they target the wrong platform.

macvim: delete incorrect manpages

427a98e

We don't provide `eview` so we need to get rid of that manpage, and there's also a bad `man/man1/mvim.1` symlink that goes nowhere.

macvim: ensure we can change install names

3693c2d

Pass `-headerpad_max_install_names` again. This is something we used to do, and then the flag got removed at some point. For some reason we need to provide it both to configure and to Xcode.

macvim: resign the binary on aarch64-darwin

f7c3c92

We need to resign the binary after running `install_name_tool` or it will have an invalid code signature.

macvim: 179 -> 181

5ece9f3

Updates macvim to r181, including bumping the Ruby version to 3.4 since that's what MacVim r181 is built against. There's still 3 CVEs in the `knownVulnerabilities` list that are newer than this release, but this is the latest MacVim release available right now.

lilyball mentioned this pull request Oct 23, 2025

macvim: fix CVE list #454720

Closed

13 tasks

lilyball added 2 commits October 23, 2025 03:00

maintainers: add lilyball

8b45a0a

macvim: add lilyball as maintainer

f20cb87

nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 23, 2025

nixpkgs-ci bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Oct 23, 2025

SuperSandro2000 reviewed Oct 23, 2025

View reviewed changes

nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 23, 2025

emilazy reviewed Oct 23, 2025

View reviewed changes

emilazy approved these changes Oct 23, 2025

View reviewed changes

emilazy added this pull request to the merge queue Oct 23, 2025

Merged via the queue into NixOS:master with commit 7ae51e0 Oct 23, 2025
28 of 32 checks passed

lilyball deleted the push-rknvkztwnpqy branch October 24, 2025 02:07

	rev = "release-${finalAttrs.version}";
	tag = "release-${finalAttrs.version}";

Uh oh!

Conversation

lilyball commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

lilyball commented Oct 23, 2025

Uh oh!

lilyball commented Oct 23, 2025

Uh oh!

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilazy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilyball commented Oct 23, 2025

Uh oh!

emilazy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

lilyball commented Oct 23, 2025 •

edited

Loading