third_party: import the 0magnet modules instead of copying them - #3951
Merged
Conversation
The copies under third_party/0magnet were staging: sources vendored by hand
so the browser-terminal work could move without a round trip through another
repository. That trade has stopped paying. The copies had already fallen
behind — xterm-go and websh gained a ResizeObserver so the terminal re-fits
to its container rather than to the window, which is exactly what the
hypervisor UI needs, since the terminal there lives inside a WinBox window,
and none of it was here.
Removes the copies and requires the modules by their own paths. No replace
directives: each fork is a module in its own right and resolves normally.
websh already requires afero, sh/v3, u-root and xterm-go itself, so those
versions come from it rather than being pinned twice.
Only four files outside the copied trees imported them, and none reached
into an internal/ package, so the rewrite is a straight path substitution.
Pins pick up two fixes that were not in the copies:
- xterm-go/websh: fit the container, not the window
- afero: the TinyGo shims are narrowed to js/wasm, where they belong.
They were gated on the whole tinygo tag, which handed ENOSYS stubs to
TinyGo's linux and wasip1 targets for os.Chmod, os.Chown and os.Link —
functions those targets implement.
…-0magnet-modules # Conflicts: # vendor/github.com/0magnet/xterm-go/xterm.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The copies under
third_party/0magnet/were staging — sources vendored by hand so the browser-terminal work could move without a round trip through another repository. That trade has stopped paying, so this removes them and requires the modules by their own paths.Why now
The copies had already drifted.
xterm-goandwebshgained aResizeObserverso the terminal re-fits to its container rather than to the window — which is precisely what the hypervisor UI needs, since the terminal there lives inside a WinBox window and the window resize event is not the same thing. None of that was in the copy. Pinning the modules picks it up.What changed
third_party/0magnet/deletedu-rootresolves indirectly throughwebsh, which already requiresafero,sh/v3,u-rootandxterm-goitself — so those versions come from one place rather than being pinned twicereplacedirectives — each fork is a module in its own right and resolves normallyinternal/package, so the rewrite is a straight path substitutionMost of the diff is git recognising the sources as renames from
third_party/intovendor/.The pins also carry an
aferofix: its TinyGo shims were gated on the wholetinygobuild tag, which handedENOSYSstubs to TinyGo's linux and wasip1 targets foros.Chmod,os.Chownandos.Link— functions those targets implement. They are now narrowed totinygo && js, which is the only target actually missing them. Same change proposed upstream as spf13/afero#650.The dependency question, stated plainly
This puts five direct requires on
github.com/0magnet/*intogo.mod. That is a real change in kind from vendored copies, and reviewers should weigh it rather than discover it: upstream's build would depend on repos outside the skycoin org.Two things bearing on it. There is precedent —
github.com/0magnet/coloredcobrais already an indirect dependency here. And the intent is to move these forks into the skycoin org, after which the paths change again. If maintainers would rather wait for that move, this can sit until then; the alternative is carrying stale hand-copied sources in the meantime, which is what prompted this.Verification
Passing:
go build .— nativeGOOS=js GOARCH=wasm go build ./cmd/websh-probe ./cmd/wasm-visortinygo build -target wasm ./cmd/websh-probe— the lane that exercises the narrowedaferoandshbuild tagsgo mod tidyandgo mod vendorcleanNot yet run:
pkg/tpviz/wasmgl, the only consumer ofcosmos-go. Worth noting it appears in no Makefile compile-check lane — neitherbuild-wasmnorbuild-wasm-tinygo— so it has no gate today. The remaining wasm artifacts are being rebuilt separately before this merges.