Refresh the Windows launchers only when their code changes, not their toolchain stamp

[vharseko]

Problem

On 23.09 and 24.09 the Package/Deploy workflow committed "Refresh the Windows native launchers" twice (a8a18f0, then c6919ea). The second commit reverted the binaries to exactly the blobs of a611c10. Nothing under opendj-server-legacy/src/build-tools/windows changed in between.

The Windows job of each Build run had landed on a different runner image:

Commit	Build run	windows-2025-vs2026 image	cl / link build
a611c10	33984133926	20260824.214	14.51.36256
a8a18f0	35869911632	20260922.246	14.51.36257
c6919ea	35973304427	20260907.229	14.51.36256

/Brepro hashes the build numbers of the tools into the output. The two sets of launchers have identical .text, .data, .rsrc, .reloc and .fptable sections. They differ in about 70 bytes per file, all of them toolchain stamp:

• the Rich header: the cl (0x0104), link (0x0102) and cvtres (0x00ff) records move from 36256 to 36257, and the XOR key of the header changes with them;
• the 32-byte REPRO debug hash;
• the COFF and debug directory timestamps, which /Brepro takes from that hash;
• the PE checksum.

GitHub rolls a new image out over days. Until the rollout finishes, windows-latest hands out either image, and a byte comparison refreshes the committed launchers back and forth on every push.

Change

• .github/scripts/same-pe-code.py compares two PE images with that stamp left out of both. It exits 0 if nothing else differs, 1 if something does, and 2 if a file cannot be read as a PE image. It also leaves out the zero padding after the DOS stub and after the section table, and e_lfanew, which only says where the first of them ends. A patch release may pad differently: 14.51.36252 put the PE header at 0x100 and 14.51.36256 at 0xf0, around the same code. The docstring lists every field it leaves out.
• .github/scripts/refresh-launchers.sh BUILT LIB copies each rebuilt launcher unless it differs from the committed one only in the stamp. A real difference, a new launcher, or a file that fails to parse all take the rebuilt file: when in doubt, it refreshes.
• deploy.yml calls refresh-launchers.sh instead of a plain cp. A launcher that differs only in the stamp stays as committed, and the log says so. A checked-out branch without the script (sustaining/4.10.x) takes the rebuilt files as before.
• .github/scripts/test-refresh-launchers.sh checks both scripts against launchers committed on master. build.yml runs it on the Linux Java 11 leg, so both scripts are checked before merge and not only after a push.
• The /Brepro comment in the Makefile and the artifact comment in build.yml now say what the workflow compares.

Verification

• test-refresh-launchers.sh, green at the head:
  • the 36256 and 36257 sets compare equal both ways, for all three launchers;
  • the 05.09 refresh (a611c10^ → a611c10, link 36252 → 36256) compares equal for launcher_administrator and winlauncher, whose code did not change, and as a difference for opendj_service, whose code did;
  • 74cb4f5 against a8a18f0 (a change of the launcher sources) is a difference for all three;
  • winlauncher against launcher_administrator is a difference, and so is one flipped .text byte;
  • a change of the headers only is a difference too: the NX_COMPAT flag, a byte of the DOS stub, or the flags of the first section, each flipped;
  • two copies that differ only in the PDB GUID and age of a CodeView entry compare equal. No committed launcher has a PDB, so the test retypes the POGO debug entry as CodeView;
  • a file that is not a PE image exits 2;
  • the loop keeps a launcher that differs only in the stamp, refreshes one whose code changed, adds one that is not committed yet, and refreshes one whose committed file is not a PE image.
• Mutants, each of which fails the check:
  • an empty normalize();
  • the REPRO hash, the Rich header, the COFF timestamp or the debug timestamp compared again;
  • CheckSum blanked at the wrong offset;
  • the padding compared again;
  • only the headers compared;
  • only the sections compared, or the DOS stub, the COFF and optional headers, or the section table left out;
  • the CodeView GUID and age compared again, or blanked at the wrong offset;
  • main() exiting 0 on a file that is not a PE image;
  • the loop condition inverted, or the loop refreshing on exit 1 only.
• The deploy step, its shell cut out of deploy.yml and run in a scratch directory:
  • with the script: the stamp-only launcher is kept and the changed one is refreshed;
  • without it: both are copied.

The deploy step itself only runs after merge, on the next push to master.

[maximthomas]

praise: the fallback is safe in every direction, and the fix works on the pairs that flapped.

• deploy.yml:117 takes the rebuilt file on exit 1 and exit 2 alike, so a parse failure, a missing file or a branch without the script falls back to the old unconditional refresh, never to a stale launcher.
• Reproduced locally: same-pe-code.py exits 0 for launcher_administrator.exe and winlauncher.exe at c6919ea against a8a18f0 (14.51.36256 against 36257), and 1 for a real code change (74cb4f5 against a8a18f0).

---

question (non-blocking): Should a 14.51.36252 → 36256 rebuild count as a code change? Its code is identical, but its PE header moved from 0x100 to 0xf0.

.github/scripts/same-pe-code.py:35-37, :129

normalize() compares at fixed offsets, so anything that moves e_lfanew shifts every header byte. The docstring gives a Rich header gaining or losing an entry as the only cause. For launcher_administrator.exe and winlauncher.exe at a611c10^ (link 36252) against a611c10 (36256), both Rich headers have 10 entries and end at 0xe8. Only the padding before the PE header differs. The sections are identical from 0x400 on, and so are the headers once aligned at e_lfanew. The script exits 1 for both launchers, so two runner images like these would still refresh the launchers back and forth. The description's "genuine 05.09 refresh" is a code change for opendj_service.exe only (177152 → 175104 bytes). As it stands this is minor: it errs towards a refresh, which is the pre-PR behaviour. It is worth fixing here if the script is meant to absorb every patch-release rotation, as the title says. The change below makes both launcher pairs exit 0. opendj_service.exe's 05.09 pair and winlauncher against launcher_administrator still exit 1, and the pin in the next comment stays green.

        blank(b, opt + 64, 4)                                    # CheckSum
        headers_end = u32(b, opt + 60)                           # SizeOfHeaders
...
    except struct.error as e:
        raise NotPE(str(e))
    # The linker pads the DOS stub up to e_lfanew and the headers up to SizeOfHeaders,
    # and it may pad differently from one patch release to the next: 14.51.36252 put the
    # PE header at 0x100, 14.51.36256 at 0xf0. Compare what the padding surrounds.
    return (bytes(b[:0x3C]) + bytes(b[0x40:pe]).rstrip(b"\0")
            + bytes(b[pe:headers_end]).rstrip(b"\0") + bytes(b[headers_end:]))

---

suggestion (non-blocking): No check before merge runs same-pe-code.py.

.github/scripts/same-pe-code.py:66

The script's only caller is deploy.yml:117, after a successful push build. Suppose a later edit blanks too much, e.g. return bytes() for return bytes(b). The step then keeps a launcher whose code changed, and the zips ship it with CI green. An edit that blanks too little brings the flip-flop back. The 20 cases in the description are not in the tree. The step body below, run locally, is green at the head. It turns red under return bytes(), with the REPRO blank dropped, and with CheckSum blanked at opt + 60. build-maven already checks out with fetch-depth: 0.

    - name: Check same-pe-code.py against known launcher pairs
      if: runner.os == 'Linux' && matrix.java == '11'
      shell: bash
      run: |
        set -e
        t=$(mktemp -d)
        rc() { python3 .github/scripts/same-pe-code.py "$1" "$2" && echo 0 || echo $?; }
        for f in launcher_administrator winlauncher; do
          for c in c6919eaaf4 a8a18f000d 74cb4f5a84; do
            git show "$c:opendj-server-legacy/lib/$f.exe" > "$t/$f-$c.exe"
          done
          # 14.51.36256 against 14.51.36257: the toolchain stamp only
          test "$(rc "$t/$f-c6919eaaf4.exe" "$t/$f-a8a18f000d.exe")" = 0
          # before and after a change of the launcher sources
          test "$(rc "$t/$f-74cb4f5a84.exe" "$t/$f-a8a18f000d.exe")" = 1
        done

Pin: keep both launchers. winlauncher.exe has CheckSum 0 in both builds, so only launcher_administrator.exe kills the CheckSum mutant.

---

suggestion (non-blocking): The keep-or-copy loop runs only after merge. An inverted condition would ship stale launchers with every check green.

.github/workflows/deploy.yml:112-124

With if [ -f "$committed" ] && ! python3 …, the loop keeps a launcher whose code changed and refreshes one that differs only in the stamp. Nothing fails before the next push to master. The [ -f ] guard is not a survivor: without it, open() raises, main() returns 2, and the loop still copies. To pin the loop, move it unchanged into .github/scripts/refresh-launchers.sh BUILT LIB (with $1/$2 for $BUILT and opendj-server-legacy/lib), call it from deploy.yml, and run the check below before merge. On the loop copied verbatim, the check is green; with the condition inverted, it is red.

set -e
t=$(mktemp -d); mkdir "$t/built" "$t/lib"
g() { git show "$1:opendj-server-legacy/lib/$2.exe"; }
g a8a18f000d winlauncher > "$t/built/winlauncher.exe"
g c6919eaaf4 winlauncher > "$t/lib/winlauncher.exe"
cp "$t/lib/winlauncher.exe" "$t/stamp-only.exe"
g a8a18f000d launcher_administrator > "$t/built/launcher_administrator.exe"
g 74cb4f5a84 launcher_administrator > "$t/lib/launcher_administrator.exe"
bash .github/scripts/refresh-launchers.sh "$t/built" "$t/lib"
cmp "$t/lib/winlauncher.exe" "$t/stamp-only.exe"                               # kept
cmp "$t/lib/launcher_administrator.exe" "$t/built/launcher_administrator.exe"  # refreshed

---

nitpick (non-blocking): build.yml still says deploy.yml commits the windows-exe-11 contents back to the branch.

.github/workflows/build.yml:87-89

After this PR, a rebuild that differs only in the stamp is not committed, so the committed lib/*.exe need not match the artifact byte for byte. The PR rewords the matching comments in deploy.yml and the Makefile, but not this one.

    # Also the source of truth for the committed opendj-server-legacy/lib/*.exe: on a
    # successful push build, deploy.yml downloads windows-exe-11 from this very run and
    # commits each launcher that differs from the committed one in more than the toolchain
    # stamp (.github/scripts/same-pe-code.py).

---

nitpick (non-blocking): The docstring calls its list of blanked fields exact, but the list leaves out the CodeView GUID and age.

.github/scripts/same-pe-code.py:32, :125-126

normalize() also blanks 20 bytes of an RSDS CodeView entry. The committed launchers have none today (debug types 13 and 16 only), so this is text only. Add the field to the list or drop "exactly".

[vharseko]

I took all five points in 1fd9477. The branch is also rebased onto the current master (2ba918a), with no conflicts.

1. 36252 → 36256. Reproduced. For launcher_administrator.exe and winlauncher.exe at a611c10^ against a611c10, the Rich headers both end at 0xe8, e_lfanew is 0x100 against 0xf0, and the script exited 1. normalize() now returns your version: the Rich header is blanked and drops out together with the padding after the DOS stub, and so does the padding after the section table up to SizeOfHeaders. It also raises NotPE when SizeOfHeaders is not between e_lfanew and the end of the file. Both pairs now exit 0; opendj_service.exe's 05.09 pair, 74cb4f5 against a8a18f0 and winlauncher against launcher_administrator still exit 1. That changes what the docstring said about a Rich header that gains or loses an entry: on its own it no longer counts, because it comes with a change of the objects linked in, which moves the code too. The docstring now says so. The PR description no longer calls the whole 05.09 refresh a code change; that holds for opendj_service.exe only.

2 and 3. A check before merge. The loop is now .github/scripts/refresh-launchers.sh BUILT LIB, moved over unchanged, and .github/scripts/test-refresh-launchers.sh checks both scripts. build.yml runs it right after the checkout, on the Linux Java 11 leg. It checks:

• the 36256/36257 pairs of all three launchers, both ways (exit 0);
• the 36252/36256 pairs of launcher_administrator and winlauncher (exit 0) and of opendj_service (exit 1);
• 74cb4f5 against a8a18f0 for all three (exit 1), and winlauncher against launcher_administrator (exit 1);
• winlauncher 36257 with one byte of .text flipped (exit 1);
• your loop case (keep winlauncher, refresh launcher_administrator), plus a launcher that is not committed yet (added).

I needed the flipped .text byte because one mutant survived without it. That mutant compares only up to SizeOfHeaders. Every real code change in the history also moves a size in the headers, so it passes on history alone.

Every mutant below makes the check fail, and the head passes:

• return b'';
• no REPRO blank;
• CheckSum blanked at opt + 60;
• no Rich header blank;
• no COFF timestamp blank;
• no debug directory timestamp blank;
• the old return bytes(b);
• headers only (the survivor above);
• the loop condition inverted.

As you said, removing the [ -f ] guard changes nothing the check can see.

deploy.yml checks out head_branch, and that can be sustaining/4.10.x, which has neither script. So the step calls refresh-launchers.sh when the file exists and does a plain cp otherwise, which is the pre-PR behaviour. I ran the step's shell, cut out of deploy.yml, in a scratch directory both ways:

• with the script: winlauncher is kept and launcher_administrator is refreshed;
• without it: both are copied.

4. The build.yml comment now says what gets committed, and names refresh-launchers.sh.

5. The docstring lists every field left out of the comparison, including the PDB GUID and age of an RSDS CodeView entry, and no longer says "exactly".

[maximthomas]

praise: Round 2 closes every round-1 point, and both scripts now run before merge.

• same-pe-code.py:141-142 compares what the padding surrounds, so the 05.09 launcher pairs (link 36252 vs 36256, PE header at 0x100 vs 0xf0) now compare equal, and test-refresh-launchers.sh:47-49 pins that.
• build.yml:59-64 runs both scripts on the Linux Java 11 leg against history blobs. The empty-normalize() mutant fails at the first stamp-only pair.
• The keep-or-copy loop is now refresh-launchers.sh, and deploy.yml and the test both call that one file.

---

issue (non-blocking): No test case differs only in the headers, so the DOS stub, the PE headers or the section table can be dropped from the comparison and CI stays green.

.github/scripts/test-refresh-launchers.sh:52-61, .github/scripts/same-pe-code.py:141-142

Every history pair that the test expects to differ also differs past SizeOfHeaders, and the one synthetic case flips a .text byte. I ran four mutants locally and all 15 checks stayed green under each one. The first has normalize() return bytes(b[headers_end:]). The other three each drop one narrower part: the DOS stub, the COFF and optional headers, or the section table. After a regression like that, a Makefile linker-flag change (/NXCOMPAT, /DYNAMICBASE, /SUBSYSTEM) leaves the committed launcher as it was. The test's header comment says it guards against exactly that case: keeping a launcher that changed. With the pin below, the head passes 20/20 checks and exits 1 for all three flips. Each of the four mutants exits 0 on at least one flip.

# A change confined to the headers must show: a linker flag, the DOS stub, a section flag.
for field in nx stub section; do
  python3 - "$t/winlauncher-36257.exe" "$t/winlauncher-$field.exe" $field <<'PY'
import struct, sys
b = bytearray(open(sys.argv[1], "rb").read())
pe = struct.unpack_from("<I", b, 0x3C)[0]
table = pe + 24 + struct.unpack_from("<H", b, pe + 20)[0]
off = {"nx": pe + 24 + 71,                  # DllCharacteristics, high byte: NX_COMPAT
       "stub": 0x4E,                        # "This program cannot be run in DOS mode."
       "section": table + 39}[sys.argv[3]]  # the first section's Characteristics, top byte
b[off] ^= 1
open(sys.argv[2], "wb").write(b)
PY
  expect 1 winlauncher-36257.exe winlauncher-$field.exe "$field: a change of the headers only"
done

Pin: after test-refresh-launchers.sh:61, three one-byte flips of winlauncher-36257.exe, each expected to differ.

---

suggestion (non-blocking): No test gives either script a file that is not a PE image, so nothing pins exit 2 or the refresh that follows it.

.github/scripts/same-pe-code.py:154-156, .github/scripts/refresh-launchers.sh:26-28, .github/scripts/test-refresh-launchers.sh:37-75

Every expect call expects 0 or 1, and the loop fixture holds only valid images. Two mutants each keep the test green (15/15): main() returning 0 on NotPE/OSError at :156, and refresh-launchers.sh:28 refreshing only on exit 1. Under either one, a committed launcher that does not parse stays as committed. Nothing triggers this road today. The pin below kills both mutants: the first fails at expect 2, the second at cmp.

# A file that is not a PE image exits 2, and the loop takes the rebuilt file over it.
head -c 64 "$t/winlauncher-36257.exe" > "$t/truncated.exe"
expect 2 truncated.exe winlauncher-36257.exe "a file that is not a PE image"
mkdir "$t/built2" "$t/lib2"
cp "$t/winlauncher-36257.exe" "$t/built2/winlauncher.exe"
cp "$t/truncated.exe" "$t/lib2/winlauncher.exe"
bash "$here/refresh-launchers.sh" "$t/built2" "$t/lib2"
cmp "$t/lib2/winlauncher.exe" "$t/built2/winlauncher.exe"
echo "ok: refresh-launchers.sh refreshes over a file that is not a PE image"

Pin: at the end of test-refresh-launchers.sh. The SizeOfHeaders guard (same-pe-code.py:107) stays unpinned because the truncated file fails the PE-signature check first. A hand-crafted SizeOfHeaders of 0 would reach the guard.

---

suggestion (non-blocking): None of the pinned launchers has an RSDS CodeView entry, so no test reaches the code that blanks the PDB GUID and age.

.github/scripts/same-pe-code.py:134-135

The docstring (:36) lists this blank, but none of the 12 history blobs the test reads contains RSDS. The Makefile passes neither /Zi nor /DEBUG, so the debug directory holds only the POGO and REPRO entries. With the blank replaced by pass, the test stays green (15/15). A broken blank only ever causes an extra refresh. The pin below retypes the POGO entry as CodeView in two copies that differ only in GUID and age. It exits 0 at the head, and 1 if the blank is removed or moved to raw.

# No committed launcher has a PDB; make two that differ only in its GUID and age.
for n in 1 2; do
  python3 - "$t/winlauncher-36257.exe" "$t/winlauncher-rsds$n.exe" $n <<'PY'
import struct, sys
b = bytearray(open(sys.argv[1], "rb").read())
pe = struct.unpack_from("<I", b, 0x3C)[0]
opt = pe + 24
dirs = opt + (96 if struct.unpack_from("<H", b, opt)[0] == 0x10B else 112)
table = opt + struct.unpack_from("<H", b, pe + 20)[0]
rva = struct.unpack_from("<I", b, dirs + 6 * 8)[0]            # the debug directory
for i in range(struct.unpack_from("<H", b, pe + 6)[0]):
    va, vsize, raw = (struct.unpack_from("<I", b, table + 40 * i + o)[0] for o in (12, 8, 20))
    if va <= rva < va + vsize:
        entry = raw + rva - va
while struct.unpack_from("<I", b, entry + 12)[0] != 13:       # the POGO entry
    entry += 28
struct.pack_into("<I", b, entry + 12, 2)                       # retyped as CodeView
data = struct.unpack_from("<I", b, entry + 24)[0]
b[data:data + 24] = b"RSDS" + bytes([int(sys.argv[3])]) * 20  # GUID and age differ
open(sys.argv[2], "wb").write(b)
PY
done
expect 0 winlauncher-rsds1.exe winlauncher-rsds2.exe "the PDB GUID and age of a CodeView entry"

Or: while no launcher has a PDB, remove this code and its docstring line.

---

nitpick (non-blocking): The docstring's exit-0 sentence and its list of what is left out both leave out e_lfanew and the padding.

.github/scripts/same-pe-code.py:21-22, :33-39

normalize() returns b[:0x3C] + b[0x40:pe].rstrip(0) + … (:141-142), so e_lfanew never reaches the comparison, and only the comment at :138-140 says so. The commit message and the PR body say the docstring lists every field the script leaves out. :21 still mentions only the build stamp, yet winlauncher.exe 36252 vs 36256 (e_lfanew 0x100 vs 0xf0) exits 0, and test-refresh-launchers.sh:47-49 pins that.

Exits 0 when the two files are identical once the build stamp and the header padding
are left out of both, 1 when they differ anywhere else, and 2 when either one cannot be
read as a PE image.
...
- the zero padding the linker puts after the DOS stub, up to the PE header, and after
  the section table, up to SizeOfHeaders, and e_lfanew, which only says where the first
  of them ends. A patch release may pad differently:
  14.51.36252 put the PE header at 0x100, 14.51.36256 at 0xf0, around the same code.

[vharseko]

I took all four points in 2495d5c. The branch is also rebased onto the current master (d30ff78), with no conflicts.

I ran your mutants against the old test first. Each of the eight stayed green at 15/15:

• only the sections compared;
• the DOS stub, the COFF and optional headers, or the section table left out;
• main() exiting 0 on NotPE;
• the loop refreshing on exit 1 only;
• the CodeView blank replaced by pass, or moved to raw.

1. A change of the headers only. Your three flips (NX_COMPAT, a byte of the DOS stub, the flags of the first section) now follow the .text flip, each expected to exit 1.

2. Exit 2. expect 2 on the file cut to 64 bytes comes right after the synthetic cases. The loop case got a fourth launcher, unreadable.exe, instead of a second built/lib pair: its committed file is the cut one, and the loop must refresh it. I left the SizeOfHeaders guard unpinned, as you suggested. It errs the same way as every other parse failure: towards a refresh.

3. CodeView. I kept the blank and added your fixture: the POGO entry retyped as CodeView in two copies that differ only in GUID and age, expected to exit 0. The POGO data is 732 bytes, so the 24 bytes of RSDS, GUID and age fit. I did not remove the code. If /DEBUG is ever added, a PDB GUID linked under /Brepro depends on the same inputs as the REPRO hash, and without the blank the launchers would flip back and forth again, with every check green.

4. The exit-0 sentence of the docstring now leaves out the header padding as well as the build stamp, and the list names e_lfanew. The PR description says the same.

The test is now 20/20 at the head. Every mutant below makes it fail, the eight above and the nine from round 1:

• return b'';
• no REPRO blank;
• CheckSum blanked at opt + 60;
• no Rich header blank;
• no COFF timestamp blank;
• no debug directory timestamp blank;
• return bytes(b);
• headers only;
• the loop condition inverted.

The Verification section of the description lists the new cases and mutants.

[maximthomas]

praise: Round 3 takes every round-2 point, and you checked each new case against the mutant it is meant to kill.

• test-refresh-launchers.sh:62-77 flips NX_COMPAT, a DOS-stub byte and a section flag, so a change confined to the headers now fails the check. You ran all eight round-2 mutants against the old test first.
• test-refresh-launchers.sh:102-115 gives both scripts a truncated image: same-pe-code.py exits 2, and the loop refreshes over it.
• same-pe-code.py:21-23 and :38-40 now say that the header padding and e_lfanew are left out of the comparison.