feat(linux): improve dev setup for Linux platform#364
Conversation
- Add make setup-tokenizers target that downloads the pre-built libtokenizers.a from GitHub Releases using the version pinned in go.mod, auto-detecting Linux x86_64 or macOS ARM64/x86_64 — no Rust/cargo required. Inspired by src/scripts/build_linux.sh. - Fix ONNX Runtime install commands in dev guide to include Linux .so variants alongside the existing macOS .dylib commands - Replace outdated tokenizer build steps with make setup-tokenizers - Add troubleshooting entries for ruff not found and golangci-lint v2 Closes dataiku#362 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@Davidnet Do we need |
|
Thank you @s3bc40 for your PR. we'll review it on Monday and get back to you. |
| sudo systemctl status kiji-proxy | ||
| ``` | ||
|
|
||
| > **Note (Linux):** The binary resolves the data directory (`~/.kiji-proxy/`) via the home directory of the user it runs as. The `useradd -r` flag creates a system user with no home directory, which causes a startup failure. Create the home directory manually before starting the service: |
There was a problem hiding this comment.
Actually I was thinking that maybe we could use something like a DynamicUser:
[Service]
ExecStart=/opt/kiji-privacy-proxy/kiji-proxy
DynamicUser=yes
# This automatically creates /var/lib/kiji-proxy with correct permissions
StateDirectory=kiji-proxy
# Tell your app to use that directory
Environment=KIJI_DATA_PATH=/var/lib/kiji-proxy
and then we check for paths in this order:
-
A CLI flag (e.g., --data-dir)
-
An environment variable (e.g., KIJI_DATA_PATH)
-
XDG_DATA_HOME (Defaults to ~/.local/share if unset)
-
~/.kiji-proxy
-
then /var/lib/kiji-proxy
What would be your opinion, my idea will be to avoid creating a user folder in the system
There was a problem hiding this comment.
I see! I was not aware of the DynamicUser solution, and it's way better than manually adding everything in home dir. If we follow along XDG system while keeping potential legacy (of chosen location .kiji-proxy/), that would cover a lot of potential issues 👌
I was thinking for the implementation to update the AppDataDir() in the Go backend (path/path.go) if it fits.
Something like this if I understood:
func AppDataDir() string {
// macOS
if runtime.GOOS == "darwin" {
homeDir, _ := os.UserHomeDir()
return filepath.Join(homeDir, "Library", "Application Support", "Kiji Privacy Proxy")
}
// Linux
if p := os.Getenv("KIJI_DATA_PATH"); p != "" {
return p
}
if xdg := os.Getenv("XDG_DATA_HOME"); xdg != "" {
return filepath.Join(xdg, "kiji-proxy")
}
if homeDir, err := os.UserHomeDir(); err == nil {
return filepath.Join(homeDir, ".kiji-proxy")
}
return "/var/lib/kiji-proxy"
}(I'll seriously learn Go soon, to avoid any potential errors and understand the Go best practices)
|
Warning This PR touches 3+ distinct areas of the codebase. Consider splitting into smaller, focused PRs — each covering a single semantic type. Categories found: docs:
code:
chore:
test:
|
|
@hanneshapke & @Davidnet thanks for your feedback. I applied the following changes to make it more suitable to your needs. ###go list for tokenizers version (
|
|
Hi @s3bc40 I will be starting the review now |
| @@ -104,9 +102,13 @@ source .venv/bin/activate | |||
| # Install ONNX Runtime | |||
| pip install onnxruntime | |||
There was a problem hiding this comment.
Would you mind doing something like:
GO_VER=$(go list -m -f '{{.Version}}' github.com/yalue/onnxruntime_go | sed 's/^v//') && \
pip install "onnxruntime==${GO_VER}"
just to have the versions synced
There was a problem hiding this comment.
I will sync the macos in another branch
There was a problem hiding this comment.
I tried to run the command but it seems there is a difference between PyPi version and Go version of onnxruntime
Right now on PyPi it is v1.25.1 -> https://pypi.org/project/onnxruntime/
But Go runtime output:
-> % echo $GO_VER
1.27.0Which triggers the following mismatch:
-> % GO_VER=$(go list -m -f '{{.Version}}' github.com/yalue/onnxruntime_go | sed 's/^v//') && \
uv pip install "onnxruntime==${GO_VER}"
× No solution found when resolving dependencies:
╰─▶ Because there is no version of onnxruntime==1.27.0 and you require onnxruntime==1.27.0, we can
conclude that your requirements are unsatisfiable.Do you want me to run another or fix anything on my end ? Thanks for your review 👍
There was a problem hiding this comment.
Yeah it seems that the author is suggesting that we hardcode:
maybe then:
uv pip install "onnxruntime==1.25.0"
and we leave a link to the above discussion, let me know and then we can merge this,
Thanks for the PR.
There was a problem hiding this comment.
@Davidnet updated as requested ! I checked the version in each commands to set v1.25.0 and let a note about the ONNX runtime lib, with the link you sent me and this discussion for context.
Summary
Changes
Makefile: added 23 lines of Linux dev setup targets/commandsdocs/01-getting-started.md: updated with Linux platform guidance (+13/-7 lines)docs/02-development-guide.md: expanded Linux development instructions (+31/-14 lines)