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.

Sign up for GitHub

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

Jump to bottom

CUE silently ignores undefined tool builtins #1581

Closed
mxey opened this issue Mar 14, 2022 · 6 comments
Closed

CUE silently ignores undefined tool builtins #1581

mxey opened this issue Mar 14, 2022 · 6 comments
Labels
NeedsFix

Comments

@mxey
Copy link
Contributor

mxey commented Mar 14, 2022

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

$ cue version
cue version v0.4.2 darwin/amd64

Does this issue reproduce with the latest release?

v0.4.2 is the latest release.

What did you do?

testscript <<EOF
exec cue cmd build

-- build_tool.cue --
package foo

import "tool/file"

command: build: {
	task: missing: file.SomethingThatDoesNotExist & {
	}
}
EOF

What did you expect to see?

An error

What did you see instead?

PASS
@mxey mxey added NeedsInvestigation Triage Requires triage/attention labels Mar 14, 2022
@verdverm
Copy link

verdverm commented Mar 14, 2022 •
edited
Loading

This is more generally about imported packages still being open. @mpvl mentioned that there probably is not a good reason for this, but it remains an open issue. (which I cannot find a.t.m.)

That is to say this error / issue happens with regular CUE files / packages as well.

@eonpatapon
Copy link
Contributor

Probably dup of #629

@mpvl mpvl added this to the v0.4.x milestone Mar 22, 2022
@mpvl
Copy link
Member

mpvl commented Mar 22, 2022

It is indeed related to #629 , but in this case it should indeed still give an "incomplete" error, as the error is not coalesced away by a disjunction.

@kcburge kcburge mentioned this issue May 12, 2022
Closed
@myitcv myitcv added the zVerify CUE team to check if fixed or not label Apr 27, 2023
@myitcv myitcv removed this from the v0.4.x milestone Apr 27, 2023
@myitcv myitcv added the zGarden label Jun 13, 2023
@mvdan
Copy link
Member

mvdan commented Jun 13, 2023

Still appears to reproduce as of 12908b3.

@mvdan mvdan removed the zVerify CUE team to check if fixed or not label Jun 13, 2023
@mvdan mvdan removed the zGarden label Feb 8, 2024
cueckoo pushed a commit that referenced this issue Feb 24, 2024
@nickfiggins
tools/flow: check and return errors from task values
d9722d4 
Currently if an identifier doesn't exist within a _tool.cue file,
it's ignored and no error is thrown.

This updates the logic in the flow controller to check and return
errors from task values during initialization.

Fixes #1581.

Change-Id: Iaf1a6262b7f44b07525b9e56079b804047dada9d
Signed-off-by: Nick Figgins <figginsn@gmail.com>
cueckoo pushed a commit that referenced this issue Feb 24, 2024
@nickfiggins
tools/flow: check and return errors from task values
c8a8e08 
Currently if an identifier doesn't exist within a _tool.cue file,
it's ignored and no error is thrown.

This updates the logic in the flow controller to check and return
errors from task values during initialization.

Fixes #1581.

Change-Id: Iaf1a6262b7f44b07525b9e56079b804047dada9d
Signed-off-by: Nick Figgins <figginsn@gmail.com>
cueckoo pushed a commit that referenced this issue Feb 25, 2024
@nickfiggins
tools/flow: check and return errors from task values
92dc8e2 
Currently if an identifier doesn't exist within a _tool.cue file,
it's ignored and no error is thrown.

This updates the logic in the flow controller to check and return
errors from task values during initialization.

Fixes #1581.

Change-Id: Iaf1a6262b7f44b07525b9e56079b804047dada9d
Signed-off-by: Nick Figgins <figginsn@gmail.com>
@mvdan
Copy link
Member

mvdan commented Feb 26, 2024

@nickfiggins thanks for the fix! It's already paid for itself - it spotted a bug in our release script :) https://review.gerrithub.io/c/cue-lang/cue/+/1177345

cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
89e41af 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and updates
the `issue2517` test case so that it illustrates the way that
the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
60757af 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and updates
the `issue2517` test case so that it illustrates the way that
the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
f44bc3d 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and updates
the `issue2517` test case so that it illustrates the way that
the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
c025a43 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and updates
the `issue2517` test case so that it illustrates the way that
the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

