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

cue/load: regression on load.Instances() with unrelated error message #3213

Closed
AntoineThebaud opened this issue Jun 11, 2024 · 1 comment
Closed
Labels
modules Issues related to CUE modules and the experimental implementation NeedsFix

Comments

@AntoineThebaud
Copy link

AntoineThebaud commented Jun 11, 2024

What version of CUE are you using (cue version)?

cuelang.org/go v0.9.0

Does this issue reproduce with the latest stable release?

It appeared with it. The issue didn't exist in the v0.9.0-alpha.5 we were using before.

What did you do?

I have the two following files under the same folder:

gauge.cue

package model

import (
	"github.com/perses/perses/cue/schemas/common"
)

kind: "GaugeChart"
spec: close({
	calculation: common.#calculation
	format?:     common.#format
	thresholds?: common.#thresholds
	max?:        number
})

migrate.cue

if #panel.type != _|_ if #panel.type == "gauge" {
	kind: "GaugeChart"
	spec: {
		// transformation logic
	}
},

Then, at some point in my program, I'm using the Go API of Cue to process the content of the model package only:

buildInstances := load.Instances([]string{}, &load.Config{Dir: schemaPath, Package: "model"})

(just in case, full code available here)

What did you expect to see?

I expect the load.Instances() call to work, just like it was in v0.9.0-alpha.5 & prior.

What did you see instead?

The first (& only as expected) build instance returned by load.Instances() reports an error:

import failed: [...]\\perses\\cue\\schemas\\panels\\gauge\\gauge.cue:17:2: no dependency found for package \"github.com/perses/perses/cue/schemas/common\"

The error message seems totally unrelated to the root cause here because if ever I delete the file migrate.cue that has no package clause (so that I expect to be ignored thanks to the provided load.Config), then the load.Instances() call works. Before latest release it was properly ignored.

@rogpeppe
Copy link
Member

Thanks for the report! I've got a fix in the pipeline for this. If you need a workaround in the meantime, the following code should work and be equivalent to your current code:

buildInstances := load.Instances([]string{".:model"}, &load.Config{Dir: schemaPath})

@rogpeppe rogpeppe added modules Issues related to CUE modules and the experimental implementation NeedsFix and removed NeedsInvestigation Triage Requires triage/attention labels Jun 11, 2024
cueckoo pushed a commit that referenced this issue Jun 12, 2024
Issue #3213 exposed a shortcoming in the modules
logic when there is a Package field explicitly specified
in the cue/load.Config struct. This CL adds some tests
to expose existing behavior in this respect
before a subsequent CL will apply a fix.

For #3213.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: If101f7b6729acdc87a9ae62ebd932044fb276e51
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196177
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
cueckoo pushed a commit that referenced this issue Jun 12, 2024
Issue #3213 exposed a shortcoming in the modules
logic when there is a Package field explicitly specified
in the cue/load.Config struct. This CL adds some tests
to expose existing behavior in this respect
before a subsequent CL will apply a fix.

For #3213.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: If101f7b6729acdc87a9ae62ebd932044fb276e51
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196177
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196231
Reviewed-by: Paul Jolly <paul@myitcv.io>
cueckoo pushed a commit that referenced this issue Jun 12, 2024
When a caller of `cue/load.Instances`, specifies an explicit package
to load by setting `Config.Package`, it should be essentially the
same as specifying an explicit package qualifier in all package arguments
that don't already have one. That's not the case currently. This change
fixes that.

Fixes #3213.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I740a16911340e083776303fb0e454b4533e66f2f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196178
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196232
Reviewed-by: Paul Jolly <paul@myitcv.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issues related to CUE modules and the experimental implementation NeedsFix
Projects
Archived in project
Status: v0.9.1
Development

No branches or pull requests

2 participants