kcl-lang/kcl-go

Request: allow disabling shared lib download behavior at build time

Closed this issue · 8 comments

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:

  • Turn UseKCLPluginEnvVar, LibHomeEnvVar and DisableArtifactEnvVar into actual bool values to be set at build time, instead of variable names intended for runtime usage.
  • Allow more flexible fine-tuning of the shared lib location. It should be {configurable_prefix}/{configurable_path}/<shared_lib_file> (usually /usr/lib) instead of {libHome}/bin/<shared_lib_file>

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. ❤️

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

#259

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:

libPath := filepath.Join(root, "bin", fullLibName)
if !utils.FileExists(libPath) {
libPath = filepath.Join(root, "lib", fullLibName)
}
and
if env.GetDisableUseArtifactInPath() {
// Must use the artifact installed by kcl-go
// Get the install lib path.
g_KclvmRoot = findInstalledArtifactRoot()
} else {
// Get kclvm root path in PATH
g_KclvmRoot = findKclvmRoot()
}
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.

I see. I will open another PR to handle variable injection for the build time. The PR is here #267

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 😄

Sorry, I forgot to set them as string fields. #270

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.

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.