Skip to content
Merged
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
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- **BREAKING -- the exit-code contract now matches partspec and netspec**
([A3]). `0` no differences, `1` differences (with `--fail-on-diff`), `2` the
comparison could not be completed, `4` an input could not be read or parsed,
`64` usage (`EX_USAGE`). Previously `2` meant a parse error and `1` doubled as
both "differences found" and "could not write the output file". A gate written
`[ $? -eq 0 ]` is unaffected; one that treated `2` as a parse failure must now
read `4`.
- **Geometry report schema `version` 2 -> 3**, adding `summary.outcome`,
`summary.unrepresented` and a per-layer `unrepresented`. Additive: every v2
field keeps its meaning.

### Added

- **`geomdiff` has a third outcome, and says what it could not model** ([A3]) --
`identical` | `different` | `indeterminate`. A stroke drawn with a macro or
block aperture is not modelled by the geometry engine, and used to vanish: a
board with an added trace reported `0 changes` at exit `0`, with JSON
byte-identical to comparing a board against a copy of itself, while the raster
engine reported the change. The two engines answered the same question
differently and the geometry one gave the dangerous answer. Such operations are
now counted in `summary.unrepresented`, named on stderr, and make the outcome
`indeterminate` at exit `2` -- which does not wait for `--fail-on-diff`, since
that flag chooses whether a *difference* fails, not whether the tool could look.
`different` outranks `indeterminate`, so one unmodellable stroke cannot mask a
trace that moved.

### Fixed

- **Draw operations using an undefined aperture are no longer dropped in
Expand Down Expand Up @@ -686,6 +714,7 @@ merge_tolerance) -> SingleLayerDiff`.
- 2 smoke tests in `tests/test_scaffold.py`.

[#17]: https://github.com/heibench/gerberdiff/issues/17
[A3]: https://heibench.com/adjudications.html
[Unreleased]: https://github.com/heibench/gerberdiff/compare/v0.29.1...HEAD
[0.29.1]: https://github.com/heibench/gerberdiff/compare/v0.29.0...v0.29.1
[0.29.0]: https://github.com/heibench/gerberdiff/compare/9ffd4c8f...v0.29.0
Expand Down
17 changes: 17 additions & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
# CLI reference


## Exit codes

Shared across the org's tools, so a script can branch on them the same way for
`gerberdiff`, `partspec` and `netspec`.

| code | meaning |
| ---- | -------- |
| `0` | no differences, and everything was modelled |
| `1` | differences found (with `--fail-on-diff`) |
| `2` | part of the comparison could not be made -- **not** a statement about the boards |
| `4` | an input could not be read or parsed -- also not a statement about the boards |
| `64` | usage (`EX_USAGE`): bad arguments, or an output file that exists without `--overwrite` |

`2` does not wait for `--fail-on-diff`. That flag chooses whether a *difference* is a
failure; it has no bearing on whether the tool could look.

## `parse` -- inspect a single file

```sh
Expand Down
8 changes: 7 additions & 1 deletion docs/geometry-diff.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,14 @@ documented so the two engines' outputs are interpreted correctly:

## Known limitations

- Strokes drawn with macro or block apertures are skipped (the raster
- Strokes drawn with macro or block apertures are not modelled (the raster
engine draws them as hairlines; neither engine models them properly).
**They are reported, not skipped in silence.** Each one is counted in
`summary.unrepresented`, makes the layer `indeterminate`, and gives the
comparison an `outcome` of `"indeterminate"` at exit `2`. This limitation was
documented here long before the output admitted to it: a board with such a
stroke reported `0 changes` at exit `0`, with JSON byte-identical to comparing
a board against a copy of itself, while the raster engine reported the change.
- `rerouted` classification is deferred: a redrawn trace reports as
removed + added segments (or `resized` when endpoints stay close).
- Geometry changes are op-granular: a moved multi-op footprint reports
Expand Down
21 changes: 17 additions & 4 deletions docs/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ All coordinate values are in **inches**.

---

## Schema version 2 (geometry diff)
## Schema version 3 (geometry diff)

Coordinates (`centroid_x`, `centroid_y`) are in **inches** (matching v1);
areas are in **mm^2** and displacements in **mm**.
Expand All @@ -107,7 +107,7 @@ areas are in **mm^2** and displacements in **mm**.

| Field | Type | Description |
| ------------ | ---------------------------- | -------------------------------------------------- |
| `version` | `integer` | Always `2`. |
| `version` | `integer` | Always `3`. |
| `generator` | `string` | Always `"gerberdiff"`. |
| `mode` | `string` | Always `"geometry"`. |
| `summary` | `object` | Aggregate counts (see below). |
Expand All @@ -121,6 +121,13 @@ areas are in **mm^2** and displacements in **mm**.
| `changed_layers` | `integer` | Layers with any change, non-matched status, or area. |
| `total_changes` | `integer` | Total attributed change records across all layers. |
| `has_changes` | `boolean` | `true` when anything differs. |
| `outcome` | `string` | `"identical"` \| `"different"` \| `"indeterminate"`. **Branch on this, not on `has_changes`.** |
| `unrepresented` | `object` | Reason -> count of operations the engine could not model, summed across layers. Empty when the comparison was complete. |

`has_changes` cannot express "could not tell": it is `false` both for two identical
boards and for a board carrying geometry the engine does not model. `outcome` is
`"indeterminate"` in the second case, and the CLI exits `2`. `"different"` outranks
`"indeterminate"`, so a change that *was* found is still reported as one.

### `tolerances` object

Expand Down Expand Up @@ -161,10 +168,16 @@ areas are in **mm^2** and displacements in **mm**.

```json
{
"version": 2,
"version": 3,
"generator": "gerberdiff",
"mode": "geometry",
"summary": { "changed_layers": 1, "total_changes": 2, "has_changes": true },
"summary": {
"changed_layers": 1,
"total_changes": 2,
"has_changes": true,
"outcome": "different",
"unrepresented": {}
},
"tolerances": {
"move_tol_mm": 0.005,
"gate_radius_mm": 0.2,
Expand Down
71 changes: 53 additions & 18 deletions gerberdiff/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@
from gerberdiff.diff.layer_matcher import EXCELLON_SUFFIXES
from gerberdiff.types import Diagnostic, DiagnosticSeverity, LayerStatus

# The exit-code contract, aligned with partspec and netspec across the org (A1/A3).
# 2 is where "could not tell" belongs; a differ needs it as much as a checker does,
# because what the engine could not model must never be reported as no difference.
EXIT_OK = 0
"""No differences, and everything was modelled."""
EXIT_DIFFERENT = 1
"""Differences found (with --fail-on-diff)."""
EXIT_INDETERMINATE = 2
"""Part of the comparison could not be made. Not a statement about the boards."""
EXIT_ERROR = 4
"""Could not read or parse an input. Not a statement about the boards either."""
EXIT_USAGE = 64
"""EX_USAGE: bad arguments, or an output file that exists without --overwrite."""

_MEMORY_WARN_PIXELS = 16_777_216 # 4096^2


Expand Down Expand Up @@ -39,7 +53,7 @@ def parse_cmd(file: Path, dump_ir: bool, quiet: bool, verbose: bool) -> None:
content = file.read_text(errors="replace")
except OSError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(1)
sys.exit(EXIT_ERROR)

if file.suffix.lower() in EXCELLON_SUFFIXES:
img = parse_excellon(content, source_path=file)
Expand Down Expand Up @@ -89,7 +103,7 @@ def parse_cmd(file: Path, dump_ir: bool, quiet: bool, verbose: bool) -> None:
}
click.echo(json.dumps(ir, indent=2))

sys.exit(2 if has_errors else 0)
sys.exit(EXIT_ERROR if has_errors else EXIT_OK)


@cli.command("render")
Expand Down Expand Up @@ -135,13 +149,13 @@ def render_cmd(
f"error: output file already exists: {out_png} (use --overwrite to replace)",
err=True,
)
sys.exit(1)
sys.exit(EXIT_USAGE)

try:
content = file.read_text(errors="replace")
except OSError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(1)
sys.exit(EXIT_ERROR)

if file.suffix.lower() in EXCELLON_SUFFIXES:
img = parse_excellon(content, source_path=file)
Expand All @@ -160,7 +174,7 @@ def render_cmd(
click.echo(f"info: {diag.message}", err=True)

if has_errors:
sys.exit(2)
sys.exit(EXIT_ERROR)

vp = compute_viewport(img.bounding_box, width, height)

Expand All @@ -173,7 +187,7 @@ def render_cmd(
surface.write_to_png(str(out_png))
except OSError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(1)
sys.exit(EXIT_ERROR)

if not quiet:
click.echo(f"rendered {width}x{height} -> {out_png}")
Expand Down Expand Up @@ -270,7 +284,7 @@ def diff_cmd(
"error: --align-offset must be two comma-separated floats (e.g. '0.5,0')",
err=True,
)
sys.exit(2)
sys.exit(EXIT_USAGE)

# Memory warning
total_pixels = width * height
Expand Down Expand Up @@ -323,13 +337,13 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
)
except GerberParseError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(2)
sys.exit(EXIT_ERROR)
except FileExistsError as exc:
click.echo(f"error: {exc} (use --overwrite to replace)", err=True)
sys.exit(1)
sys.exit(EXIT_USAGE)
except OSError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(1)
sys.exit(EXIT_ERROR)

elapsed_total = time.perf_counter() - t_start

Expand All @@ -351,7 +365,7 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
write_report(diff_result, out_json, overwrite=overwrite)
except FileExistsError as exc:
click.echo(f"error: {exc} (use --overwrite to replace)", err=True)
sys.exit(1)
sys.exit(EXIT_USAGE)

elapsed_ms = f"({elapsed_total * 1000:.0f} ms)"
if not quiet:
Expand All @@ -364,7 +378,7 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
if out_json:
click.echo(f"report: {out_json}")

sys.exit(1 if fail_on_diff and diff_result.has_changes else 0)
sys.exit(EXIT_DIFFERENT if fail_on_diff and diff_result.has_changes else EXIT_OK)


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -445,6 +459,7 @@ def geomdiff_cmd(
from gerberdiff.export.json_report import write_geometry_report
from gerberdiff.export.svg_export import write_geometry_svg
from gerberdiff.geometry import compute_geometry_diff
from gerberdiff.geometry.types import DiffOutcome
from gerberdiff.types import GerberParseError

def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
Expand All @@ -468,10 +483,10 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
)
except GerberParseError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(2)
sys.exit(EXIT_ERROR)
except OSError as exc:
click.echo(f"error: {exc}", err=True)
sys.exit(1)
sys.exit(EXIT_ERROR)
elapsed_total = time.perf_counter() - t_start

if verbose:
Expand Down Expand Up @@ -501,7 +516,7 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
write_geometry_report(result, out_json, tolerances=tolerances, overwrite=overwrite)
except FileExistsError as exc:
click.echo(f"error: {exc} (use --overwrite to replace)", err=True)
sys.exit(1)
sys.exit(EXIT_USAGE)

if out_svg_dir is not None:
try:
Expand All @@ -513,19 +528,39 @@ def _on_diagnostic(path: Path, diag: Diagnostic) -> None:
)
except FileExistsError as exc:
click.echo(f"error: {exc} (use --overwrite to replace)", err=True)
sys.exit(1)
sys.exit(EXIT_USAGE)

# What the engine could not model goes to stderr even under --quiet: it is the
# difference between "no changes" and "no changes that I could see", and suppressing
# it is the failure this reports on.
unrepresented = result.unrepresented
if unrepresented:
from gerberdiff.geometry.layer_geometry import UNREPRESENTED_REASONS

click.echo(
"warning: this comparison is incomplete; "
f"{sum(unrepresented.values())} operation(s) could not be modelled:",
err=True,
)
for reason, count in sorted(unrepresented.items()):
click.echo(f" {count} x {UNREPRESENTED_REASONS[reason]}", err=True)

if not quiet:
changed_layers = sum(1 for layer_diff in result.layers if layer_diff.has_changes)
total_changes = sum(len(layer_diff.changes) for layer_diff in result.layers)
click.echo(
f"geomdiff: {changed_layers}/{len(result.layers)} layers changed, "
f"geomdiff: {result.outcome}, "
f"{changed_layers}/{len(result.layers)} layers changed, "
f"{total_changes} changes ({elapsed_total * 1000:.0f} ms)"
)
if out_json:
click.echo(f"report: {out_json}")

sys.exit(1 if fail_on_diff and result.has_changes else 0)
# Indeterminate does not wait for --fail-on-diff. That flag chooses whether a
# *difference* is a failure; it has no bearing on whether the tool could look.
if result.outcome == DiffOutcome.Indeterminate:
sys.exit(EXIT_INDETERMINATE)
sys.exit(EXIT_DIFFERENT if fail_on_diff and result.has_changes else EXIT_OK)


if __name__ == "__main__":
Expand Down
8 changes: 7 additions & 1 deletion gerberdiff/export/json_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from gerberdiff.types import DiffResult, LayerDiffResult, LayerStatus, Region

_SCHEMA_VERSION = 1
_GEOMETRY_SCHEMA_VERSION = 2
_GEOMETRY_SCHEMA_VERSION = 3
_GENERATOR = "gerberdiff"


Expand Down Expand Up @@ -92,6 +92,11 @@ def build_geometry_report(
"changed_layers": sum(1 for layer in result.layers if layer.has_changes),
"total_changes": sum(len(layer.changes) for layer in result.layers),
"has_changes": result.has_changes,
# has_changes cannot carry "could not tell": it was False both for two
# identical boards and for a board whose geometry the engine could not
# model. Branch on outcome, not on has_changes.
"outcome": str(result.outcome),
"unrepresented": result.unrepresented,
},
"layers": [_serialize_geometry_layer(layer) for layer in result.layers],
}
Expand Down Expand Up @@ -140,6 +145,7 @@ def _serialize_geometry_layer(layer: LayerGeometryDiff) -> dict[str, Any]:
"resized": layer.count("resized"),
},
"changes": [_serialize_geometry_change(c) for c in layer.changes],
"unrepresented": layer.unrepresented,
}


Expand Down
8 changes: 8 additions & 0 deletions gerberdiff/geometry/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def _build(path: Path) -> LayerGeometry:
status=pair.status,
added_area_mm2=total_mm2 if pair.status == LayerStatus.Added else 0.0,
removed_area_mm2=total_mm2 if pair.status == LayerStatus.Removed else 0.0,
unrepresented=dict(geometry.unrepresented),
)
)
continue
Expand Down Expand Up @@ -195,6 +196,12 @@ def _diff_layer_pair(
# raster engine's region ordering.
changes.sort(key=lambda c: (-c.centroid_y, c.centroid_x))

# Merged across both revisions: an operation the engine could not model on either
# side leaves the comparison unable to say the pair is identical.
unrepresented = dict(geom_a.unrepresented)
for reason, n in geom_b.unrepresented.items():
unrepresented[reason] = unrepresented.get(reason, 0) + n

return LayerGeometryDiff(
name=name,
layer_type=layer_type,
Expand All @@ -203,6 +210,7 @@ def _diff_layer_pair(
unchanged_count=unchanged_count,
added_area_mm2=added_geom.area * MM2_PER_IN2,
removed_area_mm2=removed_geom.area * MM2_PER_IN2,
unrepresented=unrepresented,
)


Expand Down
Loading