Guard test and docs subdirectories with EXISTS checks #3200

2026-09-17T15:02:44-05:00

ilg-ul commented

2026-09-17 15:02:44 -05:00

(Migrated from github.com)

Allows the top-level CMakeLists.txt to configure successfully when the test or docs directories are not present, such as in a partial or stripped-down SDK checkout.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-09-17 15:05:10 -05:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

🟡 Changes recommended

The current EXISTS checks only verify directories, which can still allow add_subdirectory() to fail if the directories exist but are incomplete (missing CMakeLists.txt).

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR updates the top-level SDK CMake configuration to avoid hard failures when optional test/ and docs/ subdirectories are missing, supporting partial/stripped-down SDK checkouts.

Changes:

Guard add_subdirectory(test) behind PICO_SDK_TESTS_ENABLED plus an existence check.
Guard add_subdirectory(docs) behind an existence check so configuration succeeds without docs.

File summaries

File	Description
CMakeLists.txt	Adds conditional `EXISTS` guards around adding `test/` and `docs/` subdirectories to allow configuring without those directories.

Review details

Suppressed comments (1)

CMakeLists.txt:65

Similar to the test guard, checking only for the docs directory may still allow add_subdirectory(docs) to fail if the directory exists but is incomplete. Consider checking for docs/CMakeLists.txt instead to fully meet the goal of configuring successfully in stripped-down checkouts.

    if (EXISTS "${CMAKE_CURRENT_LIST_DIR}/docs")

Files reviewed: 1/1 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

### 🟡 Changes recommended The current `EXISTS` checks only verify directories, which can still allow `add_subdirectory()` to fail if the directories exist but are incomplete (missing `CMakeLists.txt`). *Get a fresh assessment by requesting another Copilot review.* <details> <summary>Pull request overview</summary> This PR updates the top-level SDK CMake configuration to avoid hard failures when optional `test/` and `docs/` subdirectories are missing, supporting partial/stripped-down SDK checkouts. **Changes:** - Guard `add_subdirectory(test)` behind `PICO_SDK_TESTS_ENABLED` plus an existence check. - Guard `add_subdirectory(docs)` behind an existence check so configuration succeeds without docs. </details> <details> <summary>File summaries</summary> | File | Description | | ---- | ----------- | | CMakeLists.txt | Adds conditional `EXISTS` guards around adding `test/` and `docs/` subdirectories to allow configuring without those directories. | </details> <details> <summary>Review details</summary> ### Suppressed comments (1) **CMakeLists.txt:65** * Similar to the `test` guard, checking only for the `docs` directory may still allow `add_subdirectory(docs)` to fail if the directory exists but is incomplete. Consider checking for `docs/CMakeLists.txt` instead to fully meet the goal of configuring successfully in stripped-down checkouts. ``` if (EXISTS "${CMAKE_CURRENT_LIST_DIR}/docs") ``` - **Files reviewed:** 1/1 changed files - **Comments generated:** 1 - **Review effort level:** Lite </details> --- 💡 <a href="/raspberrypi/pico-sdk/new/develop?filename=.github/skills/code-review/SKILL.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add a `code-review` agent skill</a> or configure MCP servers for context-aware, tailored reviews. <a href="https://docs.github.com/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review?tool=webui#mcp-servers-and-agent-skills" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn more in the docs.</a>

CMakeLists.txt

					
				@ -58,3 +58,3 @@

				    endif()

				    if (PICO_SDK_TESTS_ENABLED)

				    if (PICO_SDK_TESTS_ENABLED AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/test")

				        add_subdirectory(test)

The EXISTS guard checks only for the presence of the test directory, but add_subdirectory(test) will still fail if the directory exists without a CMakeLists.txt (e.g., an incomplete/stripped checkout). Checking for test/CMakeLists.txt makes the guard reliably prevent configure-time errors.

This issue also appears on line 65 of the same file.

The EXISTS guard checks only for the presence of the `test` directory, but `add_subdirectory(test)` will still fail if the directory exists without a `CMakeLists.txt` (e.g., an incomplete/stripped checkout). Checking for `test/CMakeLists.txt` makes the guard reliably prevent configure-time errors. This issue also appears on line 65 of the same file.

ilg-ul (Migrated from github.com) reviewed

2026-09-17 15:13:30 -05:00

CMakeLists.txt

					
				@ -58,3 +58,3 @@

				    endif()

				    if (PICO_SDK_TESTS_ENABLED)

				    if (PICO_SDK_TESTS_ENABLED AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/test")

				        add_subdirectory(test)

Although in theory this is a possibilty, in practice, when the SDK is stripped down to be published in lighter packages (for example @xpack-3rd-party/raspberrypi-pico-sdk), the entire folders are removed, not only the content.

But if you think that this is a valid point, I can update the PR.

Although in theory this is a possibilty, in practice, when the SDK is stripped down to be published in lighter packages (for example [@xpack-3rd-party/raspberrypi-pico-sdk](https://www.npmjs.com/package/@xpack-3rd-party/raspberrypi-pico-sdk)), the entire folders are removed, not only the content. But if you think that this is a valid point, I can update the PR.

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'micro-os-plus/develop:micro-os-plus/develop'

git switch 'micro-os-plus/develop'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'develop'

git merge --no-ff 'micro-os-plus/develop'

git switch 'micro-os-plus/develop'

git rebase 'develop'

git switch 'develop'

git merge --ff-only 'micro-os-plus/develop'

git switch 'micro-os-plus/develop'

git rebase 'develop'

git switch 'develop'

git merge --no-ff 'micro-os-plus/develop'

git switch 'develop'

git merge --squash 'micro-os-plus/develop'