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.

Sign up for GitHub

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

Jump to bottom

MacVim does not properly copy runtime files to the app bundle #1417

Closed
ychin opened this issue Jul 10, 2023 · 0 comments · Fixed by #1430
Closed

MacVim does not properly copy runtime files to the app bundle #1417

ychin opened this issue Jul 10, 2023 · 0 comments · Fixed by #1430
Milestone
Release 178

Comments

@ychin
Copy link
Member

ychin commented Jul 10, 2023 •
edited

Currently MacVim does not properly install the Vim runtime files to the app bundle. It simply grabs the runtime folder and copy it over but that folder contains files that aren't for deploying to the target runtime folder. In regular Vim installation, Vim's make install step is in charge of the logic to only copy the necessary files over to runtime and also copy some others to other locations like man pages, and the testdir/ files in syntax and indent are not installed anywhere.

We should fix the script to simply use make install instead (and do whatever changes we need if we don't want certain files). This prevents copying over unnecessary files.

Note: #1418 fixed part of this by manually cleaning up some files (mostly the testdir ones) but there are quite a few others, and we should not manually clean them up like this.

One design decision is how to handle things like man pages and so on. Currently MacVim does not install any of that even for say Homebrew builds. We could expose an option to explicit install it. This is similar to #1404 which proposes exposing a functionality to install the mvim CLI tool to PATH.
@ychin ychin added this to the Release 178 milestone Jul 10, 2023
ychin added a commit to ychin/macvim that referenced this issue Jul 10, 2023
@ychin
Clean up runtime dir after copying them to the app bundle
0222294 
This cleans up some glaring ones like the syntax/indent's testdir
folders in the runtime directory. The real issue is that MacVim just
blindly copies the runtime folder to the app bundler but they should
really be "installed" using the `make install` command which properly
copies the necessary files only. See macvim-dev#1417 which will fix this.
@ychin ychin mentioned this issue Jul 10, 2023
Merged
ychin added a commit to ychin/macvim that referenced this issue Sep 11, 2023
@ychin
Install Vim runtime to the MacVim app bundle properly
556a274 
Previously, MacVim's build process simply used an Xcode post-build
script to copy the runtime folder to the target app bundle's
Contents/Resources/vim/runtime folder and called it a day. However,
that's actually not the correct procedure because the runtime folder
contains misc files (e.g. Makefile/testdir for testing) and they should
go through a install step using `make install` to properly deploy the
proper files to the target folder.

Fix this by changing the post-build script to call the relevant make
targets instead of just blindly copying it over. We still copy the
vim/view/etc and vimtutor binaries separately for now, because the mvim
script is a custom script for MacVim, although that could change in
future.

One of the corollary of using the builtin installation scripts is that
man pages (for CLI vim/gvim usage) are now properly generated. They are
now stored under `MacVim.app/Contents/man`, and a user can set MANPATH to
it if they so wish.

Another corollary is that we now bundle xxd with MacVim like most Vim
distributions. It was probably an oversight before, and now it's built
and bundled in the `MacVim.app/Contents/bin` folder like the
vim/view/mvim scripts.

One annoying thing with Xcode is that in order for incremental builds to
work properly we want it to only run this installation step if the
runtime folder has changed (it takes a couple secs to finish) and
Xcode's input file lists doesn't support recursive folder search. To fix
this, add a build step to manually generate the list of all runtime
folders called runtime_folder_list.xcfilelist which we pass to the build
step.

Fix macvim-dev#1417
@ychin ychin mentioned this issue Sep 11, 2023
Merged
ychin added a commit to ychin/macvim that referenced this issue Sep 11, 2023
@ychin
Install Vim runtime to the MacVim app bundle properly
438c94e 
Previously, MacVim's build process simply used an Xcode post-build
script to copy the runtime folder to the target app bundle's
Contents/Resources/vim/runtime folder and called it a day. However,
that's actually not the correct procedure because the runtime folder
contains misc files (e.g. Makefile/testdir for testing) and they should
go through a install step using `make install` to properly deploy the
proper files to the target folder.

Fix this by changing the post-build script to call the relevant make
targets instead of just blindly copying it over. We still copy the
vim/view/etc and vimtutor binaries separately for now, because the mvim
script is a custom script for MacVim, although that could change in
future.

One of the corollary of using the builtin installation scripts is that
man pages (for CLI vim/gvim usage) are now properly generated. They are
now stored under `MacVim.app/Contents/man`, and a user can set MANPATH to
it if they so wish.

Another corollary is that we now bundle xxd with MacVim like most Vim
distributions. It was probably an oversight before, and now it's built
and bundled in the `MacVim.app/Contents/bin` folder like the
vim/view/mvim scripts.

One annoying thing with Xcode is that in order for incremental builds to
work properly we want it to only run this installation step if the
runtime folder has changed (it takes a couple secs to finish) and
Xcode's input file lists doesn't support recursive folder search. To fix
this, add a build step to manually generate the list of all runtime
folders called runtime_folder_list.xcfilelist which we pass to the build
step.

Fix macvim-dev#1417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 178
Development

Successfully merging a pull request may close this issue.

Install Vim runtime to the MacVim app bundle properly ychin/macvim
1 participant
@ychin