Skip to content

fix(docker): copy stb submodule into dev image to fix Python bindings#341

Closed
Geff115 wants to merge 1 commit into
Ryan-Millard:mainfrom
Geff115:fix/docker-container-build-dependencies-fix
Closed

fix(docker): copy stb submodule into dev image to fix Python bindings#341
Geff115 wants to merge 1 commit into
Ryan-Millard:mainfrom
Geff115:fix/docker-container-build-dependencies-fix

Conversation

@Geff115
Copy link
Copy Markdown
Contributor

@Geff115 Geff115 commented May 2, 2026

Summary

The dev Docker image was missing the stb submodule, causing the Python
bindings build to fail with:

fatal error: stb/stb_image.h: No such file or directory

This meant that running uv sync --no-build-isolation inside the container
would always error out, blocking any contributor from setting up the Python
bindings locally.

Root Cause

Dockerfile.dev copied third_party/dawn into the image, but never copied
third_party/stb. Since the Python bindings built via CMake depend on STB
for image loading, the build failed at compile time.

Note: Running git submodule update --init --recursive is not a viable
workaround because Dawn's nested sub-submodules include a Google-internal
repository (chrome-internal.googlesource.com) that requires special
credentials inaccessible to external contributors.

Fix

Added COPY third_party/stb third_party/stb to Dockerfile.dev, alongside
the existing Dawn copy. Contributors must have the stb submodule initialized
on their host machine before building the image:

git submodule update --init third_party/stb

Testing

Verified the full setup sequence inside the container succeeds:

uv venv
uv pip install scikit_build_core numpy
uv sync --no-build-isolation
uv run python3 -c "import img2num;"

All steps completed without errors.

Related

Raised by @Ryan-Millard in issue #332 thread. This should close the #340 issue created by @coderabbitai in the thread.

@Geff115 Geff115 requested a review from Ryan-Millard as a code owner May 2, 2026 13:11
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a15922c1-60c3-4a19-b5dd-672724986f95

📥 Commits

Reviewing files that changed from the base of the PR and between fce52c6 and a37409e.

📒 Files selected for processing (1)
  • Dockerfile.dev
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
Dockerfile*

⚙️ CodeRabbit configuration file

Dockerfile*: Review with Hadolint rules. Prefer multi-stage builds, minimal base images,
and pinned image tags. Ensure no secrets are baked into layers.

Files:

  • Dockerfile.dev
🔇 Additional comments (1)
Dockerfile.dev (1)

62-62: Good fix for missing bindings dependency.

Adding COPY third_party/stb third_party/stb correctly makes stb headers available in the dev image and directly addresses the build failure root cause.


Release Notes

  • Fixed Docker dev image build failures: Added third_party/stb submodule to Dockerfile.dev to enable successful Python bindings compilation. The stb header files (stb_image.h) are required by the CMake build process for the Python bindings and were previously missing from the container.

Contribution Summary

Author File(s) Lines Added Lines Removed
Geff115 Dockerfile.dev 1 0

Walkthrough

The Docker development image is updated to include the third_party/stb directory during the container build, alongside the existing third_party/dawn directory. This ensures the stb third-party library sources are available in the build context.

Changes

Docker Build Context

Layer / File(s) Summary
Build Artifact Inclusion
Dockerfile.dev
Adds COPY third_party/stb third_party/stb to include stb headers in the development image alongside third_party/dawn.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested labels

build-system, python, c/cpp

Suggested reviewers

  • Ryan-Millard
  • Krasner

Poem

🐰 With stb now bundled tight,
The Docker build shines bright!
Headers packed, containers blessed,
One simple line—build context blessed! ✨

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Coderabbit Config Needs Update ❓ Inconclusive Unable to execute git commands to verify PR changes without access to the repository environment. Please run the git diff commands in your local repository to verify the actual changes in this PR.
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific and descriptive, following Conventional Commit format (fix:) and clearly describes the actual change: copying the stb submodule into the Docker dev image to fix Python bindings.
Description check ✅ Passed The description is detailed and comprehensive, explaining the root cause, the implemented fix, testing verification, and related issues—all directly relevant to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Ai Slop Pr Description ✅ Passed PR description provides specific technical details about the problem, root cause, exact fix, and testing procedures rather than generic boilerplate.
No Strangely-Named Root Markdown Files ✅ Passed The pull request does not add any unconventional markdown files to the repository root. All markdown files present are standard and explicitly listed as acceptable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added c/cpp Changes to C or C++ files python changes to python bindings or python code labels May 2, 2026
@Ryan-Millard
Copy link
Copy Markdown
Owner

Hi.

Please will you explain more about the problem you faced here.

STB was intentionally not included in the image because of the type of library it is (a header-only C++ library). Dawn, however, was included and built into the image because it takes about 30-45 minutes on a normal computer (the image and repository take long enough to get locally).

If you weren't able to use STB, I'd like to first verify that it wasn't an accident that occurred when setting up your environment (maybe due to outdated documentation). The Docker container mounts to the project’s directory, so it should've just worked without any problems
problems. My suspicion is a submodule-related problem because they behave weirdly sometimes - when you pulled, did you ensure that it recursed the submodules?

@Geff115
Copy link
Copy Markdown
Contributor Author

Geff115 commented May 2, 2026

Hi @Ryan-Millard, thank you for the clarification!

You're right! It was a submodule initialization issue. When I cloned the repo, I didn't initialize the submodules, so third_party/stb was an empty directory on my host machine. Since the container mounts the project directory, it saw the empty folder too, which caused the missing stb_image.h error during the build.
Running git submodule update --init third_party/stb on the host resolved it immediately.

So rather than a Dockerfile change, the fix should be a documentation update, adding a note to the setup/contributing docs to remind contributors to initialize submodules after cloning. Would you like me to update the PR to reflect that instead?

@Ryan-Millard
Copy link
Copy Markdown
Owner

Hi @Ryan-Millard, thank you for the clarification!

You're right! It was a submodule initialization issue. When I cloned the repo, I didn't initialize the submodules, so third_party/stb was an empty directory on my host machine. Since the container mounts the project directory, it saw the empty folder too, which caused the missing stb_image.h error during the build. Running git submodule update --init third_party/stb on the host resolved it immediately.

So rather than a Dockerfile change, the fix should be a documentation update, adding a note to the setup/contributing docs to remind contributors to initialize submodules after cloning. Would you like me to update the PR to reflect that instead?

Sure, I'd love that. Thank you so much.

I'd prefer it if you'd rather open up another PR, though. Closing this one adds traceability and makes things clean and easy to look back at in the future (for example, cross-referencing this PR in a future conversation).

I'm going to close this PR since it doesn't fix a problem in the repository. Please will you reference it when you make the new one.

Have a great evening further!🦔🦔🦔

@Ryan-Millard
Copy link
Copy Markdown
Owner

@coderabbitai, please create an issue for this. Look at the conversation and see that the problem related to this PR was a documentation issue and that it needs to be fixed. Please make sure to link it when the new PR is created to fix the documentation on the Setup & Dependencies (docs/docs/contributing/setup-and-dependencies/index.md) page for contributors.

Additionally, I noticed that I didn't update all the admonitions with Docusaurus's latest version and it looks like they are now broken on all of the pages (specifically when the admonition has a title). Please create a separate issue for this and do not reference this PR in that issue. You may reference the PR that updates the Docusaurus version (#327) and the Setup & Dependencies page as an example of the problem (specifically the "Docker Dev Environment" admonition on line 25).

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create Issue 1: Documentation - submodule initialization missing from setup docs
gh issue create \
  --repo Ryan-Millard/Img2Num \
  --title "docs: add submodule initialization instructions to Setup & Dependencies page" \
  --assignee Ryan-Millard \
  --body "## Summary

The [Setup & Dependencies](https://ryan-millard.github.io/Img2Num/info/docs/next/contributing/setup-and-dependencies/) page (\`docs/docs/contributing/setup-and-dependencies/index.md\`) does not currently instruct contributors to initialize Git submodules after cloning the repository.

## Problem

As discovered in PR `#341` (https://github.com/Ryan-Millard/Img2Num/pull/341), contributors who clone the repository without initializing submodules end up with empty \`third_party/\` directories (e.g., \`third_party/stb\`). Because the Docker dev container mounts the project directory, it inherits the empty folder, causing build failures such as:

\`\`\`
fatal error: stb/stb_image.h: No such file or directory
\`\`\`

The fix was simply running:
\`\`\`bash
git submodule update --init third_party/stb
\`\`\`

But contributors have no way of knowing this from the current documentation.

## Requested Change

Update \`docs/docs/contributing/setup-and-dependencies/index.md\` to include a clear note or step instructing contributors to initialize the required submodules after cloning. The new PR addressing this change should reference PR `#341` (https://github.com/Ryan-Millard/Img2Num/pull/341) for traceability.

## Related

- PR `#341`: https://github.com/Ryan-Millard/Img2Num/pull/341 (original report, now closed)
- Requested by `@Ryan-Millard`"

echo "---"

# Create Issue 2: Broken Docusaurus admonitions with titles
gh issue create \
  --repo Ryan-Millard/Img2Num \
  --title "docs: fix broken admonition titles after Docusaurus version upgrade" \
  --assignee Ryan-Millard \
  --body "## Summary

Following the Docusaurus version upgrade (PR `#327`: https://github.com/Ryan-Millard/Img2Num/pull/327), admonitions that include a custom title are rendering incorrectly across the documentation site.

## Problem

The admonition syntax for titled admonitions appears to have changed in the newer version of Docusaurus. As a result, titled admonitions are broken on all affected pages.

## Example

A concrete example can be seen on the [Setup & Dependencies](https://ryan-millard.github.io/Img2Num/info/docs/next/contributing/setup-and-dependencies/) page — specifically the **\"Docker Dev Environment\"** admonition defined at [line 25 of \`docs/docs/contributing/setup-and-dependencies/index.md\`](https://github.com/Ryan-Millard/Img2Num/blob/fce52c6779013ee13717f31000b431eb67a2ec42/docs/docs/contributing/setup-and-dependencies/index.md?plain=1#L25).

## Requested Change

Audit all documentation pages for admonitions with custom titles and update their syntax to comply with the format expected by the currently installed version of Docusaurus.

## Related

- PR `#327` (Docusaurus version upgrade): https://github.com/Ryan-Millard/Img2Num/pull/327
- Requested by `@Ryan-Millard`"

Length of output: 106


@Ryan-Millard: Both issues have been created and assigned to you:

  1. #343 — docs: add submodule initialization instructions to Setup & Dependencies page

    • Documents the submodule initialization problem discovered in this PR, requests an update to docs/docs/contributing/setup-and-dependencies/index.md, and notes that the new PR fixing it should reference #341 for traceability.
  2. #344 — docs: fix broken admonition titles after Docusaurus version upgrade

    • References PR #327 (the Docusaurus version upgrade) and the "Docker Dev Environment" admonition at line 25 of the Setup & Dependencies page as a concrete example. PR #341 is not referenced in this issue.

Have a great evening! 🦔

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

Labels

build-system c/cpp Changes to C or C++ files python changes to python bindings or python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants