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

cmd/go: module replacements have confusing (lack of) aliasing for package-level state #26607

Closed
bcmills opened this issue Jul 25, 2018 · 3 comments
Closed

cmd/go: module replacements have confusing (lack of) aliasing for package-level state #26607

bcmills opened this issue Jul 25, 2018 · 3 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2018

If I replace two different modules with the same replacement (say, because I'm planning to merge a fork back into the original package), I expect the two import paths to actually refer to the same package, including the same package state.

However, that is currently not the case: instead, the program receives two distinct copies of the module that do not share state.

(It may be that my expectations are off, but if so, we should clarify the documentation.)

$ go version
go version devel +5f5402b723 Tue Jul 24 23:54:08 2018 +0000 linux/amd64

$ cat go.mod
module example.com/replace

require (
        example.com/a v0.0.0
        example.com/b v0.0.0
)

replace (
        example.com/a => ./p
        example.com/b => ./p
)

$ cat p/p.go
package p

var Global string = "p"

$ cat replace.go
package main

import (
        "fmt"

        a "example.com/a"
        b "example.com/b"
)

func main() {
        a.Global = "a"
        fmt.Println(b.Global)
}

$ go build .

$ ./replace
p

CC: @rsc @myitcv
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules labels Jul 25, 2018
@bcmills bcmills modified the milestones: Go1.11, Go1.12 Jul 25, 2018
@bcmills
Copy link
Contributor Author

bcmills commented Jul 26, 2018

Per in-person discussion with @rsc, perhaps for now we should require that the go.mod for the replacement module must declare the same path as the module it is replacing. That supports local forks (the overwhelmingly common use-case for replacements) without committing to any other particular aliasing behavior.

@bcmills
Copy link
Contributor Author

bcmills commented Jul 26, 2018

Or, as another alternative, perhaps we should enforce that paths found in replace directives are unique: you can replace any module with any other module or path, but you can't also require the replacement by its original name.

@bcmills bcmills self-assigned this Jul 26, 2018
@gopherbot
Copy link

Change https://golang.org/cl/126156 mentions this issue: cmd/go: forbid use of one module with two different paths

jeet-parekh pushed a commit to jeet-parekh/go that referenced this issue Jul 31, 2018
@jeet-parekh
cmd/go: forbid use of one module with two different paths
115bddf 
If a single module is imported via two different paths (e.g., as itself and as a
replacement for something else), some users may be surprised if the two paths
do not share the same package-level state. Others may be surprised if the two
paths do share state.

Punt on the question for now by rejecting that condition explicitly.

Fixes golang#26607.

Change-Id: I15c3889f61f8dd4ba5e5c48ca33ad63aeecac04e
Reviewed-on: https://go-review.googlesource.com/126156
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@bcmills bcmills mentioned this issue Aug 9, 2018
Open
@bcmills bcmills mentioned this issue Oct 12, 2018
Open
@bcmills bcmills mentioned this issue Jan 9, 2019
Closed
@golang golang locked and limited conversation to collaborators Jul 31, 2019
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
2 participants
@bcmills @gopherbot