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

runtime/cgo: do not change directory during runtime/cgo init #58225

Open
bcmills opened this issue Feb 1, 2023 · 2 comments
Open

runtime/cgo: do not change directory during runtime/cgo init #58225

bcmills opened this issue Feb 1, 2023 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. mobile Android, iOS, and x/mobile NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 1, 2023

In CL 8784, runtime/cgo on ios was given special-case code to change the current working directory to the app root.

That high-level behavior seems awfully presumptuous and out-of-place in a low-level package like runtime/cgo, and it causes runtime/cgo to depend on the CoreFoundation framework on ios when it does not need that framework on other darwin platforms.

If the init_working_dir behavior belongs in the standard library at all, arguably an init function in the os package would be much more appropriate than an ad-hoc side effect in runtime/cgo.

(attn @golang/ios; CC @golang/runtime)

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 1, 2023
@bcmills bcmills added OS-Darwin mobile Android, iOS, and x/mobile labels Feb 1, 2023
@bcmills bcmills added this to the Backlog milestone Feb 1, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Feb 1, 2023

Concretely, this special case complicates things like misc/cgo/testcarchive, which then needs its own special-case code to deal with the inappropriate dependency.

Moreover, it causes a Go library linked into a larger ios program to change the current working directory, which is not at all an appropriate library behavior!

@gopherbot
Copy link

Change https://go.dev/cl/463752 mentions this issue: cmd/dist,internal/platform: reenable the c-archive build mode on ios

gopherbot pushed a commit that referenced this issue Feb 1, 2023
Also fix misc/cgo/testcarchive to provide a missing CoreFoundation
dependency at link time.

Fixes #58221.
Updates #58172.
Updates #58225.

Change-Id: Ib8b6e52ed2914596615da4c427df2fe984722de6
Reviewed-on: https://go-review.googlesource.com/c/go/+/463752
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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 Feb 1, 2023
johanbrandhorst pushed a commit to Pryz/go that referenced this issue Feb 12, 2023
Also fix misc/cgo/testcarchive to provide a missing CoreFoundation
dependency at link time.

Fixes golang#58221.
Updates golang#58172.
Updates golang#58225.

Change-Id: Ib8b6e52ed2914596615da4c427df2fe984722de6
Reviewed-on: https://go-review.googlesource.com/c/go/+/463752
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. mobile Android, iOS, and x/mobile NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Projects
Development

No branches or pull requests

3 participants