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

Jump to bottom

go/build: sourcehut requires characters considered unsafe for cgo names #32260

Closed
ddevault opened this issue May 26, 2019 · 10 comments
Closed

go/build: sourcehut requires characters considered unsafe for cgo names #32260

ddevault opened this issue May 26, 2019 · 10 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.14

Comments

@ddevault
Copy link

ddevault commented May 26, 2019 •
edited by odeke-em

build.go has a set of safe characters which can be present in CFLAGS:

go/src/go/build/build.go

Lines 1553 to 1559 in c290cb6

// NOTE: $ is not safe for the shell, but it is allowed here because of linker options like -Wl,$ORIGIN.
// We never pass these arguments to a shell (just to programs we construct argv for), so this should be okay.
// See golang.org/issue/6038.
// The @ is for OS X. See golang.org/issue/13720.
// The % is for Jenkins. See golang.org/issue/16959.
// The ! is because module paths may use them. See golang.org/issue/26716.
const safeString = "+-.,/0123456789=ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz:$@%! "

Sourcehut packages always have ~ in the URL, and in the future will likely have ^ as well. Importing a package from sr.ht which uses ${SRCDIR} in its CFLAGS accordingly causes this error:

widgets/terminal.go:11:2: /home/sircmpwn/.local/share/go/pkg/mod/git.sr.ht/~sircmpwn/go-libvterm@v0.0.0-20190526184212-d093d527d591/vterm.go: malformed #cgo argument: -I/home/sircmpwn/.local/share/go/pkg/mod/git.sr.ht/~sircmpwn/go-libvterm@v0.0.0-20190526184212-d093d527d591/libvterm/include/

~ has a special meaning in the shell, but it is URL safe and, per the comments in build.go, this string is never expanded with a shell.

EDIT: added permalink
@AlexRouSg
Copy link
Contributor

ping @ianlancetaylor

@ianlancetaylor ianlancetaylor changed the title cgo can fail when used with packages on sourcehut due to unsafe characters go/build: sourcehut requires characters considered unsafe for cgo names May 27, 2019
@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels May 27, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone May 27, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@ddevault
Copy link
Author

ddevault commented Oct 8, 2019

Bump? This should be easy to fix, is definitely a bug in golang, and shows unfair favoritism towards other platforms.

@ianlancetaylor
Copy link
Contributor

Want to send in a patch?

@ddevault
Copy link
Author

ddevault commented Oct 9, 2019 •
edited

I don't really want to sign the CLA to be honest :/ I've pointed out the line of code which needs updating and it should be a one-line change, nay, a one-character change.

@gopherbot
Copy link

Change https://golang.org/cl/199918 mentions this issue: go/build: add ~ and ^ to safe character whitelist

@mvdan
Copy link
Member

mvdan commented Oct 9, 2019

Please note that these changes often aren't a trivial one-line fix. At minimum, it should include a small regression test. And the author should ensure that no other features or tests break because of the change.

@ALTree ALTree added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 9, 2019
@ddevault
Copy link
Author

ddevault commented Oct 9, 2019

Please note that these changes often aren't a trivial one-line fix. At minimum, it should include a small regression test. And the author should ensure that no other features or tests break because of the change.

Yeah, I know. The point I was getting at is that this is about as simple of a change as Golang can get. Thanks a lot for putting up the patch, I appreciate it!

@ddevault
Copy link
Author

ddevault commented Oct 9, 2019

Thank you!

@ThreeFx
Copy link

ThreeFx commented Jan 16, 2020

FWIW: This does not affect only sourcehut repositories, it also comes up when packaging Go libraries and programs which use cgo in Debian, since in some cases the version will include tilde characters (e.g. 0.0~git20190526.b7d861d-1), which will in turn mess with git-buildpackage.

@zhangyoufu
Copy link
Contributor

This also affects 8.3 short path on Windows. Glad to see it was fixed.

--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/mod_case_cgo (0.31s)
        script_test.go:183:
            > [!cgo] skip
            > env GO111MODULE=on
            > go get rsc.io/CGO
            [stderr]
            go: finding rsc.io/CGO v1.0.0
            go: downloading rsc.io/CGO v1.0.0
            go: extracting rsc.io/CGO v1.0.0
            package rsc.io/CGO: $WORK\gopath\pkg\mod\rsc.io\!c!g!o@v1.0.0\cgo.go: malformed #cgo argument: -IC:/DOCUME~1/ADMINI~1/LOCALS~1/Temp/cmd-go-test-056881027/script-mod_case_cgo/gopath/pkg/mod/rsc.io/!c!g!o@v1.0.0
            [exit status 1]
            FAIL: testdata\script\mod_case_cgo.txt:5: unexpected command failure

@golang golang locked and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
9 participants
@andybons @ddevault @zhangyoufu @ianlancetaylor @mvdan @ALTree @AlexRouSg @ThreeFx @gopherbot