From 4ddf99e0ce5c866f90d610b0d5acfbd9670f61c3 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:28:54 +0100 Subject: [PATCH 01/20] SYS-8665 fix changed files --- action.yml | 11 ++++-- main/githooks.py | 90 ++++++++++++++++++++++++++++++------------------ 2 files changed, 66 insertions(+), 35 deletions(-) diff --git a/action.yml b/action.yml index 095af22..18672ba 100644 --- a/action.yml +++ b/action.yml @@ -82,11 +82,18 @@ runs: if [ ${#CHANGED_FILES[@]} -gt 0 ]; then echo "Checking ${#CHANGED_FILES[@]} changed file(s) for header compliance..." + CHECK_ROOT="$(mktemp -d)" + trap 'rm -rf "$CHECK_ROOT"' EXIT + for file in "${CHANGED_FILES[@]}"; do + if [ -f "$file" ]; then + mkdir -p "$CHECK_ROOT/$(dirname "$file")" + cp -- "$file" "$CHECK_ROOT/$file" + fi + done copywrite headers \ --config "$GITHUB_ACTION_PATH/main/copywrite/.copywrite.hcl" \ --plan \ - -- \ - "${CHANGED_FILES[@]}" + --dirPath "$CHECK_ROOT" else echo "No added or modified files found to check for header compliance." fi diff --git a/main/githooks.py b/main/githooks.py index 839e24f..ac2bbfe 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -22,7 +22,7 @@ from collections import defaultdict from io import StringIO from pathlib import Path -from tempfile import NamedTemporaryFile +from tempfile import NamedTemporaryFile, TemporaryDirectory from unittest.mock import patch import os import platform @@ -1076,12 +1076,6 @@ def run_copywrite(files): return 1 is_check_mode = mode in ['check', 'plan', 'verify'] - cmd = [copywrite_exe, 'headers', f'--config={config_path}'] - if is_check_mode: - cmd.append('--plan') - cmd.append('--') - cmd.extend(files) - try: if not is_check_mode: unstaged = subprocess.run( @@ -1098,27 +1092,53 @@ def run_copywrite(files): _fail(f'Unable to inspect unstaged changes:\n{unstaged.stderr.strip()}') return 1 - proc = subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - env=env - ) - if proc.returncode != 0: - details = '\n'.join(output.strip() for output in [proc.stdout, proc.stderr] if output.strip()) - _fail(f'Copyright header update failed:\n{details}') - return 1 - if not is_check_mode: - restage = subprocess.run( - ['git', 'add', '--'] + files, - stdout=subprocess.DEVNULL, + with TemporaryDirectory() as temp_dir: + check_root = Path(temp_dir) + for filename in files: + relative_path = Path(filename) + if relative_path.is_absolute() or '..' in relative_path.parts: + _fail(f'Cannot check file outside the repository: {filename}') + return 1 + if relative_path.is_file(): + staged_path = check_root / relative_path + staged_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(relative_path, staged_path) + + cmd = [ + copywrite_exe, + 'headers', + f'--config={config_path}', + f'--dirPath={check_root}' + ] + if is_check_mode: + cmd.append('--plan') + + proc = subprocess.run( + cmd, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, - text=True + text=True, + env=env ) - if restage.returncode != 0: - _fail(f'Unable to restage files updated by Copywrite:\n{restage.stderr.strip()}') + if proc.returncode != 0: + details = '\n'.join(output.strip() for output in [proc.stdout, proc.stderr] if output.strip()) + _fail(f'Copyright header update failed:\n{details}') return 1 + if not is_check_mode: + for filename in files: + updated_path = check_root / filename + if updated_path.is_file(): + shutil.copy2(updated_path, filename) + + restage = subprocess.run( + ['git', 'add', '--'] + files, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + text=True + ) + if restage.returncode != 0: + _fail(f'Unable to restage files updated by Copywrite:\n{restage.stderr.strip()}') + return 1 except (OSError, subprocess.SubprocessError) as error: _fail(f'Failed to run Copywrite: {error}') return 1 @@ -1140,7 +1160,8 @@ def test_missing_executable_is_soft_failure(self, _config, _which): @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'check']) @patch('githooks.subprocess.run') - def test_check_failure_blocks_commit(self, run, _config, _which, _is_file): + @patch('githooks.shutil.copy2') + def test_check_failure_blocks_commit(self, _copy, run, _config, _which, _is_file): run.return_value = subprocess.CompletedProcess([], 1, 'stdout', 'stderr') self.assertEqual(1, run_copywrite(['example.py'])) @@ -1148,24 +1169,26 @@ def test_check_failure_blocks_commit(self, run, _config, _which, _is_file): @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - def test_fix_restages_files(self, run, _config, _which, _is_file): + @patch('githooks.shutil.copy2') + def test_fix_restages_files(self, _copy, run, _config, _which, _is_file): run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), ] self.assertEqual(0, run_copywrite(['example.py'])) - self.assertEqual( - ['copywrite', 'headers', unittest.mock.ANY, '--', 'example.py'], - run.call_args_list[1].args[0] - ) + copywrite_cmd = run.call_args_list[1].args[0] + self.assertEqual(['copywrite', 'headers'], copywrite_cmd[:2]) + self.assertTrue(copywrite_cmd[2].startswith('--config=')) + self.assertTrue(copywrite_cmd[3].startswith('--dirPath=')) self.assertEqual(['git', 'add', '--', 'example.py'], run.call_args_list[-1].args[0]) @patch('githooks.Path.is_file', return_value=True) @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - def test_restage_failure_blocks_commit(self, run, _config, _which, _is_file): + @patch('githooks.shutil.copy2') + def test_restage_failure_blocks_commit(self, _copy, run, _config, _which, _is_file): run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), @@ -1186,7 +1209,8 @@ def test_fix_rejects_partially_staged_files(self, run, _config, _which, _is_file @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'check']) @patch('githooks.subprocess.run', side_effect=OSError('cannot execute')) - def test_subprocess_error_blocks_commit(self, _run, _config, _which, _is_file): + @patch('githooks.shutil.copy2') + def test_subprocess_error_blocks_commit(self, _copy, _run, _config, _which, _is_file): self.assertEqual(1, run_copywrite(['example.py'])) From 066b30bdb268f64bddc4b658f77c0cf17089729e Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:54:10 +0100 Subject: [PATCH 02/20] SYS-8665 fix whitespace splitting --- main/commit-msg | 2 +- main/pre-commit | 2 +- main/pre-merge-commit | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/main/commit-msg b/main/commit-msg index ebdc690..50f7c8a 100755 --- a/main/commit-msg +++ b/main/commit-msg @@ -35,4 +35,4 @@ case "$OSTYPE" in ;; esac -"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]%.*}.py" "$@" +"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]}.py" "$@" diff --git a/main/pre-commit b/main/pre-commit index ebdc690..50f7c8a 100755 --- a/main/pre-commit +++ b/main/pre-commit @@ -35,4 +35,4 @@ case "$OSTYPE" in ;; esac -"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]%.*}.py" "$@" +"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]}.py" "$@" diff --git a/main/pre-merge-commit b/main/pre-merge-commit index ebdc690..50f7c8a 100755 --- a/main/pre-merge-commit +++ b/main/pre-merge-commit @@ -35,4 +35,4 @@ case "$OSTYPE" in ;; esac -"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]%.*}.py" "$@" +"${PYTHON_EXECUTABLE[@]}" "${BASH_SOURCE[0]}.py" "$@" From 276a4e4410ab4f405ba368d2ab65604aa0310f20 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Sat, 29 Aug 2026 10:41:41 +0100 Subject: [PATCH 03/20] SYS-8665 copywrite alternative --- .github/workflows/quality_check.yml | 2 +- README.md | 17 ++- action.yml | 20 +--- main/copywrite/.copywrite.hcl | 37 ------- main/githooks.py | 133 ++++++----------------- main/license_headers.py | 161 ++++++++++++++++++++++++++++ test/test_license_headers.py | 55 ++++++++++ 7 files changed, 255 insertions(+), 170 deletions(-) delete mode 100644 main/copywrite/.copywrite.hcl create mode 100644 main/license_headers.py create mode 100644 test/test_license_headers.py diff --git a/.github/workflows/quality_check.yml b/.github/workflows/quality_check.yml index 050b7e6..8ec6674 100644 --- a/.github/workflows/quality_check.yml +++ b/.github/workflows/quality_check.yml @@ -20,4 +20,4 @@ jobs: flake8 . --count --exit-zero --max-complexity=10 --statistics - name: Pytest run: | - pytest main/githooks.py + pytest main/githooks.py test/test_license_headers.py diff --git a/README.md b/README.md index cc73bad..9fbb0d3 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ The commit will be flagged if it includes certain text files with: * Tabs * Missing terminating newline for certain files * Certain C++ #include patterns and std::exception -* Missing or non-compliant CCDC copyright and license headers (when using the GitHub Action or local copywrite integration) +* Missing or non-compliant CCDC copyright and licence headers (when header validation is enabled) The commit will also be flagged if the commit message does not include a Jira ID (unless marked with NO_JIRA or a Copilot Autofix co-author line), or if the @@ -67,18 +67,15 @@ To enable CCDC commit checks (Jira ID, CRLF, line endings, DO NOT COMMIT, file s ```bash git config --global core.hooksPath /main ``` -3. (Optional) Install `copywrite` to automatically add and format CCDC copyright headers on commit: - * **Windows:** `choco install copywrite` - * **macOS:** `brew install hashicorp/tap/copywrite` - * **Linux:** `go install github.com/hashicorp/copywrite@latest` +3. (Optional) Enable automatic CCDC copyright and licence header formatting as described below. -> **Note:** If `copywrite` is not installed on your machine, native hooks will continue to run all other standard checks and display a gentle warning without failing your commit. +## Configuring Licence Header Behavior -## Configuring Copywrite Behavior +Developers can customise the licence header hook using Git configuration. The +existing `hooks.copywrite` names are retained for compatibility and do not +require the Copywrite executable. -Developers can customise the copywrite hook using Git configuration: - -* **Enable / Disable Copywrite:** +* **Enable / Disable Header Formatting:** ```bash git config --global hooks.copywrite true # opt-in: enable copywrite integration git config --global hooks.copywrite false # default: disabled diff --git a/action.yml b/action.yml index 18672ba..ea4eeee 100644 --- a/action.yml +++ b/action.yml @@ -27,19 +27,12 @@ inputs: runs: using: composite steps: - - name: Install copywrite - if: ${{ inputs.licenseCheck == 'true' }} - uses: hashicorp/setup-copywrite@v1.1.3 - - name: Validate Header Compliance if: ${{ inputs.licenseCheck == 'true' }} shell: bash env: - COPYWRITE_HOOK_ROOT: ${{ github.action_path }} GITHUB_EVENT_BEFORE: ${{ github.event.before }} run: | - copywrite --version - CHANGED_FILES=() if [ "${GITHUB_EVENT_NAME:-}" = "pull_request" ] && [ -n "${GITHUB_BASE_REF:-}" ]; then DIFF_REF="origin/${GITHUB_BASE_REF}" @@ -82,18 +75,7 @@ runs: if [ ${#CHANGED_FILES[@]} -gt 0 ]; then echo "Checking ${#CHANGED_FILES[@]} changed file(s) for header compliance..." - CHECK_ROOT="$(mktemp -d)" - trap 'rm -rf "$CHECK_ROOT"' EXIT - for file in "${CHANGED_FILES[@]}"; do - if [ -f "$file" ]; then - mkdir -p "$CHECK_ROOT/$(dirname "$file")" - cp -- "$file" "$CHECK_ROOT/$file" - fi - done - copywrite headers \ - --config "$GITHUB_ACTION_PATH/main/copywrite/.copywrite.hcl" \ - --plan \ - --dirPath "$CHECK_ROOT" + python3 "$GITHUB_ACTION_PATH/main/license_headers.py" check -- "${CHANGED_FILES[@]}" else echo "No added or modified files found to check for header compliance." fi diff --git a/main/copywrite/.copywrite.hcl b/main/copywrite/.copywrite.hcl deleted file mode 100644 index c76c316..0000000 --- a/main/copywrite/.copywrite.hcl +++ /dev/null @@ -1,37 +0,0 @@ -schema_version = 1 - -project { - copyright_holder = "The Cambridge Crystallographic Data Centre (CCDC)" - - header_ignore = [ - ".git/**", - ".github/**", - "test/**", - "tests/**", - "templates/**", - "**/bin/**", - "**/obj/**", - "**/packages/**", - "**/node_modules/**", - "**/dist/**", - "**/build/**", - "**/.venv/**", - "**/venv/**", - "**/__pycache__/**", - "**/*.Designer.cs", - "**/*.g.cs", - "**/*.generated.*", - "**/*.min.js", - "**/*.lock", - ] -} - -rule { - paths = ["**/*.py", "**/*.sh", "**/*.bash", "**/*.yaml", "**/*.yml"] - license_header = "${COPYWRITE_HOOK_ROOT}/main/copywrite/headers/ccdc_hash.tmpl" -} - -rule { - paths = ["**/*.js", "**/*.ts", "**/*.cs", "**/*.cpp", "**/*.cxx", "**/*.cc", "**/*.h", "**/*.hpp"] - license_header = "${COPYWRITE_HOOK_ROOT}/main/copywrite/headers/ccdc_slash.tmpl" -} \ No newline at end of file diff --git a/main/githooks.py b/main/githooks.py index ac2bbfe..3168891 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -22,12 +22,12 @@ from collections import defaultdict from io import StringIO from pathlib import Path -from tempfile import NamedTemporaryFile, TemporaryDirectory +from tempfile import NamedTemporaryFile from unittest.mock import patch +import license_headers import os import platform import re -import shutil import subprocess import unittest import sys @@ -216,7 +216,7 @@ def get_commit_files(): else: commands += ['HEAD~..', '--'] else: - commands = ['git', 'diff-index', '--ignore-submodules', 'HEAD', '--cached'] + commands = ['git', 'diff', '--cached', '--ignore-submodules', '--name-status', '--'] output = _get_output(commands) result = defaultdict(list) @@ -289,7 +289,7 @@ def get_changed_lines(modified_file): else: commands += ['HEAD~', '--', modified_file] else: - commands = [f'git', 'diff-index', 'HEAD', '--unified=0', '--', f'{modified_file}'] + commands = [f'git', 'diff', '--cached', '--unified=0', '--', f'{modified_file}'] output = _get_output(commands) lines = [] @@ -1034,7 +1034,7 @@ def _test(input, is_good=True): def run_copywrite(files): - '''Run copywrite to automatically check or fix license headers. + '''Check or fix complete CCDC licence headers. Configurable via git config: - `git config --global hooks.copywrite true|false` (default: false, opt-in) @@ -1042,7 +1042,7 @@ def run_copywrite(files): - fix: automatically adds/updates headers and restages files - check: checks header compliance and warns/fails without modifying - If copywrite is not installed and the hook is enabled, print a soft warning and return 0 (do not block commit). + The existing hooks.copywrite setting is retained for compatibility. ''' if not files: return 0 @@ -1052,24 +1052,6 @@ def run_copywrite(files): if enabled_setting is None or enabled_setting.lower() not in ['true', '1', 'yes', 'on']: return 0 - copywrite_exe = shutil.which('copywrite') - if not copywrite_exe: - print(' WARNING: "copywrite" not found on PATH. Skipping copyright header check.') - print(' To enable automatic copyright formatting, install copywrite:') - print(' - Windows: choco install copywrite') - print(' - macOS: brew install hashicorp/tap/copywrite') - print(' - Linux: go install github.com/hashicorp/copywrite@latest') - return 0 - - hook_root = Path(__file__).resolve().parent.parent - config_path = hook_root / 'main' / 'copywrite' / '.copywrite.hcl' - if not config_path.is_file(): - _fail(f'Copywrite configuration not found: {config_path}') - return 1 - - env = os.environ.copy() - env['COPYWRITE_HOOK_ROOT'] = str(hook_root) - mode = (get_config_setting('hooks.copywriteMode') or 'fix').lower() if mode not in ['fix', 'check', 'plan', 'verify']: _fail(f'Unsupported hooks.copywriteMode value: {mode}') @@ -1092,55 +1074,22 @@ def run_copywrite(files): _fail(f'Unable to inspect unstaged changes:\n{unstaged.stderr.strip()}') return 1 - with TemporaryDirectory() as temp_dir: - check_root = Path(temp_dir) - for filename in files: - relative_path = Path(filename) - if relative_path.is_absolute() or '..' in relative_path.parts: - _fail(f'Cannot check file outside the repository: {filename}') - return 1 - if relative_path.is_file(): - staged_path = check_root / relative_path - staged_path.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(relative_path, staged_path) - - cmd = [ - copywrite_exe, - 'headers', - f'--config={config_path}', - f'--dirPath={check_root}' - ] - if is_check_mode: - cmd.append('--plan') - - proc = subprocess.run( - cmd, - stdout=subprocess.PIPE, + if license_headers.process_files(files, fix=not is_check_mode) != 0: + _fail('Copyright and licence header check failed.') + return 1 + + if not is_check_mode: + restage = subprocess.run( + ['git', 'add', '--'] + files, + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, - text=True, - env=env + text=True ) - if proc.returncode != 0: - details = '\n'.join(output.strip() for output in [proc.stdout, proc.stderr] if output.strip()) - _fail(f'Copyright header update failed:\n{details}') + if restage.returncode != 0: + _fail(f'Unable to restage files updated by the licence header fixer:\n{restage.stderr.strip()}') return 1 - if not is_check_mode: - for filename in files: - updated_path = check_root / filename - if updated_path.is_file(): - shutil.copy2(updated_path, filename) - - restage = subprocess.run( - ['git', 'add', '--'] + files, - stdout=subprocess.DEVNULL, - stderr=subprocess.PIPE, - text=True - ) - if restage.returncode != 0: - _fail(f'Unable to restage files updated by Copywrite:\n{restage.stderr.strip()}') - return 1 - except (OSError, subprocess.SubprocessError) as error: - _fail(f'Failed to run Copywrite: {error}') + except (OSError, subprocess.SubprocessError, UnicodeError) as error: + _fail(f'Failed to process licence headers: {error}') return 1 return 0 @@ -1151,66 +1100,44 @@ class TestRunCopywrite(unittest.TestCase): def test_disabled(self, _config): self.assertEqual(0, run_copywrite(['example.py'])) - @patch('githooks.shutil.which', return_value=None) - @patch('githooks.get_config_setting', return_value='true') - def test_missing_executable_is_soft_failure(self, _config, _which): - self.assertEqual(0, run_copywrite(['example.py'])) - - @patch('githooks.Path.is_file', return_value=True) - @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.subprocess.run') - @patch('githooks.shutil.copy2') - def test_check_failure_blocks_commit(self, _copy, run, _config, _which, _is_file): - run.return_value = subprocess.CompletedProcess([], 1, 'stdout', 'stderr') + @patch('githooks.license_headers.process_files', return_value=1) + def test_check_failure_blocks_commit(self, process_files, _config): self.assertEqual(1, run_copywrite(['example.py'])) + process_files.assert_called_once_with(['example.py'], fix=False) - @patch('githooks.Path.is_file', return_value=True) - @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.shutil.copy2') - def test_fix_restages_files(self, _copy, run, _config, _which, _is_file): + @patch('githooks.license_headers.process_files', return_value=0) + def test_fix_restages_files(self, process_files, run, _config): run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), - subprocess.CompletedProcess([], 0, '', ''), ] self.assertEqual(0, run_copywrite(['example.py'])) - copywrite_cmd = run.call_args_list[1].args[0] - self.assertEqual(['copywrite', 'headers'], copywrite_cmd[:2]) - self.assertTrue(copywrite_cmd[2].startswith('--config=')) - self.assertTrue(copywrite_cmd[3].startswith('--dirPath=')) + process_files.assert_called_once_with(['example.py'], fix=True) self.assertEqual(['git', 'add', '--', 'example.py'], run.call_args_list[-1].args[0]) - @patch('githooks.Path.is_file', return_value=True) - @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.shutil.copy2') - def test_restage_failure_blocks_commit(self, _copy, run, _config, _which, _is_file): + @patch('githooks.license_headers.process_files', return_value=0) + def test_restage_failure_blocks_commit(self, _process_files, run, _config): run.side_effect = [ - subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 1, '', 'cannot add'), ] self.assertEqual(1, run_copywrite(['example.py'])) - @patch('githooks.Path.is_file', return_value=True) - @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - def test_fix_rejects_partially_staged_files(self, run, _config, _which, _is_file): + def test_fix_rejects_partially_staged_files(self, run, _config): run.return_value = subprocess.CompletedProcess([], 1, '', '') self.assertEqual(1, run_copywrite(['example.py'])) run.assert_called_once() - @patch('githooks.Path.is_file', return_value=True) - @patch('githooks.shutil.which', return_value='copywrite') @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.subprocess.run', side_effect=OSError('cannot execute')) - @patch('githooks.shutil.copy2') - def test_subprocess_error_blocks_commit(self, _copy, _run, _config, _which, _is_file): + @patch('githooks.license_headers.process_files', side_effect=OSError('cannot read')) + def test_processing_error_blocks_commit(self, _process_files, _config): self.assertEqual(1, run_copywrite(['example.py'])) diff --git a/main/license_headers.py b/main/license_headers.py new file mode 100644 index 0000000..524d38b --- /dev/null +++ b/main/license_headers.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +# +# This code is Copyright (C) 2026 The Cambridge Crystallographic Data Centre (CCDC) +# of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This +# code may not be used, reproduced, translated, modified, disassembled or +# copied, except in accordance with a valid licence agreement with CCDC and +# may NOT be disclosed or redistributed in any form, either in whole or in +# part, to any third party. All copies of this code made in accordance with a +# valid licence agreement as referred to above must contain this copyright +# notice. +# +# No representations, warranties, or liabilities are expressed or implied in +# the supply of this code by CCDC, its servants or agents, except where such +# exclusion or limitation is prohibited, void or unenforceable under governing +# law. +# +'''Check and fix full CCDC copyright and licence headers.''' + +import argparse +from datetime import datetime +from pathlib import Path, PurePosixPath +import re +import sys + + +HASH_EXTENSIONS = {'.py', '.sh', '.bash', '.yaml', '.yml'} +SLASH_EXTENSIONS = {'.js', '.ts', '.cs', '.cpp', '.cxx', '.cc', '.h', '.hpp'} +IGNORED_DIRECTORIES = { + '.git', '.github', 'test', 'tests', 'templates', 'bin', 'obj', 'packages', + 'node_modules', 'dist', 'build', '.venv', 'venv', '__pycache__' +} +IGNORED_SUFFIXES = ('.designer.cs', '.g.cs', '.min.js', '.lock') +TEMPLATE_DIRECTORY = Path(__file__).resolve().parent / 'copywrite' / 'headers' + + +def _comment_style(filename): + path = PurePosixPath(str(filename).replace('\\', '/')) + lower_name = path.name.lower() + if any(part.lower() in IGNORED_DIRECTORIES for part in path.parts): + return None + if lower_name.endswith(IGNORED_SUFFIXES) or '.generated.' in lower_name: + return None + if path.suffix.lower() in HASH_EXTENSIONS: + return 'hash' + if path.suffix.lower() in SLASH_EXTENSIONS: + return 'slash' + return None + + +def _render_header(style, newline='\n', year=None): + template_path = TEMPLATE_DIRECTORY / f'ccdc_{style}.tmpl' + template = template_path.read_text(encoding='utf-8') + rendered = template.replace('{{ .Year }}', str(year or datetime.now().year)) + return rendered.replace('\n', newline) + + +def _header_offset(filename, text, style): + if style != 'hash': + return 0 + + offset = 0 + if text.startswith('#!'): + newline_offset = text.find('\n') + offset = len(text) if newline_offset == -1 else newline_offset + 1 + + if str(filename).lower().endswith('.py'): + first_line = text[offset:].split('\n', 1)[0] + if re.match(r'^#.*coding[:=]\s*[-\w.]+', first_line): + offset += len(first_line) + (1 if offset + len(first_line) < len(text) else 0) + return offset + + +def _existing_header_end(text, offset, style): + marker = '#' if style == 'hash' else '//' + position = offset + lines = [] + for line in text[offset:].splitlines(keepends=True): + stripped = line.strip() + if stripped and not stripped.startswith(marker): + break + lines.append((position, position + len(line), stripped)) + position += len(line) + + block = text[offset:position] + if 'Copyright' not in block or 'Cambridge Crystallographic Data Centre' not in block: + return offset + + for index, (_, line_end, stripped) in enumerate(lines): + if stripped.endswith('law.'): + if index + 1 < len(lines) and lines[index + 1][2] == marker: + return lines[index + 1][1] + return line_end + return position + + +def check_content(filename, data, year=None): + '''Return an error message when a supported file lacks the exact header.''' + style = _comment_style(filename) + if style is None: + return None + try: + text = data.decode('utf-8') if isinstance(data, bytes) else data + except UnicodeDecodeError: + return 'file is not UTF-8 encoded' + + newline = '\r\n' if '\r\n' in text else '\n' + offset = _header_offset(filename, text, style) + expected = _render_header(style, newline, year) + if text.startswith(expected, offset): + return None + return 'missing or non-compliant CCDC copyright and licence header' + + +def fix_content(filename, data, year=None): + '''Return content with the full rendered header for supported files.''' + style = _comment_style(filename) + if style is None: + return data + + was_bytes = isinstance(data, bytes) + text = data.decode('utf-8') if was_bytes else data + newline = '\r\n' if '\r\n' in text else '\n' + offset = _header_offset(filename, text, style) + expected = _render_header(style, newline, year) + if text.startswith(expected, offset): + return data + + header_end = _existing_header_end(text, offset, style) + fixed = text[:offset] + expected + text[header_end:] + return fixed.encode('utf-8') if was_bytes else fixed + + +def process_files(files, fix=False, year=None): + failures = 0 + for filename in files: + path = Path(filename) + if not path.is_file() or _comment_style(filename) is None: + continue + data = path.read_bytes() + issue = check_content(filename, data, year) + if issue is None: + continue + if fix: + path.write_bytes(fix_content(filename, data, year)) + print(f'Updated CCDC licence header: {filename}') + else: + print(f'HEADER FAIL: {filename}: {issue}') + failures += 1 + return failures + + +def main(argv=None): + parser = argparse.ArgumentParser() + parser.add_argument('mode', choices=['check', 'fix']) + parser.add_argument('files', nargs='*') + arguments = parser.parse_args(argv) + return process_files(arguments.files, fix=arguments.mode == 'fix') + + +if __name__ == '__main__': + sys.exit(main()) \ No newline at end of file diff --git a/test/test_license_headers.py b/test/test_license_headers.py new file mode 100644 index 0000000..067e17e --- /dev/null +++ b/test/test_license_headers.py @@ -0,0 +1,55 @@ +from pathlib import Path +import sys + + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'main')) +import license_headers + + +def test_complete_hash_header_passes(): + header = license_headers._render_header('hash', year=2026) + assert license_headers.check_content('example.py', header + 'print("ok")\n', 2026) is None + + +def test_literal_year_token_fails(): + header = license_headers._render_header('hash', year=2026).replace('2026', '{{ .Year }}') + assert license_headers.check_content('example.py', header, 2026) is not None + + +def test_removed_licence_line_fails_and_is_repaired(): + header = license_headers._render_header('hash', year=2026) + broken = header.replace('# copied, except in accordance with a valid licence agreement with CCDC and\n', '') + content = broken + 'print("ok")\n' + assert license_headers.check_content('example.py', content, 2026) is not None + fixed = license_headers.fix_content('example.py', content, 2026) + assert fixed == header + 'print("ok")\n' + + +def test_missing_header_is_added_after_shebang(): + fixed = license_headers.fix_content('script.py', '#!/usr/bin/env python3\nprint("ok")\n', 2026) + assert fixed.startswith('#!/usr/bin/env python3\n#\n# This code is Copyright (C) 2026') + assert fixed.endswith('print("ok")\n') + + +def test_python_encoding_declaration_is_preserved_before_header(): + fixed = license_headers.fix_content('script.py', '# -*- coding: latin-1 -*-\nprint("ok")\n', 2026) + assert fixed.startswith('# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026') + + +def test_copywrite_one_line_header_is_replaced(): + old_header = '# Copyright The Cambridge Crystallographic Data Centre (CCDC) 2021, 2026\n\n' + fixed = license_headers.fix_content('example.py', old_header + 'print("ok")\n', 2026) + assert fixed.startswith('#\n# This code is Copyright (C) 2026') + assert old_header not in fixed + + +def test_slash_header_is_added(): + fixed = license_headers.fix_content('example.cpp', 'int main() {}\n', 2026) + assert fixed.startswith('//\n// This code is Copyright (C) 2026') + assert license_headers.check_content('example.cpp', fixed, 2026) is None + + +def test_ignored_and_unsupported_files_are_skipped(): + assert license_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None + assert license_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None + assert license_headers.check_content('README.md', '# Read me\n', 2026) is None \ No newline at end of file From 106e921f49eb7a4375adf1f09a8572a7de3a67a6 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:23:46 +0100 Subject: [PATCH 04/20] SYS-8665 co-pilot comments --- main/license_headers.py | 52 +++++++++++++++++++++++++----------- test/test_license_headers.py | 35 +++++++++++++++++++++++- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/main/license_headers.py b/main/license_headers.py index 524d38b..5246c15 100644 --- a/main/license_headers.py +++ b/main/license_headers.py @@ -18,9 +18,11 @@ import argparse from datetime import datetime +from io import BytesIO from pathlib import Path, PurePosixPath import re import sys +import tokenize HASH_EXTENSIONS = {'.py', '.sh', '.bash', '.yaml', '.yml'} @@ -31,6 +33,11 @@ } IGNORED_SUFFIXES = ('.designer.cs', '.g.cs', '.min.js', '.lock') TEMPLATE_DIRECTORY = Path(__file__).resolve().parent / 'copywrite' / 'headers' +PYTHON_ENCODING_PATTERN = re.compile(r'^[ \t\f]*#.*?coding[:=][ \t]*[-\w.]+') +LEGACY_HEADER_PATTERN = re.compile( + r'^(#|//) Copyright The Cambridge Crystallographic Data Centre ' + r'\(CCDC\) \d{4}(?:, \d{4})?\r?\n?$' +) def _comment_style(filename): @@ -54,20 +61,29 @@ def _render_header(style, newline='\n', year=None): return rendered.replace('\n', newline) +def _decode_content(filename, data): + if not isinstance(data, bytes): + return data, None + encoding = 'utf-8' + if str(filename).lower().endswith('.py'): + encoding, _ = tokenize.detect_encoding(BytesIO(data).readline) + return data.decode(encoding), encoding + + def _header_offset(filename, text, style): if style != 'hash': return 0 - offset = 0 - if text.startswith('#!'): - newline_offset = text.find('\n') - offset = len(text) if newline_offset == -1 else newline_offset + 1 + lines = text.splitlines(keepends=True) + has_shebang = bool(lines and lines[0].startswith('#!')) + if not str(filename).lower().endswith('.py'): + return len(lines[0]) if has_shebang else 0 - if str(filename).lower().endswith('.py'): - first_line = text[offset:].split('\n', 1)[0] - if re.match(r'^#.*coding[:=]\s*[-\w.]+', first_line): - offset += len(first_line) + (1 if offset + len(first_line) < len(text) else 0) - return offset + candidate_indexes = [1] if has_shebang else range(min(2, len(lines))) + for index in candidate_indexes: + if index < len(lines) and PYTHON_ENCODING_PATTERN.match(lines[index]): + return sum(len(line) for line in lines[:index + 1]) + return len(lines[0]) if has_shebang else 0 def _existing_header_end(text, offset, style): @@ -90,7 +106,12 @@ def _existing_header_end(text, offset, style): if index + 1 < len(lines) and lines[index + 1][2] == marker: return lines[index + 1][1] return line_end - return position + + if lines and LEGACY_HEADER_PATTERN.match(text[lines[0][0]:lines[0][1]]): + if len(lines) > 1 and lines[1][2] == '': + return lines[1][1] + return lines[0][1] + return offset def check_content(filename, data, year=None): @@ -99,9 +120,9 @@ def check_content(filename, data, year=None): if style is None: return None try: - text = data.decode('utf-8') if isinstance(data, bytes) else data - except UnicodeDecodeError: - return 'file is not UTF-8 encoded' + text, _ = _decode_content(filename, data) + except (LookupError, SyntaxError, UnicodeDecodeError): + return 'file encoding could not be decoded' newline = '\r\n' if '\r\n' in text else '\n' offset = _header_offset(filename, text, style) @@ -117,8 +138,7 @@ def fix_content(filename, data, year=None): if style is None: return data - was_bytes = isinstance(data, bytes) - text = data.decode('utf-8') if was_bytes else data + text, encoding = _decode_content(filename, data) newline = '\r\n' if '\r\n' in text else '\n' offset = _header_offset(filename, text, style) expected = _render_header(style, newline, year) @@ -127,7 +147,7 @@ def fix_content(filename, data, year=None): header_end = _existing_header_end(text, offset, style) fixed = text[:offset] + expected + text[header_end:] - return fixed.encode('utf-8') if was_bytes else fixed + return fixed.encode(encoding) if encoding else fixed def process_files(files, fix=False, year=None): diff --git a/test/test_license_headers.py b/test/test_license_headers.py index 067e17e..bf76454 100644 --- a/test/test_license_headers.py +++ b/test/test_license_headers.py @@ -36,11 +36,44 @@ def test_python_encoding_declaration_is_preserved_before_header(): assert fixed.startswith('# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026') +def test_second_line_python_encoding_declaration_preserves_comment_prefix(): + source = '# generated source\n# coding=latin-1\nprint("ok")\n' + fixed = license_headers.fix_content('script.py', source, 2026) + prefix = '# generated source\n# coding=latin-1\n#\n# This code is Copyright (C) 2026' + assert fixed.startswith(prefix) + assert license_headers.check_content('script.py', fixed, 2026) is None + + +def test_encoding_declaration_after_shebang_is_preserved(): + source = '#!/usr/bin/env python3\n# -*- coding: latin-1 -*-\nprint("ok")\n' + fixed = license_headers.fix_content('script.py', source, 2026) + prefix = '#!/usr/bin/env python3\n# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026' + assert fixed.startswith(prefix) + assert license_headers.check_content('script.py', fixed, 2026) is None + + +def test_non_utf8_python_file_preserves_declared_encoding(): + source = '# generated\n# coding=latin-1\nname = "caf\xe9"\n'.encode('latin-1') + fixed = license_headers.fix_content('script.py', source, 2026) + assert isinstance(fixed, bytes) + assert b'# coding=latin-1\n#\n# This code is Copyright (C) 2026' in fixed + assert b'caf\xe9' in fixed + assert license_headers.check_content('script.py', fixed, 2026) is None + + +def test_empty_python_file_gets_header(): + fixed = license_headers.fix_content('empty.py', b'', 2026) + assert fixed.startswith(b'#\n# This code is Copyright (C) 2026') + assert license_headers.check_content('empty.py', fixed, 2026) is None + + def test_copywrite_one_line_header_is_replaced(): old_header = '# Copyright The Cambridge Crystallographic Data Centre (CCDC) 2021, 2026\n\n' - fixed = license_headers.fix_content('example.py', old_header + 'print("ok")\n', 2026) + source_comment = '# keep this source comment\n' + fixed = license_headers.fix_content('example.py', old_header + source_comment + 'print("ok")\n', 2026) assert fixed.startswith('#\n# This code is Copyright (C) 2026') assert old_header not in fixed + assert fixed.endswith(source_comment + 'print("ok")\n') def test_slash_header_is_added(): From b3c5b44902b950e048a0aa0f5092dddefd588490 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:34:24 +0100 Subject: [PATCH 05/20] SYS-8665 fix BOM issue --- main/license_headers.py | 78 +++++++++++++++++++++++++----------- test/test_license_headers.py | 30 ++++++++++++++ 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/main/license_headers.py b/main/license_headers.py index 5246c15..70e21eb 100644 --- a/main/license_headers.py +++ b/main/license_headers.py @@ -17,6 +17,7 @@ '''Check and fix full CCDC copyright and licence headers.''' import argparse +import codecs from datetime import datetime from io import BytesIO from pathlib import Path, PurePosixPath @@ -64,9 +65,12 @@ def _render_header(style, newline='\n', year=None): def _decode_content(filename, data): if not isinstance(data, bytes): return data, None - encoding = 'utf-8' - if str(filename).lower().endswith('.py'): + if data.startswith(codecs.BOM_UTF8): + encoding = 'utf-8-sig' + elif str(filename).lower().endswith('.py'): encoding, _ = tokenize.detect_encoding(BytesIO(data).readline) + else: + encoding = 'utf-8' return data.decode(encoding), encoding @@ -86,32 +90,57 @@ def _header_offset(filename, text, style): return len(lines[0]) if has_shebang else 0 -def _existing_header_end(text, offset, style): - marker = '#' if style == 'hash' else '//' +def _header_line_matches(actual, expected): + if 'This code is Copyright (C)' in expected: + marker = expected.split(' ', 1)[0] + pattern = ( + rf'{re.escape(marker)} This code is Copyright \(C\) ' + r'(?:\d{4}(?:, \d{4})?|\{\{ \.Year \}\}) ' + r'The Cambridge Crystallographic Data Centre \(CCDC\)' + ) + return re.fullmatch(pattern, actual) is not None + return actual == expected + + +def _known_header_prefix_end(text, offset, expected): + actual_lines = [] position = offset - lines = [] for line in text[offset:].splitlines(keepends=True): - stripped = line.strip() - if stripped and not stripped.startswith(marker): - break - lines.append((position, position + len(line), stripped)) + actual_lines.append((position, position + len(line), line.strip())) position += len(line) - block = text[offset:position] - if 'Copyright' not in block or 'Cambridge Crystallographic Data Centre' not in block: + expected_lines = [line.strip() for line in expected.splitlines()] + if len(actual_lines) < 2 or len(expected_lines) < 2: + return offset + if actual_lines[0][2] != expected_lines[0]: return offset + if not _header_line_matches(actual_lines[1][2], expected_lines[1]): + return offset + + header_end = actual_lines[1][1] + expected_index = 2 + for _, line_end, actual in actual_lines[2:]: + if actual == '': + continue + match_index = next( + (index for index in range(expected_index, len(expected_lines)) + if _header_line_matches(actual, expected_lines[index])), + None + ) + if match_index is None: + break + expected_index = match_index + 1 + header_end = line_end + return header_end - for index, (_, line_end, stripped) in enumerate(lines): - if stripped.endswith('law.'): - if index + 1 < len(lines) and lines[index + 1][2] == marker: - return lines[index + 1][1] - return line_end - if lines and LEGACY_HEADER_PATTERN.match(text[lines[0][0]:lines[0][1]]): - if len(lines) > 1 and lines[1][2] == '': - return lines[1][1] - return lines[0][1] - return offset +def _existing_header_end(text, offset, style, expected): + lines = text[offset:].splitlines(keepends=True) + if lines and LEGACY_HEADER_PATTERN.match(lines[0]): + if len(lines) > 1 and lines[1].strip() == '': + return offset + len(lines[0]) + len(lines[1]) + return offset + len(lines[0]) + return _known_header_prefix_end(text, offset, expected) def check_content(filename, data, year=None): @@ -145,8 +174,11 @@ def fix_content(filename, data, year=None): if text.startswith(expected, offset): return data - header_end = _existing_header_end(text, offset, style) - fixed = text[:offset] + expected + text[header_end:] + header_end = _existing_header_end(text, offset, style, expected) + prefix = text[:offset] + if prefix and not prefix.endswith(('\n', '\r')): + prefix += newline + fixed = prefix + expected + text[header_end:] return fixed.encode(encoding) if encoding else fixed diff --git a/test/test_license_headers.py b/test/test_license_headers.py index bf76454..9564dae 100644 --- a/test/test_license_headers.py +++ b/test/test_license_headers.py @@ -67,6 +67,26 @@ def test_empty_python_file_gets_header(): assert license_headers.check_content('empty.py', fixed, 2026) is None +def test_utf8_bom_remains_at_byte_zero_for_supported_non_python_files(): + for filename in ['example.yml', 'example.js', 'example.cpp']: + fixed = license_headers.fix_content(filename, b'\xef\xbb\xbfvalue\n', 2026) + assert fixed.startswith(b'\xef\xbb\xbf') + assert fixed.count(b'\xef\xbb\xbf') == 1 + assert license_headers.check_content(filename, fixed, 2026) is None + + +def test_unterminated_shebang_is_separated_from_header(): + for filename, shebang in [('script.py', b'#!/usr/bin/env python3'), ('script.sh', b'#!/bin/sh')]: + fixed = license_headers.fix_content(filename, shebang, 2026) + assert fixed.startswith(shebang + b'\n#\n# This code is Copyright (C) 2026') + + +def test_unterminated_encoding_declaration_is_separated_from_header(): + declaration = b'# coding=latin-1' + fixed = license_headers.fix_content('script.py', declaration, 2026) + assert fixed.startswith(declaration + b'\n#\n# This code is Copyright (C) 2026') + + def test_copywrite_one_line_header_is_replaced(): old_header = '# Copyright The Cambridge Crystallographic Data Centre (CCDC) 2021, 2026\n\n' source_comment = '# keep this source comment\n' @@ -76,6 +96,16 @@ def test_copywrite_one_line_header_is_replaced(): assert fixed.endswith(source_comment + 'print("ok")\n') +def test_truncated_full_header_is_replaced_without_losing_source_comment(): + header = license_headers._render_header('hash', year=2026) + old_header = license_headers._render_header('hash', year=2025) + truncated = old_header.split('# law.\n', 1)[0] + source = '# keep this source comment\nprint("ok")\n' + fixed = license_headers.fix_content('example.py', truncated + source, 2026) + assert fixed == header + source + assert fixed.count('This code is Copyright (C)') == 1 + + def test_slash_header_is_added(): fixed = license_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') From 37327b307cc146ced0922ca4a396e1ced67c647d Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:38:44 +0100 Subject: [PATCH 06/20] SYS-8665 heal broken headers --- main/license_headers.py | 25 +++++++++++++++++++++++++ test/test_license_headers.py | 24 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/main/license_headers.py b/main/license_headers.py index 70e21eb..7e10897 100644 --- a/main/license_headers.py +++ b/main/license_headers.py @@ -134,12 +134,37 @@ def _known_header_prefix_end(text, offset, expected): return header_end +def _damaged_full_header_end(text, offset, style): + marker = '#' if style == 'hash' else '//' + lines = text[offset:].splitlines(keepends=True) + if len(lines) < 2 or lines[0].strip() != marker: + return offset + if not lines[1].strip().startswith(f'{marker} This code is Copyright'): + return offset + + position = offset + line_positions = [] + for line in lines: + line_positions.append((position, position + len(line), line.strip())) + position += len(line) + + for index, (_, line_end, stripped) in enumerate(line_positions[2:], start=2): + if stripped.startswith(marker) and stripped.endswith('law.'): + if index + 1 < len(line_positions) and line_positions[index + 1][2] == marker: + return line_positions[index + 1][1] + return line_end + return offset + + def _existing_header_end(text, offset, style, expected): lines = text[offset:].splitlines(keepends=True) if lines and LEGACY_HEADER_PATTERN.match(lines[0]): if len(lines) > 1 and lines[1].strip() == '': return offset + len(lines[0]) + len(lines[1]) return offset + len(lines[0]) + damaged_header_end = _damaged_full_header_end(text, offset, style) + if damaged_header_end != offset: + return damaged_header_end return _known_header_prefix_end(text, offset, expected) diff --git a/test/test_license_headers.py b/test/test_license_headers.py index 9564dae..6ddbf8b 100644 --- a/test/test_license_headers.py +++ b/test/test_license_headers.py @@ -106,6 +106,30 @@ def test_truncated_full_header_is_replaced_without_losing_source_comment(): assert fixed.count('This code is Copyright (C)') == 1 +def test_heavily_damaged_full_header_is_replaced_without_duplication(): + damaged = '''# +# This code is CopyrightCrystallographic Data Centre (CCDC) +# of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This +# code may not be used, reproduced,sassembled or +# copied, except in accordance with a valid licence agreement with CCDC and +# may NOT be disclosed or redistributhole or in +# part, toust contain this copyright +# notice. + +# No representations, warranties, or liabilities are expressed or implied in +# the supply servants or agents, except where such +# exclusion or limitation is prohibited, void or unenforceable under governing +# law. +# + +''' + source = '# keep this source comment\nhello\n' + header = license_headers._render_header('hash', year=2026) + fixed = license_headers.fix_content('example.py', damaged + source, 2026) + assert fixed == header + '\n' + source + assert fixed.count('This code is Copyright') == 1 + + def test_slash_header_is_added(): fixed = license_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') From 506ffa929c3c1fe9a1045a180c5cb99aadcb6dd8 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:42:21 +0100 Subject: [PATCH 07/20] SYS-8665 properly repair corruption of headers --- main/license_headers.py | 6 +++++- test/test_license_headers.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/main/license_headers.py b/main/license_headers.py index 7e10897..4dd048a 100644 --- a/main/license_headers.py +++ b/main/license_headers.py @@ -139,7 +139,11 @@ def _damaged_full_header_end(text, offset, style): lines = text[offset:].splitlines(keepends=True) if len(lines) < 2 or lines[0].strip() != marker: return offset - if not lines[1].strip().startswith(f'{marker} This code is Copyright'): + identity_line = lines[1].strip() + if ( + not identity_line.startswith(f'{marker} This code is ') + or 'Crystallographic Data Centre (CCDC)' not in identity_line + ): return offset position = offset diff --git a/test/test_license_headers.py b/test/test_license_headers.py index 6ddbf8b..b66a1bc 100644 --- a/test/test_license_headers.py +++ b/test/test_license_headers.py @@ -130,6 +130,31 @@ def test_heavily_damaged_full_header_is_replaced_without_duplication(): assert fixed.count('This code is Copyright') == 1 +def test_damaged_copyright_identity_line_is_repaired_without_duplication(): + damaged = '''# +# This code is Cop Crystallographic Data Centre (CCDC) +# of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This +# code may not beteded, disassembled or +# copied, except in accordance with a valid licence agreement with CCDC and + +# part, to any third party. All copies of this code made in accordance with a +# valid licence agreement as referred to above must contain this copyright +# notice. +# +# No representations, warranties, or liabilities are expressed or implied in +# the supply of this cod servants or agents, except where such +# exclusion or limitation is prohibited, vrceable under governing +# law. +# + +''' + source = '# keep this source comment\nhello\n' + header = license_headers._render_header('hash', year=2026) + fixed = license_headers.fix_content('example.py', damaged + source, 2026) + assert fixed == header + '\n' + source + assert fixed.count('This code is Copyright') == 1 + + def test_slash_header_is_added(): fixed = license_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') From fb2d13d1752f484d0f08938d76c435a97c9a691e Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:24:17 +0100 Subject: [PATCH 08/20] SYS-8665 allow for both licence and license --- .github/workflows/quality_check.yml | 2 +- .github/workflows/status_check.yml | 2 +- README.md | 4 +- action.yml | 10 ++- main/githooks.py | 12 ++-- ...{license_headers.py => licence_headers.py} | 2 +- templates/compliance.yml | 2 +- ...nse_headers.py => test_licence_headers.py} | 70 +++++++++---------- 8 files changed, 54 insertions(+), 50 deletions(-) rename main/{license_headers.py => licence_headers.py} (99%) rename test/{test_license_headers.py => test_licence_headers.py} (71%) diff --git a/.github/workflows/quality_check.yml b/.github/workflows/quality_check.yml index 8ec6674..0c11ffd 100644 --- a/.github/workflows/quality_check.yml +++ b/.github/workflows/quality_check.yml @@ -20,4 +20,4 @@ jobs: flake8 . --count --exit-zero --max-complexity=10 --statistics - name: Pytest run: | - pytest main/githooks.py test/test_license_headers.py + pytest main/githooks.py test/test_licence_headers.py diff --git a/.github/workflows/status_check.yml b/.github/workflows/status_check.yml index a86b48f..86a0853 100644 --- a/.github/workflows/status_check.yml +++ b/.github/workflows/status_check.yml @@ -26,4 +26,4 @@ jobs: - uses: ./ with: commitMessage: ${{ env.commit_message }} - licenseCheck: true + licenceCheck: true diff --git a/README.md b/README.md index 9fbb0d3..31efd8a 100644 --- a/README.md +++ b/README.md @@ -52,8 +52,8 @@ headers and file compliance rules in CI. - uses: ccdc-opensource/commit-hooks@v8 with: commitMessage: ${{ env.commit_message }} - # Optional: enable CCDC license header validation on PR changed files - licenseCheck: true # default: false (opt-in) + # Optional: enable CCDC licence header validation on PR changed files + licenceCheck: true # default: false (opt-in) ``` A complete workflow template for CI is available in [templates/compliance.yml](templates/compliance.yml). diff --git a/action.yml b/action.yml index ea4eeee..de92503 100644 --- a/action.yml +++ b/action.yml @@ -20,15 +20,19 @@ inputs: commitMessage: description: 'The commit message' required: true + licenceCheck: + description: 'Validate CCDC copyright and licence headers on changed files (true/false)' + required: false + default: 'false' licenseCheck: - description: 'Validate CCDC copyright and license headers on changed files (true/false)' + description: 'Alias for licenceCheck (true/false)' required: false default: 'false' runs: using: composite steps: - name: Validate Header Compliance - if: ${{ inputs.licenseCheck == 'true' }} + if: ${{ inputs.licenceCheck == 'true' || inputs.licenseCheck == 'true' }} shell: bash env: GITHUB_EVENT_BEFORE: ${{ github.event.before }} @@ -75,7 +79,7 @@ runs: if [ ${#CHANGED_FILES[@]} -gt 0 ]; then echo "Checking ${#CHANGED_FILES[@]} changed file(s) for header compliance..." - python3 "$GITHUB_ACTION_PATH/main/license_headers.py" check -- "${CHANGED_FILES[@]}" + python3 "$GITHUB_ACTION_PATH/main/licence_headers.py" check -- "${CHANGED_FILES[@]}" else echo "No added or modified files found to check for header compliance." fi diff --git a/main/githooks.py b/main/githooks.py index 3168891..d17b8d9 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -24,7 +24,7 @@ from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch -import license_headers +import licence_headers import os import platform import re @@ -1074,7 +1074,7 @@ def run_copywrite(files): _fail(f'Unable to inspect unstaged changes:\n{unstaged.stderr.strip()}') return 1 - if license_headers.process_files(files, fix=not is_check_mode) != 0: + if licence_headers.process_files(files, fix=not is_check_mode) != 0: _fail('Copyright and licence header check failed.') return 1 @@ -1101,14 +1101,14 @@ def test_disabled(self, _config): self.assertEqual(0, run_copywrite(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.license_headers.process_files', return_value=1) + @patch('githooks.licence_headers.process_files', return_value=1) def test_check_failure_blocks_commit(self, process_files, _config): self.assertEqual(1, run_copywrite(['example.py'])) process_files.assert_called_once_with(['example.py'], fix=False) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.license_headers.process_files', return_value=0) + @patch('githooks.licence_headers.process_files', return_value=0) def test_fix_restages_files(self, process_files, run, _config): run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), @@ -1120,7 +1120,7 @@ def test_fix_restages_files(self, process_files, run, _config): @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.license_headers.process_files', return_value=0) + @patch('githooks.licence_headers.process_files', return_value=0) def test_restage_failure_blocks_commit(self, _process_files, run, _config): run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), @@ -1136,7 +1136,7 @@ def test_fix_rejects_partially_staged_files(self, run, _config): run.assert_called_once() @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.license_headers.process_files', side_effect=OSError('cannot read')) + @patch('githooks.licence_headers.process_files', side_effect=OSError('cannot read')) def test_processing_error_blocks_commit(self, _process_files, _config): self.assertEqual(1, run_copywrite(['example.py'])) diff --git a/main/license_headers.py b/main/licence_headers.py similarity index 99% rename from main/license_headers.py rename to main/licence_headers.py index 4dd048a..dbc911f 100644 --- a/main/license_headers.py +++ b/main/licence_headers.py @@ -239,4 +239,4 @@ def main(argv=None): if __name__ == '__main__': - sys.exit(main()) \ No newline at end of file + sys.exit(main()) diff --git a/templates/compliance.yml b/templates/compliance.yml index fc618cd..3e48b65 100644 --- a/templates/compliance.yml +++ b/templates/compliance.yml @@ -36,4 +36,4 @@ jobs: uses: ccdc-opensource/commit-hooks@v8 with: commitMessage: ${{ env.commit_message }} - licenseCheck: true + licenceCheck: true diff --git a/test/test_license_headers.py b/test/test_licence_headers.py similarity index 71% rename from test/test_license_headers.py rename to test/test_licence_headers.py index b66a1bc..1f3c1a4 100644 --- a/test/test_license_headers.py +++ b/test/test_licence_headers.py @@ -3,105 +3,105 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'main')) -import license_headers +import licence_headers def test_complete_hash_header_passes(): - header = license_headers._render_header('hash', year=2026) - assert license_headers.check_content('example.py', header + 'print("ok")\n', 2026) is None + header = licence_headers._render_header('hash', year=2026) + assert licence_headers.check_content('example.py', header + 'print("ok")\n', 2026) is None def test_literal_year_token_fails(): - header = license_headers._render_header('hash', year=2026).replace('2026', '{{ .Year }}') - assert license_headers.check_content('example.py', header, 2026) is not None + header = licence_headers._render_header('hash', year=2026).replace('2026', '{{ .Year }}') + assert licence_headers.check_content('example.py', header, 2026) is not None def test_removed_licence_line_fails_and_is_repaired(): - header = license_headers._render_header('hash', year=2026) + header = licence_headers._render_header('hash', year=2026) broken = header.replace('# copied, except in accordance with a valid licence agreement with CCDC and\n', '') content = broken + 'print("ok")\n' - assert license_headers.check_content('example.py', content, 2026) is not None - fixed = license_headers.fix_content('example.py', content, 2026) + assert licence_headers.check_content('example.py', content, 2026) is not None + fixed = licence_headers.fix_content('example.py', content, 2026) assert fixed == header + 'print("ok")\n' def test_missing_header_is_added_after_shebang(): - fixed = license_headers.fix_content('script.py', '#!/usr/bin/env python3\nprint("ok")\n', 2026) + fixed = licence_headers.fix_content('script.py', '#!/usr/bin/env python3\nprint("ok")\n', 2026) assert fixed.startswith('#!/usr/bin/env python3\n#\n# This code is Copyright (C) 2026') assert fixed.endswith('print("ok")\n') def test_python_encoding_declaration_is_preserved_before_header(): - fixed = license_headers.fix_content('script.py', '# -*- coding: latin-1 -*-\nprint("ok")\n', 2026) + fixed = licence_headers.fix_content('script.py', '# -*- coding: latin-1 -*-\nprint("ok")\n', 2026) assert fixed.startswith('# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026') def test_second_line_python_encoding_declaration_preserves_comment_prefix(): source = '# generated source\n# coding=latin-1\nprint("ok")\n' - fixed = license_headers.fix_content('script.py', source, 2026) + fixed = licence_headers.fix_content('script.py', source, 2026) prefix = '# generated source\n# coding=latin-1\n#\n# This code is Copyright (C) 2026' assert fixed.startswith(prefix) - assert license_headers.check_content('script.py', fixed, 2026) is None + assert licence_headers.check_content('script.py', fixed, 2026) is None def test_encoding_declaration_after_shebang_is_preserved(): source = '#!/usr/bin/env python3\n# -*- coding: latin-1 -*-\nprint("ok")\n' - fixed = license_headers.fix_content('script.py', source, 2026) + fixed = licence_headers.fix_content('script.py', source, 2026) prefix = '#!/usr/bin/env python3\n# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026' assert fixed.startswith(prefix) - assert license_headers.check_content('script.py', fixed, 2026) is None + assert licence_headers.check_content('script.py', fixed, 2026) is None def test_non_utf8_python_file_preserves_declared_encoding(): source = '# generated\n# coding=latin-1\nname = "caf\xe9"\n'.encode('latin-1') - fixed = license_headers.fix_content('script.py', source, 2026) + fixed = licence_headers.fix_content('script.py', source, 2026) assert isinstance(fixed, bytes) assert b'# coding=latin-1\n#\n# This code is Copyright (C) 2026' in fixed assert b'caf\xe9' in fixed - assert license_headers.check_content('script.py', fixed, 2026) is None + assert licence_headers.check_content('script.py', fixed, 2026) is None def test_empty_python_file_gets_header(): - fixed = license_headers.fix_content('empty.py', b'', 2026) + fixed = licence_headers.fix_content('empty.py', b'', 2026) assert fixed.startswith(b'#\n# This code is Copyright (C) 2026') - assert license_headers.check_content('empty.py', fixed, 2026) is None + assert licence_headers.check_content('empty.py', fixed, 2026) is None def test_utf8_bom_remains_at_byte_zero_for_supported_non_python_files(): for filename in ['example.yml', 'example.js', 'example.cpp']: - fixed = license_headers.fix_content(filename, b'\xef\xbb\xbfvalue\n', 2026) + fixed = licence_headers.fix_content(filename, b'\xef\xbb\xbfvalue\n', 2026) assert fixed.startswith(b'\xef\xbb\xbf') assert fixed.count(b'\xef\xbb\xbf') == 1 - assert license_headers.check_content(filename, fixed, 2026) is None + assert licence_headers.check_content(filename, fixed, 2026) is None def test_unterminated_shebang_is_separated_from_header(): for filename, shebang in [('script.py', b'#!/usr/bin/env python3'), ('script.sh', b'#!/bin/sh')]: - fixed = license_headers.fix_content(filename, shebang, 2026) + fixed = licence_headers.fix_content(filename, shebang, 2026) assert fixed.startswith(shebang + b'\n#\n# This code is Copyright (C) 2026') def test_unterminated_encoding_declaration_is_separated_from_header(): declaration = b'# coding=latin-1' - fixed = license_headers.fix_content('script.py', declaration, 2026) + fixed = licence_headers.fix_content('script.py', declaration, 2026) assert fixed.startswith(declaration + b'\n#\n# This code is Copyright (C) 2026') def test_copywrite_one_line_header_is_replaced(): old_header = '# Copyright The Cambridge Crystallographic Data Centre (CCDC) 2021, 2026\n\n' source_comment = '# keep this source comment\n' - fixed = license_headers.fix_content('example.py', old_header + source_comment + 'print("ok")\n', 2026) + fixed = licence_headers.fix_content('example.py', old_header + source_comment + 'print("ok")\n', 2026) assert fixed.startswith('#\n# This code is Copyright (C) 2026') assert old_header not in fixed assert fixed.endswith(source_comment + 'print("ok")\n') def test_truncated_full_header_is_replaced_without_losing_source_comment(): - header = license_headers._render_header('hash', year=2026) - old_header = license_headers._render_header('hash', year=2025) + header = licence_headers._render_header('hash', year=2026) + old_header = licence_headers._render_header('hash', year=2025) truncated = old_header.split('# law.\n', 1)[0] source = '# keep this source comment\nprint("ok")\n' - fixed = license_headers.fix_content('example.py', truncated + source, 2026) + fixed = licence_headers.fix_content('example.py', truncated + source, 2026) assert fixed == header + source assert fixed.count('This code is Copyright (C)') == 1 @@ -124,8 +124,8 @@ def test_heavily_damaged_full_header_is_replaced_without_duplication(): ''' source = '# keep this source comment\nhello\n' - header = license_headers._render_header('hash', year=2026) - fixed = license_headers.fix_content('example.py', damaged + source, 2026) + header = licence_headers._render_header('hash', year=2026) + fixed = licence_headers.fix_content('example.py', damaged + source, 2026) assert fixed == header + '\n' + source assert fixed.count('This code is Copyright') == 1 @@ -149,19 +149,19 @@ def test_damaged_copyright_identity_line_is_repaired_without_duplication(): ''' source = '# keep this source comment\nhello\n' - header = license_headers._render_header('hash', year=2026) - fixed = license_headers.fix_content('example.py', damaged + source, 2026) + header = licence_headers._render_header('hash', year=2026) + fixed = licence_headers.fix_content('example.py', damaged + source, 2026) assert fixed == header + '\n' + source assert fixed.count('This code is Copyright') == 1 def test_slash_header_is_added(): - fixed = license_headers.fix_content('example.cpp', 'int main() {}\n', 2026) + fixed = licence_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') - assert license_headers.check_content('example.cpp', fixed, 2026) is None + assert licence_headers.check_content('example.cpp', fixed, 2026) is None def test_ignored_and_unsupported_files_are_skipped(): - assert license_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None - assert license_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None - assert license_headers.check_content('README.md', '# Read me\n', 2026) is None \ No newline at end of file + assert licence_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None + assert licence_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None + assert licence_headers.check_content('README.md', '# Read me\n', 2026) is None \ No newline at end of file From 2b14bdd0d9bece72aa2634f1344de55cfdf5f695 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:36:19 +0100 Subject: [PATCH 09/20] SYS-8665 use hooks.licencecheck --- .gitignore | 4 ++++ README.md | 12 +++++------- main/githooks.py | 34 ++++++++++++++++------------------ test/test_licence_headers.py | 2 +- 4 files changed, 26 insertions(+), 26 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..ae90a5f --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +__pycache__/ +*.py[cod] +*$py.class +.pytest_cache/ diff --git a/README.md b/README.md index 31efd8a..977042c 100644 --- a/README.md +++ b/README.md @@ -71,20 +71,18 @@ To enable CCDC commit checks (Jira ID, CRLF, line endings, DO NOT COMMIT, file s ## Configuring Licence Header Behavior -Developers can customise the licence header hook using Git configuration. The -existing `hooks.copywrite` names are retained for compatibility and do not -require the Copywrite executable. +Developers can customise the licence header hook using Git configuration: * **Enable / Disable Header Formatting:** ```bash - git config --global hooks.copywrite true # opt-in: enable copywrite integration - git config --global hooks.copywrite false # default: disabled + git config --global hooks.licenceCheck true # opt-in: enable licence header formatting + git config --global hooks.licenceCheck false # default: disabled ``` * **Set Mode (`fix` vs `check`):** ```bash - git config --global hooks.copywriteMode fix # default: automatically inserts/updates headers on commit - git config --global hooks.copywriteMode check # read-only check (warns/fails if headers are missing) + git config --global hooks.licenceCheckMode fix # default: automatically inserts/updates headers on commit + git config --global hooks.licenceCheckMode check # read-only check (warns/fails if headers are missing) ``` ## Recommended settings diff --git a/main/githooks.py b/main/githooks.py index d17b8d9..a7f4107 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -1033,28 +1033,26 @@ def _test(input, is_good=True): _test('Close but no cigar abc-1234', False) -def run_copywrite(files): +def run_licence_check(files): '''Check or fix complete CCDC licence headers. Configurable via git config: - - `git config --global hooks.copywrite true|false` (default: false, opt-in) - - `git config --global hooks.copywriteMode fix|check` (default: fix) + - `git config --global hooks.licenceCheck true|false` (default: false, opt-in) + - `git config --global hooks.licenceCheckMode fix|check` (default: fix) - fix: automatically adds/updates headers and restages files - check: checks header compliance and warns/fails without modifying - - The existing hooks.copywrite setting is retained for compatibility. ''' if not files: return 0 # Opt-in: only run if explicitly enabled in git config - enabled_setting = get_config_setting('hooks.copywrite') + enabled_setting = get_config_setting('hooks.licenceCheck') if enabled_setting is None or enabled_setting.lower() not in ['true', '1', 'yes', 'on']: return 0 - mode = (get_config_setting('hooks.copywriteMode') or 'fix').lower() + mode = (get_config_setting('hooks.licenceCheckMode') or 'fix').lower() if mode not in ['fix', 'check', 'plan', 'verify']: - _fail(f'Unsupported hooks.copywriteMode value: {mode}') + _fail(f'Unsupported hooks.licenceCheckMode value: {mode}') return 1 is_check_mode = mode in ['check', 'plan', 'verify'] @@ -1067,8 +1065,8 @@ def run_copywrite(files): text=True ) if unstaged.returncode == 1: - _fail('Copywrite fix mode cannot run with unstaged changes in staged files. ' - 'Stage or stash those changes, or use hooks.copywriteMode check.') + _fail('Licence check fix mode cannot run with unstaged changes in staged files. ' + 'Stage or stash those changes, or use hooks.licenceCheckMode check.') return 1 if unstaged.returncode != 0: _fail(f'Unable to inspect unstaged changes:\n{unstaged.stderr.strip()}') @@ -1095,15 +1093,15 @@ def run_copywrite(files): return 0 -class TestRunCopywrite(unittest.TestCase): +class TestRunLicenceCheck(unittest.TestCase): @patch('githooks.get_config_setting', return_value=None) def test_disabled(self, _config): - self.assertEqual(0, run_copywrite(['example.py'])) + self.assertEqual(0, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) @patch('githooks.licence_headers.process_files', return_value=1) def test_check_failure_blocks_commit(self, process_files, _config): - self.assertEqual(1, run_copywrite(['example.py'])) + self.assertEqual(1, run_licence_check(['example.py'])) process_files.assert_called_once_with(['example.py'], fix=False) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @@ -1114,7 +1112,7 @@ def test_fix_restages_files(self, process_files, run, _config): subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), ] - self.assertEqual(0, run_copywrite(['example.py'])) + self.assertEqual(0, run_licence_check(['example.py'])) process_files.assert_called_once_with(['example.py'], fix=True) self.assertEqual(['git', 'add', '--', 'example.py'], run.call_args_list[-1].args[0]) @@ -1126,19 +1124,19 @@ def test_restage_failure_blocks_commit(self, _process_files, run, _config): subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 1, '', 'cannot add'), ] - self.assertEqual(1, run_copywrite(['example.py'])) + self.assertEqual(1, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') def test_fix_rejects_partially_staged_files(self, run, _config): run.return_value = subprocess.CompletedProcess([], 1, '', '') - self.assertEqual(1, run_copywrite(['example.py'])) + self.assertEqual(1, run_licence_check(['example.py'])) run.assert_called_once() @patch('githooks.get_config_setting', side_effect=['true', 'check']) @patch('githooks.licence_headers.process_files', side_effect=OSError('cannot read')) def test_processing_error_blocks_commit(self, _process_files, _config): - self.assertEqual(1, run_copywrite(['example.py'])) + self.assertEqual(1, run_licence_check(['example.py'])) def commit_hook(merge=False): @@ -1156,7 +1154,7 @@ def commit_hook(merge=False): staged_files = files['M'] + files['A'] print(' Check and update copyright headers ...') - retval += run_copywrite(staged_files) + retval += run_licence_check(staged_files) print(' Check filenames ...') retval += check_filenames(staged_files) diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 1f3c1a4..8dbb89c 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -164,4 +164,4 @@ def test_slash_header_is_added(): def test_ignored_and_unsupported_files_are_skipped(): assert licence_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None assert licence_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None - assert licence_headers.check_content('README.md', '# Read me\n', 2026) is None \ No newline at end of file + assert licence_headers.check_content('README.md', '# Read me\n', 2026) is None From 6ab8d52a75e676119f566969c23d9d5a3cd14f86 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:48:05 +0100 Subject: [PATCH 10/20] SYS-8665 Skip symlinks before reading or writing --- main/licence_headers.py | 5 +---- test/test_licence_headers.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/main/licence_headers.py b/main/licence_headers.py index dbc911f..4815165 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -75,9 +75,6 @@ def _decode_content(filename, data): def _header_offset(filename, text, style): - if style != 'hash': - return 0 - lines = text.splitlines(keepends=True) has_shebang = bool(lines and lines[0].startswith('#!')) if not str(filename).lower().endswith('.py'): @@ -215,7 +212,7 @@ def process_files(files, fix=False, year=None): failures = 0 for filename in files: path = Path(filename) - if not path.is_file() or _comment_style(filename) is None: + if path.is_symlink() or not path.is_file() or _comment_style(filename) is None: continue data = path.read_bytes() issue = check_content(filename, data, year) diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 8dbb89c..01b94e5 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -31,6 +31,15 @@ def test_missing_header_is_added_after_shebang(): assert fixed.endswith('print("ok")\n') +def test_missing_slash_header_is_added_after_shebang(): + for filename in ['cli.js', 'cli.ts']: + source = '#!/usr/bin/env node\nconsole.log("ok");\n' + fixed = licence_headers.fix_content(filename, source, 2026) + assert fixed.startswith('#!/usr/bin/env node\n//\n// This code is Copyright (C) 2026') + assert fixed.endswith('console.log("ok");\n') + assert licence_headers.check_content(filename, fixed, 2026) is None + + def test_python_encoding_declaration_is_preserved_before_header(): fixed = licence_headers.fix_content('script.py', '# -*- coding: latin-1 -*-\nprint("ok")\n', 2026) assert fixed.startswith('# -*- coding: latin-1 -*-\n#\n# This code is Copyright (C) 2026') @@ -79,6 +88,10 @@ def test_unterminated_shebang_is_separated_from_header(): for filename, shebang in [('script.py', b'#!/usr/bin/env python3'), ('script.sh', b'#!/bin/sh')]: fixed = licence_headers.fix_content(filename, shebang, 2026) assert fixed.startswith(shebang + b'\n#\n# This code is Copyright (C) 2026') + for filename in ['script.js', 'script.ts']: + shebang = b'#!/usr/bin/env node' + fixed = licence_headers.fix_content(filename, shebang, 2026) + assert fixed.startswith(shebang + b'\n//\n// This code is Copyright (C) 2026') def test_unterminated_encoding_declaration_is_separated_from_header(): @@ -165,3 +178,22 @@ def test_ignored_and_unsupported_files_are_skipped(): assert licence_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None assert licence_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None assert licence_headers.check_content('README.md', '# Read me\n', 2026) is None + + +def test_process_files_skips_symlinks(tmp_path): + target = tmp_path / 'target.py' + target_content = 'print("target")\n' + target.write_text(target_content, encoding='utf-8') + + symlink = tmp_path / 'link.py' + try: + symlink.symlink_to(target) + except OSError: + import pytest + pytest.skip('Symlinks not supported in current environment or permissions') + + # Check mode should skip symlink and report 0 failures + assert licence_headers.process_files([str(symlink)], fix=False, year=2026) == 0 + # Fix mode should skip symlink without modifying target file + assert licence_headers.process_files([str(symlink)], fix=True, year=2026) == 0 + assert target.read_text(encoding='utf-8') == target_content From 69ac1c1be653c61f3c19ec189cf10813b61c8c04 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:53:36 +0100 Subject: [PATCH 11/20] SYS-8665 stop damaged header scanning at the first non comment, non blank line --- main/githooks.py | 23 ++++++++++++++++++++--- main/licence_headers.py | 2 ++ test/test_licence_headers.py | 12 ++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/main/githooks.py b/main/githooks.py index a7f4107..712ea8d 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -221,9 +221,19 @@ def get_commit_files(): output = _get_output(commands) result = defaultdict(list) for line in output.splitlines(): - parts = line.split() - if parts[-2] in ['M', 'A']: - result[parts[-2]].append(parts[-1]) + if not line: + continue + if '\t' in line: + parts = line.split('\t') + status = parts[0].strip() + path = parts[-1].strip() + else: + parts = line.split(None, 1) + if len(parts) != 2: + continue + status, path = parts[0].strip(), parts[1].strip() + if status in ['M', 'A']: + result[status].append(path) return result @@ -337,6 +347,13 @@ def test_push_without_before_uses_previous_commit(self): ['git', 'diff', '--unified=0', 'HEAD~', '--', 'example.py'] ) + def test_files_with_spaces(self): + with patch.dict(os.environ, {}, clear=True), patch('githooks._get_output') as get_output: + get_output.return_value = 'M\tpath/with spaces/file.py\nA\tanother file.js\n' + files = get_commit_files() + self.assertEqual(['path/with spaces/file.py'], files['M']) + self.assertEqual(['another file.js'], files['A']) + def yield_changed_lines(changed_lines): '''Yield individual line numbers from list returned by get_changed_lines''' diff --git a/main/licence_headers.py b/main/licence_headers.py index 4815165..24db0c5 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -150,6 +150,8 @@ def _damaged_full_header_end(text, offset, style): position += len(line) for index, (_, line_end, stripped) in enumerate(line_positions[2:], start=2): + if stripped and not stripped.startswith(marker): + break if stripped.startswith(marker) and stripped.endswith('law.'): if index + 1 < len(line_positions) and line_positions[index + 1][2] == marker: return line_positions[index + 1][1] diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 01b94e5..243aab2 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -168,6 +168,18 @@ def test_damaged_copyright_identity_line_is_repaired_without_duplication(): assert fixed.count('This code is Copyright') == 1 +def test_truncated_header_does_not_consume_source_code_before_law_comment(): + truncated = '''# +# This code is Copyright (C) 2026 The Cambridge Crystallographic Data Centre (CCDC) +# of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This +''' + source = 'def calculate():\n return 42\n# according to the law.\n' + fixed = licence_headers.fix_content('example.py', truncated + source, 2026) + expected_header = licence_headers._render_header('hash', year=2026) + assert fixed == expected_header + source + assert 'def calculate():' in fixed + + def test_slash_header_is_added(): fixed = licence_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') From 4a7e7fc67b54f0d9e253627810de71c6d3a06b0f Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:13:08 +0100 Subject: [PATCH 12/20] SYS-8665 do away with copywrite references --- main/githooks.py | 21 +++++++++++++++----- main/{copywrite => }/headers/ccdc_hash.tmpl | 0 main/{copywrite => }/headers/ccdc_slash.tmpl | 0 main/licence_headers.py | 17 +++++----------- test/test_licence_headers.py | 13 +++++++++++- 5 files changed, 33 insertions(+), 18 deletions(-) rename main/{copywrite => }/headers/ccdc_hash.tmpl (100%) rename main/{copywrite => }/headers/ccdc_slash.tmpl (100%) diff --git a/main/githooks.py b/main/githooks.py index 712ea8d..340b181 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -226,14 +226,16 @@ def get_commit_files(): if '\t' in line: parts = line.split('\t') status = parts[0].strip() - path = parts[-1].strip() + path = parts[-1] else: parts = line.split(None, 1) if len(parts) != 2: continue - status, path = parts[0].strip(), parts[1].strip() + status, path = parts[0].strip(), parts[1] if status in ['M', 'A']: result[status].append(path) + elif status.startswith(('R', 'C')): + result['A'].append(path) return result @@ -347,12 +349,21 @@ def test_push_without_before_uses_previous_commit(self): ['git', 'diff', '--unified=0', 'HEAD~', '--', 'example.py'] ) - def test_files_with_spaces(self): + def test_files_with_spaces_and_renames(self): with patch.dict(os.environ, {}, clear=True), patch('githooks._get_output') as get_output: - get_output.return_value = 'M\tpath/with spaces/file.py\nA\tanother file.js\n' + get_output.return_value = ( + 'M\tpath/with spaces/file.py\n' + 'A\tanother file.js\n' + 'R100\told/name.py\tnew/renamed file.py\n' + 'C100\tsrc.h\tdst/copied file.h\n' + 'A\tbad/end/space.txt \n' + ) files = get_commit_files() self.assertEqual(['path/with spaces/file.py'], files['M']) - self.assertEqual(['another file.js'], files['A']) + self.assertEqual( + ['another file.js', 'new/renamed file.py', 'dst/copied file.h', 'bad/end/space.txt '], + files['A'] + ) def yield_changed_lines(changed_lines): diff --git a/main/copywrite/headers/ccdc_hash.tmpl b/main/headers/ccdc_hash.tmpl similarity index 100% rename from main/copywrite/headers/ccdc_hash.tmpl rename to main/headers/ccdc_hash.tmpl diff --git a/main/copywrite/headers/ccdc_slash.tmpl b/main/headers/ccdc_slash.tmpl similarity index 100% rename from main/copywrite/headers/ccdc_slash.tmpl rename to main/headers/ccdc_slash.tmpl diff --git a/main/licence_headers.py b/main/licence_headers.py index 24db0c5..43671a1 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -33,7 +33,7 @@ 'node_modules', 'dist', 'build', '.venv', 'venv', '__pycache__' } IGNORED_SUFFIXES = ('.designer.cs', '.g.cs', '.min.js', '.lock') -TEMPLATE_DIRECTORY = Path(__file__).resolve().parent / 'copywrite' / 'headers' +TEMPLATE_DIRECTORY = Path(__file__).resolve().parent / 'headers' PYTHON_ENCODING_PATTERN = re.compile(r'^[ \t\f]*#.*?coding[:=][ \t]*[-\w.]+') LEGACY_HEADER_PATTERN = re.compile( r'^(#|//) Copyright The Cambridge Crystallographic Data Centre ' @@ -115,18 +115,11 @@ def _known_header_prefix_end(text, offset, expected): return offset header_end = actual_lines[1][1] - expected_index = 2 - for _, line_end, actual in actual_lines[2:]: - if actual == '': - continue - match_index = next( - (index for index in range(expected_index, len(expected_lines)) - if _header_line_matches(actual, expected_lines[index])), - None - ) - if match_index is None: + for expected_index, (_, line_end, actual) in enumerate(actual_lines[2:], start=2): + if expected_index >= len(expected_lines): + break + if not _header_line_matches(actual, expected_lines[expected_index]): break - expected_index = match_index + 1 header_end = line_end return header_end diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 243aab2..44283b4 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -100,7 +100,7 @@ def test_unterminated_encoding_declaration_is_separated_from_header(): assert fixed.startswith(declaration + b'\n#\n# This code is Copyright (C) 2026') -def test_copywrite_one_line_header_is_replaced(): +def test_legacy_one_line_header_is_replaced(): old_header = '# Copyright The Cambridge Crystallographic Data Centre (CCDC) 2021, 2026\n\n' source_comment = '# keep this source comment\n' fixed = licence_headers.fix_content('example.py', old_header + source_comment + 'print("ok")\n', 2026) @@ -180,6 +180,17 @@ def test_truncated_header_does_not_consume_source_code_before_law_comment(): assert 'def calculate():' in fixed +def test_truncated_header_preserves_source_comment_matching_later_header_line(): + truncated = '''# +# This code is Copyright (C) 2026 The Cambridge Crystallographic Data Centre (CCDC) +''' + source = '# notice.\nprint("ok")\n' + fixed = licence_headers.fix_content('example.py', truncated + source, 2026) + expected_header = licence_headers._render_header('hash', year=2026) + assert fixed == expected_header + source + assert '# notice.\n' in fixed + + def test_slash_header_is_added(): fixed = licence_headers.fix_content('example.cpp', 'int main() {}\n', 2026) assert fixed.startswith('//\n// This code is Copyright (C) 2026') From 2ffe726c0f7193cb37d5f930fdd756eea71d67c0 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:29:12 +0100 Subject: [PATCH 13/20] SYS-8665 make licence_headers import not dependent --- main/githooks.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/main/githooks.py b/main/githooks.py index 340b181..fd2489f 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -24,7 +24,6 @@ from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch -import licence_headers import os import platform import re @@ -32,6 +31,11 @@ import unittest import sys +try: + import licence_headers +except ImportError: + licence_headers = None + # Absolute file size limit (in MB) - it's 100MB on github.com HARD_SIZE_THRESHOLD = 99.0 @@ -1084,6 +1088,18 @@ def run_licence_check(files): return 1 is_check_mode = mode in ['check', 'plan', 'verify'] + global licence_headers + if licence_headers is None: + try: + import licence_headers as _licence_headers + licence_headers = _licence_headers + except ImportError: + _fail( + 'Licence header checking is enabled but the ' + '"licence_headers" module is not installed.' + ) + return 1 + try: if not is_check_mode: unstaged = subprocess.run( @@ -1166,6 +1182,17 @@ def test_fix_rejects_partially_staged_files(self, run, _config): def test_processing_error_blocks_commit(self, _process_files, _config): self.assertEqual(1, run_licence_check(['example.py'])) + @patch('githooks.get_config_setting', return_value=None) + @patch('githooks.licence_headers', None) + def test_missing_module_allowed_when_disabled(self, _config): + self.assertEqual(0, run_licence_check(['example.py'])) + + @patch('githooks.get_config_setting', side_effect=['true', 'check']) + @patch('githooks.licence_headers', None) + @patch('builtins.__import__', side_effect=ImportError('No module named licence_headers')) + def test_missing_module_fails_when_enabled(self, _import, _config): + self.assertEqual(1, run_licence_check(['example.py'])) + def commit_hook(merge=False): retval = 0 From bee3d890a683d9aec4324ea73c9b655b81f32408 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:40:09 +0100 Subject: [PATCH 14/20] SYS-8665 add unit tests --- main/licence_headers.py | 6 ++- test/test_licence_headers.py | 78 +++++++++++++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/main/licence_headers.py b/main/licence_headers.py index 43671a1..a39c66f 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -214,8 +214,10 @@ def process_files(files, fix=False, year=None): if issue is None: continue if fix: - path.write_bytes(fix_content(filename, data, year)) - print(f'Updated CCDC licence header: {filename}') + fixed = fix_content(filename, data, year) + if fixed != data: + path.write_bytes(fixed) + print(f'Updated CCDC licence header: {filename}') else: print(f'HEADER FAIL: {filename}: {issue}') failures += 1 diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 44283b4..d9999f3 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -62,12 +62,16 @@ def test_encoding_declaration_after_shebang_is_preserved(): def test_non_utf8_python_file_preserves_declared_encoding(): - source = '# generated\n# coding=latin-1\nname = "caf\xe9"\n'.encode('latin-1') - fixed = licence_headers.fix_content('script.py', source, 2026) - assert isinstance(fixed, bytes) - assert b'# coding=latin-1\n#\n# This code is Copyright (C) 2026' in fixed - assert b'caf\xe9' in fixed - assert licence_headers.check_content('script.py', fixed, 2026) is None + for enc in ['latin-1', 'iso-8859-15', 'cp1252', 'utf-8-sig']: + source = f'# coding={enc}\nname = "café"\n'.encode(enc) + fixed = licence_headers.fix_content('script.py', source, 2026) + assert isinstance(fixed, bytes) + assert f'# coding={enc}'.encode('ascii') in fixed + assert licence_headers.check_content('script.py', fixed, 2026) is None + # Verify decoding with original encoding roundtrips cleanly + decoded_text, detected_enc = licence_headers._decode_content('script.py', fixed) + assert 'This code is Copyright' in decoded_text + assert 'café' in decoded_text def test_empty_python_file_gets_header(): @@ -201,6 +205,68 @@ def test_ignored_and_unsupported_files_are_skipped(): assert licence_headers.check_content('.github/workflows/check.yml', 'name: check\n', 2026) is None assert licence_headers.check_content('templates/check.yml', 'name: check\n', 2026) is None assert licence_headers.check_content('README.md', '# Read me\n', 2026) is None + assert licence_headers.check_content('notes.txt', 'some notes\n', 2026) is None + assert licence_headers.check_content('package.json', '{"name": "app"}\n', 2026) is None + assert licence_headers.check_content('node_modules/pkg/index.js', 'console.log();\n', 2026) is None + assert licence_headers.check_content('dist/bundle.js', 'console.log();\n', 2026) is None + assert licence_headers.check_content('.venv/lib/module.py', 'print("ok")\n', 2026) is None + + +def test_generated_files_are_excluded(): + generated_files = [ + 'Form.Designer.cs', + 'Model.g.cs', + 'bundle.min.js', + 'packages.lock', + 'service.generated.ts', + 'codegen.generated.cpp', + ] + for filename in generated_files: + assert licence_headers.check_content(filename, 'var x = 1;\n', 2026) is None + assert licence_headers.fix_content(filename, 'var x = 1;\n', 2026) == 'var x = 1;\n' + + +def test_crlf_line_endings_are_preserved(): + source = 'int main() {\r\n return 0;\r\n}\r\n' + fixed = licence_headers.fix_content('main.cpp', source, 2026) + assert '\r\n' in fixed + assert '\n' not in fixed.replace('\r\n', '') + assert licence_headers.check_content('main.cpp', fixed, 2026) is None + + +def test_legacy_slash_header_is_replaced(): + old_header = '// Copyright The Cambridge Crystallographic Data Centre (CCDC) 2020\r\n\r\n' + source = 'const x = 42;\r\n' + fixed = licence_headers.fix_content('app.js', old_header + source, 2026) + assert fixed.startswith('//\r\n// This code is Copyright (C) 2026') + assert old_header not in fixed + assert fixed.endswith(source) + + +def test_damaged_slash_header_is_repaired(): + damaged = ( + '//\n' + '// This code is Copyright (C) 2024 The Cambridge Crystallographic Data Centre (CCDC)\n' + '// of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This\n' + '// broken line...\n' + '// law.\n' + '//\n' + ) + source = 'const app = 1;\n' + fixed = licence_headers.fix_content('app.ts', damaged + source, 2026) + expected_header = licence_headers._render_header('slash', year=2026) + assert fixed == expected_header + source + + +def test_process_files_avoids_write_when_already_compliant(tmp_path): + test_file = tmp_path / 'compliant.py' + header = licence_headers._render_header('hash', year=2026) + content = (header + 'print("hello")\n').encode('utf-8') + test_file.write_bytes(content) + + initial_mtime = test_file.stat().st_mtime_ns + assert licence_headers.process_files([str(test_file)], fix=True, year=2026) == 0 + assert test_file.stat().st_mtime_ns == initial_mtime def test_process_files_skips_symlinks(tmp_path): From bd7ee2b2b64dea4866b29ffe377e69ab1f97beb0 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:45:20 +0100 Subject: [PATCH 15/20] SYS-8665 cleanup unused parameters and arguments --- main/licence_headers.py | 20 +++++++++++++++++--- test/test_licence_headers.py | 10 ++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/main/licence_headers.py b/main/licence_headers.py index a39c66f..fb0aa19 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -92,7 +92,7 @@ def _header_line_matches(actual, expected): marker = expected.split(' ', 1)[0] pattern = ( rf'{re.escape(marker)} This code is Copyright \(C\) ' - r'(?:\d{4}(?:, \d{4})?|\{\{ \.Year \}\}) ' + r'\d{4}(?:, \d{4})? ' r'The Cambridge Crystallographic Data Centre \(CCDC\)' ) return re.fullmatch(pattern, actual) is not None @@ -164,6 +164,20 @@ def _existing_header_end(text, offset, style, expected): return _known_header_prefix_end(text, offset, expected) +def _has_valid_header(text, offset, expected): + '''Check if text starting at offset contains a valid compliant header (accepting any valid year).''' + if text.startswith(expected, offset): + return True + actual_lines = [line.strip() for line in text[offset:].splitlines()] + expected_lines = [line.strip() for line in expected.splitlines()] + if len(actual_lines) < len(expected_lines): + return False + for actual, exp in zip(actual_lines[:len(expected_lines)], expected_lines): + if not _header_line_matches(actual, exp): + return False + return True + + def check_content(filename, data, year=None): '''Return an error message when a supported file lacks the exact header.''' style = _comment_style(filename) @@ -177,7 +191,7 @@ def check_content(filename, data, year=None): newline = '\r\n' if '\r\n' in text else '\n' offset = _header_offset(filename, text, style) expected = _render_header(style, newline, year) - if text.startswith(expected, offset): + if _has_valid_header(text, offset, expected): return None return 'missing or non-compliant CCDC copyright and licence header' @@ -192,7 +206,7 @@ def fix_content(filename, data, year=None): newline = '\r\n' if '\r\n' in text else '\n' offset = _header_offset(filename, text, style) expected = _render_header(style, newline, year) - if text.startswith(expected, offset): + if _has_valid_header(text, offset, expected): return data header_end = _existing_header_end(text, offset, style, expected) diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index d9999f3..fb3ffcd 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -11,6 +11,16 @@ def test_complete_hash_header_passes(): assert licence_headers.check_content('example.py', header + 'print("ok")\n', 2026) is None +def test_existing_file_with_earlier_year_passes_and_is_not_modified(): + # An existing file with 2020 should pass and NOT be bumped to 2026 + header_2020 = licence_headers._render_header('hash', year=2020) + source = header_2020 + 'print("ok")\n' + assert licence_headers.check_content('example.py', source, year=2026) is None + fixed = licence_headers.fix_content('example.py', source, year=2026) + assert fixed == source + assert 'Copyright (C) 2020' in fixed + + def test_literal_year_token_fails(): header = licence_headers._render_header('hash', year=2026).replace('2026', '{{ .Year }}') assert licence_headers.check_content('example.py', header, 2026) is not None From f04a59672dbbb417da15f36ab656ec435d4a13a5 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:08:00 +0100 Subject: [PATCH 16/20] SYS-8665 prevents incorrect header placement --- README.md | 19 +++++++++++++------ main/licence_headers.py | 12 +++++++++++- test/test_licence_headers.py | 8 ++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 977042c..13a9432 100644 --- a/README.md +++ b/README.md @@ -60,18 +60,25 @@ A complete workflow template for CI is available in [templates/compliance.yml](t # Native Git Hooks -To enable CCDC commit checks (Jira ID, CRLF, line endings, DO NOT COMMIT, file size, and automatic copyright headers) globally for all repositories on your machine: +To enable CCDC commit checks (Jira ID, CRLF, line endings, DO NOT COMMIT, file size, and automatic copyright headers): 1. Clone this repository. -2. Run: - ```bash - git config --global core.hooksPath /main - ``` +2. Configure `core.hooksPath`: + * **Globally (for all repositories on your machine):** + ```bash + git config --global core.hooksPath /main + ``` + * **Locally (for a single repository only):** + Run inside the target repository: + ```bash + git config core.hooksPath /main + ``` + *(Only use `--global` if you want the hooks applied across all repositories.)* 3. (Optional) Enable automatic CCDC copyright and licence header formatting as described below. ## Configuring Licence Header Behavior -Developers can customise the licence header hook using Git configuration: +Developers can customise the licence header hook using Git configuration (use `--global` for all repos, or omit it within a specific repo): * **Enable / Disable Header Formatting:** ```bash diff --git a/main/licence_headers.py b/main/licence_headers.py index fb0aa19..0e8c3b4 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -80,7 +80,17 @@ def _header_offset(filename, text, style): if not str(filename).lower().endswith('.py'): return len(lines[0]) if has_shebang else 0 - candidate_indexes = [1] if has_shebang else range(min(2, len(lines))) + candidate_indexes = [1] if has_shebang else [0] + if ( + not has_shebang + and len(lines) > 1 + and ( + lines[0].strip() == '' + or lines[0].lstrip().startswith('#') + ) + ): + candidate_indexes.append(1) + for index in candidate_indexes: if index < len(lines) and PYTHON_ENCODING_PATTERN.match(lines[index]): return sum(len(line) for line in lines[:index + 1]) diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index fb3ffcd..7846c24 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -63,6 +63,14 @@ def test_second_line_python_encoding_declaration_preserves_comment_prefix(): assert licence_headers.check_content('script.py', fixed, 2026) is None +def test_second_line_encoding_after_code_is_not_treated_as_prefix(): + source = 'print("x")\n# coding=utf-8\n' + fixed = licence_headers.fix_content('script.py', source, 2026) + header = licence_headers._render_header('hash', year=2026) + assert fixed.startswith(header) + assert fixed.endswith(source) + + def test_encoding_declaration_after_shebang_is_preserved(): source = '#!/usr/bin/env python3\n# -*- coding: latin-1 -*-\nprint("ok")\n' fixed = licence_headers.fix_content('script.py', source, 2026) From 871ebeaecb15241af18e5911e008665ccdedfd60 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:15:38 +0100 Subject: [PATCH 17/20] SYS-8665 fix unpadded header duplication issue --- main/licence_headers.py | 41 +++++++++++++++++++++++++----------- test/test_licence_headers.py | 22 +++++++++++++++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/main/licence_headers.py b/main/licence_headers.py index 0e8c3b4..fa96a9b 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -117,29 +117,45 @@ def _known_header_prefix_end(text, offset, expected): position += len(line) expected_lines = [line.strip() for line in expected.splitlines()] - if len(actual_lines) < 2 or len(expected_lines) < 2: + if not actual_lines or not expected_lines: return offset - if actual_lines[0][2] != expected_lines[0]: - return offset - if not _header_line_matches(actual_lines[1][2], expected_lines[1]): + + if actual_lines[0][2] == expected_lines[0] and len(actual_lines) > 1: + if not _header_line_matches(actual_lines[1][2], expected_lines[1]): + return offset + header_end = actual_lines[1][1] + start_idx = 2 + exp_start_idx = 2 + elif _header_line_matches(actual_lines[0][2], expected_lines[1]): + header_end = actual_lines[0][1] + start_idx = 1 + exp_start_idx = 2 + else: return offset - header_end = actual_lines[1][1] - for expected_index, (_, line_end, actual) in enumerate(actual_lines[2:], start=2): - if expected_index >= len(expected_lines): + for actual_idx, exp_idx in enumerate(range(exp_start_idx, len(expected_lines)), start=start_idx): + if actual_idx >= len(actual_lines): break - if not _header_line_matches(actual, expected_lines[expected_index]): + if not _header_line_matches(actual_lines[actual_idx][2], expected_lines[exp_idx]): break - header_end = line_end + header_end = actual_lines[actual_idx][1] return header_end def _damaged_full_header_end(text, offset, style): marker = '#' if style == 'hash' else '//' lines = text[offset:].splitlines(keepends=True) - if len(lines) < 2 or lines[0].strip() != marker: + if not lines: return offset - identity_line = lines[1].strip() + + first_line = lines[0].strip() + if first_line == marker and len(lines) > 1: + identity_line = lines[1].strip() + start_index = 2 + else: + identity_line = first_line + start_index = 1 + if ( not identity_line.startswith(f'{marker} This code is ') or 'Crystallographic Data Centre (CCDC)' not in identity_line @@ -152,7 +168,8 @@ def _damaged_full_header_end(text, offset, style): line_positions.append((position, position + len(line), line.strip())) position += len(line) - for index, (_, line_end, stripped) in enumerate(line_positions[2:], start=2): + for index in range(start_index, len(line_positions)): + _, line_end, stripped = line_positions[index] if stripped and not stripped.startswith(marker): break if stripped.startswith(marker) and stripped.endswith('law.'): diff --git a/test/test_licence_headers.py b/test/test_licence_headers.py index 7846c24..7b96484 100644 --- a/test/test_licence_headers.py +++ b/test/test_licence_headers.py @@ -165,6 +165,28 @@ def test_heavily_damaged_full_header_is_replaced_without_duplication(): assert fixed.count('This code is Copyright') == 1 +def test_unpadded_header_without_leading_and_trailing_marker_is_repaired(): + unpadded = '''# This code is Copyright (C) 2026 The Cambridge Crystallographic Data Centre (CCDC) +# of 12 Union Road, Cambridge CB2 1EZ, UK and a proprietary work of CCDC. This +# code may not be used, reproduced, translated, modified, disassembled or +# copied, except in accordance with a valid licence agreement with CCDC and +# may NOT be disclosed or redistributed in any form, either in whole or in +# part, to any third party. All copies of this code made in accordance with a +# valid licence agreement as referred to above must contain this copyright +# notice. + +# No representations, warranties, or liabilities are expressed or implied in +# the supply of this code by CCDC, its servants or agents, except where such +# exclusion or limitation is prohibited, void or unenforceable under governing +# law. +''' + source = 'print("hello world")\n' + fixed = licence_headers.fix_content('example.py', unpadded + '\n' + source, 2026) + expected_header = licence_headers._render_header('hash', year=2026) + assert fixed == expected_header + '\n' + source + assert fixed.count('This code is Copyright') == 1 + + def test_damaged_copyright_identity_line_is_repaired_without_duplication(): damaged = '''# # This code is Cop Crystallographic Data Centre (CCDC) From a64660c9d01479fb68d33f651a53811963ae342a Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:28:00 +0100 Subject: [PATCH 18/20] SYS-8665 less strict checking for unpadded headings --- main/licence_headers.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/main/licence_headers.py b/main/licence_headers.py index fa96a9b..bc0b053 100644 --- a/main/licence_headers.py +++ b/main/licence_headers.py @@ -172,10 +172,16 @@ def _damaged_full_header_end(text, offset, style): _, line_end, stripped = line_positions[index] if stripped and not stripped.startswith(marker): break - if stripped.startswith(marker) and stripped.endswith('law.'): - if index + 1 < len(line_positions) and line_positions[index + 1][2] == marker: - return line_positions[index + 1][1] - return line_end + if (stripped.startswith(marker) and stripped.endswith('law.')) or 'void or unenforceable under governing' in stripped: + if stripped.endswith('law.'): + if index + 1 < len(line_positions) and line_positions[index + 1][2] == marker: + return line_positions[index + 1][1] + return line_end + elif index + 1 < len(line_positions) and line_positions[index + 1][2].startswith(marker) and line_positions[index + 1][2].endswith('law.'): + next_index = index + 1 + if next_index + 1 < len(line_positions) and line_positions[next_index + 1][2] == marker: + return line_positions[next_index + 1][1] + return line_positions[next_index][1] return offset From d52fdcf1bf5e9ac4157c4a926f783ed2c5a41300 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:46:46 +0100 Subject: [PATCH 19/20] SYS-8665 improve importing licence_headers --- main/githooks.py | 64 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/main/githooks.py b/main/githooks.py index fd2489f..d9f6e6d 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -24,6 +24,7 @@ from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch +import importlib import os import platform import re @@ -31,10 +32,7 @@ import unittest import sys -try: - import licence_headers -except ImportError: - licence_headers = None +licence_headers = None # Absolute file size limit (in MB) - it's 100MB on github.com @@ -1091,8 +1089,7 @@ def run_licence_check(files): global licence_headers if licence_headers is None: try: - import licence_headers as _licence_headers - licence_headers = _licence_headers + licence_headers = importlib.import_module('licence_headers') except ImportError: _fail( 'Licence header checking is enabled but the ' @@ -1138,32 +1135,59 @@ def run_licence_check(files): class TestRunLicenceCheck(unittest.TestCase): + def setUp(self): + # Reset module-level cached import between tests for complete test isolation + import githooks + githooks.licence_headers = None + @patch('githooks.get_config_setting', return_value=None) def test_disabled(self, _config): self.assertEqual(0, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.licence_headers.process_files', return_value=1) - def test_check_failure_blocks_commit(self, process_files, _config): + @patch('githooks.importlib.import_module') + def test_check_failure_blocks_commit(self, import_module, _config): + fake_module = unittest.mock.Mock() + fake_module.process_files.return_value = 1 + import_module.return_value = fake_module + self.assertEqual(1, run_licence_check(['example.py'])) - process_files.assert_called_once_with(['example.py'], fix=False) + fake_module.process_files.assert_called_once_with(['example.py'], fix=False) + + @patch('githooks.get_config_setting', side_effect=['true', 'check']) + @patch('githooks.importlib.import_module') + def test_check_mode_allows_unstaged_changes(self, import_module, _config): + fake_module = unittest.mock.Mock() + fake_module.process_files.return_value = 0 + import_module.return_value = fake_module + + self.assertEqual(0, run_licence_check(['example.py'])) + fake_module.process_files.assert_called_once_with(['example.py'], fix=False) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.licence_headers.process_files', return_value=0) - def test_fix_restages_files(self, process_files, run, _config): + @patch('githooks.importlib.import_module') + def test_fix_restages_files(self, import_module, run, _config): + fake_module = unittest.mock.Mock() + fake_module.process_files.return_value = 0 + import_module.return_value = fake_module + run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), ] self.assertEqual(0, run_licence_check(['example.py'])) - process_files.assert_called_once_with(['example.py'], fix=True) + fake_module.process_files.assert_called_once_with(['example.py'], fix=True) self.assertEqual(['git', 'add', '--', 'example.py'], run.call_args_list[-1].args[0]) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.licence_headers.process_files', return_value=0) - def test_restage_failure_blocks_commit(self, _process_files, run, _config): + @patch('githooks.importlib.import_module') + def test_restage_failure_blocks_commit(self, import_module, run, _config): + fake_module = unittest.mock.Mock() + fake_module.process_files.return_value = 0 + import_module.return_value = fake_module + run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 1, '', 'cannot add'), @@ -1178,18 +1202,20 @@ def test_fix_rejects_partially_staged_files(self, run, _config): run.assert_called_once() @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.licence_headers.process_files', side_effect=OSError('cannot read')) - def test_processing_error_blocks_commit(self, _process_files, _config): + @patch('githooks.importlib.import_module') + def test_processing_error_blocks_commit(self, import_module, _config): + fake_module = unittest.mock.Mock() + fake_module.process_files.side_effect = OSError('cannot read') + import_module.return_value = fake_module + self.assertEqual(1, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', return_value=None) - @patch('githooks.licence_headers', None) def test_missing_module_allowed_when_disabled(self, _config): self.assertEqual(0, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.licence_headers', None) - @patch('builtins.__import__', side_effect=ImportError('No module named licence_headers')) + @patch('githooks.importlib.import_module', side_effect=ImportError('No module named licence_headers')) def test_missing_module_fails_when_enabled(self, _import, _config): self.assertEqual(1, run_licence_check(['example.py'])) From a1fbc20b6d15a957c8d108ae04f1f17efe139012 Mon Sep 17 00:00:00 2001 From: Manish Maharjan <112875432+mmaharjan-ccdc@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:58:12 +0100 Subject: [PATCH 20/20] SYS-8665 revert to import licence_headers --- main/githooks.py | 84 ++++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 52 deletions(-) diff --git a/main/githooks.py b/main/githooks.py index d9f6e6d..05bc266 100644 --- a/main/githooks.py +++ b/main/githooks.py @@ -24,7 +24,6 @@ from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch -import importlib import os import platform import re @@ -32,7 +31,10 @@ import unittest import sys -licence_headers = None +try: + import licence_headers +except ImportError: + licence_headers = None # Absolute file size limit (in MB) - it's 100MB on github.com @@ -1086,16 +1088,12 @@ def run_licence_check(files): return 1 is_check_mode = mode in ['check', 'plan', 'verify'] - global licence_headers if licence_headers is None: - try: - licence_headers = importlib.import_module('licence_headers') - except ImportError: - _fail( - 'Licence header checking is enabled but the ' - '"licence_headers" module is not installed.' - ) - return 1 + _fail( + 'Licence header checking is enabled but the ' + '"licence_headers" module is not installed.' + ) + return 1 try: if not is_check_mode: @@ -1135,59 +1133,42 @@ def run_licence_check(files): class TestRunLicenceCheck(unittest.TestCase): - def setUp(self): - # Reset module-level cached import between tests for complete test isolation - import githooks - githooks.licence_headers = None - @patch('githooks.get_config_setting', return_value=None) def test_disabled(self, _config): self.assertEqual(0, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.importlib.import_module') - def test_check_failure_blocks_commit(self, import_module, _config): - fake_module = unittest.mock.Mock() - fake_module.process_files.return_value = 1 - import_module.return_value = fake_module - + @patch('githooks.licence_headers') + def test_check_failure_blocks_commit(self, mock_licence, _config): + mock_licence.process_files.return_value = 1 self.assertEqual(1, run_licence_check(['example.py'])) - fake_module.process_files.assert_called_once_with(['example.py'], fix=False) + mock_licence.process_files.assert_called_once_with(['example.py'], fix=False) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.importlib.import_module') - def test_check_mode_allows_unstaged_changes(self, import_module, _config): - fake_module = unittest.mock.Mock() - fake_module.process_files.return_value = 0 - import_module.return_value = fake_module - + @patch('githooks.licence_headers') + def test_check_mode_allows_unstaged_changes(self, mock_licence, _config): + mock_licence.process_files.return_value = 0 self.assertEqual(0, run_licence_check(['example.py'])) - fake_module.process_files.assert_called_once_with(['example.py'], fix=False) + mock_licence.process_files.assert_called_once_with(['example.py'], fix=False) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.importlib.import_module') - def test_fix_restages_files(self, import_module, run, _config): - fake_module = unittest.mock.Mock() - fake_module.process_files.return_value = 0 - import_module.return_value = fake_module - + @patch('githooks.licence_headers') + def test_fix_restages_files(self, mock_licence, run, _config): + mock_licence.process_files.return_value = 0 run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 0, '', ''), ] self.assertEqual(0, run_licence_check(['example.py'])) - fake_module.process_files.assert_called_once_with(['example.py'], fix=True) + mock_licence.process_files.assert_called_once_with(['example.py'], fix=True) self.assertEqual(['git', 'add', '--', 'example.py'], run.call_args_list[-1].args[0]) @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - @patch('githooks.importlib.import_module') - def test_restage_failure_blocks_commit(self, import_module, run, _config): - fake_module = unittest.mock.Mock() - fake_module.process_files.return_value = 0 - import_module.return_value = fake_module - + @patch('githooks.licence_headers') + def test_restage_failure_blocks_commit(self, mock_licence, run, _config): + mock_licence.process_files.return_value = 0 run.side_effect = [ subprocess.CompletedProcess([], 0, '', ''), subprocess.CompletedProcess([], 1, '', 'cannot add'), @@ -1196,27 +1177,26 @@ def test_restage_failure_blocks_commit(self, import_module, run, _config): @patch('githooks.get_config_setting', side_effect=['true', 'fix']) @patch('githooks.subprocess.run') - def test_fix_rejects_partially_staged_files(self, run, _config): + @patch('githooks.licence_headers') + def test_fix_rejects_partially_staged_files(self, _mock_licence, run, _config): run.return_value = subprocess.CompletedProcess([], 1, '', '') self.assertEqual(1, run_licence_check(['example.py'])) run.assert_called_once() @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.importlib.import_module') - def test_processing_error_blocks_commit(self, import_module, _config): - fake_module = unittest.mock.Mock() - fake_module.process_files.side_effect = OSError('cannot read') - import_module.return_value = fake_module - + @patch('githooks.licence_headers') + def test_processing_error_blocks_commit(self, mock_licence, _config): + mock_licence.process_files.side_effect = OSError('cannot read') self.assertEqual(1, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', return_value=None) + @patch('githooks.licence_headers', None) def test_missing_module_allowed_when_disabled(self, _config): self.assertEqual(0, run_licence_check(['example.py'])) @patch('githooks.get_config_setting', side_effect=['true', 'check']) - @patch('githooks.importlib.import_module', side_effect=ImportError('No module named licence_headers')) - def test_missing_module_fails_when_enabled(self, _import, _config): + @patch('githooks.licence_headers', None) + def test_missing_module_fails_when_enabled(self, _config): self.assertEqual(1, run_licence_check(['example.py']))