Skip to content

feat: add per-remote LFS scoping via install --remote#64

Open
thevinchi wants to merge 1 commit into
awslabs:mainfrom
fduplex-forks:feat/lfs-install-per-remote
Open

feat: add per-remote LFS scoping via install --remote#64
thevinchi wants to merge 1 commit into
awslabs:mainfrom
fduplex-forks:feat/lfs-install-per-remote

Conversation

@thevinchi
Copy link
Copy Markdown

Issue #, if available: #32

Description of changes:

git-lfs-s3 install sets lfs.standalonetransferagent globally, so git-lfs invokes git-lfs-s3 for every remote in the repo. Any push/pull to a non-S3 remote that uses LFS (GitHub, GitLab, etc.) fails because the agent rejects non-S3 URLs.

The root cause is that git-lfs parses s3:// as a bare SSH URL and attempts SSH endpoint discovery before consulting per-URL scoped agent config, so the existing workaround of writing lfs.<s3-url>.standalonetransferagent never takes effect (lfsapi/endpoint_finder.go:234-274, tq/manifest.go:270-284).

This PR adds git-lfs-s3 install --remote <name>, which sets remote.<name>.lfsurl to a synthetic HTTPS URL (https://lfs-alias.git-remote-s3.test/<bucket>/<prefix>). RemoteEndpoint() checks lfsurl before falling back to the remote URL, so the HTTPS-shaped value short-circuits SSH discovery. The scoped agent lookup then finds the per-URL standalonetransferagent and selects git-lfs-s3 for that remote only. The synthetic URL is never contacted (IsStandaloneTransfer() skips the LFS server entirely); the .test TLD is RFC 6761-reserved for non-resolvable use.

Changes:

  • git_remote_s3/common.py — adds LFS_ALIAS_HOST constant and synthetic_lfs_url(bucket, prefix) helper.
  • git_remote_s3/lfs.py
    • Refactors install() to accept an optional --remote <name> argument.
    • _install_unscoped() — preserves existing behavior; adds a stderr warning when more than one remote is configured.
    • _install_scoped() — writes remote.<name>.lfsurl, lfs.<url>.standalonetransferagent, and lfs.customtransfer.git-lfs-s3.path scoped to the named remote; errors if lfsurl is already set to a conflicting value; warns if unscoped lfs.standalonetransferagent is already set.
    • Switches both code paths from --add (non-idempotent) to --replace-all.
    • Adds _git_config_get, _git_config_set, _list_git_remotes, _resolve_s3_remote helpers.
  • test/lfs_install_test.py (new) — 11 tests using real temp git repos:
    • Bare install, one remote → unscoped config written, no warning.
    • Bare install, multiple remotes → unscoped config written + warning.
    • --remote nonexistent remote → exit 1.
    • --remote non-S3 remote → exit 1.
    • --remote happy path (s3://) → three scoped keys, no global agent key.
    • --remote happy path (s3+zip://) → same.
    • Idempotency: re-running produces no duplicate values.
    • Conflicting pre-existing lfsurl → exit 1, original value preserved.
    • Pre-existing unscoped agent → warning emitted, scoped keys still written.
    • synthetic_lfs_url golden-value and RFC-reserved-TLD tests.
  • README.md — updates LFS section to recommend --remote; documents the scoped install form and explains the synthetic URL; updates code examples.

Verification:

  • pytest test/ — 59 passed (the pre-existing test_simultaneous_pushes_single_bundle_remains failure on main is unrelated to this change).
  • black --check — clean on changed files.
  • flake8 — clean on changed files.
  • mypy — same 4 pre-existing errors as main; no new errors introduced.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Adds \`git-lfs-s3 install --remote <name>\` to scope the LFS transfer
agent to a single S3 remote. Fixes coexistence with non-S3 LFS remotes
that break when lfs.standalonetransferagent is configured globally.

- Writes remote.<name>.lfsurl and lfs.<url>.standalonetransferagent
instead of the global lfs.standalonetransferagent
- Bare install preserved (back-compat) with a warning when multiple
remotes are configured
- Switches --add to --replace-all for idempotency in both paths
- 11 new tests; updated README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant