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

cmd/go: consider erroring if go list -f requests a field that can't be available #51638

Open
mvdan opened this issue Mar 12, 2022 · 5 comments
Open
Labels
GoCommand NeedsDecision

Comments

@mvdan
Copy link
Member

@mvdan mvdan commented Mar 12, 2022

go version devel go1.19-c1f22134f2 Fri Mar 11 06:03:26 2022 +0000 linux/amd64

I just spent a couple of minutes wondering why this printed nothing:

go list -f {{.BuildID}}

I was of course forgetting the -export flag, which fills fields like BuildID.

Instead of silently using the field that will always be unset, it would be nice if cmd/go failed with an obvious message, like: did you forget to set the -export flag?

@mvdan mvdan added the NeedsDecision label Mar 12, 2022
@seankhliao seankhliao added the GoCommand label Mar 12, 2022
@robpike
Copy link
Contributor

@robpike robpike commented Mar 12, 2022

Is this a problem specific to BuildID alone? The field is available, it's just empty. Not sure why that's a bug in general.

% go list -f '{{.Asdf}}'
template: main:1:2: executing "main" at <.Asdf>: can't evaluate field Asdf in type *load.PackagePublic
% 

@mvdan
Copy link
Member Author

@mvdan mvdan commented Mar 12, 2022

Some fields are only set if certain flags are passed. I do understand the fields exist, but without the flag, they're always unset. From go help list:

        Export        string   // file containing export data (when using -export)
        BuildID       string   // build ID of the compiled package (when using -export)

You have the same situation with -compiled. If a user tries to use {{.BuildID}} but they didn't pass -export, that certainly sounds like a bug to me - I'm not sure what purpose it could have.

If we want to make this an error, one way could be to look at the template syntax tree to spot any references to fields that we know are unset.

@jimmyfrasche
Copy link
Member

@jimmyfrasche jimmyfrasche commented Mar 13, 2022

Would it suffice to set any fields like that with something informative when the flags are not present, like "<BuildID only available when using -export>"?

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 15, 2022

If we want to make this an error, one way could be to look at the template syntax tree to spot any references to fields that we know are unset.

I would rather not try to analyze the syntax tree directly, but I think it would be fine to convert these fields to methods that can throw an error at runtime.

@mvdan
Copy link
Member Author

@mvdan mvdan commented Mar 15, 2022

@jimmyfrasche that would have been enough for my use case. I only worry that "print gibberish and succeed" may not be as evident and helpful as "print an error and fail".

@bcmills my first thinking (on Slack) was indeed to swap these fields for methods that can return an error. @dominikh argued that that could potentially break some user templates, even though the syntax to use the field is the same as calling the method by the same name, and I haven't thought too hard about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand NeedsDecision
Projects
None yet
Development

No branches or pull requests

5 participants