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: add release note for rejection of non-ASCII import paths in module mode #43052

Closed
bcmills opened this issue Dec 7, 2020 · 10 comments
Closed
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Dec 7, 2020

In CL 251878 (for #29101), we tightened up the import-path check in module mode so that it now disallows all non-ASCII package import paths, not just non-ASCII module paths (see #29101 (comment)).

Although we believe the number of affected packages to be very small, we should add a release note for this change so that users can easily verify that the change was intentional (see #43035).

CC @jayconrod @matloob @maruel

@bcmills bcmills added Documentation NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Dec 7, 2020
@bcmills bcmills added this to the Go1.16 milestone Dec 7, 2020
@dmitshur dmitshur added the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Dec 10, 2020
@toothrot toothrot removed the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Dec 17, 2020
maruel added a commit to maruel/panicparse that referenced this issue Dec 18, 2020
This slightly change "panic utf8" output.

Package path with unicode is officially not supported in go1.16. It was
only mostly working before but with go1.16 it'll be hard broken.

golang/go#43052

Tested with go @ tag go1.16beta1. Confirmed the package do not compile
without this fix.
@jamie-digital
Copy link

jamie-digital commented Dec 20, 2020

This appears to disagree with the current language spec:

ImportPath       = string_lit .

Implementation restriction: A compiler may restrict ImportPaths to non-empty strings using only characters belonging to Unicode's L, M, N, P, and S general categories (the Graphic characters without spaces) and may also exclude the characters !"#$%&'()*,:;<=>?[]^`{|} and the Unicode replacement character U+FFFD.

I have a main package etld+1 that falls foul of this, but testing this in the playground indicates that + meets the requirements in the spec. Please could either the spec be updated to agree with the change, or the change adjusted to meet the spec? Many thanks.

@matloob matloob self-assigned this Jan 5, 2021
@matloob
Copy link
Contributor

matloob commented Jan 7, 2021

@jamie-digital You bring up a subtle issue: the changes that we made don't affect the compiler, but just the go command and its build system. And there are other restrictions that the build system places above those in the compiler. But I agree that it may be confusing that there are two different sets of restrictions.

@gopherbot
Copy link

Change https://golang.org/cl/282194 mentions this issue: doc/go1.16: add release note for disallowing non-ASCII import paths

@jamie-digital
Copy link

@matloob Thanks for the clarification, that makes sense. It would be really helpful to have a clear set of rules the go command will accept.

@jayconrod
Copy link
Contributor

@jamie-digital Module paths and versions is a list of restrictions on module paths. File path and size constraints is a list of restrictions on file and directory names within a module.

@jamie-digital
Copy link

@jayconrod Thanks, those are really useful references. It's worth noting that although golang.org/x/mod/module.CheckPath doesn't allow the '+', the Module paths and versions section does:

A module path must satisfy the following requirements:
...

  • Each path element is a non-empty string made of up ASCII letters, ASCII digits, and limited ASCII punctuation (+, -, ., _, and ~).

@bcmills
Copy link
Contributor Author

bcmills commented Jan 7, 2021

For + specifically see #31376, particularly #31376 (comment).

@bcmills
Copy link
Contributor Author

bcmills commented Jan 7, 2021

Probably we should remove + from the “Module paths and versions” documentation.

@gopherbot
Copy link

Change https://golang.org/cl/282512 mentions this issue: content/static/doc: remove "+" from allowed module path characters

@matloob
Copy link
Contributor

matloob commented Jan 7, 2021

@bcmills I sent CL 282512 to remove + from the "Module paths and versions" doc.

gopherbot pushed a commit to golang/website that referenced this issue Jan 8, 2021
Go 1.16 will no longer accept "+" as a character in a module or import
path. Amend the module docs to reflect that change.

Fixes golang/go#31376
For golang/go#43052

Change-Id: Ie0b58888cf5023c69f112dcc32137fc69af6c659
Reviewed-on: https://go-review.googlesource.com/c/website/+/282512
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Mar 27, 2022
@rsc rsc unassigned matloob Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

8 participants
@toothrot @jayconrod @dmitshur @bcmills @gopherbot @jamie-digital @matloob and others