From 5adbeb13c5ae590ea31bf5599c54b9b604edc380 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Fri, 12 Apr 2019 17:46:36 +0200 Subject: [PATCH 01/14] update.nix: use ThreadPoolExecutor Not sure why I chose ProcessPoolExecutor in the first place. --- maintainers/scripts/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index eb7d0ef2647..b9e17736bed 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -31,7 +31,7 @@ def main(max_workers, keep_going, packages): eprint() eprint('Running update for:') - with concurrent.futures.ProcessPoolExecutor(max_workers=max_workers) as executor: + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: for package in packages: updates[executor.submit(run_update_script, package)] = package From d351cea9f3a041c88d79fe9be5467bc7faabb4a4 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Fri, 12 Apr 2019 18:22:06 +0200 Subject: [PATCH 02/14] common-updater-scripts: add --print-changes flag Printing the changed file and new version can be used to commit the changes to git. --- pkgs/common-updater/scripts/update-source-version | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkgs/common-updater/scripts/update-source-version b/pkgs/common-updater/scripts/update-source-version index 6a66f94597f..ba628fd2a44 100755 --- a/pkgs/common-updater/scripts/update-source-version +++ b/pkgs/common-updater/scripts/update-source-version @@ -11,7 +11,7 @@ die() { usage() { echo "Usage: $scriptName [] []" echo " [--version-key=] [--system=] [--file=]" - echo " [--ignore-same-hash]" + echo " [--ignore-same-hash] [--print-changes]" } args=() @@ -33,6 +33,9 @@ for arg in "$@"; do --ignore-same-hash) ignoreSameHash="true" ;; + --print-changes) + printChanges="true" + ;; --help) usage exit 0 @@ -102,6 +105,9 @@ fi if [[ "$oldVersion" = "$newVersion" ]]; then echo "$scriptName: New version same as old version, nothing to do." >&2 + if [ -n "$printChanges" ]; then + printf '[]\n' + fi exit 0 fi @@ -197,3 +203,7 @@ fi rm -f "$nixFile.bak" rm -f "$attr.fetchlog" + +if [ -n "$printChanges" ]; then + printf '[{"attrPath":"%s","oldVersion":"%s","newVersion":"%s","files":["%s"]}]\n' "$attr" "$oldVersion" "$newVersion" "$nixFile" +fi From 1efc042d92c0f20be6261be73e462789596fff09 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Fri, 12 Apr 2019 19:32:44 +0200 Subject: [PATCH 03/14] maintainers/scripts/update.nix: Add support for auto-commiting changes Update scripts can now declare features using passthru.updateScript = { command = [ ../../update.sh pname ]; supportedFeatures = [ "commit" ]; }; A `commit` feature means that when the update script finishes successfully, it will print a JSON list like the following: [ { "attrPath": "volume_key", "oldVersion": "0.3.11", "newVersion": "0.3.12", "files": [ "/path/to/nixpkgs/pkgs/development/libraries/volume-key/default.nix" ] } ] and data from that will be used when update.nix is run with --argstr commit true to create commits. We will create a new git worktree for each thread in the pool and run the update script there. Then we will commit the change and cherry pick it in the main repo, releasing the worktree for a next change. --- doc/stdenv/stdenv.xml | 44 +++++++++++++++++++++++++-- maintainers/scripts/update.nix | 12 ++++++-- maintainers/scripts/update.py | 55 +++++++++++++++++++++++++++++----- 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index f97c2a145af..ccf5bb440d3 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -475,10 +475,48 @@ passthru.updateScript = writeScript "update-zoom-us" '' passthru.updateScript = [ ../../update.sh pname "--requested-release=unstable" ]; + Finally, the attribute can be an attribute set, listing the extra supported features among other things. + +passthru.updateScript = { + command = [ ../../update.sh pname ]; + supportedFeatures = [ "commit" ]; +}; + + + + The script will be usually run from the root of the Nixpkgs repository but you should not rely on that. Also note that the update scripts will be run in parallel by default; you should avoid running git commit or any other commands that cannot handle that. + + - - The script will be usually run from the root of the Nixpkgs repository but you should not rely on that. Also note that the update scripts will be run in parallel by default; you should avoid running git commit or any other commands that cannot handle that. - + + Supported features + + + commit + + + + Whenever the update script exits with 0 return + status, it is expected to print a JSON list containing an object for + each updated attribute. Empty list can be returned when the script did + not update any files: for example, when the attribute is already the + latest version. The required keys can be seen below: + +[ + { + "attrPath": "volume_key", + "oldVersion": "0.3.11", + "newVersion": "0.3.12", + "files": [ + "/path/to/nixpkgs/pkgs/development/libraries/volume-key/default.nix" + ] + } +] + + + + + For information about how to run the updates, execute nix-shell maintainers/scripts/update.nix. diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index 9568c6cbbcc..e11e2450bd0 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -4,6 +4,7 @@ , max-workers ? null , include-overlays ? false , keep-going ? null +, commit ? null }: # TODO: add assert statements @@ -132,19 +133,26 @@ let --argstr keep-going true to continue running when a single update fails. + + You can also make the updater automatically commit on your behalf from updateScripts + that support it by adding + + --argstr commit true ''; packageData = package: { name = package.name; pname = lib.getName package; - updateScript = map builtins.toString (lib.toList package.updateScript); + updateScript = map builtins.toString (lib.toList (package.updateScript.command or package.updateScript)); + supportedFeatures = package.updateScript.supportedFeatures or []; }; packagesJson = pkgs.writeText "packages.json" (builtins.toJSON (map packageData packages)); optionalArgs = lib.optional (max-workers != null) "--max-workers=${max-workers}" - ++ lib.optional (keep-going == "true") "--keep-going"; + ++ lib.optional (keep-going == "true") "--keep-going" + ++ lib.optional (commit == "true") "--commit"; args = [ packagesJson ] ++ optionalArgs; diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index b9e17736bed..b440f9defe4 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -1,22 +1,45 @@ import argparse +import contextlib import concurrent.futures import json import os import subprocess import sys +import tempfile +import threading updates = {} +thread_name_prefix='UpdateScriptThread' + def eprint(*args, **kwargs): print(*args, file=sys.stderr, **kwargs) -def run_update_script(package): +def run_update_script(package, commit): + if commit and 'commit' in package['supportedFeatures']: + thread_name = threading.current_thread().name + worktree, _branch, lock = temp_dirs[thread_name] + lock.acquire() + package['thread'] = thread_name + else: + worktree = None + eprint(f" - {package['name']}: UPDATING ...") - subprocess.run(package['updateScript'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, check=True) + return subprocess.run(package['updateScript'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True, cwd=worktree) +@contextlib.contextmanager +def make_worktree(): + with tempfile.TemporaryDirectory() as wt: + branch_name = f'update-{os.path.basename(wt)}' + target_directory = f'{wt}/nixpkgs' -def main(max_workers, keep_going, packages): + subprocess.run(['git', 'worktree', 'add', '-b', branch_name, target_directory], check=True) + yield (target_directory, branch_name) + subprocess.run(['git', 'worktree', 'remove', target_directory], check=True) + subprocess.run(['git', 'branch', '-D', branch_name], check=True) + +def main(max_workers, keep_going, commit, packages): with open(sys.argv[1]) as f: packages = json.load(f) @@ -31,15 +54,29 @@ def main(max_workers, keep_going, packages): eprint() eprint('Running update for:') - with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: + with contextlib.ExitStack() as stack, concurrent.futures.ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix=thread_name_prefix) as executor: + global temp_dirs + + if commit: + temp_dirs = {f'{thread_name_prefix}_{str(i)}': (*stack.enter_context(make_worktree()), threading.Lock()) for i in range(max_workers)} + for package in packages: - updates[executor.submit(run_update_script, package)] = package + updates[executor.submit(run_update_script, package, commit)] = package for future in concurrent.futures.as_completed(updates): package = updates[future] try: - future.result() + p = future.result() + if commit and 'commit' in package['supportedFeatures']: + thread_name = package['thread'] + worktree, branch, lock = temp_dirs[thread_name] + changes = json.loads(p.stdout) + for change in changes: + subprocess.run(['git', 'add'] + change['files'], check=True, cwd=worktree) + commit_message = '{attrPath}: {oldVersion} → {newVersion}'.format(**change) + subprocess.run(['git', 'commit', '-m', commit_message], check=True, cwd=worktree) + subprocess.run(['git', 'cherry-pick', branch], check=True) eprint(f" - {package['name']}: DONE.") except subprocess.CalledProcessError as e: eprint(f" - {package['name']}: ERROR") @@ -54,6 +91,9 @@ def main(max_workers, keep_going, packages): if not keep_going: sys.exit(1) + finally: + if commit and 'commit' in package['supportedFeatures']: + lock.release() eprint() eprint('Packages updated!') @@ -65,13 +105,14 @@ def main(max_workers, keep_going, packages): parser = argparse.ArgumentParser(description='Update packages') parser.add_argument('--max-workers', '-j', dest='max_workers', type=int, help='Number of updates to run concurrently', nargs='?', default=4) parser.add_argument('--keep-going', '-k', dest='keep_going', action='store_true', help='Do not stop after first failure') +parser.add_argument('--commit', '-c', dest='commit', action='store_true', help='Commit the changes') parser.add_argument('packages', help='JSON file containing the list of package names and their update scripts') if __name__ == '__main__': args = parser.parse_args() try: - main(args.max_workers, args.keep_going, args.packages) + main(args.max_workers, args.keep_going, args.commit, args.packages) except (KeyboardInterrupt, SystemExit) as e: for update in updates: update.cancel() From 74c1b8deb249adc6d550f411f4ff95f256a24e9c Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Fri, 12 Apr 2019 19:37:17 +0200 Subject: [PATCH 04/14] gnome3.updateScript: implement commit feature --- pkgs/desktops/gnome-3/update.nix | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkgs/desktops/gnome-3/update.nix b/pkgs/desktops/gnome-3/update.nix index 1bceddf77eb..78cefdd0b0a 100644 --- a/pkgs/desktops/gnome-3/update.nix +++ b/pkgs/desktops/gnome-3/update.nix @@ -21,6 +21,9 @@ let version_policy="$3" PATH=${lib.makeBinPath [ common-updater-scripts python ]} latest_tag=$(python "${./find-latest-version.py}" "$package_name" "$version_policy" "stable" ${upperBoundFlag}) - update-source-version "$attr_path" "$latest_tag" + update-source-version "$attr_path" "$latest_tag" --print-changes ''; -in [ updateScript packageName attrPath versionPolicy ] +in { + command = [ updateScript packageName attrPath versionPolicy ]; + supportedFeatures = [ "commit" ]; +} From 17f89667b3fc8713dce98effcfe3d371d589940c Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 17 Apr 2019 04:04:32 +0200 Subject: [PATCH 05/14] maintainers/scripts/update.nix: refactoring Get rid of some globals, split main into smaller functions, rename some variables, add typehints. --- maintainers/scripts/update.py | 85 +++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index b440f9defe4..ad77d17a80a 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -1,3 +1,4 @@ +from typing import Dict, Generator, Tuple, Union import argparse import contextlib import concurrent.futures @@ -8,28 +9,30 @@ import sys import tempfile import threading -updates = {} +updates: Dict[concurrent.futures.Future, Dict] = {} -thread_name_prefix='UpdateScriptThread' +TempDirs = Dict[str, Tuple[str, str, threading.Lock]] + +thread_name_prefix = 'UpdateScriptThread' def eprint(*args, **kwargs): print(*args, file=sys.stderr, **kwargs) -def run_update_script(package, commit): +def run_update_script(package: Dict, commit: bool, temp_dirs: TempDirs) -> subprocess.CompletedProcess: + worktree: Union[None, str] = None + if commit and 'commit' in package['supportedFeatures']: thread_name = threading.current_thread().name worktree, _branch, lock = temp_dirs[thread_name] lock.acquire() package['thread'] = thread_name - else: - worktree = None eprint(f" - {package['name']}: UPDATING ...") return subprocess.run(package['updateScript'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True, cwd=worktree) @contextlib.contextmanager -def make_worktree(): +def make_worktree() -> Generator[Tuple[str, str], None, None]: with tempfile.TemporaryDirectory() as wt: branch_name = f'update-{os.path.basename(wt)}' target_directory = f'{wt}/nixpkgs' @@ -39,7 +42,40 @@ def make_worktree(): subprocess.run(['git', 'worktree', 'remove', target_directory], check=True) subprocess.run(['git', 'branch', '-D', branch_name], check=True) -def main(max_workers, keep_going, commit, packages): +def commit_changes(worktree: str, branch: str, execution: subprocess.CompletedProcess) -> None: + changes = json.loads(execution.stdout) + for change in changes: + subprocess.run(['git', 'add'] + change['files'], check=True, cwd=worktree) + commit_message = '{attrPath}: {oldVersion} → {newVersion}'.format(**change) + subprocess.run(['git', 'commit', '-m', commit_message], check=True, cwd=worktree) + subprocess.run(['git', 'cherry-pick', branch], check=True) + +def merge_changes(package: Dict, future: concurrent.futures.Future, commit: bool, keep_going: bool, temp_dirs: TempDirs) -> None: + try: + execution = future.result() + if commit and 'commit' in package['supportedFeatures']: + thread_name = package['thread'] + worktree, branch, lock = temp_dirs[thread_name] + commit_changes(worktree, branch, execution) + eprint(f" - {package['name']}: DONE.") + except subprocess.CalledProcessError as e: + eprint(f" - {package['name']}: ERROR") + eprint() + eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") + eprint() + eprint(e.stdout.decode('utf-8')) + with open(f"{package['pname']}.log", 'wb') as logfile: + logfile.write(e.stdout) + eprint() + eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") + + if not keep_going: + sys.exit(1) + finally: + if commit and 'commit' in package['supportedFeatures']: + lock.release() + +def main(max_workers: int, keep_going: bool, commit: bool, packages: Dict) -> None: with open(sys.argv[1]) as f: packages = json.load(f) @@ -55,45 +91,18 @@ def main(max_workers, keep_going, commit, packages): eprint('Running update for:') with contextlib.ExitStack() as stack, concurrent.futures.ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix=thread_name_prefix) as executor: - global temp_dirs - if commit: temp_dirs = {f'{thread_name_prefix}_{str(i)}': (*stack.enter_context(make_worktree()), threading.Lock()) for i in range(max_workers)} + else: + temp_dirs = {} for package in packages: - updates[executor.submit(run_update_script, package, commit)] = package + updates[executor.submit(run_update_script, package, commit, temp_dirs)] = package for future in concurrent.futures.as_completed(updates): package = updates[future] - try: - p = future.result() - if commit and 'commit' in package['supportedFeatures']: - thread_name = package['thread'] - worktree, branch, lock = temp_dirs[thread_name] - changes = json.loads(p.stdout) - for change in changes: - subprocess.run(['git', 'add'] + change['files'], check=True, cwd=worktree) - commit_message = '{attrPath}: {oldVersion} → {newVersion}'.format(**change) - subprocess.run(['git', 'commit', '-m', commit_message], check=True, cwd=worktree) - subprocess.run(['git', 'cherry-pick', branch], check=True) - eprint(f" - {package['name']}: DONE.") - except subprocess.CalledProcessError as e: - eprint(f" - {package['name']}: ERROR") - eprint() - eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") - eprint() - eprint(e.stdout.decode('utf-8')) - with open(f"{package['pname']}.log", 'wb') as f: - f.write(e.stdout) - eprint() - eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") - - if not keep_going: - sys.exit(1) - finally: - if commit and 'commit' in package['supportedFeatures']: - lock.release() + merge_changes(package, future, commit, keep_going, temp_dirs) eprint() eprint('Packages updated!') From 01b9d5371c5ce1f8d622ff00bd67c9defb656c79 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Fri, 18 Sep 2020 22:22:42 +0200 Subject: [PATCH 06/14] maintainers/scripts/update.nix: switch to asyncio This will make it cleaner and also better respect SIGTERM. --- maintainers/scripts/update.py | 182 +++++++++++++++++++++------------- 1 file changed, 114 insertions(+), 68 deletions(-) diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index ad77d17a80a..51a19164a17 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -1,35 +1,65 @@ -from typing import Dict, Generator, Tuple, Union +from __future__ import annotations +from typing import Dict, Generator, List, Optional, Tuple import argparse +import asyncio import contextlib -import concurrent.futures import json import os import subprocess import sys import tempfile -import threading -updates: Dict[concurrent.futures.Future, Dict] = {} - -TempDirs = Dict[str, Tuple[str, str, threading.Lock]] - -thread_name_prefix = 'UpdateScriptThread' +class CalledProcessError(Exception): + process: asyncio.subprocess.Process def eprint(*args, **kwargs): print(*args, file=sys.stderr, **kwargs) -def run_update_script(package: Dict, commit: bool, temp_dirs: TempDirs) -> subprocess.CompletedProcess: - worktree: Union[None, str] = None +async def check_subprocess(*args, **kwargs): + """ + Emulate check argument of subprocess.run function. + """ + process = await asyncio.create_subprocess_exec(*args, **kwargs) + returncode = await process.wait() - if commit and 'commit' in package['supportedFeatures']: - thread_name = threading.current_thread().name - worktree, _branch, lock = temp_dirs[thread_name] - lock.acquire() - package['thread'] = thread_name + if returncode != 0: + error = CalledProcessError() + error.process = process + + raise error + + return process + +async def run_update_script(merge_lock: asyncio.Lock, temp_dir: Optional[Tuple[str, str]], package: Dict, keep_going: bool): + worktree: Optional[str] = None + + if temp_dir is not None: + worktree, _branch = temp_dir eprint(f" - {package['name']}: UPDATING ...") - return subprocess.run(package['updateScript'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True, cwd=worktree) + try: + update_process = await check_subprocess(*package['updateScript'], stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) + update_info = await update_process.stdout.read() + + await merge_changes(merge_lock, package, update_info, temp_dir) + except KeyboardInterrupt as e: + eprint('Cancelling…') + raise asyncio.exceptions.CancelledError() + except CalledProcessError as e: + eprint(f" - {package['name']}: ERROR") + eprint() + eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") + eprint() + stderr = await e.process.stderr.read() + eprint(stderr.decode('utf-8')) + with open(f"{package['pname']}.log", 'wb') as logfile: + logfile.write(stderr) + eprint() + eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") + + if not keep_going: + raise asyncio.exceptions.CancelledError() @contextlib.contextmanager def make_worktree() -> Generator[Tuple[str, str], None, None]: @@ -37,46 +67,76 @@ def make_worktree() -> Generator[Tuple[str, str], None, None]: branch_name = f'update-{os.path.basename(wt)}' target_directory = f'{wt}/nixpkgs' - subprocess.run(['git', 'worktree', 'add', '-b', branch_name, target_directory], check=True) + subprocess.run(['git', 'worktree', 'add', '-b', branch_name, target_directory]) yield (target_directory, branch_name) - subprocess.run(['git', 'worktree', 'remove', target_directory], check=True) - subprocess.run(['git', 'branch', '-D', branch_name], check=True) + subprocess.run(['git', 'worktree', 'remove', '--force', target_directory]) + subprocess.run(['git', 'branch', '-D', branch_name]) -def commit_changes(worktree: str, branch: str, execution: subprocess.CompletedProcess) -> None: - changes = json.loads(execution.stdout) +async def commit_changes(merge_lock: asyncio.Lock, worktree: str, branch: str, update_info: str) -> None: + changes = json.loads(update_info) for change in changes: - subprocess.run(['git', 'add'] + change['files'], check=True, cwd=worktree) - commit_message = '{attrPath}: {oldVersion} → {newVersion}'.format(**change) - subprocess.run(['git', 'commit', '-m', commit_message], check=True, cwd=worktree) - subprocess.run(['git', 'cherry-pick', branch], check=True) + # Git can only handle a single index operation at a time + async with merge_lock: + await check_subprocess('git', 'add', *change['files'], cwd=worktree) + commit_message = '{attrPath}: {oldVersion} → {newVersion}'.format(**change) + await check_subprocess('git', 'commit', '--quiet', '-m', commit_message, cwd=worktree) + await check_subprocess('git', 'cherry-pick', branch) -def merge_changes(package: Dict, future: concurrent.futures.Future, commit: bool, keep_going: bool, temp_dirs: TempDirs) -> None: - try: - execution = future.result() - if commit and 'commit' in package['supportedFeatures']: - thread_name = package['thread'] - worktree, branch, lock = temp_dirs[thread_name] - commit_changes(worktree, branch, execution) - eprint(f" - {package['name']}: DONE.") - except subprocess.CalledProcessError as e: - eprint(f" - {package['name']}: ERROR") - eprint() - eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") - eprint() - eprint(e.stdout.decode('utf-8')) - with open(f"{package['pname']}.log", 'wb') as logfile: - logfile.write(e.stdout) - eprint() - eprint(f"--- SHOWING ERROR LOG FOR {package['name']} ----------------------") +async def merge_changes(merge_lock: asyncio.Lock, package: Dict, update_info: str, temp_dir: Optional[Tuple[str, str]]) -> None: + if temp_dir is not None: + worktree, branch = temp_dir + await commit_changes(merge_lock, worktree, branch, update_info) + eprint(f" - {package['name']}: DONE.") - if not keep_going: - sys.exit(1) - finally: - if commit and 'commit' in package['supportedFeatures']: - lock.release() +async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, packages_to_update: asyncio.Queue[Optional[Dict]], keep_going: bool, commit: bool): + while True: + package = await packages_to_update.get() + if package is None: + # A sentinel received, we are done. + return -def main(max_workers: int, keep_going: bool, commit: bool, packages: Dict) -> None: - with open(sys.argv[1]) as f: + if not ('commit' in package['supportedFeatures']): + temp_dir = None + + await run_update_script(merge_lock, temp_dir, package, keep_going) + +async def start_updates(max_workers: int, keep_going: bool, commit: bool, packages: List[Dict]): + merge_lock = asyncio.Lock() + packages_to_update: asyncio.Queue[Optional[Dict]] = asyncio.Queue() + + with contextlib.ExitStack() as stack: + temp_dirs: List[Optional[Tuple[str, str]]] = [] + + # Do not create more workers than there are packages. + num_workers = min(max_workers, len(packages)) + + # Set up temporary directories when using auto-commit. + for i in range(num_workers): + temp_dir = stack.enter_context(make_worktree()) if commit else None + temp_dirs.append(temp_dir) + + # Fill up an update queue, + for package in packages: + await packages_to_update.put(package) + + # Add sentinels, one for each worker. + # A workers will terminate when it gets sentinel from the queue. + for i in range(num_workers): + await packages_to_update.put(None) + + # Prepare updater workers for each temp_dir directory. + # At most `num_workers` instances of `run_update_script` will be running at one time. + updaters = asyncio.gather(*[updater(temp_dir, merge_lock, packages_to_update, keep_going, commit) for temp_dir in temp_dirs]) + + try: + # Start updater workers. + await updaters + except asyncio.exceptions.CancelledError as e: + # When one worker is cancelled, cancel the others too. + updaters.cancel() + +def main(max_workers: int, keep_going: bool, commit: bool, packages_path: str) -> None: + with open(packages_path) as f: packages = json.load(f) eprint() @@ -90,19 +150,7 @@ def main(max_workers: int, keep_going: bool, commit: bool, packages: Dict) -> No eprint() eprint('Running update for:') - with contextlib.ExitStack() as stack, concurrent.futures.ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix=thread_name_prefix) as executor: - if commit: - temp_dirs = {f'{thread_name_prefix}_{str(i)}': (*stack.enter_context(make_worktree()), threading.Lock()) for i in range(max_workers)} - else: - temp_dirs = {} - - for package in packages: - updates[executor.submit(run_update_script, package, commit, temp_dirs)] = package - - for future in concurrent.futures.as_completed(updates): - package = updates[future] - - merge_changes(package, future, commit, keep_going, temp_dirs) + asyncio.run(start_updates(max_workers, keep_going, commit, packages)) eprint() eprint('Packages updated!') @@ -122,8 +170,6 @@ if __name__ == '__main__': try: main(args.max_workers, args.keep_going, args.commit, args.packages) - except (KeyboardInterrupt, SystemExit) as e: - for update in updates: - update.cancel() - - sys.exit(e.code if isinstance(e, SystemExit) else 130) + except KeyboardInterrupt as e: + # Let’s cancel outside of the main loop too. + sys.exit(130) From 4a161ddb3bdfab0b09597456bd541cbbe6c84b07 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sat, 19 Sep 2020 16:44:17 +0200 Subject: [PATCH 07/14] maintainers/scripts/update.nix: support auto-committing by passing attrPath Instead of having the updateScript support returning JSON object, it should be sufficient to specify attrPath in passthru.updateScript. It is much easier to use. The former is now considered experimental. --- doc/stdenv/stdenv.xml | 10 +++++++--- maintainers/scripts/update.nix | 2 ++ maintainers/scripts/update.py | 29 +++++++++++++++++++++++++---- pkgs/desktops/gnome-3/update.nix | 4 ++-- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index ccf5bb440d3..060dc80c915 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -475,11 +475,12 @@ passthru.updateScript = writeScript "update-zoom-us" '' passthru.updateScript = [ ../../update.sh pname "--requested-release=unstable" ]; - Finally, the attribute can be an attribute set, listing the extra supported features among other things. + Finally, the attribute can be an attribute set, listing the attribute path and extra supported features in addition to command. passthru.updateScript = { command = [ ../../update.sh pname ]; - supportedFeatures = [ "commit" ]; + attrPath = pname; + supportedFeatures = [ … ]; }; @@ -488,9 +489,12 @@ passthru.updateScript = { + + maintainers/scripts/update.nix also supports automatically creating commits by running it with --argstr commit true but it requires either declaring the attrPath, or adding a commit to supportedFeatures and modifying the script accordingly. + Supported features - + commit diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index e11e2450bd0..3d6f3500f5c 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -143,8 +143,10 @@ let packageData = package: { name = package.name; pname = lib.getName package; + oldVersion = lib.getVersion package; updateScript = map builtins.toString (lib.toList (package.updateScript.command or package.updateScript)); supportedFeatures = package.updateScript.supportedFeatures or []; + attrPath = package.updateScript.attrPath or null; }; packagesJson = pkgs.writeText "packages.json" (builtins.toJSON (map packageData packages)); diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index 51a19164a17..bca554a5d82 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -72,8 +72,7 @@ def make_worktree() -> Generator[Tuple[str, str], None, None]: subprocess.run(['git', 'worktree', 'remove', '--force', target_directory]) subprocess.run(['git', 'branch', '-D', branch_name]) -async def commit_changes(merge_lock: asyncio.Lock, worktree: str, branch: str, update_info: str) -> None: - changes = json.loads(update_info) +async def commit_changes(name: str, merge_lock: asyncio.Lock, worktree: str, branch: str, changes: List[Dict]) -> None: for change in changes: # Git can only handle a single index operation at a time async with merge_lock: @@ -85,7 +84,29 @@ async def commit_changes(merge_lock: asyncio.Lock, worktree: str, branch: str, u async def merge_changes(merge_lock: asyncio.Lock, package: Dict, update_info: str, temp_dir: Optional[Tuple[str, str]]) -> None: if temp_dir is not None: worktree, branch = temp_dir - await commit_changes(merge_lock, worktree, branch, update_info) + + if 'commit' in package['supportedFeatures']: + changes = json.loads(update_info) + elif 'attrPath' in package: + attr_path = package['attrPath'] + obtain_new_version_process = await check_subprocess('nix-instantiate', '--expr', f'with import ./. {{}}; lib.getVersion {attr_path}', '--eval', '--strict', '--json', stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) + new_version = json.loads((await obtain_new_version_process.stdout.read()).decode('utf-8')) + changed_files_process = await check_subprocess('git', 'diff', '--name-only', stdout=asyncio.subprocess.PIPE, cwd=worktree) + changed_files = (await changed_files_process.stdout.read()).splitlines() + if len(changed_files) > 0: + changes = [ + { + 'files': changed_files, + 'oldVersion': package['oldVersion'], + 'newVersion': new_version, + 'attrPath': attr_path, + } + ] + else: + changes = [] + + await commit_changes(package['name'], merge_lock, worktree, branch, changes) + eprint(f" - {package['name']}: DONE.") async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, packages_to_update: asyncio.Queue[Optional[Dict]], keep_going: bool, commit: bool): @@ -95,7 +116,7 @@ async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, # A sentinel received, we are done. return - if not ('commit' in package['supportedFeatures']): + if not ('commit' in package['supportedFeatures'] or 'attrPath' in package): temp_dir = None await run_update_script(merge_lock, temp_dir, package, keep_going) diff --git a/pkgs/desktops/gnome-3/update.nix b/pkgs/desktops/gnome-3/update.nix index 78cefdd0b0a..5c3e5c24785 100644 --- a/pkgs/desktops/gnome-3/update.nix +++ b/pkgs/desktops/gnome-3/update.nix @@ -21,9 +21,9 @@ let version_policy="$3" PATH=${lib.makeBinPath [ common-updater-scripts python ]} latest_tag=$(python "${./find-latest-version.py}" "$package_name" "$version_policy" "stable" ${upperBoundFlag}) - update-source-version "$attr_path" "$latest_tag" --print-changes + update-source-version "$attr_path" "$latest_tag" ''; in { command = [ updateScript packageName attrPath versionPolicy ]; - supportedFeatures = [ "commit" ]; + inherit attrPath; } From b828285933c42e914cf99448bd18cbe219567971 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sat, 19 Sep 2020 23:40:04 +0200 Subject: [PATCH 08/14] maintainers/scripts/update.nix: support filling in auto-commit attributes We can determine all of them when attrPath is present so we might jsut as well do it. --- doc/stdenv/stdenv.xml | 3 ++ maintainers/scripts/update.py | 54 +++++++++++++++++++++-------------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index 060dc80c915..2a148a9c2a1 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -518,6 +518,9 @@ passthru.updateScript = { ] + + When the returned array contains exactly one object (e.g. [{}]), keys can be omitted and will be determined automatically. Finding out newVersion requires attrPath to be present either in the update script output or passed to the passthru.updateScript attribute set. + diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index bca554a5d82..00d86311528 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -81,30 +81,42 @@ async def commit_changes(name: str, merge_lock: asyncio.Lock, worktree: str, bra await check_subprocess('git', 'commit', '--quiet', '-m', commit_message, cwd=worktree) await check_subprocess('git', 'cherry-pick', branch) +async def check_changes(package: Dict, worktree: str, update_info: str): + if 'commit' in package['supportedFeatures']: + changes = json.loads(update_info) + else: + changes = [{}] + + # Try to fill in missing attributes when there is just a single change. + if len(changes) == 1: + # Dynamic data from updater take precedence over static data from passthru.updateScript. + if 'attrPath' not in changes[0]: + if 'attrPath' in package: + changes[0]['attrPath'] = package['attrPath'] + + if 'oldVersion' not in changes[0]: + # update.nix is always passing oldVersion + changes[0]['oldVersion'] = package['oldVersion'] + + if 'newVersion' not in changes[0]: + attr_path = changes[0]['attrPath'] + obtain_new_version_process = await check_subprocess('nix-instantiate', '--expr', f'with import ./. {{}}; lib.getVersion {attr_path}', '--eval', '--strict', '--json', stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) + changes[0]['newVersion'] = json.loads((await obtain_new_version_process.stdout.read()).decode('utf-8')) + + if 'files' not in changes[0]: + changed_files_process = await check_subprocess('git', 'diff', '--name-only', stdout=asyncio.subprocess.PIPE, cwd=worktree) + changed_files = (await changed_files_process.stdout.read()).splitlines() + changes[0]['files'] = changed_files + + if len(changed_files) == 0: + return [] + + return changes + async def merge_changes(merge_lock: asyncio.Lock, package: Dict, update_info: str, temp_dir: Optional[Tuple[str, str]]) -> None: if temp_dir is not None: worktree, branch = temp_dir - - if 'commit' in package['supportedFeatures']: - changes = json.loads(update_info) - elif 'attrPath' in package: - attr_path = package['attrPath'] - obtain_new_version_process = await check_subprocess('nix-instantiate', '--expr', f'with import ./. {{}}; lib.getVersion {attr_path}', '--eval', '--strict', '--json', stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) - new_version = json.loads((await obtain_new_version_process.stdout.read()).decode('utf-8')) - changed_files_process = await check_subprocess('git', 'diff', '--name-only', stdout=asyncio.subprocess.PIPE, cwd=worktree) - changed_files = (await changed_files_process.stdout.read()).splitlines() - if len(changed_files) > 0: - changes = [ - { - 'files': changed_files, - 'oldVersion': package['oldVersion'], - 'newVersion': new_version, - 'attrPath': attr_path, - } - ] - else: - changes = [] - + changes = await check_changes(package, worktree, update_info) await commit_changes(package['name'], merge_lock, worktree, branch, changes) eprint(f" - {package['name']}: DONE.") From b351de0971619d0bd21b347401fa3e4815bae8be Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sat, 19 Sep 2020 23:52:31 +0200 Subject: [PATCH 09/14] maintainers/scripts/update.nix: mention when there were no changes committed --- maintainers/scripts/update.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index 00d86311528..373818d303b 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -117,9 +117,13 @@ async def merge_changes(merge_lock: asyncio.Lock, package: Dict, update_info: st if temp_dir is not None: worktree, branch = temp_dir changes = await check_changes(package, worktree, update_info) - await commit_changes(package['name'], merge_lock, worktree, branch, changes) - eprint(f" - {package['name']}: DONE.") + if len(changes) > 0: + await commit_changes(package['name'], merge_lock, worktree, branch, changes) + else: + eprint(f" - {package['name']}: DONE, no changes.") + else: + eprint(f" - {package['name']}: DONE.") async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, packages_to_update: asyncio.Queue[Optional[Dict]], keep_going: bool, commit: bool): while True: From c21a85c6a08df971b49adba13428abcf71097e41 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 20 Sep 2020 00:20:34 +0200 Subject: [PATCH 10/14] maintainers/scripts/update.nix: auto-detect attrPath --- doc/stdenv/stdenv.xml | 2 +- maintainers/scripts/update.nix | 53 +++++++++++++++++--------------- maintainers/scripts/update.py | 4 +-- pkgs/desktops/gnome-3/update.nix | 5 +-- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index 2a148a9c2a1..6962c57a6ac 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -490,7 +490,7 @@ passthru.updateScript = { - maintainers/scripts/update.nix also supports automatically creating commits by running it with --argstr commit true but it requires either declaring the attrPath, or adding a commit to supportedFeatures and modifying the script accordingly. + maintainers/scripts/update.nix also supports automatically creating commits by running it with --argstr commit true. Neither declaring the attrPath attribute, or adding a commit to supportedFeatures and modifying the script accordingly is required. It might be useful if you want to customize the values to something else than what update.nix detects. Supported features diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index 3d6f3500f5c..c60860629a9 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -32,27 +32,31 @@ let in [x] ++ nubOn f xs; - packagesWithPath = relativePath: cond: return: pathContent: + packagesWithPath = rootPath: cond: pkgs: let - result = builtins.tryEval pathContent; - - dedupResults = lst: nubOn (pkg: pkg.updateScript) (lib.concatLists lst); - in - if result.success then + packagesWithPathInner = relativePath: pathContent: let - pathContent = result.value; + result = builtins.tryEval pathContent; + + dedupResults = lst: nubOn ({ package, attrPath }: package.updateScript) (lib.concatLists lst); in - if lib.isDerivation pathContent then - lib.optional (cond relativePath pathContent) (return relativePath pathContent) - else if lib.isAttrs pathContent then - # If user explicitly points to an attrSet or it is marked for recursion, we recur. - if relativePath == [] || pathContent.recurseForDerivations or false || pathContent.recurseForRelease or false then - dedupResults (lib.mapAttrsToList (name: elem: packagesWithPath (relativePath ++ [name]) cond return elem) pathContent) - else [] - else if lib.isList pathContent then - dedupResults (lib.imap0 (i: elem: packagesWithPath (relativePath ++ [i]) cond return elem) pathContent) - else [] - else []; + if result.success then + let + pathContent = result.value; + in + if lib.isDerivation pathContent then + lib.optional (cond relativePath pathContent) { attrPath = lib.concatStringsSep "." relativePath; package = pathContent; } + else if lib.isAttrs pathContent then + # If user explicitly points to an attrSet or it is marked for recursion, we recur. + if relativePath == rootPath || pathContent.recurseForDerivations or false || pathContent.recurseForRelease or false then + dedupResults (lib.mapAttrsToList (name: elem: packagesWithPathInner (relativePath ++ [name]) elem) pathContent) + else [] + else if lib.isList pathContent then + dedupResults (lib.imap0 (i: elem: packagesWithPathInner (relativePath ++ [i]) elem) pathContent) + else [] + else []; + in + packagesWithPathInner rootPath pkgs; packagesWith = packagesWithPath []; @@ -73,18 +77,17 @@ let else false ) ) - (relativePath: pkg: pkg) pkgs; packagesWithUpdateScript = path: let - pathContent = lib.attrByPath (lib.splitString "." path) null pkgs; + prefix = lib.splitString "." path; + pathContent = lib.attrByPath prefix null pkgs; in if pathContent == null then builtins.throw "Attribute path `${path}` does not exists." else - packagesWith (relativePath: pkg: builtins.hasAttr "updateScript" pkg) - (relativePath: pkg: pkg) + packagesWithPath prefix (relativePath: pkg: builtins.hasAttr "updateScript" pkg) pathContent; packageByName = name: @@ -96,7 +99,7 @@ let else if ! builtins.hasAttr "updateScript" package then builtins.throw "Package with an attribute name `${name}` does not have a `passthru.updateScript` attribute defined." else - package; + { attrPath = name; inherit package; }; packages = if package != null then @@ -140,13 +143,13 @@ let --argstr commit true ''; - packageData = package: { + packageData = { package, attrPath }: { name = package.name; pname = lib.getName package; oldVersion = lib.getVersion package; updateScript = map builtins.toString (lib.toList (package.updateScript.command or package.updateScript)); supportedFeatures = package.updateScript.supportedFeatures or []; - attrPath = package.updateScript.attrPath or null; + attrPath = package.updateScript.attrPath or attrPath; }; packagesJson = pkgs.writeText "packages.json" (builtins.toJSON (map packageData packages)); diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index 373818d303b..61387ee8cb3 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -91,8 +91,8 @@ async def check_changes(package: Dict, worktree: str, update_info: str): if len(changes) == 1: # Dynamic data from updater take precedence over static data from passthru.updateScript. if 'attrPath' not in changes[0]: - if 'attrPath' in package: - changes[0]['attrPath'] = package['attrPath'] + # update.nix is always passing attrPath + changes[0]['attrPath'] = package['attrPath'] if 'oldVersion' not in changes[0]: # update.nix is always passing oldVersion diff --git a/pkgs/desktops/gnome-3/update.nix b/pkgs/desktops/gnome-3/update.nix index 5c3e5c24785..1bceddf77eb 100644 --- a/pkgs/desktops/gnome-3/update.nix +++ b/pkgs/desktops/gnome-3/update.nix @@ -23,7 +23,4 @@ let latest_tag=$(python "${./find-latest-version.py}" "$package_name" "$version_policy" "stable" ${upperBoundFlag}) update-source-version "$attr_path" "$latest_tag" ''; -in { - command = [ updateScript packageName attrPath versionPolicy ]; - inherit attrPath; -} +in [ updateScript packageName attrPath versionPolicy ] From c1b05442ffd6cf3cf529cad469bebe8169b156e9 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 20 Sep 2020 00:59:03 +0200 Subject: [PATCH 11/14] doc: Undocument attr-set of passthru.updateScript We no longer need it for most use cases so I am making it experimental. I have something in mind where it might be useful in the future (customizing commit messages) but for now, it would only confuse people. --- doc/stdenv/stdenv.xml | 43 ------------------------------------------- 1 file changed, 43 deletions(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index 6962c57a6ac..d33ee382d0b 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -474,14 +474,6 @@ passthru.updateScript = writeScript "update-zoom-us" '' The attribute can also contain a list, a script followed by arguments to be passed to it: passthru.updateScript = [ ../../update.sh pname "--requested-release=unstable" ]; - - Finally, the attribute can be an attribute set, listing the attribute path and extra supported features in addition to command. - -passthru.updateScript = { - command = [ ../../update.sh pname ]; - attrPath = pname; - supportedFeatures = [ … ]; -}; @@ -489,41 +481,6 @@ passthru.updateScript = { - - maintainers/scripts/update.nix also supports automatically creating commits by running it with --argstr commit true. Neither declaring the attrPath attribute, or adding a commit to supportedFeatures and modifying the script accordingly is required. It might be useful if you want to customize the values to something else than what update.nix detects. - - - Supported features - - - commit - - - - Whenever the update script exits with 0 return - status, it is expected to print a JSON list containing an object for - each updated attribute. Empty list can be returned when the script did - not update any files: for example, when the attribute is already the - latest version. The required keys can be seen below: - -[ - { - "attrPath": "volume_key", - "oldVersion": "0.3.11", - "newVersion": "0.3.12", - "files": [ - "/path/to/nixpkgs/pkgs/development/libraries/volume-key/default.nix" - ] - } -] - - - - When the returned array contains exactly one object (e.g. [{}]), keys can be omitted and will be determined automatically. Finding out newVersion requires attrPath to be present either in the update script output or passed to the passthru.updateScript attribute set. - - - - For information about how to run the updates, execute nix-shell maintainers/scripts/update.nix. From 0c5ddf335836c521215ecc9bfbb07ba45256a7df Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 20 Sep 2020 01:15:58 +0200 Subject: [PATCH 12/14] maintainers/scripts/update.nix: run update script with UPDATE_NIX_ATTR_PATH The environment variable will contain the attribute path the script is supposed to update. --- doc/stdenv/stdenv.xml | 1 + maintainers/scripts/update.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/stdenv/stdenv.xml b/doc/stdenv/stdenv.xml index d33ee382d0b..6b74e96aad5 100644 --- a/doc/stdenv/stdenv.xml +++ b/doc/stdenv/stdenv.xml @@ -475,6 +475,7 @@ passthru.updateScript = writeScript "update-zoom-us" '' passthru.updateScript = [ ../../update.sh pname "--requested-release=unstable" ]; + The script will be run with UPDATE_NIX_ATTR_PATH environment variable set to the attribute path it is supposed to update. The script will be usually run from the root of the Nixpkgs repository but you should not rely on that. Also note that the update scripts will be run in parallel by default; you should avoid running git commit or any other commands that cannot handle that. diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index 61387ee8cb3..74e36eaf80c 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -39,7 +39,7 @@ async def run_update_script(merge_lock: asyncio.Lock, temp_dir: Optional[Tuple[s eprint(f" - {package['name']}: UPDATING ...") try: - update_process = await check_subprocess(*package['updateScript'], stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) + update_process = await check_subprocess('env', f"UPDATE_NIX_ATTR_PATH={package['attrPath']}", *package['updateScript'], stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) update_info = await update_process.stdout.read() await merge_changes(merge_lock, package, update_info, temp_dir) From 71c246c7856eafe369d673aec4ecbf7568b50a9f Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 20 Sep 2020 10:29:34 +0200 Subject: [PATCH 13/14] maintainers/scripts/update.nix: Run update scripts from the worktree `update.nix` extracts `passthru.updateScript` attributes in the main repo and when they are relative paths (e.g. `./update.sh`), Nix will resolve them to absolute paths in the main repo. Update scripts can use $(dirname $0) to get the location of files they should update but that would point to the main repo. We want them to modify the appropriate git worktree instead so we replace the prefix accordingly. `git rev-parse --show-toplevel` will resolve symlinks but, fortunately, Nix will do that as well, so the path will match: https://github.com/NixOS/nixpkgs/pull/98304#issuecomment-695761754 --- maintainers/scripts/update.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/maintainers/scripts/update.py b/maintainers/scripts/update.py index 74e36eaf80c..8cc2bcbd67c 100644 --- a/maintainers/scripts/update.py +++ b/maintainers/scripts/update.py @@ -5,6 +5,7 @@ import asyncio import contextlib import json import os +import re import subprocess import sys import tempfile @@ -30,16 +31,22 @@ async def check_subprocess(*args, **kwargs): return process -async def run_update_script(merge_lock: asyncio.Lock, temp_dir: Optional[Tuple[str, str]], package: Dict, keep_going: bool): +async def run_update_script(nixpkgs_root: str, merge_lock: asyncio.Lock, temp_dir: Optional[Tuple[str, str]], package: Dict, keep_going: bool): worktree: Optional[str] = None + update_script_command = package['updateScript'] + if temp_dir is not None: worktree, _branch = temp_dir + # Update scripts can use $(dirname $0) to get their location but we want to run + # their clones in the git worktree, not in the main nixpkgs repo. + update_script_command = map(lambda arg: re.sub(r'^{0}'.format(re.escape(nixpkgs_root)), worktree, arg), update_script_command) + eprint(f" - {package['name']}: UPDATING ...") try: - update_process = await check_subprocess('env', f"UPDATE_NIX_ATTR_PATH={package['attrPath']}", *package['updateScript'], stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) + update_process = await check_subprocess('env', f"UPDATE_NIX_ATTR_PATH={package['attrPath']}", *update_script_command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=worktree) update_info = await update_process.stdout.read() await merge_changes(merge_lock, package, update_info, temp_dir) @@ -125,7 +132,7 @@ async def merge_changes(merge_lock: asyncio.Lock, package: Dict, update_info: st else: eprint(f" - {package['name']}: DONE.") -async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, packages_to_update: asyncio.Queue[Optional[Dict]], keep_going: bool, commit: bool): +async def updater(nixpkgs_root: str, temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, packages_to_update: asyncio.Queue[Optional[Dict]], keep_going: bool, commit: bool): while True: package = await packages_to_update.get() if package is None: @@ -135,7 +142,7 @@ async def updater(temp_dir: Optional[Tuple[str, str]], merge_lock: asyncio.Lock, if not ('commit' in package['supportedFeatures'] or 'attrPath' in package): temp_dir = None - await run_update_script(merge_lock, temp_dir, package, keep_going) + await run_update_script(nixpkgs_root, merge_lock, temp_dir, package, keep_going) async def start_updates(max_workers: int, keep_going: bool, commit: bool, packages: List[Dict]): merge_lock = asyncio.Lock() @@ -147,6 +154,9 @@ async def start_updates(max_workers: int, keep_going: bool, commit: bool, packag # Do not create more workers than there are packages. num_workers = min(max_workers, len(packages)) + nixpkgs_root_process = await check_subprocess('git', 'rev-parse', '--show-toplevel', stdout=asyncio.subprocess.PIPE) + nixpkgs_root = (await nixpkgs_root_process.stdout.read()).decode('utf-8').strip() + # Set up temporary directories when using auto-commit. for i in range(num_workers): temp_dir = stack.enter_context(make_worktree()) if commit else None @@ -163,7 +173,7 @@ async def start_updates(max_workers: int, keep_going: bool, commit: bool, packag # Prepare updater workers for each temp_dir directory. # At most `num_workers` instances of `run_update_script` will be running at one time. - updaters = asyncio.gather(*[updater(temp_dir, merge_lock, packages_to_update, keep_going, commit) for temp_dir in temp_dirs]) + updaters = asyncio.gather(*[updater(nixpkgs_root, temp_dir, merge_lock, packages_to_update, keep_going, commit) for temp_dir in temp_dirs]) try: # Start updater workers. From 74a5bb4041bb4ef001875d5b6b26a2105dc24450 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 20 Sep 2020 21:12:43 +0200 Subject: [PATCH 14/14] maintainers/scripts/update.nix: Clean up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Make some arguments more fitting (the path is actually full, not just relative to prefix…). - Increase the purity of packages* functions (they now take pkgs from argument, not from scope). - Add some documentation comments. --- maintainers/scripts/update.nix | 80 ++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index c60860629a9..5bacf9dda6a 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -32,9 +32,18 @@ let in [x] ++ nubOn f xs; + /* Recursively find all packages (derivations) in `pkgs` matching `cond` predicate. + + Type: packagesWithPath :: AttrPath → (AttrPath → derivation → bool) → (AttrSet | List) → List + AttrPath :: [str] + + The packages will be returned as a list of named pairs comprising of: + - attrPath: stringified attribute path (based on `rootPath`) + - package: corresponding derivation + */ packagesWithPath = rootPath: cond: pkgs: let - packagesWithPathInner = relativePath: pathContent: + packagesWithPathInner = path: pathContent: let result = builtins.tryEval pathContent; @@ -42,24 +51,28 @@ let in if result.success then let - pathContent = result.value; + evaluatedPathContent = result.value; in - if lib.isDerivation pathContent then - lib.optional (cond relativePath pathContent) { attrPath = lib.concatStringsSep "." relativePath; package = pathContent; } - else if lib.isAttrs pathContent then + if lib.isDerivation evaluatedPathContent then + lib.optional (cond path evaluatedPathContent) { attrPath = lib.concatStringsSep "." path; package = evaluatedPathContent; } + else if lib.isAttrs evaluatedPathContent then # If user explicitly points to an attrSet or it is marked for recursion, we recur. - if relativePath == rootPath || pathContent.recurseForDerivations or false || pathContent.recurseForRelease or false then - dedupResults (lib.mapAttrsToList (name: elem: packagesWithPathInner (relativePath ++ [name]) elem) pathContent) + if path == rootPath || evaluatedPathContent.recurseForDerivations or false || evaluatedPathContent.recurseForRelease or false then + dedupResults (lib.mapAttrsToList (name: elem: packagesWithPathInner (path ++ [name]) elem) evaluatedPathContent) else [] - else if lib.isList pathContent then - dedupResults (lib.imap0 (i: elem: packagesWithPathInner (relativePath ++ [i]) elem) pathContent) + else if lib.isList evaluatedPathContent then + dedupResults (lib.imap0 (i: elem: packagesWithPathInner (path ++ [i]) elem) evaluatedPathContent) else [] else []; in packagesWithPathInner rootPath pkgs; + /* Recursively find all packages (derivations) in `pkgs` matching `cond` predicate. + */ packagesWith = packagesWithPath []; + /* Recursively find all packages in `pkgs` with updateScript by given maintainer. + */ packagesWithUpdateScriptAndMaintainer = maintainer': let maintainer = @@ -68,18 +81,19 @@ let else builtins.getAttr maintainer' lib.maintainers; in - packagesWith (relativePath: pkg: builtins.hasAttr "updateScript" pkg && - (if builtins.hasAttr "maintainers" pkg.meta - then (if builtins.isList pkg.meta.maintainers - then builtins.elem maintainer pkg.meta.maintainers - else maintainer == pkg.meta.maintainers - ) - else false - ) - ) - pkgs; + packagesWith (path: pkg: builtins.hasAttr "updateScript" pkg && + (if builtins.hasAttr "maintainers" pkg.meta + then (if builtins.isList pkg.meta.maintainers + then builtins.elem maintainer pkg.meta.maintainers + else maintainer == pkg.meta.maintainers + ) + else false + ) + ); - packagesWithUpdateScript = path: + /* Recursively find all packages under `path` in `pkgs` with updateScript. + */ + packagesWithUpdateScript = path: pkgs: let prefix = lib.splitString "." path; pathContent = lib.attrByPath prefix null pkgs; @@ -87,27 +101,31 @@ let if pathContent == null then builtins.throw "Attribute path `${path}` does not exists." else - packagesWithPath prefix (relativePath: pkg: builtins.hasAttr "updateScript" pkg) + packagesWithPath prefix (path: pkg: builtins.hasAttr "updateScript" pkg) pathContent; - packageByName = name: + /* Find a package under `path` in `pkgs` and require that it has an updateScript. + */ + packageByName = path: pkgs: let - package = lib.attrByPath (lib.splitString "." name) null pkgs; + package = lib.attrByPath (lib.splitString "." path) null pkgs; in if package == null then - builtins.throw "Package with an attribute name `${name}` does not exists." + builtins.throw "Package with an attribute name `${path}` does not exists." else if ! builtins.hasAttr "updateScript" package then - builtins.throw "Package with an attribute name `${name}` does not have a `passthru.updateScript` attribute defined." + builtins.throw "Package with an attribute name `${path}` does not have a `passthru.updateScript` attribute defined." else - { attrPath = name; inherit package; }; + { attrPath = path; inherit package; }; + /* List of packages matched based on the CLI arguments. + */ packages = if package != null then - [ (packageByName package) ] + [ (packageByName package pkgs) ] else if maintainer != null then - packagesWithUpdateScriptAndMaintainer maintainer + packagesWithUpdateScriptAndMaintainer maintainer pkgs else if path != null then - packagesWithUpdateScript path + packagesWithUpdateScript path pkgs else builtins.throw "No arguments provided.\n\n${helpText}"; @@ -143,6 +161,8 @@ let --argstr commit true ''; + /* Transform a matched package into an object for update.py. + */ packageData = { package, attrPath }: { name = package.name; pname = lib.getName package; @@ -152,6 +172,8 @@ let attrPath = package.updateScript.attrPath or attrPath; }; + /* JSON file with data for update.py. + */ packagesJson = pkgs.writeText "packages.json" (builtins.toJSON (map packageData packages)); optionalArgs =