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

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

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

Comments

@ddevault
Copy link

commented May 26, 2019

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

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

@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 this to the Go1.13 milestone May 27, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@ddevault

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Want to send in a patch?

@ddevault

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

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

This comment has been minimized.

Copy link

commented Oct 9, 2019

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

@mvdan

This comment has been minimized.

Copy link
Member

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 and removed NeedsInvestigation labels Oct 9, 2019
@ddevault

This comment has been minimized.

Copy link
Author

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!

@gopherbot gopherbot closed this in 0b204f9 Oct 9, 2019
@ddevault

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.