apple / pkl-go

Pkl bindings for the Go programming language
https://pkl-lang.org/go/current/index.html
Apache License 2.0
259 stars 24 forks source link

Fix relative project dir paths. #47

Closed eli-l closed 4 months ago

eli-l commented 5 months ago

Addresses https://github.com/apple/pkl-go/issues/48

Because of generator settings may fall back to default value (when installed globally) we can not rely on this value when building path from relative project dir.

This commit changes the behaviour of it: 1) When generator-settings.pkl is not present or is in default location (CWD) -> project dir is built from CWD, not settings dir 2) When settings path is overriden -> project dir is built from settings path, to ensure relative projectDir defined in the settings files accurately describe location.

eli-l commented 5 months ago

I doubt it will handle relative direction path properly when it is defined in setting file and is not under the same path with project.

This was initial reason this bug existed, to handle relative pathes from settings file.

Will test it later, though.

eli-l commented 5 months ago

@bioball
As I supposed this does not solve the problem for the initial case: when generator-settings.pkl is not present in the current dir or I am running pkl-gen-go as global binary (after go install).

Here is the way to reproduce it with this repository:

where .. equals parent in error below:

This leads to error r ––\nCannot find module file:///<parent>/pkl-go/codegen/awesomeProject1/config/PklProject.\n"}

As you see parent has pkl-go/codegen appended before the relative path, that was passed as --project-dir.

This is caused because projectDir flag is actually overrides within loadGeneratorSettings, as projectDir could also be defined in settings file.

Later on projectDir is constructed again from settings within newEvaluator

So proposed diff does not solve the initial problem.

eli-l commented 4 months ago

Hi @bioball Any thoughts on this?

bioball commented 4 months ago

@eli-l apologies for the late response! I've been focused on the next release, and this slipped through the cracks.

I followed the steps that you outlined (remove generator-settings.pkl, changed to another directory and run the globally installed code generator), but my solution still mostly works, albeit there is a nil deference error that needs to be fixed.

Steps:

  1. Apply the below diff
  2. Install the binary with go install cmd/pkl-gen-go/pkl-gen-go.go
  3. rm generator-settings.pkl && mkdir foo/ && cd foo/
  4. pkl-gen-go ../codegen/src/GeneratorSettings.pkl --project-dir ../codegen/src/ --generate-script ../codegen/src/Generator.pkl

If you run these five steps, you should see output that looks something like this:

Generating Go sources for module ../codegen/src/GeneratorSettings.pkl
Using custom generator script: /Users/danielchao/code/apple/pkl-go/codegen/src/Generator.pkl
/Users/danielchao/code/apple/pkl-go/foo/github.com/apple/pkl-go/cmd/pkl-gen-go/generatorsettings/init.pkl.go
/Users/danielchao/code/apple/pkl-go/foo/github.com/apple/pkl-go/cmd/pkl-gen-go/generatorsettings/GeneratorSettings.pkl.go

Here is the updated diff that avoids a nil dereference error:

diff --git a/cmd/pkl-gen-go/pkl-gen-go.go b/cmd/pkl-gen-go/pkl-gen-go.go
index 5f463cd..3a7b044 100644
--- a/cmd/pkl-gen-go/pkl-gen-go.go
+++ b/cmd/pkl-gen-go/pkl-gen-go.go
@@ -138,7 +138,7 @@ var Version = "development"

 func init() {
    info, ok := debug.ReadBuildInfo()
-   if !ok || info.Main.Version == "" || Version != "development" {
+   if !ok || info.Main.Version == "" || info.Main.Version == "(devel)" || Version != "development" {
        return
    }
    Version = strings.TrimPrefix(info.Main.Version, "v")
@@ -240,6 +240,12 @@ func init() {
    if err = flags.Parse(os.Args); err != nil && !errors.Is(err, pflag.ErrHelp) {
        panic(err)
    }
+   if projectDir != "" {
+       projectDir, err = filepath.Abs(projectDir)
+       if err != nil {
+           panic(err)
+       }
+   }
    settings, err = loadGeneratorSettings(generatorSettingsPath, projectDir)
    if err != nil {
        panic(err)
@@ -260,7 +266,10 @@ func init() {
        settings.AllowedResources = allowedResources
    }
    if projectDir != "" {
-       settings.ProjectDir = &projectDir
+       if settings.ProjectDir == nil {
+           settings.ProjectDir = new(string)
+       }
+       *settings.ProjectDir = projectDir
    }
    settings.DryRun = dryRun
 }