From 38f0f3898a20ad4aea02f972545db5b61f618207 Mon Sep 17 00:00:00 2001 From: George Lemon Date: Fri, 7 Aug 2026 15:00:42 +0300 Subject: [PATCH] Don't block on git credential prompts during version discovery Related to #1814. Version discovery probes many candidate URLs, including historical ones whose repos may be deleted or renamed. For those, git would otherwise block on an interactive username/password prompt. During discovery, set GIT_TERMINAL_PROMPT=0 when it isn't already set by the user, so dead URLs fail fast and the version is excluded; restore the previous value afterwards so actual installs keep interactive prompts for private repositories. --- src/nimblepkg/versiondiscovery.nim | 45 +++++++++++++++-------- tests/tester.nim | 1 + tests/tgitprompt.nim | 59 ++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 tests/tgitprompt.nim diff --git a/src/nimblepkg/versiondiscovery.nim b/src/nimblepkg/versiondiscovery.nim index 1245edca1..d960d3e37 100644 --- a/src/nimblepkg/versiondiscovery.nim +++ b/src/nimblepkg/versiondiscovery.nim @@ -637,18 +637,33 @@ proc collectAllVersions*(package: PackageMinimalInfo, options: Options, getMinim ## Processes top-level dependencies in parallel (default) or sequentially (with --sync). ## Each branch gets its own visited set to avoid race conditions on shared state. - result = newTable[string, PackageVersions]() - if not options.parallelDiscovery: - for pv in package.requires: - var visitedCopy = initHashSet[PkgTuple]() - let resultTable = await processRequirements(pv, visitedCopy, getMinimalPackage, preferredPackages, options, nimBin) - mergeVersionTables(result[], resultTable[]) - else: - var futures: seq[Future[TableRef[string, PackageVersions]]] = @[] - for pv in package.requires: - var visitedCopy = initHashSet[PkgTuple]() - futures.add processRequirements(pv, visitedCopy, getMinimalPackage, preferredPackages, options, nimBin) - await allFutures(futures) - for fut in futures: - if not fut.failed: - mergeVersionTables(result[], fut.read()[]) + # During discovery we must never block on an interactive git credential + # prompt: a candidate version may require a repo that was deleted or renamed, + # and git would ask for a username/password for it. Dead URLs should fail + # fast so the version gets excluded. An explicit user-set value is respected; + # only the default (unset) case is forced to non-interactive. The previous + # value is restored afterwards, so actual installs keep interactive prompts + # for private repositories. + let hadTerminalPrompt = existsEnv("GIT_TERMINAL_PROMPT") + let prevTerminalPrompt = getEnv("GIT_TERMINAL_PROMPT") + if not hadTerminalPrompt: + putEnv("GIT_TERMINAL_PROMPT", "0") + try: + result = newTable[string, PackageVersions]() + if not options.parallelDiscovery: + for pv in package.requires: + var visitedCopy = initHashSet[PkgTuple]() + let resultTable = await processRequirements(pv, visitedCopy, getMinimalPackage, preferredPackages, options, nimBin) + mergeVersionTables(result[], resultTable[]) + else: + var futures: seq[Future[TableRef[string, PackageVersions]]] = @[] + for pv in package.requires: + var visitedCopy = initHashSet[PkgTuple]() + futures.add processRequirements(pv, visitedCopy, getMinimalPackage, preferredPackages, options, nimBin) + await allFutures(futures) + for fut in futures: + if not fut.failed: + mergeVersionTables(result[], fut.read()[]) + finally: + if hadTerminalPrompt: putEnv("GIT_TERMINAL_PROMPT", prevTerminalPrompt) + else: delEnv("GIT_TERMINAL_PROMPT") diff --git a/tests/tester.nim b/tests/tester.nim index f5d2c469a..a92e13e6f 100644 --- a/tests/tester.nim +++ b/tests/tester.nim @@ -33,6 +33,7 @@ import tuninstall import tsat import tver import tversiondiscovery +import tgitprompt import tniminstall import trequireflag import tdeclarativeparser diff --git a/tests/tgitprompt.nim b/tests/tgitprompt.nim new file mode 100644 index 000000000..c982ed159 --- /dev/null +++ b/tests/tgitprompt.nim @@ -0,0 +1,59 @@ +{.used.} +# Tests for: git credential prompts are suppressed during version discovery. +# +# Version discovery probes many candidate URLs, including historical ones whose +# repos may be deleted or renamed. For those, git would otherwise block on an +# interactive `Username for 'https://github.com':` prompt. Discovery must never +# prompt: dead URLs should fail fast so the version gets excluded. Actual +# installs run outside discovery and keep interactive prompts for private +# repositories. + +import unittest, os +import std/[tables, options] +import chronos +import nimblepkg/[version, options, packageinfotypes, versiondiscovery] + +let nimBin = some("nim") + +proc collect(root: PackageMinimalInfo, mock: GetPackageMinimal): TableRef[string, PackageVersions] = + var options = initOptions() + options.parallelDiscovery = false + result = waitFor collectAllVersions(root, options, mock, nimBin = nimBin) + +suite "git credential prompts are suppressed during discovery": + test "discovery sets GIT_TERMINAL_PROMPT=0 only when the user didn't set it": + # An explicit user-set value is respected: discovery must not override it, + # and it must be restored afterwards so actual installs keep interactive + # prompts for private repositories. + var promptTotal = 0 + var promptSeenZero = 0 # discovery forced non-interactive + var promptSeenUser = 0 # discovery kept the user's value + proc mock(pv: PkgTuple, options: Options, nimBin: Option[string]): Future[seq[PackageMinimalInfo]] {.async.} = + inc promptTotal + case getEnv("GIT_TERMINAL_PROMPT") + of "0": inc promptSeenZero + of "1": inc promptSeenUser + else: discard + return @[PackageMinimalInfo(name: "dep", version: newVersion("1.0.0"))] + + let root = PackageMinimalInfo( + name: "root", version: newVersion("1.0.0"), isRoot: true, + requires: @[(name: "dep", ver: VersionRange(kind: verAny))]) + + # User set GIT_TERMINAL_PROMPT=1: it is respected, not overridden. + putEnv("GIT_TERMINAL_PROMPT", "1") + discard collect(root, mock) + check promptTotal > 0 + check promptSeenUser == promptTotal + check promptSeenZero == 0 + check getEnv("GIT_TERMINAL_PROMPT") == "1" + + # When it wasn't set, discovery forces 0 and removes it afterwards. + delEnv("GIT_TERMINAL_PROMPT") + promptTotal = 0 + promptSeenZero = 0 + promptSeenUser = 0 + discard collect(root, mock) + check promptSeenZero == promptTotal + check promptSeenUser == 0 + check not existsEnv("GIT_TERMINAL_PROMPT")