From dd8a07f4fd89857f14c8720763804bccf9eb35b2 Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 25 Oct 2026 10:15:41 -0600 Subject: [PATCH] repos.yaml: a branch URL is keyed by its Repo's URL file_rows keyed the raw URL, but stores parse_repo_url's URL, which turns a forge's branch URL (GitHub /tree/, GitLab /-/tree/) into the repo URL and a branch. The entry looked like a new Repo: its insert hit the existing Repo's name (rolling back every import of that file), and an invalid such entry did not protect its Repo from deletion. Rows and invalid keys now use the parsed URL; a URL that cannot be parsed is skipped by its raw key. Co-Authored-By: Claude Opus 5.5 --- modules/repos/lib.py | 10 ++++++-- modules/repos/test/test_config_import.py | 32 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/modules/repos/lib.py b/modules/repos/lib.py index f25992a2..1dddd1b2 100644 --- a/modules/repos/lib.py +++ b/modules/repos/lib.py @@ -1403,11 +1403,17 @@ class ReposSection(ConfigSection): # Counted as skipped; never a Repo's key. invalid.add(f'\0{position}') continue - key = repo_url_key(url) + try: + parsed = parse_repo_url(url) + except Exception as e: + logger.error(f'Skipping invalid repo in config: {data}', exc_info=e) + invalid.add(repo_url_key(url)) + continue + # The URL that is stored: a forge's branch URL is its Repo's URL (and a branch). + key = repo_url_key(parsed.url) try: if key in rows: raise InvalidRepo(f'Another entry has the same repo: {url}') - parsed = parse_repo_url(url) mode = data.get('mode') or 'full' _validate_mode(mode) frequency = data.get('frequency') diff --git a/modules/repos/test/test_config_import.py b/modules/repos/test/test_config_import.py index 8ff712ed..caf400d3 100644 --- a/modules/repos/test/test_config_import.py +++ b/modules/repos/test/test_config_import.py @@ -117,3 +117,35 @@ async def test_failed_import_creates_nothing(test_session, saved_repos, monkeypa lib.get_repos_config().import_config() assert set(repos(test_session)) == {ONE, TWO} + + +@pytest.mark.asyncio +async def test_branch_url_is_its_repo(test_session, saved_repos): + """A forge's branch URL (GitHub `/tree/`, GitLab `/-/tree/`) is its Repo, with that branch; + the next import writes nothing.""" + edit_repo(saved_repos, TWO, url=f'{TWO}/tree/main', branch=None) + edit_config(saved_repos, lambda data: data['repos'].append( + dict(url='https://gitlab.com/group/sub/project/-/tree/dev'))) + + config = lib.get_repos_config() + config.import_config() + + assert config.successful_import is True + found = repos(test_session) + assert set(found) == {ONE, TWO, 'https://gitlab.com/group/sub/project'} + assert (found[TWO].branch, found['https://gitlab.com/group/sub/project'].branch) == ('main', 'dev') + with statements() as collected: + config.import_config() + assert writes(collected) == [] + + +@pytest.mark.asyncio +async def test_invalid_branch_url_entry_is_kept(test_session, saved_repos): + """An invalid entry named by its Repo's branch URL keeps that Repo.""" + edit_repo(saved_repos, TWO, url=f'{TWO}/tree/main', mode='bogus') + + config = lib.get_repos_config() + config.import_config() + + assert config.import_skipped == 1 + assert set(repos(test_session)) == {ONE, TWO}