kcl-lang / kcl-go

KCL Go SDK
https://pkg.go.dev/kcl-lang.io/kcl-go@main
Apache License 2.0
52 stars 26 forks source link

Request: allow disabling shared lib download behavior at build time #257

Closed folliehiyuki closed 7 months ago

folliehiyuki commented 8 months ago

Feature Request

Is your feature request related to a problem? Please describe:

I want to package KCL programs (the Go CLI and the Rust shared lib + LSP server) for the Linux distributions I use, namely AlpineLinux and NixOS. For a proper packaging workflow, I need the Go CLI not to arbitrarily download the pre-built Rust shared object, and instead rely on the one built by the distro at the defined location. The downloaded artifact doesn't work on AlpineLinux anyway, due to the usage of musl libc. To ensure a consistent user experience, stopping this behavior at build time is desired.

Looking through the code, the artifact download behavior and the lookup location can be configured at runtime via some environment variables: KCL_GO_DISABLE_ARTIFACT, KCL_LIB_HOME and KCL_GO_USE_PLUGIN. The common lib https://github.com/kcl-lang/lib/blob/main/install.go then puts the shared object at hardcoded subdirectory bin inside that home directory.

Describe the feature you'd like:

Peefy commented 8 months ago

A very good suggestion, do we need another issue to track the Alpine version of building the KCL Rust library?

I will try to implement it next week, but until then, PRs are very welcome. ❤️

Peefy commented 8 months ago

I opened a PR to preliminarily implement it, is this the way you want it?

https://github.com/kcl-lang/kcl-go/pull/259

folliehiyuki commented 7 months ago

Sorry for my misleading suggestion! What I meant by setting at build time is is to use -ldflags of go build command with -X option to inject the values of internal variables, which only supports string type.

Maybe something like this, I don't know:

var (
    LibHome                  string
    DisableInstallArtifact   string
    DisableUseArtifactInPath string
)

# then check whether the above variables are empty string, or already set by runtime env KCL_*

Minor comments: https://github.com/kcl-lang/kcl-go/blob/c67b1275bd0297938a35788826c5e2546671e719/pkg/native/loader.go#L27-L30 and https://github.com/kcl-lang/kcl-go/blob/c67b1275bd0297938a35788826c5e2546671e719/pkg/runtime/kclvm.go#L26-L33 feel kind of forceful, but they cover the common cases, so I don't mind.

Also, I think DisableUseArtifactInPath isn't really useful. Would downloading artifacts into the same directory as kcl for both MacOS and Linux make it simpler, and more consistent? Then we only need to require kclvm_cli is inside PATH.

Peefy commented 7 months ago

I see. I will open another PR to handle variable injection for the build time. The PR is here https://github.com/kcl-lang/kcl-go/pull/267

folliehiyuki commented 7 months ago

Sorry for another complaint! The new PR doesn't work. disableInstallArtifact and disableArtifactInPath are boolean, while -X flag only supports variables of string type (-ldflags "-X kcl-lang.io/kcl-go/pkg/env.disableArtifactInPath=true" won't affect the value). I can set libHome normally though, as it's ... string :smile:

Peefy commented 7 months ago

Sorry, I forgot to set them as string fields. https://github.com/kcl-lang/kcl-go/pull/270

folliehiyuki commented 7 months ago

Thanks for the work! I'll try building the latest tag this weekend alongside the Rust lib and give you the feedback when I'm done testing stuff.

folliehiyuki commented 7 months ago

Sorry for the delay! I've briefly tested the latest CLI version, and the variables seem to work correctly.

AlpineLinux is going through a major Python version bump, so I'm not able to build the Rust lib to test things thoroughly (at least for a week or so), but this issue can be closed. I'll open another issue on the main repo if a problem comes up.