Skip to content

fix(dev): mount build.rs into cryptify-fileshare so PG_CORE_VERSION compiles#248

Closed
rubenhensen wants to merge 1 commit into
mainfrom
fix/cryptify-dev-build-rs
Closed

fix(dev): mount build.rs into cryptify-fileshare so PG_CORE_VERSION compiles#248
rubenhensen wants to merge 1 commit into
mainfrom
fix/cryptify-dev-build-rs

Conversation

@rubenhensen

Copy link
Copy Markdown
Contributor

Summary

  • cryptify/build.rs reads Cargo.lock to extract the pg-core version and exposes it as PG_CORE_VERSION, which src/email.rs:37 consumes via env!() to stamp the X-PostGuard mail header that the Outlook add-in's OnMessageRead filter relies on.
  • The cryptify-fileshare dev service only mounted src/ and templates/, so cargo watch -x run saw a package with no build script and failed with environment variable PG_CORE_VERSION not defined at compile time.
  • Mount build.rs, Cargo.toml, and Cargo.lock read-only so the build script runs at container startup, and a future pg-core bump in Cargo.lock retriggers the rebuild via cargo:rerun-if-changed=Cargo.lock.

Test plan

  • docker compose up -d --build cryptify-fileshare
  • docker compose logs -f cryptify-fileshare — confirm cargo builds without the PG_CORE_VERSION error and the service comes up healthy
  • Send a test mail via the dev stack and confirm the X-PostGuard header carries the pg-core version from Cargo.lock

cryptify/build.rs reads Cargo.lock to extract the pg-core version and
sets PG_CORE_VERSION, which src/email.rs consumes via env!() to stamp
the X-PostGuard mail header. The dev container only mounted src/ and
templates/, so cargo-watch saw a package with no build script and
compilation failed with "environment variable PG_CORE_VERSION not
defined at compile time".

Mount build.rs, Cargo.toml, and Cargo.lock read-only so the build
script runs and a future pg-core bump in Cargo.lock retriggers the
rebuild via cargo:rerun-if-changed=Cargo.lock.
@dobby-coder

dobby-coder Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Dobby has received the request! Routing to the right specialist now...

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review

PR is mostly redundant — the build.rs and Cargo.lock mounts already landed on main via #246 (d52ee5b). mergeable=CONFLICTING. Only the Cargo.toml mount is genuinely new. Rebase, drop the duplicates, and update title/body to reflect that only the Cargo.toml sync remains.

Rule compliance

Two rule issues: the PR body over-justifies what reduces to a one-line change post-rebase (no-justification-paragraphs-for-simple-changes), and the conflicting state needs the git diff origin/main --stat no-op check after resolution (conflict-resolution-check-noop).

Comment thread docker-compose.yml
- ./cryptify/templates:/app/templates
- ./cryptify/build.rs:/app/build.rs:ro
- ./cryptify/Cargo.toml:/app/Cargo.toml:ro
- ./cryptify/Cargo.lock:/app/Cargo.lock:ro

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Code review] build.rs (line 21) and Cargo.lock (line 23) mounts duplicate work already on main via #246 (d52ee5b, 2026-06-03, which also bumped cryptify to 63066a1). Only the Cargo.toml mount on line 22 is genuinely new. Rebase onto main and drop these two lines.

Comment thread docker-compose.yml
- ./cryptify/conf/config.dev.toml:/app/config.toml:ro
- ./cryptify/src:/app/src
- ./cryptify/templates:/app/templates
- ./cryptify/build.rs:/app/build.rs:ro

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Code review] PR branch's cryptify submodule pointer is 4c30e53, which predates the addition of build.rs upstream (cryptify#170, present from 7816df8). A fresh gh pr checkout 248 && git submodule update --init leaves ./cryptify/build.rs missing, so the bind mount has no source file and Docker will create an empty path or refuse to start — the stated test plan can't be executed against the branch as-shipped. Bump the submodule pointer or rebase onto main (already at 63066a1).

Comment thread docker-compose.yml
- ./cryptify/src:/app/src
- ./cryptify/templates:/app/templates
- ./cryptify/build.rs:/app/build.rs:ro
- ./cryptify/Cargo.toml:/app/Cargo.toml:ro

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Rule: no-justification-paragraphs-for-simple-changes] After rebasing onto main (which already has the build.rs and Cargo.lock mounts), this PR collapses to a single added line. The three-bullet rationale in the PR body is more explanation than the change warrants — trim to one or two bullets and let the diff speak.

@rubenhensen rubenhensen closed this Jun 4, 2026
@rubenhensen rubenhensen deleted the fix/cryptify-dev-build-rs branch June 4, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant