Skip to content

Add offline Samba4X cross-answer configure mode#118

Merged
jamesyc merged 4 commits into
mainfrom
offline-builds
May 18, 2026
Merged

Add offline Samba4X cross-answer configure mode#118
jamesyc merged 4 commits into
mainfrom
offline-builds

Conversation

@jamesyc
Copy link
Copy Markdown
Owner

@jamesyc jamesyc commented May 18, 2026

Add offline Samba4X cross-answer configure mode

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a cross-answer file mechanism for Samba 4.x builds, enabling offline configuration by providing pre-recorded responses to Waf probes. It includes new answer files for NetBSD 4 and 7, updates the build script to manage these files, and adds a comprehensive test suite. Feedback suggests defining SAMBA4X_VERSION in the example environment file and improving the robustness of path calculations for the default answers file by avoiding reliance on an undefined SCRIPT_DIR variable.

Comment thread build/.env.example
Comment thread build/_samba4x.sh Outdated
@jamesyc
Copy link
Copy Markdown
Owner Author

jamesyc commented May 18, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for offline Samba 4.x builds by utilizing Waf cross-answer files, which eliminates the need for a live SSH connection to a Time Capsule during the configuration phase. The changes include new environment variables in .env.example, the addition of pre-computed answer files for NetBSD 4 (BE/LE) and NetBSD 7, and updates to the _samba4x.sh build script to manage these files. Additionally, a comprehensive Python test suite has been added to verify the build script logic. Feedback provided suggests improving error handling in the script by explicitly checking the return code of the lane selection function to ensure descriptive failures.

Comment thread build/_samba4x.sh Outdated
}

prepare_samba4x_cross_answers() {
lane="$(samba4x_cross_answer_lane)"
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.

medium

The return code of samba4x_cross_answer_lane is not checked. If it fails (e.g., due to an unsupported configuration), the script will continue with an empty lane variable, which leads to a less descriptive "Missing Samba 4.x cross-answers file" error later in preflight_samba4x_cross_answers. Explicitly checking the return status ensures the script exits immediately with the specific error message already printed to stderr by the function.

Suggested change
lane="$(samba4x_cross_answer_lane)"
if ! lane="$(samba4x_cross_answer_lane)"; then
exit 1
fi

@jamesyc
Copy link
Copy Markdown
Owner Author

jamesyc commented May 18, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a cross-answer mechanism for Samba 4.x builds, enabling offline configuration for NetBSD targets without requiring a live device via SSH. The changes include new build scripts, pre-generated answer files for NetBSD 4 and 7, and a suite of unit tests for the build logic. Review feedback suggests refining the Python-based duplicate answer detection in build/_samba4x.sh to provide clearer conflict reports by aggregating unique values per key.

Comment thread build/_samba4x.sh
@jamesyc jamesyc merged commit bfd8806 into main May 18, 2026
12 checks passed
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