golang/go

cmd/go: support for Git LFS via new VCS qualifier

jpap opened this issue · 11 comments

jpap commented

Background

  • When Git LFS is installed on the client machine, cmd/go can work well with Git LFS, but only when using modules not via GOPROXY and GOPATH packages (called "packages" hereon for brevity).

  • When Git LFS is not installed, go get will download the repository but not smudge, so any files stored in Git LFS are left as text-file refs. This can break builds when such files are required for the build (e.g. binary syso files).

  • The Google-hosted proxy.golang.org uses cmd/go to download package contents. That service does not have Git LFS installed, and so every module retrieved via the proxy lacks Git LFS content. This can break builds, and can also result in checksum mismatches when a user tries to verify a module they previously downloaded using cmd/go in direct mode while having Git LFS installed.

  • For further detailed background information, see #47241.

Proposal

  1. A new VCS named git-lfs that prescribes the use of Git LFS when fetching a Git repo.

  2. New cmd/go support for the new VCS: using a built-in Git LFS client implementation, or one that relies on Git LFS installed on the client machine. For the latter:

    • If not installed, cmd/go should inform the user that Git LFS is required before erroring out. i.e. cmd/go should never leave un-smudged Git LFS ref files in a fetched package.

    • Have Git LFS installed on proxy.golang.org.

  3. Module (package) owners opt-in to mandatory Git LFS package retrieval using the following:

    • Specifying git-lfs as the vcs argument in the content attribute of the go-import meta tag.

    • Using the new VCS qualifier,

      • in the module directive in a go.mod file, e.g. module go.jpap.org/example.git-lfs.

        Here the .git-lfs suffix forms part of the module path when used with the GOPROXY protocol and the local module caches ($GOPATH/pkg/mod).

      • in the import comment in a package, e.g. package example // import "go.jpap.org/example.git-lfs".

      In both cases above, the .git-lfs suffix should be ignored when matching the module path (package import path) against an import statement in a Go source file. With reference to the examples above, the Go source having import statement import "go.jpap.org/example" should match even though it does not specify the VCS qualifier.

  4. Module (package) users can explicitly qualify a module path (package import path) using the proposed VCS so that Git LFS is used to access the repository. This is helpful when there is no go-import HTTP server available to resolve the repository URL.

  5. Where Git LFS is installed, explicitly disable smudging on cmd/go fetches of modules (packages) not opted into the git-lfs VCS qualifier as outlined in Item 3. This prevents the checksum mismatch problem outlined in #47241.

  6. Irrespective of whether Git LFS is installed, when fetching a module (package) cmd/go should error out when it detects that a package has opted into Git LFS and the VCS qualifier is not git-lfs. This forces module (package) users to also opt-in to using Git LFS, and prevents broken builds due to files not being smudged appropriately when Git LFS is not installed.

bcmills commented

in the import comment in a package, e.g. package example // import "go.jpap.org/example.git-lfs".

FWIW, import comments are ignored in module mode. If this behavior falls out naturally from the implementation that's fine, but I think the only places we should explicitly support the extension are in the module path:

$ go get go.jpap.org/example.git-lfs/cmd/example

and in the go-import metadata:

<meta name="go-import" content="go.jpap.org/example git-lfs https://github.com/jpap/example.git">

.

bcmills commented

When fetching a module (package), cmd/go should "upgrade" to Git LFS when it detects that the package has opted into Git LFS. In that case, the module path (package import path) should be treated as if the user had appended the .git-lfs suffix.

I don't think we should do this part — it's too much of a special case — but I do agree that we need to provide a smoother migration path for existing projects that don't control the go-import metadata for their import path, such as users with github.com import paths.

(I'm actively working on a proposal for a more general solution to the “renamed module path” use-case, so stay tuned!)

  • 👍1
bcmills commented

All in all, though, I agree that adding a git-lfs VCS type seems like the cleanest path forward, and explicitly disabling LFS for the regular git VCS type is the least likely to break existing checksums.

jpap commented

in the import comment in a package, e.g. package example // import "go.jpap.org/example.git-lfs".

FWIW, import comments are ignored in module mode. If this behavior falls out naturally from the implementation that's fine, but I think the only places we should explicitly support the extension are in the module path:

That was specified in support of GOPATH packages. I understand that GOPATH is deprecated, but is it going to be removed and become strictly unsupported?

["upgrade" stuff]

I don't think we should do this part — it's too much of a special case...

I don't feel strongly about that: I've dropped Item 5 of the proposal, and appended an "error out" item in its place.

... but I do agree that we need to provide a smoother migration path for existing projects that don't control the go-import metadata for their import path, such as users with github.com import paths.

(I'm actively working on a proposal for a more general solution to the “renamed module path” use-case, so stay tuned!)

You mentioned this back in #47241 -- are you able to outline some of what you had in mind for those like me who are curious? I hope this is related to #39536!

  • 👍1
bcmills commented

That was specified in support of GOPATH packages. I understand that GOPATH is deprecated, but is it going to be removed and become strictly unsupported?

We're not doing new feature work on it, and don't plan to. I think we should implement git-lfs support in module mode, and if that happens to also make it work in GOPATH mode all the better — but we shouldn't do extra work (or define extra semantics) just to support the new protocol in the old mode.

  • 👍1
rsc commented

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

bcmills commented

I think we should accept this. Specifically:

  • git-lfs is treated as a new, independent VCS, meaning “Git with LFS definitely enabled, and fail if it isn't supported”
  • git is treated as “Git with LFS explicitly disabled” (as proxy.golang.org does implicitly today by virtue of not having LFS installed)

I don't think we should try to detect when a repo served as git is configured for LFS — those repos may already exist and happen to work today and we shouldn't break them.

That does leave the hard-coded hosting providers like github.com in a bit of an awkward state, in that users can't seamlessly migrate them from git to git-lfs without changing the module path. But I think we can address that separately with follow-up proposals (#26134 is related, and #26904 may be sufficient once the .git-lfs suffix is recognized).

rsc commented

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

rsc commented

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

  • 🎉1