You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replaced legacy Ant build system with pure Gradle implementation
Added comprehensive Gradle documentation in .gradle-docs/ directory
Removed obsolete Ant configuration files and Eclipse launch configurations
Implemented modern build tasks for PHP module packaging and verification
Diagram Walkthrough
flowchart LR
A["Legacy Ant Build<br/>build.xml<br/>sigcheck.xml"] -->|Replace| B["Pure Gradle Build<br/>build.gradle<br/>gradle.properties"]
B -->|Generate| C["Build Artifacts<br/>PHP Packages<br/>Archives"]
D["Comprehensive Docs<br/>.gradle-docs/"] -->|Support| B
E["Tasks<br/>release<br/>verify<br/>clean"] -->|Execute| B
Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance.
Status: Audit logging: The new Gradle build documentation and examples add helper functions and tasks but do not demonstrate or enforce logging of critical actions with user ID, timestamp, action, and outcome, which may be acceptable for build docs but cannot be verified from diff alone.
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br>
**Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R639-R655'><strong>Error handling</strong></a>: Examples show throwing GradleException and running external commands but do not <br>consistently demonstrate handling edge cases (e.g., download failures, extraction errors) <br>with contextual messaging across helper functions; as only docs are added, underlying <br>implementation cannot be verified.<br>
<details open><summary>Referred Code</summary>
```markdown
throw new GradleException('Error message')
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Secure Logging Practices</strong></summary><br>
**Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R610-R616'><strong>Log content</strong></a>: The Logger API examples and println usage show generic messages without sensitive data, <br>but the documentation does not state redaction policies or safeguards to prevent logging <br>secrets when downloading or processing files.<br>
<details open><summary>Referred Code</summary>
```markdown
logger.error('Error message')
logger.warn('Warning message')
logger.lifecycle('Lifecycle message')
logger.quiet('Quiet message')
logger.info('Info message')
logger.debug('Debug message')
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br>
**Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R202-R213'><strong>Input validation</strong></a>: Documentation examples accept external URLs and file paths for downloads and extraction <br>without explicit validation or integrity checks (e.g., signature/hash verification), but <br>as these are docs, the actual build script enforcement cannot be confirmed.<br>
<details open><summary>Referred Code</summary>
```markdown
def archive = downloadFile(
'https://example.com/file.zip',
file("${buildTmpPath}/downloads")
)
Process:
Extract filename from URL
Check if file already exists
If not exists, download using URL.withInputStream
Return File object
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td align="center" colspan="2">
- [ ] Update <!-- /compliance --update_compliance=true -->
</td></tr></tbody></table>
<details><summary>Compliance status legend</summary>
🟢 - Fully Compliant<br>
🟡 - Partial Compliant<br>
🔴 - Not Compliant<br>
⚪ - Requires Further Human Verification<br>
🏷️ - Compliance label<br>
</details>
___
#### Previous compliance checks
<details>
<summary>Compliance check up to commit <a href='https://github.com/Bearsampp/module-php/commit/49858c140f1c1db9553d1fbc61a403a1064bf058'>49858c1</a></summary><br>
<table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr>
<tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary>
No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
</details></td></tr>
<tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr>
<tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary>
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->
</details></td></tr>
<tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr>
<tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary>
Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks.
</details></td></tr>
<tr><td colspan='2'><strong>Custom Compliance</strong></td></tr>
<tr><td rowspan=6>⚪</td>
<td><details>
<summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br>
**Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R610-R616'><strong>No audit logs</strong></a>: The PR adds documentation only and does not implement logging for critical actions, so <br>there is no evidence that critical actions are being logged with required context.<br>
<details open><summary>Referred Code</summary>
```markdown
logger.error('Error message')
logger.warn('Warning message')
logger.lifecycle('Lifecycle message')
logger.quiet('Quiet message')
logger.info('Info message')
logger.debug('Debug message')
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br>
**Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R1-R744'><strong>Not applicable</strong></a>: This PR primarily adds documentation and configuration with no new executable code <br>identifiers to assess; naming appears descriptive but full code context is absent.<br>
<details open><summary>Referred Code</summary>
```markdown
# API Reference
Complete API reference for the Bearsampp Module PHP Gradle build system.
---
## Table of Contents
- [Build Script API](#build-script-api)
- [Helper Functions](#helper-functions)
- [Extension Points](#extension-points)
- [Properties API](#properties-api)
- [Task API](#task-api)
---
## Build Script API
### Project Configuration
#### `group`
... (clipped 723 lines)
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation
Status: Error handling docs: The PR documents throwing GradleException and exec usage but does not add executable code where error handling can be validated, so robustness cannot be fully assessed from docs alone.
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Secure Error Handling</strong></summary><br>
**Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R610-R641'><strong>Potential exposure</strong></a>: Examples use println and logger.* with literal messages but do not show user-facing <br>boundaries, making it unclear whether internal details could be exposed in user-facing <br>contexts.<br>
<details open><summary>Referred Code</summary>
```markdown
logger.error('Error message')
logger.warn('Warning message')
logger.lifecycle('Lifecycle message')
logger.quiet('Quiet message')
logger.info('Info message')
logger.debug('Debug message')
Conditional Logging
if (logger.isInfoEnabled()) {
logger.info('Info message')
}
if (logger.isDebugEnabled()) {
logger.debug('Debug message')
}
... (clipped 11 lines)
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Secure Logging Practices</strong></summary><br>
**Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R610-R631'><strong>Logging examples</strong></a>: Logging examples demonstrate various levels but do not demonstrate redaction or safeguards <br>against logging sensitive data, which cannot be verified from documentation alone.<br>
<details open><summary>Referred Code</summary>
```markdown
logger.error('Error message')
logger.warn('Warning message')
logger.lifecycle('Lifecycle message')
logger.quiet('Quiet message')
logger.info('Info message')
logger.debug('Debug message')
Conditional Logging
if (logger.isInfoEnabled()) {
logger.info('Info message')
}
if (logger.isDebugEnabled()) {
logger.debug('Debug message')
}
... (clipped 1 lines)
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td><details>
<summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br>
**Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br>
**Status:** <br><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R202-R236'><strong>External inputs</strong></a>: Documentation references downloading from URLs and executing commands but does not show <br>validation/sanitization steps in executable code, so security posture cannot be confirmed <br>from docs.<br>
<details open><summary>Referred Code</summary>
```markdown
def archive = downloadFile(
'https://example.com/file.zip',
file("${buildTmpPath}/downloads")
)
Process:
Extract filename from URL
Check if file already exists
If not exists, download using URL.withInputStream
Return File object
extractArchive(File archive, File destDir)
Description: Extract archive (ZIP or 7z) to destination directory
Parameters:
| Parameter | Type | Description |
... (clipped 14 lines)
</details>
> Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a>
</details></td></tr>
<tr><td align="center" colspan="2">
<!-- /compliance --update_compliance=true -->
</td></tr></tbody></table>
</details>
Resolve the conflicting descriptions of the output archive structure between TASKS.md and other documentation files. The TASKS.md file incorrectly describes the archive root, which should be corrected for consistency.
**Process:**
...
8. Outputs prepared bundle under `bearsampp-build/tmp/bundles_prep/bins/php/php{version}/`
-9. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`+9. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `php{version}/`
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a critical inconsistency in the documentation regarding the final archive structure. This is a major issue that would confuse users, and correcting it is essential for the documentation's accuracy and usability.
High
Avoid programmatic task execution
Avoid documenting programmatic task execution via task.execute(). Instead, show idiomatic Gradle approaches like task dependencies (dependsOn) to manage execution order.
### Task Execution
```groovy
-// Execute task programmatically-tasks.getByName('taskName').execute()+// Prefer expressing execution order via task dependencies+tasks.register('taskB') {+ dependsOn('taskA')+ doLast {+ // Executed after taskA completes within Gradle's lifecycle+ }+}-// Execute task actions-tasks.getByName('taskName').actions.each { action ->- action.execute(tasks.getByName('taskName'))+// Or configure ordering without forcing execution+tasks.named('taskB') {+ mustRunAfter('taskA')
}
++// Invoke tasks via the command line: gradle taskB
- [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 -->
<details><summary>Suggestion importance[1-10]: 8</summary>
__
Why: The suggestion correctly identifies that documenting `task.execute()` is promoting a significant Gradle anti-pattern which can lead to incorrect and fragile builds. Replacing it with idiomatic dependency management examples is a critical improvement for the API documentation.
</details></details></td><td align=center>Medium
</td></tr><tr><td rowspan=3>General</td>
<td>
<details><summary>Guard PowerShell exec with checks</summary>
___
**Make the PowerShell execution example cross-platform safe by adding an OS check <br>to ensure it only runs on Windows. Also, add explicit error handling for the <br>command's exit code.**
[.gradle-docs/API.md [591-601]](https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R591-R601)
```diff
### PowerShell Execution
```groovy
-def output = new ByteArrayOutputStream()
-exec {
- executable 'powershell.exe'
- args '-NoLogo', '-NoProfile', '-Command', 'Get-Date'
- standardOutput = output
+def result = null
+if (org.gradle.internal.os.OperatingSystem.current().isWindows()) {
+ def output = new ByteArrayOutputStream()
+ def execResult = exec {
+ executable 'powershell.exe'
+ args '-NoLogo', '-NoProfile', '-Command', 'Get-Date'
+ standardOutput = output
+ errorOutput = new ByteArrayOutputStream()
+ ignoreExitValue = true
+ }
+ if (execResult.exitValue != 0) {
+ throw new GradleException("PowerShell command failed with exit code ${execResult.exitValue}")
+ }
+ result = output.toString().trim()
+} else {
+ logger.warn('PowerShell is only available on Windows; skipping command.')
}
-def result = output.toString().trim()
- [ ] **Apply / Chat** <!-- /improve --apply_suggestion=2 -->
<details><summary>Suggestion importance[1-10]: 7</summary>
__
Why: The suggestion correctly points out that the `PowerShell` example is not cross-platform, which contradicts the project's stated goals. Adding an OS check and error handling makes the example safer and more aligned with the project's documentation.
</details></details></td><td align=center>Medium
</td></tr><tr><td>
<details><summary>Harden archive extraction handling</summary>
___
**Improve the archive extraction examples by adding error handling, path quoting <br>for <code>7z</code>, and input validation. This makes the examples more robust and suitable <br>for real-world build scripts.**
[.gradle-docs/API.md [557-571]](https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R557-R571)
```diff
### Archive Operations
```groovy
-// Extract ZIP
-copy {
- from zipTree('archive.zip')
- into 'destination'
+// Extract ZIP safely
+tasks.register('extractZip') {
+ inputs.file('archive.zip')
+ outputs.dir(layout.projectDirectory.dir('destination'))
+ doLast {
+ if (!file('archive.zip').exists()) {
+ throw new GradleException("ZIP not found: ${project.file('archive.zip')}")
+ }
+ copy {
+ from zipTree('archive.zip')
+ into 'destination'
+ }
+ }
}
-// Extract 7z
-exec {
- executable '7z'
- args 'x', 'archive.7z', '-odestination', '-y'
+// Extract 7z with error handling and quoted paths
+tasks.register('extract7z') {
+ inputs.file('archive.7z')
+ outputs.dir(layout.projectDirectory.dir('destination'))
+ doLast {
+ if (!file('archive.7z').exists()) {
+ throw new GradleException("7z archive not found: ${project.file('archive.7z')}")
+ }
+ exec {
+ executable '7z'
+ args 'x', project.file('archive.7z').absolutePath, "-o${project.file('destination').absolutePath}", '-y'
+ isIgnoreExitValue = false
+ }
+ }
}
- [ ] **Apply / Chat** <!-- /improve --apply_suggestion=3 -->
<details><summary>Suggestion importance[1-10]: 6</summary>
__
Why: The suggestion improves the example code by making it more robust with error handling and proper path management. This promotes better build script practices for users of the documentation.
</details></details></td><td align=center>Low
</td></tr><tr><td>
<details><summary>Validate mkdir and delete operations</summary>
___
**Improve the file operation examples for creating and deleting directories. Use <br>Gradle's <code>Delete</code> task and add checks to ensure the operations succeed, preventing <br>silent failures.**
[.gradle-docs/API.md [541-544]](https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R541-R544)
```diff
### File Operations
```groovy
-// Create directory
-file.mkdirs()
+// Create directory with validation
+def dir = layout.projectDirectory.dir('path/to/dir').asFile
+if (!dir.exists() && !dir.mkdirs()) {
+ throw new GradleException("Failed to create directory: ${dir}")
+}
-// Delete file/directory
-delete file
+// Delete file/directory using Gradle API and validate
+tasks.register('purgeTemp', Delete) {
+ delete 'path/to/fileOrDir'
+ doLast {
+ if (file('path/to/fileOrDir').exists()) {
+ throw new GradleException("Failed to delete: ${project.file('path/to/fileOrDir')}")
+ }
+ }
+}
`[To ensure code accuracy, apply this suggestion manually]`
<details><summary>Suggestion importance[1-10]: 5</summary>
__
Why: The suggestion correctly advocates for more robust file operations by checking return values and using Gradle's APIs. This encourages better, more reliable build scripting practices in the documentation.
</details></details></td><td align=center>Low
</td></tr>
<tr><td align="center" colspan="2">
- [ ] More <!-- /improve --more_suggestions=true -->
</td><td></td></tr></tbody></table>
___
#### Previous suggestions
<details><summary>Suggestions up to commit 52cafd0</summary>
<br><table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>General</td>
<td>
<details><summary>Fix archive layout inconsistency</summary>
___
**Correct the <code>release</code> task's process description in <code>TASKS.md</code> to align with the <br>archive structure (<code>php{version}/</code> at the root) defined in other documentation <br>files.**
[.gradle-docs/TASKS.md [49-51]](https://github.com/Bearsampp/module-php/pull/58/files#diff-62b7322bfc2ee8cb7cc871417275fe4347be0188d8603733679d5c3b9bc25f6cR49-R51)
```diff
**Process:**
...
8. Outputs prepared bundle under `bearsampp-build/tmp/bundles_prep/bins/php/php{version}/`
-9. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`
+9. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `php{version}/`
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a critical inconsistency in the documentation regarding the final archive structure, which could lead to integration problems for users.
Medium
Possible issue
Avoid programmatic task execution
Replace the unsafe programmatic task execution example with the recommended approach of using task dependencies to avoid breaking Gradle's build lifecycle.
-// Execute task programmatically-tasks.getByName('taskName').execute()--// Execute task actions-tasks.getByName('taskName').actions.each { action ->- action.execute(tasks.getByName('taskName'))+// Prefer wiring tasks via dependencies or mustRunAfter/shouldRunAfter+tasks.register('taskB') {+ dependsOn('taskA')+ doLast {+ // Executed after taskA+ }
}
+// Or, if you need to launch tasks from another task, use Gradle's tooling+// API externally or advise users to run: gradle taskA taskB+
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that the documentation promotes a dangerous Gradle anti-pattern (task.execute()) and provides the correct, safe alternative of using task dependencies.
Medium
Harden PE header validation
Enhance the DLL validation logic to include checks for MZ and PE signatures and handle potentially corrupt files to make the process more robust.
-validateDllArchitecture(file('ext/php_redis.dll'))+def validateDllArchitecture = { File dll ->+ if (!dll.exists()) throw new GradleException("DLL not found: ${dll}")+ byte[] bytes = dll.bytes+ if (bytes.length < 64) throw new GradleException("Invalid DLL (too small): ${dll.name}")-**Process:**-1. Read DLL file bytes-2. Parse PE header-3. Check machine type (0x8664 = 64-bit)-4. Throw exception if not 64-bit+ // Check DOS header 'MZ'+ if (!(bytes[0] == (byte)0x4D && bytes[1] == (byte)0x5A)) {+ throw new GradleException("Invalid PE file (missing MZ): ${dll.name}")+ }+ int peOffset = java.nio.ByteBuffer.wrap(bytes, 60, 4).order(java.nio.ByteOrder.LITTLE_ENDIAN).int+ if (peOffset + 6 >= bytes.length) throw new GradleException("Invalid PE header offset in ${dll.name}")+ // Check 'PE\0\0'+ if (!(bytes[peOffset] == 0x50 && bytes[peOffset+1] == 0x45 && bytes[peOffset+2] == 0x00 && bytes[peOffset+3] == 0x00)) {+ throw new GradleException("Invalid PE signature in ${dll.name}")+ }++ int machine = java.nio.ByteBuffer.wrap(bytes, peOffset + 4, 2).order(java.nio.ByteOrder.LITTLE_ENDIAN).short & 0xFFFF+ if (machine != 0x8664) {+ throw new GradleException("DLL ${dll.name} is not 64-bit (machine=0x${Integer.toHexString(machine)})")+ }+}+
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that the documented process for DLL validation is naive and could fail on corrupt files, proposing a more robust implementation with proper header checks.
Medium
Validate 7-Zip execution failures
Improve the 7z execution example by adding robust error handling to check for tool presence and handle non-zero exit codes, preventing silent failures.
-exec {- executable '7z'- args 'x', 'archive.7z', '-odestination', '-y'+def output = new ByteArrayOutputStream()+def error = new ByteArrayOutputStream()+def sevenZip = '7z' // or System.getenv('SEVEN_ZIP') ?: '7z'++def result = exec {+ executable sevenZip+ args 'x', 'archive.7z', "-o${file('destination')}", '-y'+ standardOutput = output+ errorOutput = error+ ignoreExitValue = true+}+if (result.exitValue != 0) {+ logger.error("7z extraction failed:\n${error.toString('UTF-8')}")+ throw new GradleException("7z not available or extraction failed (exit ${result.exitValue})")
}
Suggestion importance[1-10]: 6
__
Why: The suggestion improves the example code for executing 7z by adding robust error handling, which is a good practice to teach in documentation to prevent silent build failures.
Low
Don’t ignore exec failures
Update the exec example to demonstrate capturing the ExecResult and checking the exit value, rather than simply ignoring failures with ignoreExitValue = true.
-exec {+def out = new ByteArrayOutputStream()+def err = new ByteArrayOutputStream()+def result = exec {
executable 'command'
args 'arg1', 'arg2'
workingDir file('directory')
- standardOutput = new ByteArrayOutputStream()+ standardOutput = out+ errorOutput = err
ignoreExitValue = true
}
+if (result.exitValue != 0) {+ logger.error("Command failed:\n${err.toString('UTF-8')}")+ throw new GradleException("Command exited with ${result.exitValue}")+}
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies that using ignoreExitValue = true without checking the result is a bad practice and provides a more robust example with explicit error handling.
Replace the example of programmatic task execution using task.execute() with the recommended Gradle patterns of using task dependencies (dependsOn) or task configuration.
-// Execute task programmatically-tasks.getByName('taskName').execute()--// Execute task actions-tasks.getByName('taskName').actions.each { action ->- action.execute(tasks.getByName('taskName'))+// Prefer task dependencies or configuration over imperative execution+tasks.register('consumerTask') {+ dependsOn 'taskName'+ doLast {+ // This runs after taskName completes+ }
}
+// Or configure existing task without forcing execution+tasks.named('taskName') {+ doLast {+ // Add additional action safely+ }+}+
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that using task.execute() is a discouraged practice in Gradle and provides examples of better, idiomatic approaches. This improves the quality of the API documentation by promoting best practices.
Medium
Correct 7z package root documentation
Correct the description for the packageRelease7z task to state that the archive root folder is php{version}/, not {bundle.release}/, to align with other documentation files.
### `packageRelease7z`
**Group:** build
-**Description:** Package release into a `.7z` archive (requires 7-Zip in PATH). Ensures the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`.+**Description:** Package release into a `.7z` archive (requires 7-Zip in PATH). Ensures the top-level folder inside the archive is `php{version}/` (e.g., `php8.3.15/`).
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a significant inconsistency in the documentation regarding the archive structure, which could confuse users. Aligning the documentation improves its correctness and clarity.
Low
Correct zip package root documentation
Correct the description for the packageReleaseZip task to state that the archive root folder is php{version}/, not {bundle.release}/, to align with other documentation files.
### `packageReleaseZip`
**Group:** build
-**Description:** Package release into a `.zip` archive using Gradle's native Zip task. Ensures the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`.+**Description:** Package release into a `.zip` archive using Gradle's native Zip task. Ensures the top-level folder inside the archive is `php{version}/` (e.g., `php8.3.15/`).
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a significant inconsistency in the documentation regarding the archive structure, which could confuse users. Aligning the documentation improves its correctness and clarity.
Low
Remove hardcoded absolute paths
Replace the hardcoded absolute Windows paths for build.properties and gradle.properties with relative paths to make the documentation platform-agnostic.
Why: The suggestion correctly identifies that hardcoded, platform-specific absolute paths in documentation are not ideal. Replacing them with relative paths makes the documentation more universally applicable and less confusing for users on different operating systems.
Correct the archive layout description in the release task process to state that the top-level folder is php{version}/, ensuring consistency with other documentation.
7. Outputs prepared bundle under `bearsampp-build/tmp/bundles_prep/bins/php/php{version}/`
-8. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`+8. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/`, ensuring the top-level folder inside the archive is `php{version}/`
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies an inconsistency in the documentation regarding the archive's root directory structure, which could confuse users. Aligning this with other documentation files (README.md) improves clarity and accuracy.
Medium
Correct package layout to php{version} root
Update the packageRelease task's archive layout description to specify php{version}/ as the root folder, resolving a conflict with other documentation.
### `packageRelease`
...
- Chooses archiver based on `bundle.format` in `build.properties` (`7z` or `zip`).
-- Archive content layout mirrors module-bruno:- - Root folder: `{bundle.release}/` (e.g., `2025.10.31/`)- - Inside root: `bin/php{version}/` and `bin/archived/`- - Optional: `releases.properties` at the same level as `bin/`+- Archive content layout:+ - Root folder: `php{version}/` (e.g., `php8.3.15/`)+ - Contents: PHP binaries, ext/, optional imagick/, pear/ as prepared+ - Optional: sidecar hash files generated alongside the archive
- Output: `bearsampp-build/bins/php/{bundle.release}/bearsampp-php-{version}-{bundle.release}.{7z|zip}`
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies an inconsistency in the documentation for the packageRelease task regarding the archive's root directory. Correcting this to php{version}/ makes it consistent with other parts of the documentation, improving its overall quality and preventing user confusion.
Medium
Align 7z packaging description with layout
Correct the packageRelease7z task description to state that the archive's top-level folder is php{version}/, aligning it with the rest of the documentation.
### `packageRelease7z`
**Group:** build
-**Description:** Package release into a `.7z` archive (requires 7-Zip in PATH). Ensures the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`.+**Description:** Package release into a `.7z` archive (requires 7-Zip in PATH). Ensures the top-level folder inside the archive is `php{version}/`.
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out an inconsistency in the packageRelease7z task description concerning the archive's root directory. Aligning the description to use php{version}/ as the root folder ensures consistency across the documentation.
Medium
Correct ZIP packaging layout description
Update the packageReleaseZip task description to specify php{version}/ as the archive's root folder, ensuring consistency with other documentation.
### `packageReleaseZip`
**Group:** build
-**Description:** Package release into a `.zip` archive using Gradle's native Zip task. Ensures the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`.+**Description:** Package release into a `.zip` archive using Gradle's native Zip task. Ensures the top-level folder inside the archive is `php{version}/`.
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies an inconsistency in the packageReleaseZip task description regarding the archive's root directory. Changing it to php{version}/ makes the documentation consistent and more reliable for users.
Medium
Clarify temporary directories naming
Clarify the temporary directory structure in the README.md file to resolve inconsistencies with other parts of the documentation.
Why: The suggestion correctly identifies a minor inconsistency in the comments describing the temporary directory structure. While the TASKS.md file provides a more detailed and seemingly correct path, aligning the README.md improves clarity and reduces potential confusion for users.
Why: The suggestion correctly identifies a critical inconsistency in the documentation where the required Java version is incompatible with the required Gradle version, preventing build failures for users.
High
Stabilize Gradle JVM memory
Add more specific JVM arguments to gradle.properties, including initial heap size, G1GC, and OOM handling, to improve Gradle's performance and stability.
Why: The suggestion provides more robust JVM arguments for Gradle, which can improve build performance and stability, though the existing settings are not incorrect.
Low
General
Make 7-Zip detection cross-platform
Enhance the find7ZipExecutable function to support Unix-like systems by searching for the 7z binary using which, making the 7-Zip detection cross-platform.
-// Helper function to find 7-Zip executable+// Helper function to find 7-Zip executable (Windows/Linux/macOS)
def find7ZipExecutable() {
- // Check environment variable+ // 1) Environment variable
def sevenZipHome = System.getenv('7Z_HOME')
if (sevenZipHome) {
- def exe = file("${sevenZipHome}/7z.exe")- if (exe.exists()) {- return exe.absolutePath- }+ def exeWin = file("${sevenZipHome}/7z.exe")+ def exeNix = file("${sevenZipHome}/7z")+ if (exeWin.exists()) return exeWin.absolutePath+ if (exeNix.exists()) return exeNix.absolutePath
}
- // Check common installation paths+ // 2) Common Windows installation paths
def commonPaths = [
'C:/Program Files/7-Zip/7z.exe',
'C:/Program Files (x86)/7-Zip/7z.exe',
'D:/Program Files/7-Zip/7z.exe',
'D:/Program Files (x86)/7-Zip/7z.exe'
]
-
for (path in commonPaths) {
def exe = file(path)
- if (exe.exists()) {- return exe.absolutePath- }+ if (exe.exists()) return exe.absolutePath
}
- // Try to find in PATH+ // 3) Try PATH on Windows
try {
def process = ['where', '7z.exe'].execute()
process.waitFor()
if (process.exitValue() == 0) {
def output = process.text.trim()
- if (output) {- return output.split('\n')[0].trim()- }+ if (output) return output.split('\\r?\\n')[0].trim()
}
- } catch (Exception e) {- // Ignore- }+ } catch (Exception ignored) { }++ // 4) Try PATH on Unix-like systems+ try {+ def process = ['which', '7z'].execute()+ process.waitFor()+ if (process.exitValue() == 0) {+ def output = process.text.trim()+ if (output) return output.split('\\r?\\n')[0].trim()+ }+ } catch (Exception ignored) { }
return null
}
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies that the find7ZipExecutable function is Windows-specific and proposes a valid cross-platform solution, which improves the build script's portability and robustness.
Low
Remove UTF-8 BOM character
Remove the leading UTF-8 BOM (Byte Order Mark) from .gradle-docs/INDEX.md to prevent potential issues with Markdown renderers and other tools.
✅ Fix undefined temp path variableSuggestion Impact:The committed diff for the releaseAll task still shows mysqlTmpPrepPath being used, but the entire file was effectively replaced with a minimal/no-op (only 1 line remains after the patch), meaning the problematic code was removed. This eliminates the runtime error stemming from the undefined variable, aligning with the suggestion's goal, though not by directly renaming the variable.
code diff:
@@ -1,1133 +1 @@
In the releaseAll task, fix a runtime error by replacing the undefined variable mysqlTmpPrepPath with the correctly defined bundleTmpPrepPath.
-// Load build properties+// Load build properties safely
def buildProps = new Properties()
-file('build.properties').withInputStream { buildProps.load(it) }+def buildPropsFile = file('build.properties')+if (buildPropsFile.exists()) {+ buildPropsFile.withInputStream { buildProps.load(it) }+} else {+ println "[WARNING] build.properties not found at ${buildPropsFile.absolutePath}. Using defaults."+}
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that the script will crash during the configuration phase if build.properties is missing, improving robustness and user experience.
Medium
Align Gradle version requirement
Update the Gradle version compatibility comment in gradle.properties to require 8.0+ to align with the project's documentation.
# Gradle Build Properties for Bearsampp Module PHP
# Gradle daemon configuration
org.gradle.daemon=true
org.gradle.parallel=true
org.gradle.caching=true
# JVM settings for Gradle
org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m -XX:+HeapDumpOnOutOfMemoryError
# Configure console output
org.gradle.console=auto
org.gradle.warning.mode=all
# Build performance
org.gradle.configureondemand=false
# Gradle version compatibility
-# This project is compatible with Gradle 7.0++# This project requires Gradle 8.0+ to build
Suggestion importance[1-10]: 4
__
Why: The suggestion correctly identifies an inconsistency between a comment in gradle.properties and the required Gradle version mentioned in other documentation files, improving clarity and preventing potential user confusion.
Low
General
Make 7-Zip detection cross-platform
Modify the find7ZipExecutable function to support non-Windows operating systems by checking for 7z on the system PATH, making the build script more portable.
-// Helper function to find 7-Zip executable+// Helper function to find 7-Zip executable (cross-platform)
def find7ZipExecutable() {
- // Check environment variable+ // If explicit home provided (Windows-style), try both exe and non-exe
def sevenZipHome = System.getenv('7Z_HOME')
if (sevenZipHome) {
- def exe = file("${sevenZipHome}/7z.exe")- if (exe.exists()) {- return exe.absolutePath- }+ def exeWin = file("${sevenZipHome}/7z.exe")+ if (exeWin.exists()) return exeWin.absolutePath+ def exeNix = file("${sevenZipHome}/7z")+ if (exeNix.exists()) return exeNix.absolutePath
}
- // Check common installation paths- def commonPaths = [+ // Windows common installation paths+ def commonWinPaths = [
'C:/Program Files/7-Zip/7z.exe',
'C:/Program Files (x86)/7-Zip/7z.exe',
'D:/Program Files/7-Zip/7z.exe',
'D:/Program Files (x86)/7-Zip/7z.exe'
]
-- for (path in commonPaths) {- def exe = file(path)- if (exe.exists()) {- return exe.absolutePath+ if (System.getProperty('os.name').toLowerCase().contains('win')) {+ for (path in commonWinPaths) {+ def exe = file(path)+ if (exe.exists()) return exe.absolutePath
}
+ // Try to find in PATH on Windows+ try {+ def process = ['where', '7z.exe'].execute()+ process.waitFor()+ if (process.exitValue() == 0) {+ def output = process.text.trim()+ if (output) return output.split('\r?\n')[0].trim()+ }+ } catch (ignored) { }+ } else {+ // Non-Windows: try `which 7z`+ try {+ def process = ['sh', '-c', 'command -v 7z || which 7z'].execute()+ process.waitFor()+ if (process.exitValue() == 0) {+ def output = process.text.trim()+ if (output) return output.split('\n')[0].trim()+ }+ } catch (ignored) { }
}
-- // Try to find in PATH- try {- def process = ['where', '7z.exe'].execute()- process.waitFor()- if (process.exitValue() == 0) {- def output = process.text.trim()- if (output) {- return output.split('\n')[0].trim()- }- }- } catch (Exception e) {- // Ignore- }-
return null
}
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly points out that the find7ZipExecutable function is Windows-specific and provides a cross-platform implementation, significantly improving the script's portability.
Medium
✅ Fix external build path referencesSuggestion Impact:The commit updated the Essential Directories table: replaced tmp/ with bearsampp-build/tmp/ and added bearsampp-build/bins/ with appropriate descriptions, matching the suggestion.
Why: The suggestion correctly identifies that the documentation in INDEX.md misrepresents the build's output and temporary directories, and the proposed fix is crucial for preventing user confusion when locating build artifacts.
Low
Correct directory structure documentation
Correct the directory structure diagram in README_PART1.txt to show that temporary files and build outputs are created in an external bearsampp-build/ directory.
Why: The suggestion correctly points out a significant documentation error in the directory structure diagram that would mislead users about where build artifacts are generated, and the fix aligns the diagram with the actual build process.
The packaging description states the archive root should be {bundle.release}/ with bin/ and bin/archived/, which conflicts with other docs (API/README) that show php{version}/ as the archive root. Align the documented output structure to avoid confusion and verify the build script matches the intended layout.
7. Outputs prepared bundle under `bearsampp-build/tmp/bundles_prep/bins/php/php{version}/`8. Packages the prepared folder into an archive in `bearsampp-build/bins/php/{bundle.release}/` ensuring the top-level folder inside the archive is `{bundle.release}/` containing `bin/` and `bin/archived/`**Output Locations:**- Prepared folder: `bearsampp-build/tmp/bundles_prep/bins/php/php{version}/`- Final archive: `bearsampp-build/bins/php/{bundle.release}/bearsampp-php-{version}-{bundle.release}.{7z|zip}`---### `releaseBuild`**Group:** build
**Description:** Execute the release build process
**Usage:**```bash
gradle releaseBuild -PbundleVersion=8.3.15
Parameters:
Parameter
Type
Required
Description
Example
bundleVersion
String
Yes
PHP version to build
8.3.15
Note: This task is typically called by the release task. Use release instead for normal builds.
packageRelease
Group: build Description: Package release into archive (7z or zip) including the version folder at the root of the archive
Usage:
gradle packageRelease -PbundleVersion=8.3.15
Notes:
Chooses archiver based on bundle.format in build.properties (7z or zip).
</details>
<details><summary><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-2882908f3aeed61d657698338c698f94281febf60dc07c94656ccf5370f92497R516-R525'><strong>Programmatic Task Execution</strong></a>
The docs suggest calling tasks.getByName('taskName').execute() and iterating actions to execute, which is generally discouraged in Gradle and can cause configuration/execution-phase issues. Recommend removing or warning against this pattern.
</summary>
```markdown
```groovy
// Execute task programmatically
tasks.getByName('taskName').execute()
// Execute task actions
tasks.getByName('taskName').actions.each { action ->
action.execute(tasks.getByName('taskName'))
}
</details>
<details><summary><a href='https://github.com/Bearsampp/module-php/pull/58/files#diff-964df91d9863f52c526c4793242b7b3fa5087dd9a6eb692d6c947080c719f921R186-R216'><strong>Encoding/Rendering Artifacts</strong></a>
Box-drawing characters in directory trees appear garbled; ensure UTF-8 encoding and that markdown renders properly on GitHub. Replace with ASCII where necessary.
</summary>
```markdown
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
Enhancement
Description
Replaced legacy Ant build system with pure Gradle implementation
Added comprehensive Gradle documentation in
.gradle-docs/directoryRemoved obsolete Ant configuration files and Eclipse launch configurations
Implemented modern build tasks for PHP module packaging and verification
Diagram Walkthrough
File Walkthrough
1 files
Main Gradle build script implementation2 files
Gradle project settings configurationGradle daemon and JVM performance settings6 files
Main Gradle build documentation and quick startComplete reference for all Gradle tasksConfiguration guide for build systemAPI reference for build scriptsDocumentation index and navigation guideUpdated with Gradle build system information5 files
Removed legacy Ant build configurationRemoved legacy Ant sigcheck configurationRemoved Eclipse Ant launch configurationRemoved temporary test batch fileRemoved repository list documentation