golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.21k stars 17.7k forks source link

x/mobile/cmd/gomobile: fail to `gomobile build` as "not a valid go package name" #24511

Closed hajimehoshi closed 6 years ago

hajimehoshi commented 6 years ago

What version of Go are you using (go version)?

go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hajimehoshi/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hajimehoshi/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7t/qw3np69559591s1v0mk5_p1m0000gn/T/go-build549033053=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  1. Create $GOPATH/src/github.com/hajimehoshi/12345 (the last directory name is invalid as a package name)

  2. Create $GOPATH/github.com/hajimehoshi/12345/main.go like

package main

import _ "golang.org/x/mobile/app"

func main() {}
  1. Run gomobile build github.com/hajimehoshi/12345

What did you expect to see?

Build succeed

What did you see instead?

panic: package name "12345" is not a valid go package name

goroutine 1 [running]:
main.androidPkgName(0x7ffeefbff8a6, 0x5, 0x7ffeefbff8a6, 0x5)
        /Users/hajimehoshi/go/src/golang.org/x/mobile/cmd/gomobile/build_androidapp.go:296 +0xeeb
main.goAndroidBuild(0xc420188a80, 0xc42009d280, 0x4, 0x4, 0x0, 0x0, 0x0)
        /Users/hajimehoshi/go/src/golang.org/x/mobile/cmd/gomobile/build_androidapp.go:32 +0x139
main.runBuild(0x1672960, 0x0, 0x0)
        /Users/hajimehoshi/go/src/golang.org/x/mobile/cmd/gomobile/build.go:109 +0x4d4
main.main()
        /Users/hajimehoshi/go/src/golang.org/x/mobile/cmd/gomobile/main.go:73 +0x34e
AlexRouSg commented 6 years ago

https://developer.android.com/guide/topics/manifest/manifest-element.html#package

The name may contain uppercase or lowercase letters ('A' through 'Z'), numbers, and underscores ('_'). However, individual package name parts may only start with letters.

You cannot have a package named 12345 on android.

hajimehoshi commented 6 years ago

The package name for Android is automatically generated by gomobile build based on the Go package path, and I don't think this is gomobile user's responsibility.

AlexRouSg commented 6 years ago

Sorry, I tried to compile a normal go package named 12345 and that failed too. So this isn't related to gomobile.

From reading the spec https://golang.org/ref/spec it described that package names must start with a letter.

A package clause begins each source file and defines the package to which the file belongs.

PackageClause = "package" PackageName . PackageName = identifier .

Identifiers name program entities such as variables and types. An identifier is a sequence of one or more letters and digits. The first character in an identifier must be a letter.

identifier = letter { letter | unicode_digit } .

hajimehoshi commented 6 years ago

Package path github.com/hajimehoshi/12345 is legal and its 'package name' is actually main.

AlexRouSg commented 6 years ago

/cc @hyangah

hajimehoshi commented 6 years ago

b39ed682d87b8df3698bcbc8fbaff91efbaa9d17

The package name for android is determined at androidPkgName. This should be easy to fix after determining how to fix this.

hajimehoshi commented 6 years ago

I've created a change,

diff --git a/cmd/gomobile/build_androidapp.go b/cmd/gomobile/build_androidapp.go
index a1f07cb..b4629b1 100644
--- a/cmd/gomobile/build_androidapp.go
+++ b/cmd/gomobile/build_androidapp.go
@@ -287,20 +287,15 @@ func goAndroidBuild(pkg *build.Package, androidArchs []string) (map[string]bool,
 // but not exactly same.
 func androidPkgName(name string) string {
        var res []rune
-       for i, r := range name {
+       for _, r := range name {
                switch {
-               case 'a' <= r && r <= 'z', 'A' <= r && r <= 'Z':
-                       res = append(res, r)
-               case '0' <= r && r <= '9':
-                       if i == 0 {
-                               panic(fmt.Sprintf("package name %q is not a valid go package name", name))
-                       }
+               case 'a' <= r && r <= 'z', 'A' <= r && r <= 'Z', '0' <= r && r <= '9':
                        res = append(res, r)
                default:
                        res = append(res, '_')
                }
        }
-       if len(res) == 0 || res[0] == '_' {
+       if len(res) == 0 || res[0] == '_' || ('0' <= res[0] && res[0] <= '9') {
                // Android does not seem to allow the package part starting with _.
                res = append([]rune{'g', 'o'}, res...)
        }
diff --git a/cmd/gomobile/build_test.go b/cmd/gomobile/build_test.go
index b09a167..e89764c 100644
--- a/cmd/gomobile/build_test.go
+++ b/cmd/gomobile/build_test.go
@@ -54,6 +54,7 @@ func TestAndroidPkgName(t *testing.T) {
                {".-.", "go___"},
                {"abstract", "abstract_"},
                {"Abstract", "Abstract"},
+               {"12345", "go12345"},
        }

        for _, tc := range tests {

but git codereview mail fails with:

remote: The push has been rejected because we detect that it contains a private
remote: key. Please check the following commands and confirm that it's
remote: intentional:
remote:
remote:     git show b4629b1170079341fbc8a53a1f72c4e964687a01
remote:
remote: You can use `git rev-list --objects --all` to find the files.
remote:
remote: To push these files, please run `git push -o nokeycheck`.
remote:
To https://go.googlesource.com/mobile
 ! [remote rejected] HEAD -> refs/for/master (found a private key)
error: failed to push some refs to 'https://go.googlesource.com/mobile'
(running: git push -q origin HEAD:refs/for/master)
/Users/hajimehoshi/go/bin/git-codereview: exit status 1

🤔

hajimehoshi commented 6 years ago

https://go-review.googlesource.com/c/mobile/+/102576

git push -o nokeycheck origin HEAD:refs/for/master solved that (https://groups.google.com/forum/#!topic/golang-dev/huoCHVWVufQ)

hajimehoshi commented 6 years ago

It's fixed (the gobot doesn't notify this, but I think this is because my git-push way is special)

dmitshur commented 6 years ago

the gobot doesn't notify this, but I think this is because my git-push way is special

It was because of https://go-review.googlesource.com/c/mobile/+/102576/4//COMMIT_MSG#15.