For #2965.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
9bb29ef 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and adds
a new test case (very similar to the test case for #2517)
that illustrates the way that the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

For #2965.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@rogpeppe
Revert "tools/flow: check and return errors from task values"
ea385fd 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and adds
a new test case (very similar to the test case for #2517)
that illustrates the way that the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

For #2965.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1185281
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo pushed a commit that referenced this issue Mar 20, 2024
@nickfiggins
tools/flow: return permanent errors from tasks
3b46bfe 
This change updates the task controller to return
errors from tasks which are classified as permanent.

It also updates the logic in evaluator to classify errors
as permanent one stage earlier in the evaluation progress
in some cases, to align with the classification elsewhere.

Fixes #1581.

Signed-off-by: Nick Figgins <figginsn@gmail.com>
Change-Id: I7fe0b362bb1b27db7226bdda9698b0d6e6cf08ae
cueckoo pushed a commit that referenced this issue Mar 21, 2024
@nickfiggins
tools/flow: return permanent errors from tasks
5056d87 
This change updates the task controller to return
errors from tasks which are classified as permanent.

It also updates the logic in evaluator to classify errors
as permanent one stage earlier in the evaluation progress
in some cases, to align with the classification elsewhere.

Fixes #1581.

Signed-off-by: Nick Figgins <figginsn@gmail.com>
Change-Id: I7fe0b362bb1b27db7226bdda9698b0d6e6cf08ae
cueckoo pushed a commit that referenced this issue Mar 21, 2024
@nickfiggins
tools/flow: return permanent errors from tasks
72e1946 
This change updates the task controller to return
errors from tasks which are classified as permanent.

It also updates the logic in evaluator to classify errors
as permanent one stage earlier in the evaluation progress
in some cases, to align with the classification elsewhere.

Fixes #1581.

Signed-off-by: Nick Figgins <figginsn@gmail.com>
Change-Id: I7fe0b362bb1b27db7226bdda9698b0d6e6cf08ae
@rogpeppe rogpeppe mentioned this issue Mar 25, 2024
Closed
@mvdan
Copy link
Member

mvdan commented Mar 25, 2024

@rogpeppe reverted the fix for this issue in https://review.gerrithub.io/c/cue-lang/cue/+/1185281, so we need to reopen this issue now to track a second and better fix.

@nickfiggins already marked his new CL (https://review.gerrithub.io/c/cue-lang/cue/+/1185458) as a "fixes" of this issue, so that's already the way we want it.

@mvdan mvdan reopened this Mar 25, 2024
cueckoo pushed a commit that referenced this issue Mar 26, 2024
@nickfiggins
tools/flow: return permanent errors from tasks
e6e42af 
Previously, all errors from tasks were returned
which led to inappropriate errors when encountering
an incomplete value that wasn't a part of a task. Due
to this issue, the previous fix for #1581 was reverted.

This change updates the task controller to return
errors again, however, we now only return errors that
are classified as permanent to prevent inappropriate
errors.

Fixes #1581.

Signed-off-by: Nick Figgins <figginsn@gmail.com>
Change-Id: I13de67b2a71fd4f7dab027cc3f84e4fc6ff0b077
cueckoo pushed a commit that referenced this issue Mar 26, 2024
@nickfiggins
tools/flow: return permanent errors from tasks
96b046b 
Previously, all errors from tasks were returned
which led to inappropriate errors when encountering
an incomplete value that wasn't a part of a task. Due
to this issue, the previous fix for #1581 was reverted.

This change updates the task controller to return
errors again, however, we now only return errors that
are classified as permanent to prevent inappropriate
errors.

Fixes #1581.

Signed-off-by: Nick Figgins <figginsn@gmail.com>
Change-Id: I30f1b04e3d9c585b47a543f0accefd042da4cd7d
cueckoo pushed a commit that referenced this issue Apr 3, 2024
@rogpeppe @mvdan
Revert "tools/flow: check and return errors from task values"
a55477e 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and adds
a new test case (very similar to the test case for #2517)
that illustrates the way that the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

For #2965.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1185281
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
(cherry picked from commit ea385fd)
cueckoo pushed a commit that referenced this issue Apr 3, 2024
@rogpeppe @mvdan
Revert "tools/flow: check and return errors from task values"
0630a37 
This reverts commit 9ef35eb
but keeps the `cmd_undefined` test case and adds
a new test case (very similar to the test case for #2517)
that illustrates the way that the reverted commit fails.

That commit introduced a regression that caused existing
tools/flow uses to fail. See https://cuelang.org/issue/2965.

A subsequent CL will apply a different fix for #1581.

For #2965.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I322ac3b269bc79a541c15001a166160ad9191267
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1185281
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
(cherry picked from commit ea385fd)
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1190978
Reviewed-by: Paul Jolly <paul@myitcv.io>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mxey @eonpatapon @verdverm @myitcv @mvdan @mpvl