Skip to content

Name Migration: Build the deprecation-warning 'main' binary every time #8404

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

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

HanClinto
Copy link
Collaborator

Summary

To help with tutorials and other situations, we will now always build the deprecation-warning main binaries to alert users when they are using an outdated filename.

Previously, this was only being built if a legacy main binary was detected in the build folder, and now we will build it every time.

Detailed Explanation

This PR attempts to better-resolve the issues that @gpacix raised in #8397. Some of the name-migration pain has already been mitigated through #8257 and #8283, and this PR attempts to build on the good things in those PRs and take care of some additional edge cases.

#8257 ensured that legacy binaries were removed when make clean is run.

#8283 added friendly deprecation-warning binaries to alert users if they were attempting to run a binary that was no longer supported.

(base) ➜  llama.cpp git:(legacy-binaries) ✗ ./main

WARNING: The binary 'main' is deprecated.
 Please use 'llama-cli' instead.
 See https://github.com/ggerganov/llama.cpp/tree/master/examples/deprecation-warning/README.md for more information.

However, to keep from cluttering up the build space, we only built these deprecation-warning binaries when a previously detected binary is present. In hindsight, we maybe shouldn't have been so cautious.

As pointed out in #8397, it is very easy for users to follow tutorial instructions, and run into a bit of a brick wall when they attempt to make and run ./main and nothing is present.

This PR removes the makefile's filename check for the main target only, so that the deprecation-warning main target is always built, and it always outputs the name migration warning message to the build log.

Hopefully both of these things combines will help users who are following outdated tutorials (even if they're 3rd party tutorials that we are not able to update) and get them back on the right track with minimal hassle.

…tead of only when a legacy binary is present. This is to help users of tutorials and other instruction sets from knowing what to do when the 'main' binary is missing and they are trying to follow instructions.
@HanClinto
Copy link
Collaborator Author

Modified the server target to build all the time as well, since main and server are the two most common targets that will be found in tutorials.

@HanClinto HanClinto merged commit dd07a12 into ggml-org:master Jul 10, 2024
53 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
ggml-org#8404)

* Modify the deprecation-warning 'main' binary to build every time, instead of only when a legacy binary is present. This is to help users of tutorials and other instruction sets from knowing what to do when the 'main' binary is missing and they are trying to follow instructions.

* Adjusting 'server' name-deprecation binary to build all the time, similar to the 'main' legacy name binary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants