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

html/template: jstmpllitinterp godebug documented but a no-op? #66217

Closed
rsc opened this issue Mar 9, 2024 · 7 comments
Closed

html/template: jstmpllitinterp godebug documented but a no-op? #66217

rsc opened this issue Mar 9, 2024 · 7 comments
Labels
Documentation Issues describing a change to documentation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.23

Comments

@rsc
Copy link
Contributor

rsc commented Mar 9, 2024

There is a new jstmpllitinterp godebug but the corresponding monitoring
is missing. jstmpllitinterp.IncNonDefault needs to be called when
program behavior changes from the default due to an override godebug
setting. And not just whenever it is set, but whenever the setting
actually changes the program behavior, so only if a template literal
is accepted when it would normally be rejected.

See 'go doc internal/godebug' for more details.

All that said, it looks like jstmpllitinterp is completely ignored now?
If so, the package docs should be updated, since they say it has an effect.
Also the doc/godebug.md docs should be updated.

/cc @rolandshoemaker
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Mar 9, 2024
@rsc rsc added this to the Go1.23 milestone Mar 9, 2024
@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Mar 9, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/570275 mentions this issue: internal/godebugs: test for use of IncNonDefault

gopherbot pushed a commit that referenced this issue Mar 9, 2024
@rsc
internal/godebugs: test for use of IncNonDefault
74726de 
A few recent godebugs are missing IncNonDefault uses.
Test for that, so that people remember to do it.
Filed bugs for the missing ones.

For #66215.
For #66216.
For #66217.

Change-Id: Ia3fd10fd108e1b003bb30a8bc2f83995c768fab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/570275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
@cagedmantis
Copy link
Contributor

This issue was reviewed in the release meeting. Is the documentation change the only remaining item for this issue?

@rolandshoemaker
Copy link
Member

Yes, the only remaining change is the documentation one, I'll try to get this out this week.

@cherrymui
Copy link
Member

@rolandshoemaker this is a release blocker. Any update on this? Thanks.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/584117 mentions this issue: html: update jstmpllitinterp doc

@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label May 15, 2024
@cagedmantis
Copy link
Contributor

@rolandshoemaker Is there is status update on this issue? There is a CL that has been reviewed. This came up in the release meeting.

@rolandshoemaker
Copy link
Member

@cagedmantis sorry keep completely forgetting about this, submitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.23
Development

No branches or pull requests
6 participants
@rsc @cagedmantis @dmitshur @rolandshoemaker @gopherbot @cherrymui