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

kcl: refactor #317827

Merged
merged 3 commits into from
Jun 26, 2024
Merged

kcl: refactor #317827

merged 3 commits into from
Jun 26, 2024

Conversation

selfuryon
Copy link
Contributor

@selfuryon selfuryon commented Jun 6, 2024

Description of changes

Hello! Currently, kcl doesn't work properly

Example
$ nix shell nixpkgs#kcl-cli
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish

$ cat main.k 
hello = "KCL"

$  kcl run main.k
Init kcl runtime failed, path:  /home/syakovlev/go
Tip: Have you used a binary version of KCL in your PATH that is not consistent with the KCL Go SDK? You can upgrade or reduce the KCL version or delete the KCL in your PATH
If not, you can run `rm -r /home/syakovlev/go/bin` to fix this issue
panic: unexpected EOF: stderr = Could not start dynamically linked executable: kclvm_cli
NixOS cannot run dynamically linked executables intended for generic
linux environments out of the box. For more information, see:
https://nix.dev/permalink/stub-ld


goroutine 1 [running]:
kcl-lang.io/kcl-go/pkg/runtime.initRuntime(0xc00093f6d8?)
	kcl-lang.io/kcl-go@v0.8.5/pkg/runtime/init.go:62 +0x42f
kcl-lang.io/kcl-go/pkg/runtime.GetRuntime.func1()
	kcl-lang.io/kcl-go@v0.8.5/pkg/runtime/init.go:26 +0x15
sync.(*Once).doSlow(0x0?, 0x0?)
	sync/once.go:74 +0xc2
sync.(*Once).Do(...)
	sync/once.go:65
kcl-lang.io/kcl-go/pkg/runtime.GetRuntime()
	kcl-lang.io/kcl-go@v0.8.5/pkg/runtime/init.go:26 +0x2c
kcl-lang.io/kcl-go/pkg/service.NewKclvmServiceClient()
	kcl-lang.io/kcl-go@v0.8.5/pkg/service/client_kclvm_service.go:21 +0x13
kcl-lang.io/kcl-go/pkg/kcl.runWithHooks({0xc00093f890, 0x0, 0x0}, {0x5cae350, 0x1, 0x1}, {0xc00093faa8, 0x1, 0x1})
	kcl-lang.io/kcl-go@v0.8.5/pkg/kcl/api.go:477 +0x16e
kcl-lang.io/kcl-go/pkg/kcl.run(...)
	kcl-lang.io/kcl-go@v0.8.5/pkg/kcl/api.go:486
kcl-lang.io/kcl-go/pkg/kcl.RunWithOpts({0xc00093faa8?, 0xc00093faa8?, 0xc00093fa58?})
	kcl-lang.io/kcl-go@v0.8.5/pkg/kcl/api.go:373 +0x45
kcl-lang.io/kpm/pkg/api.RunWithOpt(0xc000b87200)
	kcl-lang.io/kpm@v0.8.5/pkg/api/kpm_run.go:89 +0x36a
kcl-lang.io/cli/pkg/options.(*RunOptions).Run(0xc0008f80e0)
	kcl-lang.io/cli/pkg/options/run.go:126 +0x1e9
kcl-lang.io/cli/cmd/kcl/commands.NewRunCmd.func1(0xc00058c400?, {0xc0005a8990?, 0x4?, 0x2263e4c?})
	kcl-lang.io/cli/cmd/kcl/commands/run.go:49 +0x5a
github.com/spf13/cobra.(*Command).execute(0xc00015e908, {0xc0005a8950, 0x1, 0x1})
	github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc00015e608)
	github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra@v1.8.0/command.go:1039
main.main()
	kcl-lang.io/cli/cmd/kcl/main.go:14 +0x1c

The core problem is that kcl is a Go program that is coupled with kclvm_cli binary to work with kcl itself. The dependency graph looks like this:
kcl -> kclvm_cli -> kclvm, where

  • kclvm is a library with KCL Language Implementation
  • kclvm_cli is a low-level cli for working
  • kcl is a wrapper around kclvm_cli which used to work with kcl files

So I did these on this PR:

  • Added kclvm and kclvm_cli packages (mostly got from here)
  • Rename kcl-cli to just kcl. I'm afraid that it would be super confusing to have three KCL-related packages where two of them have cli in their names. In all examples the documentation always references the binary as kcl, so I believe it's better to call the main package exactly kcl too.
  • Add shell completion to kcl package

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@Peefy take a look on that too, pls.

Add a 👍 reaction to pull requests you find important.

Close #310770

@selfuryon selfuryon force-pushed the chore/kcl-refactor branch 2 times, most recently from 1a6c067 to 7cc4b2e Compare June 8, 2024 23:17
@selfuryon
Copy link
Contributor Author

selfuryon commented Jun 10, 2024

dunno, why I have This PR does not cleanly list package outputs after merging, and internally the error:

error: path '/nix/store/hnaydb7kpmjq1hhds1flah08gwxw26cy-source.drv' is not valid

Locally kclvm is working...

@selfuryon
Copy link
Contributor Author

dunno, why I have This PR does not cleanly list package outputs after merging, and internally the error:

error: path '/nix/store/hnaydb7kpmjq1hhds1flah08gwxw26cy-source.drv' is not valid

Locally kclvm is working...

Looks like fixed after importing Cargo.lock directly to the package. Got the idea from here #284172 (comment)

@selfuryon selfuryon force-pushed the chore/kcl-refactor branch 2 times, most recently from f5ec6ca to edaf378 Compare June 11, 2024 09:22
@selfuryon
Copy link
Contributor Author

I leave only platforms.linux because kclvm can't be compiled on darwin platforms, and I don't have darwin to check or debug why.

@aldoborrero
Copy link
Contributor

@Mic92 @zimbatm can we have some love to this PR :)

@zimbatm zimbatm merged commit 7d1463e into NixOS:master Jun 26, 2024
25 checks passed
@selfuryon selfuryon deleted the chore/kcl-refactor branch June 26, 2024 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing completion for kcl-cli package
3 participants