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

un-ignore build-info.cmake and build-info.sh #7996

Merged
merged 5 commits into from
Jun 19, 2024

Conversation

mdegans
Copy link
Contributor

@mdegans mdegans commented Jun 18, 2024

If they are ignored it can break some build tools that consider if a file is ignored, even if it's committed, it should not exist at all. I am assuming ignoring it was a mistake.

mdegans added 2 commits June 18, 2024 14:53
I am assuming that ignoring them was unintentional. If they are ignored, some tools, like cargo, will consider the files inexistent, even if they're comitted, for the purpose of publishing. This leads to the build failing in such cases.
For the same reason as the previous two files.
@slaren
Copy link
Collaborator

slaren commented Jun 18, 2024

There are a few more files affected, although they are not directly used in the build:

$ git ls-files -ci --exclude-standard                                                                                                                          
.clang-tidy
.github/workflows/build.yml
common/build-info.cpp.in
examples/chat-13B.bat
examples/llama.android/app/build.gradle.kts
examples/llama.android/build.gradle.kts
examples/llama.android/llama/build.gradle.kts
examples/llama.swiftui/llama.swiftui.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
examples/llava/android/build_64.sh
examples/sycl/build.sh
examples/sycl/win-build-sycl.bat
examples/sycl/win-run-llama2.bat
models/.editorconfig
models/ggml-vocab-aquila.gguf
models/ggml-vocab-baichuan.gguf
models/ggml-vocab-bert-bge.gguf
models/ggml-vocab-bert-bge.gguf.inp
models/ggml-vocab-bert-bge.gguf.out
models/ggml-vocab-command-r.gguf
models/ggml-vocab-command-r.gguf.inp
models/ggml-vocab-command-r.gguf.out
models/ggml-vocab-deepseek-coder.gguf
models/ggml-vocab-deepseek-coder.gguf.inp
models/ggml-vocab-deepseek-coder.gguf.out
models/ggml-vocab-deepseek-llm.gguf
models/ggml-vocab-deepseek-llm.gguf.inp
models/ggml-vocab-deepseek-llm.gguf.out
models/ggml-vocab-falcon.gguf
models/ggml-vocab-falcon.gguf.inp
models/ggml-vocab-falcon.gguf.out
models/ggml-vocab-gpt-2.gguf
models/ggml-vocab-gpt-2.gguf.inp
models/ggml-vocab-gpt-2.gguf.out
models/ggml-vocab-gpt-neox.gguf
models/ggml-vocab-gpt2.gguf
models/ggml-vocab-llama-bpe.gguf
models/ggml-vocab-llama-bpe.gguf.inp
models/ggml-vocab-llama-bpe.gguf.out
models/ggml-vocab-llama-spm.gguf
models/ggml-vocab-llama-spm.gguf.inp
models/ggml-vocab-llama-spm.gguf.out
models/ggml-vocab-mpt.gguf
models/ggml-vocab-mpt.gguf.inp
models/ggml-vocab-mpt.gguf.out
models/ggml-vocab-phi-3.gguf
models/ggml-vocab-phi-3.gguf.inp
models/ggml-vocab-phi-3.gguf.out
models/ggml-vocab-qwen2.gguf
models/ggml-vocab-qwen2.gguf.inp
models/ggml-vocab-qwen2.gguf.out
models/ggml-vocab-refact.gguf
models/ggml-vocab-refact.gguf.inp
models/ggml-vocab-refact.gguf.out
models/ggml-vocab-stablelm.gguf
models/ggml-vocab-starcoder.gguf
models/ggml-vocab-starcoder.gguf.inp
models/ggml-vocab-starcoder.gguf.out
scripts/build-info.cmake
scripts/build-info.sh
scripts/install-oneapi.bat

* Add exceptions for files mentioned by @slaren

I did leave .clang-tidy since it was explicitly ignored before.

* Add comments for organization
* Sort some lines for pretty
* Test with `make` and `cmake` builds to ensure no build artifacts might be comitted
@mdegans
Copy link
Contributor Author

mdegans commented Jun 19, 2024

I don't know if it was intentional to ignore models/.editorconfig given .clang-tidy was explicitly ignored. I'm assuming so, but I'll wait for feedback before committing again.

@slaren
Copy link
Collaborator

slaren commented Jun 19, 2024

I don't think that .clang-tidy should be in .gitignore. It looks like it was added by @ggerganov in 7ad7750.

@ggerganov Do you remember why?

@ggerganov
Copy link
Owner

I used to disable some of the warnings locally, but not anymore. It's fine to unignore

@mdegans
Copy link
Contributor Author

mdegans commented Jun 19, 2024

I used to disable some of the warnings locally, but not anymore. It's fine to unignore

'tis done.

Although on my mac this is still ignored

% git ls-files -ci --exclude-standard
examples/llama.swiftui/llama.swiftui.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist

Despite this line in .gitignore

!IDEWorkspaceChecks.plist

Not sure what's going on there. I'm on macOS so there may be something Apple-specific going on. There's nothing in my ~/gitignore_global.

Edit: Nevermind. I'm silly. There was a nested .gitignore which excluded xcshareddata I've edited that. According to this so, "xcshareddata should be added to the repo"

I don't use XCode. I'm assuming this is correct?

@slaren
Copy link
Collaborator

slaren commented Jun 19, 2024

It's because it is being overridden by examples/llama.swiftui/.gitignore. My guess is that it should be deleted.

@slaren
Copy link
Collaborator

slaren commented Jun 19, 2024

What I meant is that xcshareddata and IDEWorkspaceChecks.plist should be removed from the repository. I don't know what this file does, but that .gitignore was probably automatically generated by some tool and should be respected.

@mdegans
Copy link
Contributor Author

mdegans commented Jun 19, 2024

It's because it is being overridden by examples/llama.swiftui/.gitignore. My guess is that it should be deleted.

I have used XCode like twice in my life, and only to build something. I've no idea other than what I found on SO pertaining to xcsharedata.

@mdegans
Copy link
Contributor Author

mdegans commented Jun 19, 2024

What I meant is that xcshareddata and IDEWorkspaceChecks.plist should be removed from the repository. I don't know what this file does, but that .gitignore was probably automatically generated by some tool and should be respected.

Sounds good. I'll add it back.

@slaren slaren merged commit a785474 into ggerganov:master Jun 19, 2024
18 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jun 29, 2024
* un-ignore `build-info.cmake` and `build-info.sh`

I am assuming that ignoring them was unintentional. If they are ignored, some tools, like cargo, will consider the files inexistent, even if they're comitted, for the purpose of publishing. This leads to the build failing in such cases.

* un-ignore `build-info.cpp.in`

For the same reason as the previous two files.

* Reorganize `.gitignore`

* Add exceptions for files mentioned by @slaren

I did leave .clang-tidy since it was explicitly ignored before.

* Add comments for organization
* Sort some lines for pretty
* Test with `make` and `cmake` builds to ensure no build artifacts might be comitted

* Remove `.clang-tidy` from `.gitignore`

Per comment by @ggerganov

* Remove `IDEWorkspaceChecks.plist` from root-level `.gitignore`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants