Skip to content

manifest digest: hash the AST of dependencies.rb, not its bytes — comments and formatting must not invalidate the lock #178

Description

@JPDuchesne

Problem

update_deps_command.rb:60 stamps Digest::SHA256.file("dependencies.rb") into the lockfiles, and Staleness#manifest_message (staleness.rb:87) compares against the same byte hash. So any edit to the file — a comment, a blank line, a re-wrap — reads as "dependencies.rb changed since the lockfiles were generated — run dev update-deps" and fails dev check / dev style in CI.

Seen live on d3mlabs/cellbound-3d#172: a comment-only edit to dependencies.rb (retiring a mention of the current symlink) failed the dev style gate. The fix was to revert the comment, because the alternative — re-running update-deps — re-resolves open constraints (pip >= pins) and churns the lock for an unrelated reason. Neither outcome is what the check is for: the check exists to catch declarations drifting from the lock.

Proposal

Hash a canonical, location-free serialization of the parsed AST instead of the bytes. Comments live outside the AST (Prism::ParseResult#comments), so they vanish for free; whitespace and line breaks vanish with them. Anything that changes the declared dependencies still changes the digest.

Design notes

  • Parser: Prism is a default gem since Ruby 3.3 and dev's toolchain is ruby "4.0.6" — zero new runtime dependency. parser/ast_transform only reach dev through rspock in the test group; the Homebrew formula (dev-core.rb) vendors neither, so using them at runtime means shipping them. Prefer Prism unless the AST shape from ast_transform (rspockframework/ast-transform) buys something concrete.
  • Serialization: Prism::Node#inspect embeds locations, so it can't be hashed directly. Walk the tree emitting (node type, non-location fields) in a deterministic order — or hash the token stream from Prism.lex minus comment/whitespace/newline tokens, which is smaller but weaker (two formattings of the same expression could tokenize differently across \ line continuations).
  • Migration: existing lockfiles carry a byte digest. On the first update-deps after this ships the new digest is stamped; until then the old digest would mismatch every manifest. Options: (a) accept one dev update-deps per repo and say so in the release notes; (b) prefix the digest with a scheme tag (ast: / bare = bytes) and keep verifying legacy digests bytewise until they're rewritten. (b) mirrors how Staleness already stays quiet for lockfiles predating the digest.
  • Do not hash eval'd results (the resolved Dev::Deps.define block): declarations may be conditional on host/env, and the digest must be reproducible on every machine.

Done when

  • A comment-only or whitespace-only edit to dependencies.rb leaves dev check green against an unchanged lock.
  • Adding/removing/changing any declaration still fails it.
  • Tests for both, using real dependencies.rb fixtures in a tmpdir (no filesystem mocks).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions