From 54c1fc8fb151a0ba62aa1f228b680af8ede15c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 10:31:46 +0100 Subject: [PATCH 1/8] webdav: support extra headers Allow the caller to pass additional request headers to openDownload(). This enables conditional downloads with "If-Modified-Since". --- pym/bob/webdav.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pym/bob/webdav.py b/pym/bob/webdav.py index f0efe6e6..6680e0ab 100644 --- a/pym/bob/webdav.py +++ b/pym/bob/webdav.py @@ -120,11 +120,15 @@ def __exists(self, path): return False - def openDownload(self, path, offset=None, length=None, query=None): - return self.__retry(lambda: self.__openDownload(path, offset, length, query)) + def openDownload(self, path, offset=None, length=None, query=None, + headers=None): + return self.__retry(lambda: self.__openDownload(path, offset, length, + query, headers)) - def __openDownload(self, path, offset, length, query): + def __openDownload(self, path, offset, length, query, extraHeaders=None): headers = self._getHeaders() + if extraHeaders is not None: + headers.update(extraHeaders) if offset is not None and length is not None: headers.update({'Range': 'bytes={}-{}'.format(offset, offset + length - 1)}) From 43da4daff082214cdec7ab74dc38c327d5db5dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 10:32:31 +0100 Subject: [PATCH 2/8] webdav: add 304 handling A "304 Not Modified" response is reported as WebdavNotModifiedError so that it can be distinguished from real errors. --- pym/bob/webdav.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pym/bob/webdav.py b/pym/bob/webdav.py index 6680e0ab..b04148a9 100644 --- a/pym/bob/webdav.py +++ b/pym/bob/webdav.py @@ -18,6 +18,9 @@ class WebdavNotFoundError(WebdavError): class WebdavAlreadyExistsError(WebdavError): pass +class WebdavNotModifiedError(WebdavError): + pass + def getNetLoc(url): """Get the network location of a parsed URL without the credentials. @@ -140,6 +143,9 @@ def __openDownload(self, path, offset, length, query, extraHeaders=None): e.fp.read() if e.status == 404: raise WebdavNotFoundError() + elif e.status == 304: + # Only possible if the caller passed "If-Modified-Since" + raise WebdavNotModifiedError() else: raise WebdavError("{} {}".format(e.status, e.reason)) except (http.client.HTTPException, OSError) as e: From 90e96758ae9f46b61cdee13f1b32af61b64d3031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 08:55:38 +0100 Subject: [PATCH 3/8] scm: url: use WebDav class for http(s) transfers Replace the hand-rolled urllib requests for http and https URLs by the WebDav class. This adds support for HTTP basic authentication with the credentials embedded in the URL and keeps them out of error messages. Uploads are done with "If-None-Match" now. Losing the race against a concurrent upload is not an error. The ftp downloads still use urllib directly. The retries are still handled by UrlScm to keep the delay between the attempts and the warnings. --- pym/bob/scm/url.py | 72 ++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/pym/bob/scm/url.py b/pym/bob/scm/url.py index d95b254f..35e4e543 100644 --- a/pym/bob/scm/url.py +++ b/pym/bob/scm/url.py @@ -6,8 +6,9 @@ from .. import BOB_VERSION from ..errors import BuildError, ParseError from ..stringparser import IfExpression -from ..utils import asHexStr, hashFile, removeUserFromUrl, sslNoVerifyContext, \ - replacePath +from ..utils import asHexStr, hashFile, removeUserFromUrl, replacePath +from ..webdav import WebDav, WebdavError, WebdavAlreadyExistsError, \ + WebdavNotFoundError, WebdavNotModifiedError from .scm import Scm, ScmAudit from http.client import HTTPException from abc import abstractmethod @@ -22,12 +23,10 @@ import schema import shutil import signal -import ssl import stat import sys import tempfile import time -import urllib.error import urllib.parse import urllib.request @@ -483,23 +482,31 @@ def _getCandidateUrls(self, invoker): except ValueError as e: invoker.fail(str(e)) - def _download(self, url, destination, mode): + def __openHttpDownload(self, url, destination): headers = {} - headers["User-Agent"] = "BobBuildTool/{}".format(BOB_VERSION) headers.update(self.__headers) - context = None if self.__sslVerify else sslNoVerifyContext() - if os.path.isfile(destination) and (url.scheme in ["http", "https"]): + if os.path.isfile(destination): # Try to avoid download if possible headers["If-Modified-Since"] = time2HTTPDate(os.stat(destination).st_mtime) + return WebDav(url, self.__sslVerify).openDownload("", headers=headers) + + def __openFtpDownload(self, url): + req = urllib.request.Request(url=url.geturl()) + return urllib.request.urlopen(req) + def _download(self, url, destination, mode): tmpFileName = None - req = urllib.request.Request(url=url.geturl(), headers=headers) try: # Set default signal handler so that KeyboardInterrupt is raised. # Needed to gracefully handle ctrl+c. signal.signal(signal.SIGINT, signal.default_int_handler) - with contextlib.closing(urllib.request.urlopen(req, context=context)) as rsp: + if url.scheme in ["http", "https"]: + rsp = self.__openHttpDownload(url, destination) + else: + rsp = self.__openFtpDownload(url) + + with contextlib.closing(rsp): with tempfile.NamedTemporaryFile(dir=os.path.dirname(destination), delete=False) as f: tmpFileName = f.name read = 0 @@ -521,12 +528,13 @@ def _download(self, url, destination, mode): replacePath(tmpFileName, destination) tmpFileName = None - except urllib.error.HTTPError as e: - if e.code != 304: - return False, "HTTP error {}: {}".format(e.code, e.reason), False - else: - # HTTP 304 Not modifed -> local file up-to-date - return False, None, False + except WebdavNotModifiedError: + # HTTP 304 Not modifed -> local file up-to-date + return False, None, False + except WebdavNotFoundError: + return False, "HTTP error 404: Not Found", False + except WebdavError as e: + return False, "Failed to download: " + str(e), False except HTTPException as e: return False, "HTTP error: " + str(e), False except OSError as e: @@ -589,10 +597,7 @@ async def _fetch(self, invoker, url, destination, mode): invoker.fail("Unsupported URL scheme: " + url.scheme) def _upload(self, source, url): - headers = {} - headers["User-Agent"] = "BobBuildTool/{}".format(BOB_VERSION) - context = None if self.__sslVerify else sslNoVerifyContext() - + webdav = WebDav(url, self.__sslVerify) try: # Set default signal handler so that KeyboardInterrupt is raised. # Needed to gracefully handle ctrl+c. @@ -600,26 +605,19 @@ def _upload(self, source, url): # First check if the file already exists. Only upload if there is a # reason to do so. - req = urllib.request.Request(url=url.geturl(), method='HEAD', headers=headers) - try: - with urllib.request.urlopen(req, context=context) as rsp: - return None - except urllib.error.HTTPError as e: - if e.code != 404: - return "{}: HTTP error {}: {}".format(url.geturl(), e.code, e.reason) + if webdav.exists(""): + return None - # Ok, HEAD returned a 404. Proceed with upload... with open(source, "rb") as f: - headers["Content-Length"] = f.seek(0, os.SEEK_END) - f.seek(0) - req = urllib.request.Request(url=url.geturl(), method='PUT', data=f, headers=headers) - with urllib.request.urlopen(req, context=context): - pass + webdav.upload("", f, False) - except urllib.error.URLError as e: - return "{}: {}".format(url.geturl(), e.reason) - except HTTPException as e: - return "{}: HTTP error: {}".format(url.geturl(), str(e)) + except WebdavAlreadyExistsError: + # Lost the race with a concurrent upload. Never mind... + pass + except WebdavError as e: + return "{}: HTTP error: {}".format(webdav.getRequestURL(""), str(e)) + except OSError as e: + return "{}: {}".format(source, str(e)) finally: # Restore signals to default so that Ctrl+C kills process. Needed # to prevent ugly backtraces when user presses ctrl+c. From 3b81868326af23794b59ae7d4b76c4d8cb753cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 08:57:57 +0100 Subject: [PATCH 4/8] test: add UrlScm tests with credentials Add optional HTTP basic authentication to the HTTP server mock. Use it to verify that UrlScm downloads and mirror uploads work with credentials embedded in the URL. The password contains reserved characters to check the proper unquoting. Wrong credentials must fail. --- test/unit/mocks/http_server.py | 30 +++++++++++++- test/unit/test_input_urlscm.py | 75 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/test/unit/mocks/http_server.py b/test/unit/mocks/http_server.py index 3596ecde..efb75735 100644 --- a/test/unit/mocks/http_server.py +++ b/test/unit/mocks/http_server.py @@ -2,11 +2,25 @@ from http.server import BaseHTTPRequestHandler, HTTPServer from threading import Thread, Lock from xml.etree.ElementTree import Element, SubElement, tostring +import base64 import os, time def createHttpHandler(repoPath, args): class Handler(BaseHTTPRequestHandler): + def checkAuth(self): + if args.get('auth') is None: + return True + if self.headers.get('Authorization') == args['auth']: + return True + # Drain request body to not upset the client + length = int(self.headers.get('Content-Length', 0)) + if length: self.rfile.read(length) + self.send_response(401, "Unauthorized") + self.send_header("WWW-Authenticate", 'Basic realm="default"') + self.end_headers() + return False + def addProps(self, path, response): # trying to create basic props href = SubElement(response, 'D:href') @@ -71,6 +85,7 @@ def getCommon(self, start=None, end=None): def do_HEAD(self): self.stats.headRequests += 1 + if not self.checkAuth(): return if args.get('retryHead') and args.get('retries') > 0: self.send_error(500, "internal error (retries={})".format(args['retries'])) args['retries'] = args.get('retries') - 1 @@ -80,6 +95,7 @@ def do_HEAD(self): def do_GET(self): self.stats.getRequests += 1 + if not self.checkAuth(): return if args.get('noResponse'): self.log_error("GET: noResponse: drop connection!") self.close_connection = True @@ -105,6 +121,7 @@ def do_GET(self): def do_PUT(self): self.stats.putRequests += 1 + if not self.checkAuth(): return if args.get('noResponse'): self.log_error("PUT: noResponse: drop connection!") self.close_connection = True @@ -128,6 +145,7 @@ def do_PUT(self): def do_DELETE(self): self.stats.deleteRequests += 1 + if not self.checkAuth(): return if args.get('noResponse'): self.log_error("PUT: noResponse: drop connection!") self.close_connection = True @@ -147,6 +165,7 @@ def do_DELETE(self): def do_MKCOL(self): self.stats.mkcolRequests += 1 + if not self.checkAuth(): return if args.get('noResponse'): self.log_error("PUT: noResponse: drop connection!") self.close_connection = True @@ -175,6 +194,7 @@ def do_MKCOL(self): def do_PROPFIND(self): self.stats.propfindRequests += 1 + if not self.checkAuth(): return if args.get('noResponse'): self.log_error("PUT: noResponse: drop connection!") self.close_connection = True @@ -214,11 +234,17 @@ def __init__(self, port): self.propfindRequests = 0 class HttpServerMock(): - def __init__(self, repoPath, noResponse=False, retries=0, retryHead=False): + def __init__(self, repoPath, noResponse=False, retries=0, retryHead=False, + username=None, password=None): if not repoPath.endswith(os.sep): repoPath += os.sep + auth = None + if username is not None: + auth = 'Basic ' + base64.b64encode( + (username+":"+password).encode("utf-8")).decode("ascii") self.handler = createHttpHandler(repoPath, - {'noResponse' : noResponse, 'retries' : retries, 'retryHead' : retryHead}) + {'noResponse' : noResponse, 'retries' : retries, 'retryHead' : retryHead, + 'auth' : auth}) self.server = HTTPServer(('localhost', 0), self.handler) def __enter__(self): diff --git a/test/unit/test_input_urlscm.py b/test/unit/test_input_urlscm.py index 475eece2..db81740b 100644 --- a/test/unit/test_input_urlscm.py +++ b/test/unit/test_input_urlscm.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later from shlex import quote +from urllib.parse import quote as urlquote from unittest import TestCase, skipIf from unittest.mock import MagicMock, patch import asyncio @@ -36,6 +37,14 @@ def makeFileUrl(fn): def escapeMirrorFileName(fn): return os.path.abspath(fn).replace('\\', '\\\\') +# Use reserved characters in the password to verify the proper escaping. +USERNAME = "bob" +PASSWORD = "p@ss:w/rd" + +def makeHttpUrl(port, path, username=USERNAME, password=PASSWORD): + return "http://{}:{}@localhost:{}/{}".format(urlquote(username, safe=''), + urlquote(password, safe=''), port, path) + class DummyPackage: def getName(self): return "dummy" @@ -364,6 +373,26 @@ def testDownloadDiskFull(self, signalMock): with self.assertRaises(InvocationError): self.invokeScm(workspace, scm, executor=concurrent.futures.ThreadPoolExecutor()) + def testDownloadCredentials(self): + """Download with HTTP basic authentication credentials in the URL""" + with HttpServerMock(self.dir, username=USERNAME, password=PASSWORD) as srv: + scm = self.createUrlScm({ + "url" : makeHttpUrl(srv.port, "test.txt"), + }) + with TemporaryWorkspace() as workspace: + self.invokeScm(workspace, scm) + self.assertContent(os.path.join(workspace, "test.txt")) + + def testDownloadWrongCredentials(self): + """Download with wrong credentials fails""" + with HttpServerMock(self.dir, username=USERNAME, password=PASSWORD) as srv: + scm = self.createUrlScm({ + "url" : makeHttpUrl(srv.port, "test.txt", password="wrong"), + }) + with TemporaryWorkspace() as workspace: + with self.assertRaises(InvocationError): + self.invokeScm(workspace, scm) + self.assertEqual(srv.getRequests, 1, "URL SCM does not retry on disk full errors") class TestExtraction(TestCase): @@ -708,6 +737,52 @@ def testHttpMirrorUploadRetry(self): self.assertExists(mirrorPath) self.assertContent(mirrorPath) + def testHttpMirrorUploadCredentials(self): + """A HTTP mirror with credentials in the URL is filled""" + with tempfile.TemporaryDirectory() as mirror: + mirrorPath = os.path.join(mirror, self.fn) + self.assertNotExists(mirrorPath) + + with HttpServerMock(mirror, username=USERNAME, password=PASSWORD) as srv: + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : makeHttpUrl(srv.port, r"\1"), + "upload" : True, + }] + ) + with TemporaryWorkspace() as workspace: + self.invokeScm(workspace, scm) + + self.assertEqual(1, srv.headRequests) + self.assertEqual(1, srv.putRequests) + + self.assertExists(mirrorPath) + self.assertContent(mirrorPath) + + def testHttpMirrorUploadWrongCredentials(self): + """Uploading to a HTTP mirror with wrong credentials fails""" + with tempfile.TemporaryDirectory() as mirror: + mirrorPath = os.path.join(mirror, self.fn) + + with HttpServerMock(mirror, username=USERNAME, password=PASSWORD) as srv: + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : makeHttpUrl(srv.port, r"\1", password="wrong"), + "upload" : True, + }] + ) + with TemporaryWorkspace() as workspace: + with self.assertRaises(InvocationError): + self.invokeScm(workspace, scm) + + self.assertNotExists(mirrorPath) + def testHttpMirrorNoReplaceExisting(self): """Existing files on an HTTP mirror are not replaced""" with tempfile.TemporaryDirectory() as mirror: From 6d431f3dc1f3b3a38d19e8bbc96180f3b33dce52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 09:30:30 +0100 Subject: [PATCH 5/8] scm: url: create missing parent directories on upload Quoting RFC 4918: A PUT that would result in the creation of a resource without an appropriately scoped parent collection MUST fail with a 409 (Conflict). Like the HttpArchive, make sure that the parent directories exist before uploading a file to a HTTP mirror. --- pym/bob/scm/url.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/pym/bob/scm/url.py b/pym/bob/scm/url.py index 35e4e543..5a51d473 100644 --- a/pym/bob/scm/url.py +++ b/pym/bob/scm/url.py @@ -597,7 +597,10 @@ async def _fetch(self, invoker, url, destination, mode): invoker.fail("Unsupported URL scheme: " + url.scheme) def _upload(self, source, url): - webdav = WebDav(url, self.__sslVerify) + # Address the file relative to the server root. Otherwise the parent + # directories could not be created. + path = url.path + webdav = WebDav(url._replace(path=""), self.__sslVerify) try: # Set default signal handler so that KeyboardInterrupt is raised. # Needed to gracefully handle ctrl+c. @@ -605,17 +608,28 @@ def _upload(self, source, url): # First check if the file already exists. Only upload if there is a # reason to do so. - if webdav.exists(""): + if webdav.exists(path): return None + # Quoting RFC 4918: + # + # A PUT that would result in the creation of a resource without an + # appropriately scoped parent collection MUST fail with a 409 + # (Conflict). + # + # Thus make sure the parent directories exist. + (dirs, _, _) = path.strip("/").rpartition("/") + if dirs: + webdav.mkdir(dirs) + with open(source, "rb") as f: - webdav.upload("", f, False) + webdav.upload(path, f, False) except WebdavAlreadyExistsError: # Lost the race with a concurrent upload. Never mind... pass except WebdavError as e: - return "{}: HTTP error: {}".format(webdav.getRequestURL(""), str(e)) + return "{}: HTTP error: {}".format(webdav.getRequestURL(path), str(e)) except OSError as e: return "{}: {}".format(source, str(e)) finally: From 4156d9d6d6f48ab9c3842d6e38126b1dfb14bc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 09:30:30 +0100 Subject: [PATCH 6/8] test: verify parent directory creation on mirror upload Let the HTTP server mock fail a PUT with 409 if the parent collection is missing, as mandated by RFC 4918, instead of silently creating it. Add a test that uploads to a HTTP mirror into a missing sub-directory. --- test/unit/mocks/http_server.py | 10 ++++++++-- test/unit/test_input_urlscm.py | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/test/unit/mocks/http_server.py b/test/unit/mocks/http_server.py index efb75735..b6766bcb 100644 --- a/test/unit/mocks/http_server.py +++ b/test/unit/mocks/http_server.py @@ -136,10 +136,16 @@ def do_PUT(self): self.end_headers() return - os.makedirs(os.path.dirname(path), exist_ok=True) length = int(self.headers['Content-Length']) + data = self.rfile.read(length) + # RFC 4918: missing parent collection must fail with 409 + if not os.path.isdir(os.path.dirname(path)): + self.send_response(409) + self.end_headers() + return + with open(path, "wb") as f: - f.write(self.rfile.read(length)) + f.write(data) self.send_response(200) self.end_headers() diff --git a/test/unit/test_input_urlscm.py b/test/unit/test_input_urlscm.py index db81740b..0aaa3fe9 100644 --- a/test/unit/test_input_urlscm.py +++ b/test/unit/test_input_urlscm.py @@ -737,6 +737,30 @@ def testHttpMirrorUploadRetry(self): self.assertExists(mirrorPath) self.assertContent(mirrorPath) + def testHttpMirrorUploadSubdir(self): + """Missing parent directories on a HTTP mirror are created""" + with tempfile.TemporaryDirectory() as mirror: + mirrorPath = os.path.join(mirror, "sub", "dir", self.fn) + self.assertNotExists(mirrorPath) + + with HttpServerMock(mirror) as srv: + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : r"http://localhost:{}/sub/dir/\1".format(srv.port), + "upload" : True, + }] + ) + with TemporaryWorkspace() as workspace: + self.invokeScm(workspace, scm) + + self.assertEqual(1, srv.putRequests) + + self.assertExists(mirrorPath) + self.assertContent(mirrorPath) + def testHttpMirrorUploadCredentials(self): """A HTTP mirror with credentials in the URL is filled""" with tempfile.TemporaryDirectory() as mirror: From 1dfd24d5eea24c4477aa3aa43381c52535b5f4e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 10:14:39 +0100 Subject: [PATCH 7/8] scm: url: add ignoreUploadErrors mirror property Uploading to a mirror might not always be possible, e.g. if the mirror is not always reachable or writable. Add an "ignoreUploadErrors" property to the UrlScm mirror entries. If set, a failed upload is only reported as warning instead of failing the build. As a side effect, errors when copying to local mirrors are reported properly instead of being raised as unhandled exceptions. --- doc/manual/configuration.rst | 10 +++-- pym/bob/scm/url.py | 77 ++++++++++++++++++++++++++---------- 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/doc/manual/configuration.rst b/doc/manual/configuration.rst index ea837431..d4843474 100644 --- a/doc/manual/configuration.rst +++ b/doc/manual/configuration.rst @@ -3102,10 +3102,12 @@ interoperability between projects, layers and the local user configuration. Each mirror entry specifies the SCM type (``scm``), a regular expression to match the URL (``url``) and a replacement URL (``mirror``). Optionally, for -the URL SCM it is possible to upload to the mirror (``upload``). Currently -the URL and SVN SCMs are supported for mirrors. For the SVN SCM, mirrors are -only tried during the initial checkout of a working copy. Uploading to a SVN -mirror is not supported. +the URL SCM it is possible to upload to the mirror (``upload``). By default, a +failed upload fails the build. Set ``ignoreUploadErrors`` to ``True`` to only +print a warning instead, e.g. if the mirror is not always reachable or writable. +Currently the URL and SVN SCMs are supported for mirrors. For the SVN SCM, +mirrors are only tried during the initial checkout of a working copy. Uploading +to a SVN mirror is not supported. .. attention:: Bob assumes that SVN mirrors hold the same content as the primary URL. This diff --git a/pym/bob/scm/url.py b/pym/bob/scm/url.py index 5a51d473..18002998 100644 --- a/pym/bob/scm/url.py +++ b/pym/bob/scm/url.py @@ -334,6 +334,7 @@ class UrlScm(Scm): 'url' : re.compile, 'mirror' : str, schema.Optional('upload') : bool, + schema.Optional('ignoreUploadErrors') : bool, }) EXTENSIONS = [ @@ -403,8 +404,10 @@ def __init__(self, spec, overrides=[], stripUser=None, self.__fallbackMirrors = fallbackMirrors self.__preMirrorsUrls = spec.get("preMirrors") self.__preMirrorsUpload = spec.get("__preMirrorsUpload") + self.__preMirrorsIgnoreUploadErrors = spec.get("__preMirrorsIgnoreUploadErrors") self.__fallbackMirrorsUrls = spec.get("fallbackMirrors") self.__fallbackMirrorsUpload = spec.get("__fallbackMirrorsUpload") + self.__fallbackMirrorsIgnoreUploadErrors = spec.get("__fallbackMirrorsIgnoreUploadErrors") self.__fileMode = spec.get("fileMode", 0o600 if defaultFileMode else None) self.__separateDownload = spec.get("__separateDownload", separateDownload) self.__headers = spec.get("headers", {}) @@ -431,7 +434,9 @@ def getProperties(self, isJenkins, pretty=False): if not pretty: ret.update({ '__preMirrorsUpload' : self.__getPreMirrorsUpload(), + '__preMirrorsIgnoreUploadErrors' : self.__getPreMirrorsIgnoreUploadErrors(), '__fallbackMirrorsUpload' : self.__getFallbackMirrorsUpload(), + '__fallbackMirrorsIgnoreUploadErrors' : self.__getFallbackMirrorsIgnoreUploadErrors(), '__separateDownload': self.__separateDownload, }) @@ -445,7 +450,8 @@ def __applyMirrors(self, mirrors): if not self.isDeterministic(): return [] - return [ (re.sub(m['url'], m['mirror'], self.__url), m.get('upload', False)) + return [ (re.sub(m['url'], m['mirror'], self.__url), m.get('upload', False), + m.get('ignoreUploadErrors', False)) for m in mirrors if m['scm'] == 'url' and re.match(m['url'], self.__url) ] @@ -461,6 +467,12 @@ def __getPreMirrorsUpload(self): ret = self.__preMirrorsUpload = [ m[1] for m in self.__applyMirrors(self.__preMirrors) ] return ret + def __getPreMirrorsIgnoreUploadErrors(self): + ret = self.__preMirrorsIgnoreUploadErrors + if ret is None: + ret = self.__preMirrorsIgnoreUploadErrors = [ m[2] for m in self.__applyMirrors(self.__preMirrors) ] + return ret + def __getFallbackMirrorsUrls(self): ret = self.__fallbackMirrorsUrls if ret is None: @@ -473,12 +485,21 @@ def __getFallbackMirrorsUpload(self): ret = self.__fallbackMirrorsUpload = [ m[1] for m in self.__applyMirrors(self.__fallbackMirrors) ] return ret + def __getFallbackMirrorsIgnoreUploadErrors(self): + ret = self.__fallbackMirrorsIgnoreUploadErrors + if ret is None: + ret = self.__fallbackMirrorsIgnoreUploadErrors = [ m[2] for m in self.__applyMirrors(self.__fallbackMirrors) ] + return ret + def _getCandidateUrls(self, invoker): - ret = list(zip(self.__getPreMirrorsUrls(), self.__getPreMirrorsUpload())) + \ - [(self.__url, False)] + \ - list(zip(self.__getFallbackMirrorsUrls(), self.__getFallbackMirrorsUpload())) + ret = list(zip(self.__getPreMirrorsUrls(), self.__getPreMirrorsUpload(), + self.__getPreMirrorsIgnoreUploadErrors())) + \ + [(self.__url, False, False)] + \ + list(zip(self.__getFallbackMirrorsUrls(), self.__getFallbackMirrorsUpload(), + self.__getFallbackMirrorsIgnoreUploadErrors())) try: - return [ (parseUrl(url), upload) for (url, upload) in ret ] + return [ (parseUrl(url), upload, ignoreUploadErrors) + for (url, upload, ignoreUploadErrors) in ret ] except ValueError as e: invoker.fail(str(e)) @@ -639,7 +660,15 @@ def _upload(self, source, url): return None - async def _put(self, invoker, source, url): + async def _put(self, invoker, source, url, ignoreErrors): + err = await self.__put(invoker, source, url) + if err: + if ignoreErrors: + invoker.warn("Ignoring upload error:", err) + else: + invoker.fail(err) + + async def __put(self, invoker, source, url): if url.scheme in ['', 'file']: # Verify that host name is empty or "localhost" if url.netloc not in ['', 'localhost']: @@ -647,16 +676,19 @@ async def _put(self, invoker, source, url): # Local files: copy only if newer (u), target never is a directory # (T). Move atomically to let concurrent other Bob instances see # only fully files. - if isYounger(source, url.path): - if os.path.isdir(url.path): - invoker.fail("Destination", url.path, "is an existing directory!") - invoker.trace("", source, url.path) - destDir = os.path.dirname(url.path) - os.makedirs(destDir, exist_ok=True) - with tempfile.TemporaryDirectory(dir=destDir) as tmpDir: - tmpFileName = os.path.join(tmpDir, os.path.basename(url.path)) - shutil.copy2(source, tmpFileName) - os.replace(tmpFileName, url.path) + try: + if isYounger(source, url.path): + if os.path.isdir(url.path): + return "Destination {} is an existing directory!".format(url.path) + invoker.trace("", source, url.path) + destDir = os.path.dirname(url.path) + os.makedirs(destDir, exist_ok=True) + with tempfile.TemporaryDirectory(dir=destDir) as tmpDir: + tmpFileName = os.path.join(tmpDir, os.path.basename(url.path)) + shutil.copy2(source, tmpFileName) + os.replace(tmpFileName, url.path) + except OSError as e: + return "Failed to copy: " + str(e) elif url.scheme in ["http", "https"]: retries = self.__retries while True: @@ -665,7 +697,7 @@ async def _put(self, invoker, source, url): err = await invoker.runInExecutor(UrlScm._upload, self, source, url) if err: if retries == 0: - invoker.fail(err) + return err else: break except (concurrent.futures.CancelledError, @@ -676,6 +708,8 @@ async def _put(self, invoker, source, url): else: invoker.fail("Upload not supported for URL scheme: " + url.scheme) + return None + def canSwitch(self, oldScm): diff = self._diffSpec(oldScm) if "scm" in diff: @@ -686,7 +720,8 @@ def canSwitch(self, oldScm): # Filter irrelevant properties diff -= { "sslVerify", "retries", 'preMirrors', '__preMirrorsUpload', - 'fallbackMirrors', '__fallbackMirrorsUpload' } + '__preMirrorsIgnoreUploadErrors', 'fallbackMirrors', + '__fallbackMirrorsUpload', '__fallbackMirrorsIgnoreUploadErrors' } # As long as the SCM is deterministic and the hashes have not changed, # the concrete URL is irrelevant. @@ -735,7 +770,7 @@ async def invoke(self, invoker, workspaceCreated): # behaviour. This must apply to fetches from local mirrors too. mode = 0o600 err = None - for url, upload in urls: + for url, _, _ in urls: if err: # Output previously failed download attempt as warning invoker.warn(err) @@ -768,9 +803,9 @@ async def invoke(self, invoker, workspaceCreated): # Upload to mirrors that requested it. This is only done for stable # files, though. if downloaded: - for url, upload in urls: + for url, upload, ignoreUploadErrors in urls: if upload: - await self._put(invoker, destination, url) + await self._put(invoker, destination, url, ignoreUploadErrors) # Run optional extractors if extractor is not None: From 27fafe658637dd91ebefae99e3fe92ba7a30b495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 30 Oct 2026 10:14:39 +0100 Subject: [PATCH 8/8] test: add UrlScm mirror upload error tests Verify that failed uploads to HTTP and local mirrors fail the build by default and that they are ignored if "ignoreUploadErrors" is set. --- test/unit/test_input_urlscm.py | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/unit/test_input_urlscm.py b/test/unit/test_input_urlscm.py index 0aaa3fe9..06fb1fda 100644 --- a/test/unit/test_input_urlscm.py +++ b/test/unit/test_input_urlscm.py @@ -761,6 +761,87 @@ def testHttpMirrorUploadSubdir(self): self.assertExists(mirrorPath) self.assertContent(mirrorPath) + def testHttpMirrorUploadError(self): + """A failed upload to a HTTP mirror fails the build""" + with tempfile.TemporaryDirectory() as mirror: + with HttpServerMock(mirror, noResponse=True) as srv: + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : r"http://localhost:{}/\1".format(srv.port), + "upload" : True, + }] + ) + with TemporaryWorkspace() as workspace: + with self.assertRaises(InvocationError): + self.invokeScm(workspace, scm) + + def testHttpMirrorUploadIgnoreErrors(self): + """A failed upload to a HTTP mirror can be ignored""" + with tempfile.TemporaryDirectory() as mirror: + mirrorPath = os.path.join(mirror, self.fn) + with HttpServerMock(mirror, noResponse=True) as srv: + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : r"http://localhost:{}/\1".format(srv.port), + "upload" : True, + "ignoreUploadErrors" : True, + }] + ) + with TemporaryWorkspace() as workspace: + self.invokeScm(workspace, scm) + self.assertContent(os.path.join(workspace, self.fn)) + + self.assertEqual(1, srv.putRequests) + + self.assertNotExists(mirrorPath) + + def testFileMirrorUploadError(self): + """A failed upload to a local mirror fails the build""" + with tempfile.TemporaryDirectory() as mirror: + # Parent "directory" is a file -> upload must fail + blocker = os.path.join(mirror, "blocker") + with open(blocker, "w") as f: + pass + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : escapeMirrorFileName(blocker) + r"/\1", + "upload" : True, + }] + ) + with TemporaryWorkspace() as workspace: + with self.assertRaises(InvocationError): + self.invokeScm(workspace, scm) + + def testFileMirrorUploadIgnoreErrors(self): + """A failed upload to a local mirror can be ignored""" + with tempfile.TemporaryDirectory() as mirror: + # Parent "directory" is a file -> upload must fail + blocker = os.path.join(mirror, "blocker") + with open(blocker, "w") as f: + pass + scm = self.createUrlScm( + { "digestSHA1" : self.urlSha1 }, + fallbackMirrors = [{ + "scm" : "url", + "url" : r".*/(.*)", + "mirror" : escapeMirrorFileName(blocker) + r"/\1", + "upload" : True, + "ignoreUploadErrors" : True, + }] + ) + with TemporaryWorkspace() as workspace: + self.invokeScm(workspace, scm) + self.assertContent(os.path.join(workspace, self.fn)) + def testHttpMirrorUploadCredentials(self): """A HTTP mirror with credentials in the URL is filled""" with tempfile.TemporaryDirectory() as mirror: