Repository navigation
fix: tell a moved GitHub repository by its web page redirect - #386
Merged
Merged
Conversation
lip 0.36.1 requests a tooth.json package by the path it declares when the requested path redirects there, but asked only where the git discovery request ends up. GitHub serves the git endpoints of a transferred repository at its old path (200 for https://github.com/LiteLDev/Lepton.git/info/refs) and redirects only the web page (https://github.com/LiteLDev/Lepton answers 301 with Location https://github.com/MiracleForest/Lepton), so 0.36.1 never saw the move: the lock kept the old path and every update kept warning. GetRepositoryUrl now also asks where the repository's web page redirects (HEAD, directly rather than through the GitHub proxy, once per repository) when discovery shows no redirect. A page that does not answer with a success status, or cannot be reached, leaves the answer at the discovered endpoint, so lip falls back to the old behaviour.
Review of the web page fallback: - Only github.com repositories are asked where their web page redirects; on other hosts a page redirect may mean something else (e.g. a mirror pointing at its upstream), and GitLab-style hosts redirect the git endpoints themselves. - A 5 s timeout instead of 30 s: users of the GitHub proxy often cannot reach github.com directly. - Any failure other than cancellation means "not known", so the cached answer is never a fault. - Document that FakeGitRemote.GetRepositoryUrl stands for either kind of redirect.
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.
Follow-up to #384, found in the end-to-end check of the released v0.36.1 Windows binary.
Problem
#384 requests a tooth.json package by the path its tooth.json declares when the requested path redirects there, and detected the redirect from where the git discovery request (
/info/refs) ends up. Real GitHub does not redirect the git endpoints of a transferred repository:So with v0.36.1,
lip updateon a lock written by 0.36.0 (chain refgithub.com/LiteLDev/Lepton) still keeps the old path and still warns. The unit tests passed because the fake remote modelled a redirect of the git endpoint.Fix
GitRemoteClient.GetRepositoryUrlfirst uses the discovered endpoint (cached afterListRefs, unchanged), and when that shows no redirect, asks where the repository's web page (the repo URL without.git) redirects with aHEADrequest, directly (not through the GitHub proxy), cached once per repository. A page that does not answer with 2xx, times out or cannot be reached leaves the answer at the discovered endpoint, so lip falls back to the 0.36.0 behaviour (old path + warning). The request is only made when a tooth.json declares another path than the one requested (or forlip uninstall <old path>that names nothing else).Forks and unrelated repositories have their own page (200, no redirect), so the false-positive analysis of #384 is unchanged.
Verification
lip updateprints "github.com/LiteLDev/Lepton redirects to github.com/MiracleForest/Lepton ...; requesting github.com/MiracleForest/Lepton instead", reinstalls fromgithub.com/MiracleForest/Lepton@0.1.1, and the lock records chain refgithub.com/MiracleForest/Lepton, archive repohttps://github.com/MiracleForest/Lepton.git; a secondlip updateprints no warning.GitRemoteClientTests: git endpoint served at the old path + web page 301 (asked once, case-insensitively, never via the proxy); page 404/500; page timeout.dotnet build,dotnet test(386 / 1031 / 348 passed, 13 skipped).Review
An independent review found no false positive on GitHub (forks have their own 200 page; login/404 pages are not 2xx or do not match; case variants are covered by the GitHub repo key; MovedRequest still requires the same commit). Its should-fix items are applied in the second commit: the web page is only asked for github.com repositories (on other hosts a page redirect may mean e.g. a mirror pointing at its upstream, and GitLab-style hosts redirect the git endpoints themselves), with a 5 s timeout (proxy users often cannot reach github.com directly), and any failure other than cancellation means "not known" so the cache never holds a fault. FakeGitRemote.GetRepositoryUrl is documented as standing for either kind of redirect; a test checks that other hosts get no HEAD.
The first CI run of this PR crashed the Windows test process (0xC00000FD) with the four new client tests unreported; it did not reproduce locally, and the run after these changes passed on every platform.