Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pre-commit: Ignore time stamps in po files #708

Open
wants to merge 4 commits into
base: 2.5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 34 additions & 18 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,39 @@ jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
- uses: pre-commit/[email protected]
env:
SKIP: rstcheck
- name: "Check out repository"
uses: actions/checkout@v4
with:
# Fetch the previous commit to be able to check for changes
fetch-depth: 2

- name: "Generate patch file"
if: failure()
run: |
git diff-index -p HEAD > "${PATCH_FILE}"
[ -s "${PATCH_FILE}" ] && echo "UPLOAD_PATCH_FILE=${PATCH_FILE}" >> "${GITHUB_ENV}"
env:
PATCH_FILE: pre-commit.patch
- name: "Detect code style issues (push)"
uses: pre-commit/[email protected]
if: github.event_name == 'push'
env:
SKIP: rstcheck,ignore-pot-creation-date

- name: "Upload patch artifact"
if: failure() && env.UPLOAD_PATCH_FILE != null
uses: actions/upload-artifact@v4
with:
name: ${{ env.UPLOAD_PATCH_FILE }}
path: ${{ env.UPLOAD_PATCH_FILE }}
- name: "Detect code style issues (pull_request)"
uses: pre-commit/[email protected]
if: github.event_name == 'pull_request'
env:
SKIP: rstcheck
with:
# HEAD is the not yet integrated PR merge commit +refs/pull/xxxx/merge
# HEAD^1 is the PR target branch and HEAD^2 is the HEAD of the source branch
extra_args: --from-ref HEAD^1 --to-ref HEAD

- name: "Generate patch file"
if: failure()
run: |
git diff-index -p HEAD > "${PATCH_FILE}"
[ -s "${PATCH_FILE}" ] && echo "UPLOAD_PATCH_FILE=${PATCH_FILE}" >> "${GITHUB_ENV}"
env:
PATCH_FILE: pre-commit.patch

- name: "Upload patch artifact"
if: failure() && env.UPLOAD_PATCH_FILE != null
uses: actions/upload-artifact@v4
with:
name: ${{ env.UPLOAD_PATCH_FILE }}
path: ${{ env.UPLOAD_PATCH_FILE }}
8 changes: 7 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,18 @@ repos:
types: [rst]
entry: python tools/fixup_gh_wiki_anchors.py
language: python
- id: ignore-pot-creation-date
name: Ignore POT-Creation-Date
entry: python tools/ignore_pot_creation_date.py
language: python
files: ^.*\.po$
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.1.0
rev: v5.0.0
hooks:
- id: check-byte-order-marker
- id: check-case-conflict
- id: end-of-file-fixer
exclude: ^.tx/config$
- id: mixed-line-ending
- id: trailing-whitespace
exclude: ^source/locale/.*$
Expand Down
96 changes: 96 additions & 0 deletions tools/ignore_pot_creation_date.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import argparse
import logging
import os
import subprocess
import sys
import typing

"""
This script reverts changes in PO files when only the timestamp has changed.
It helps to create meaningful commits by ignoring unnecessary changes.
"""


def get_git_changeset(from_ref, to_ref) -> str:
"""
Constructs the changeset string for `git diff` based on from_ref and
to_ref.
"""
from_ref = (
from_ref
or os.getenv("PRE_COMMIT_FROM_REF")
or os.getenv("PRE_COMMIT_SOURCE")
or "HEAD"
)
to_ref = (
to_ref
or os.getenv("PRE_COMMIT_TO_REF")
or os.getenv("PRE_COMMIT_ORIGIN")
)
return f"{from_ref}...{to_ref}" if to_ref else from_ref


def count_meaningful_changes(changeset, po_file) -> int:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def count_meaningful_changes(changeset, po_file) -> int:
def count_meaningful_changes(changeset: str, po_file: typing.Iterable[pathlib.Path]) -> int:

"""
Counts meaningful changes in the diff for a given PO file.
"""
cmd = ["git", "diff", "--patch", "--unified=0", changeset, "--", po_file]
proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()

diff_lines = proc.stdout.splitlines()
return sum(
1
for line in diff_lines
if (line.startswith("-") or line.startswith("+"))
and "POT-Creation-Date:" not in line
and "PO-Revision-Date:" not in line
and po_file not in line
)


def revert_po_file(changeset, po_file, logger):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes no sense to pass a logger here, getLogger is cheap

"""
Reverts a PO file to its original state in the given changeset.
"""
ref = changeset.split("...")[
0
] # Use the first part of the changeset as the reference
logger.info(f"{po_file} has no meaningful changes, reverting to {ref}")
cmd = ["git", "show", f"{ref}:{po_file}"]
proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()
Comment on lines +61 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you want this:

Suggested change
proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()
output = subprocess.check_output(cmd, text=True)


with open(po_file, "w") as file:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if po_file is a Path, you can do this:

Suggested change
with open(po_file, "w") as file:
with po_file.open(mode="w") as file:

file.write(proc.stdout)


def main(argv: typing.Optional[typing.List[str]] = None) -> int:
logging.basicConfig(
format="[%(levelname)s] %(message)s", level=logging.INFO
)
logger = logging.getLogger(__name__)

parser = argparse.ArgumentParser(
description="Revert PO files with only timestamp changes."
)
parser.add_argument("--from-ref", help="Use changes since this commit.")
parser.add_argument("--to-ref", help="Use changes until this commit.")
parser.add_argument("files", nargs="*", help="Only check these files.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parser.add_argument("files", nargs="*", help="Only check these files.")
parser.add_argument("files", nargs="*", type=pathlib.Path, help="Only check these files.")

args = parser.parse_args(argv)

# Remove the first entry which is the script file name itself
files_to_process = args.files[1:] if args.files else []
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. Are you confusing this with sys.argv?


changeset = get_git_changeset(args.from_ref, args.to_ref)

for po_file in files_to_process:
count = count_meaningful_changes(changeset, po_file)
if count == 0:
revert_po_file(changeset, po_file, logger)

return 0


if __name__ == "__main__":
sys.exit(main())
Loading