Repository navigation
fix: the blame and trailer fixes that built the token dataset - #115
Draft
EllianCarlos wants to merge 8 commits into
Draft
EllianCarlos wants to merge 8 commits into
EllianCarlos wants to merge 8 commits into
Conversation
The footer_* columns came from the footers table of the cregit DB (step 4). That DB comes from the rewritten repository. There, blobExec appends "\n" + "Former-commit-id: X" to a message whose last paragraph has trailers. A git CLI message ends in "\n", so a blank line then comes before Former-commit-id, and JGit finds no other trailer. Only messages with no final newline (for example, from the GitHub web UI) kept their trailers. The curl census Parquet has 0 trailers, but git finds them on 520 of its 12,072 commits. Step 10 now reads the footers table of the original log DB (step 3), keyed by the original commit SHA. The key is the same as for original_commit_sha: commitmap.originalcid, else the cregit SHA. blobExec and the rewritten history do not change, so the commit SHAs and the blob map stay the same, and a resume stays valid. generate_dataset.py has a new required argument, --original-db. If the file is missing or has no footers table, the run stops before Phase 1, with the reason. It does not read the trailers from the cregit DB. run_pipeline_process.sh passes the argument and stops step 10 if the step 3 DB is missing. The column names, types, order, the key match and the idx order do not change. Only the 15 footer_* columns change: on libexpat and iputils, every other column is identical row by row. (cherry picked from commit acd08b4)
The typed footer_* lists and the person columns came from one subquery, which joins each trailer to emails on its address. The persons DB has one emails row for each name that an address committed with. So a trailer with such an address came out once for each row: on libexpat, 1 of 1,752 commits had its Co-authored-by twice, and on iputils, 1 of 388 commits had its Reviewed-by twice. git shows each of them once. The typed lists now come from a subquery over footers alone. The footer_personids and footer_person_names subquery is the same as before, so those two columns do not change. A trailer line that the message repeats stays repeated. (cherry picked from commit 9079a9c)
A commit message can name one person twice in one trailer key: with the same text, or with another address. In the Linux Parquet, 4,695 of 839,023 commits have one Signed-off-by value twice, and 7,135 have one name with another text, for example "Borislav Petkov (AMD) <bp@alien8.de>" and "Borislav Petkov <bp@suse.de>". git log shows these repeats in the messages. Each of the 13 typed footer_* lists now holds one entry per person. In one list, two values are one person when their addresses resolve to one personid. The address is the one that footer_personids uses: the address in <...>, else a bare address. If the address is not in emails, the lowercased address is the key. A value with no address is never merged. For each person, the list keeps the value with the lowest idx, at its place, so the list is in the order of the first line of each person. footer_personids and footer_person_names do not change. (cherry picked from commit 1f17663)
git blame -C100 takes 3 to 38 hours on one huge generated file (protobuf Java, tree-sitter parser.c, ICU stubdata.cpp). Plain blame takes about 3 minutes on the same file. formatBlame.pl now runs git with fork and exec, not through a shell, so it knows the pid and the exit status of git. After --c100-limit seconds (default CREGIT_C100_LIMIT, else 3600; 0 = no limit), it kills -C100, discards the partial output, and runs git blame --porcelain without -C and with no limit. It prints one line to STDERR: "PLAIN-BLAME <file> after <N> s of -C100". The pipeline log keeps it. formatBlame.pl now checks the exit status of git. Before, a git that stopped early (killed, out of memory) gave an empty .blame file and exit 0, so the tokens of the file were lost with no error. Now the script removes the temp file, writes no .blame file, and exits 1. blameRepoFiles.pl counts the error, and the blame step fails. A file that ends within the limit gets the same bytes as before: Read_Record and copy_file do not change. blameRepoFiles.pl does not change: its children inherit CREGIT_C100_LIMIT. (cherry picked from commit 57551d7)
git blame -C100 can stop with "fatal: unable to generate diff" and exit 128. This occurs when a commit moves a blob over 1 GiB: git reads the blob as a copy candidate, xdiff refuses a buffer over 1 GiB, and git does not skip the candidate. The census found this in one project (gitlab.inria.fr/cedar/connectionlens, a 1.33 GB model file). Since 57551d7, a git failure stops the project, so the project failed. After a -C100 exit 128, formatBlame.pl now runs plain blame, the same path as after the -C100 time limit. It prints "PLAIN-BLAME <file> after git exit 128 of -C100" to STDERR, so the file can be listed in LIMITATIONS. A git that a signal killed, or a plain blame that also fails, still gives a non-zero exit and no blame file. Tests: formatBlame.t has 5 new tests (46 in total); 3 of them fail on 57551d7. (cherry picked from commit 23e4ccb)
Two blameRepoFiles.pl runs can now use the same output directory. formatBlame.pl takes an exclusive lock on build/locks/<sha1 of the output path>.lock and holds it until it exits. When the lock is held and the .blame file already exists, the script exits 0 and does not start git. Otherwise the blame runs as before, with the same bytes. The second run can then blame the files that the first run has not reached yet, and the first run skips each file that the second run finished. The locks stay in build/, out of the output directory. (cherry picked from commit 365a47a)
blameRepoFiles.pl read `git ls-files` without -z. git C-quotes a path with a byte >= 0x80, '"', '\' or a control byte (core.quotePath), so the quoted name did not match the mask and the file was silently not blamed: it had no token rows. Now it reads `ls-files -z`. blameRepoFiles.pl gave the path to formatBlame.pl, and formatBlame.pl gave it to git blame, with no '--' before it. A path that starts with '-' was read as an option, and step 7 exited 1. Now both pass '--'. git blame --porcelain also C-quotes "filename". formatBlame.pl now compares the raw bytes with the blamed path, so a quoted file does not name itself in field 2. Field 2 keeps git's quoted form, as before. The libexpat gate Parquet is unchanged (14eb5d72, 2,196,248 bytes). tests/t/blamePaths.t covers a non-ASCII byte, '"', '\', a tab, a newline, a single quote, a leading '-' file and folder, and renames. (cherry picked from commit 14b8db4)
formatBlame.t found the real git with `command -v git` in backticks. Perl does not use a shell for a command with no shell characters, so it runs the program "command" directly. On the Ubuntu CI runner, "command" is only a shell builtin. The exec failed, $realGit was empty, each blame through the fake git failed, and the file stopped after 46 of its 56 tests. Amazon Linux has /usr/bin/command (a script from the bash package), so the test passed there. Find git with a PATH search in Perl (File::Spec) instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR collects the blame fixes and the trailer fixes that wrote the token dataset. Before this PR, no commit had all of them: 23e4ccb, 365a47a and 14b8db4 are three separate branches on 57551d7, and 9ccead6 is a fourth branch on 95cdc88. The run branches also sit on 95cdc88, which holds PR #110 and the srcML patch commit 73c6df4, so a PR from them would show those changes again.
The head branch
fix/dataset-blame-trailersstarts at master and has the 7 fix commits again, made withgit cherry-pick -x. Each commit message names its run commit ("cherry picked from commit ..."). The branch does not need PR #110. The run commits stay on GitHub as the tagsdataset-build/cregit-<sha7>, so a squash of this PR keeps them reachable.The blame step after this PR:
What changes and why
Trailers from the original log DB (acd08b4, 9079a9c, 1f17663)
Defect: step 10 read the footer_* columns from the tokenized repository. Its commit messages are not the original messages, so the trailers were wrong or missing. The emails join also repeated a trailer once for each emails row of the same address.
Change:
generate_dataset.py --original-db(required) attaches the step 3 log DB read-only and joins oncoalesce(m.originalcid, t.commit_sha). It fails before phase 1 when the file or the footers table is missing.run_pipeline_process.shpasses$DB_PATH_ORIGINAL. The typed lists have their own subquery. Each of the 13 typed lists keeps one entry per person: the key is the lowest personid of the address, else the lowercased address; an entry with no address is never merged; the first idx stays.Measured effect: the libexpat gate changed from 40511e0d (2,184,432 B) to 14eb5d72 (2,196,248 B), and only footer_* columns differ. iputils: 0 typed trailers before, 528 after, 388 of 388 commits equal to git. The one-entry rule changes 9,536 commits of Linux; a Python model of the rule agrees on 839,023 of 839,023 commits. Batches 2 to 6 got the same columns after the run (
rewrite_footer_columns.py).-C100 time limit and git exit status (57551d7; 9ccead6 is the same file on 95cdc88)
Defect:
git blame -C100takes 3 to 38 hours on one huge generated file (for example a protobuf Java file, a tree-sitter parser.c, an ICU data stub); plain blame takes about 3 minutes. The old script also read git through a shell and did not check its exit status: a killed git or an out-of-memory stop gave an empty or partial.blamefile and exit 0, so the tokens of the file were lost with no error.Change: fork and exec, an alarm, and a plain-blame fallback after the limit. A git failure now writes no
.blamefile and exits 1. A file that ends within the limit gets the same bytes as before.Measured effect: the gates did not change (14eb5d72 on 1f17663, 40511e0d on 95cdc88). At least 201 files of the 5,000+ layer (count of 2026-10-14) and 367 more files got plain blame; the later projects are not counted yet (see the known issues).
Plain blame after a git exit 128 (23e4ccb)
Defect:
git blame -C100exits 128 ("unable to generate diff") when a commit moves a blob over 1 GiB, because xdiff refuses the buffer. Since 57551d7 such a project fails.Change: after a -C100 exit 128, the script runs plain blame and prints a PLAIN-BLAME line. A git that a signal killed, or a plain blame that fails, still gives exit 1.
Measured effect: 1 project (connectionlens, 2 files with plain blame). The libexpat gate is unchanged (14eb5d72).
Lock and skip (365a47a)
Change: two
blameRepoFiles.plruns can share one output folder. Each file has an exclusive lock inbuild/locks/. When the lock is held and the.blamefile exists, the script exits 0 and does not start git. The bytes of a blamed file do not change.Why: two blame helpers shared the work of the slowest projects.
Raw paths and '--' (14b8db4)
Defects:
git ls-fileswithout-zC-quotes a path with a byte of 0x80 or more, a double quote, a backslash or a control byte; the quoted name did not match the mask, and the file got no token rows with no error. A path that starts with '-' became a git option, and step 7 exited 1. In 95cdc88 and 1f17663, a path with a single quote gave an empty.blamefile (shell string).Change:
ls-files -z, '--' before each path in both scripts, and a raw-byte compare for the porcelain "filename". Field 2 keeps the quoted form of git, as before.Measured effect: the scan found 1,709 files (2.5 MB) in 19 projects of the 5,000+ layer and 3,097 files (16.7 MB) in 18 projects of the 1,000-4,999 layer with no token rows. The path-fix rerun wrote 35 projects again. The libexpat gate is unchanged (14eb5d72).
Evidence
tests/t/formatBlame.tof 57551d7 on the 1f17663 script: 14 of 41 tests fail (partial.blamefile with exit 0).After: 41 of 41 pass on 57551d7.
After: 46 of 46 pass on 23e4ccb.
formatBlame.thas 51 tests and they pass.tests/t/blamePaths.tcovers a non-ASCII byte, a double quote, a backslash, a tab, a newline, a single quote, a leading '-' file and folder, and renames. Two projects that failed on 57551d7 give a Parquet on 14b8db4, the same bytes on a second run.tests/test_generate_dataset.py: 82 tests at 157cf8b, 99 tests at 1f17663; all pass.git cherry-pick -nof the 7 commits onto fork master 5f0b6b6, plan line set to 56; no commit made). 40f865e, the newer master, adds onlynix/srcml*files, which no fix commit changes:git merge-tree --write-tree 57551d7 9ccead6gives the tree of 57551d7 (48eaf1b8), so it adds no change.Dataset runs that used these commits (tools.lock)
formatBlame.plin place from 2026-10-11 20:05 UTC.Known issues to fix in this PR
formatBlame.pl(57551d7 lines 87, 120-121, 195-196; same in 23e4ccb, 365a47a, 14b8db4) do not stop the script. With RLIMIT_FSIZE 64 KiB the script exits 0 with 1,257 of 20,000 lines; a 252-byte name exits 0 with no file. Fix:or dieon print, close and move; in step 10, compare the record count with the token lines.--overwrite(soctp --reblamekeeps a stale file), the lock is per tool copy, and any existing file counts as finished. Fix: skip only on request; put the lock in the output tree or write a completion marker.DATASET.mdstill says that any other failure exits 1; update it.formatBlame.plleaves an orphan git; a SIGALRM after EOF and beforealarm(0)can force a needless plain blame. Fix: a TERM and INT handler; after a late alarm, use the git status.Walker.scala:892,:1105), so step 10 reports "source not found" and skips the file (generate_dataset.py:714-716). Fix: keep the raw bytes of names. 0 such names in the scan.DATASET.mdthat personid order is text order.DATASET.mdandtests/test_generate_dataset.pyuse real maintainer identities; replace them with fictitious ones.tests/t/formatBlame.t(from 23e4ccb):my ($s9) = ...at the bad-limit test masks the$s9of the exit-128 test (a Perl warning). Rename one of them.GIT_CONFIG_NOSYSTEM=1andGIT_CONFIG_GLOBAL=/dev/nullfor the git calls of the blame step, so the machine git config cannot change the blame.--plain-blame-list <file>to force plain blame for a list of files, and write one record per plain-blame file (project, file_path, mechanism) next to the Parquet. Today the only record is a STDERR line in logs that the release does not hold, and the choice depends on the machine..blamerecord. A ';' in an earlier file name (rename or -C100 source) shifts the split ingenerate_dataset.py:296: token_type gets "name.c;\tDECL" (36 rows in 2 files of rust-analyzer in the sample).DATASET.md:125: every "Key: value" line of the last paragraph, no 25% rule, no blank line needed, continuation lines dropped; and the one-entry-per-person rule.Follow-ups for code that is already on master (separate PRs)
srcml2tokenexits 0 after a xerces fatal error, and the worker keeps the partial stream. Exit non-zero on error and fatalError; refuse a stream with no end_unit.gitLogToDb.scala:159-161). Format in UTC and keep the offset.unifyPersons.scalalowercases with the default locale, never fills personname, and breaks ties by hash-set order. UseLocale.ROOT, fill personname, break ties by key.rm -rfof any existing--workfolder; FROM_STEP not range-checked; unquoted variables; a 0-byte Parquet after a failed COPY (use a temp file andos.replace); a trailing-space name gets no person id (22 rows in the local Linux file);DATASET.mderrors.tokenWorker.pl:155-159moves the memo across file systems with a copy, so another task can read a partial memo. Write a temp file in the memo folder, then rename.tokenWorker.pland--position; a ctags failure is silent; a null committer ident gives an NPE; symlinks are tokenized; add a Rust and a Java gate.Merge Danger
Door: two-way. A revert restores the old scripts. The run commits do not depend on this PR: the tags
dataset-build/cregit-<sha7>keep them on GitHub.Blast Radius: output. A new dataset build with master changes the footer_* bytes against 95cdc88, can give plain blame for slow files (the choice depends on the machine), and fails a project on a git error where the old code wrote an empty file. Rows of quoted paths come back.
The repository allows squash and rebase merges only. A squash is safe for provenance, because the tags hold the run commits and each picked commit names its run commit. Do not delete the old branches: the release docs name some of them.
Checklist
dataset-build/cregit-<sha7>of the 7 run commits (95cdc88, 9ccead6, 1f17663, 57551d7, 23e4ccb, 365a47a, 14b8db4) are on GitHub.prove tests/t/formatBlame.t tests/t/blamePaths.t tests/t/blameRepoFiles.tpasses (119 tests before the follow-ups).pytest tests/test_generate_dataset.pypasses (99 tests before the follow-ups).