Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macvim: 7.4.909 -> 8.1.1517 #62934

Merged
merged 1 commit into from Jun 16, 2019

Conversation

@lilyball
Copy link
Member

commented Jun 10, 2019

Motivation for this change

Fix up the macvim package to build again, with the latest snapshot. The patchfile has been recreated by manually reapplying all of the changes from the old patchfile, and the other changes in here were figured out by trial and error (such as the need to unset LD).

Also tweak the package to use python37 by default, and add an option to go back to python27 if desired. I'm not actually sure what the most common python configuration is these days, so this was chosen mostly because python3 is supposed to be the future.

Disable Sparkle so the user isn't prompted to update a readonly package.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This build is impure. It relies on accessing a few tools from /usr/bin (including xcodebuild), and it sets the minimum deployment target to the current OS version. I tried patching the latter but that resulted in a bunch of warnings during the build process, so I left it alone. This impurity exists for the current broken package too, of course.

CC @cstrahan

@ofborg ofborg bot added the 6.topic: vim label Jun 10, 2019

@lilyball

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

I tested some basic functionality, such as :py3 print("hello") and :ruby print("hello"). I also verified that it builds with usePython27 set (and that :py print("hello") works there). I was somewhat tempted to just enable both python2 and python3 simultaneously, which apparently works with some limitations, but decided it was better not to.

@ofborg ofborg bot requested a review from cstrahan Jun 10, 2019

@lilyball lilyball force-pushed the lilyball:macvim branch 2 times, most recently from 86298b2 to 559a67f Jun 10, 2019

@lilyball lilyball changed the title macvim: 7.4.909 -> 8.1.950 macvim: 7.4.909 -> 8.1.1517 Jun 11, 2019

@lilyball

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Updated to the just-released snapshot-156 which includes a fix for the recent Vim CVE (relating to modelines).

macvim: 7.4.909 -> 8.1.1517
Fix up the macvim package to build again, with the latest snapshot. The
patchfile has been recreated by manually reapplying all of the changes
from the old patchfile, and the other changes in here were figured out
by trial and error (such as the need to unset `LD`).

Also tweak the package to use python37 by default, and add an option to
go back to python27 if desired.

Disable Sparkle so the user isn't prompted to update a readonly package.

@lilyball lilyball force-pushed the lilyball:macvim branch from 559a67f to 95bfb99 Jun 11, 2019

@lilyball

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

I've also gone ahead and added myself as a maintainer.

@nixos-discourse

This comment has been minimized.

Copy link

commented Jun 16, 2019

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/19

@matthewbauer matthewbauer merged commit 486626b into NixOS:master Jun 16, 2019

13 of 15 checks passed

macvim on aarch64-linux No attempt
Details
macvim on x86_64-linux No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
# Full path to xcodebuild
substituteInPlace src/Makefile --replace "xcodebuild" "/usr/bin/xcodebuild"
for nib in MainMenu Preferences; do
/usr/bin/ibtool --compile src/MacVim/English.lproj/$nib.nib/keyedobjects.nib src/MacVim/English.lproj/$nib.nib

This comment has been minimized.

Copy link
@matthewbauer

matthewbauer Jun 16, 2019

Member

This could cause issues when /usr/bin/ibtool doesn't exist. Probably we should follow the Hydra builds to see. I think a darwin.ibtool might be useful to give a nice error message when this doesn't exist.

This comment has been minimized.

Copy link
@lilyball

lilyball Jun 16, 2019

Author Member

What version of macOS does Hydra run on, and what's the minimum version of macOS that Nix supports? /usr/bin/ibtool should exist on any version of macOS from the past few years at least. I suppose this could be /usr/bin/xcrun ibtool instead if we need to support a version of macOS that predates the introduction of the full set of build tools into /usr/bin.

@lilyball lilyball deleted the lilyball:macvim branch Jun 16, 2019

@lilyball lilyball referenced this pull request Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.