Skip to content

Conversation

@Scavanger
Copy link
Contributor

@Scavanger Scavanger commented Nov 26, 2025

User description

Windows SITL depends on cygwin1.dll.
To always have the correct dll available and to get rid of the SITL binaries floating around in the configurator repro.


PR Type

Enhancement


Description

  • Include cygwin1.dll in Windows SITL CI build artifacts

  • Copy cygwin1.dll from system to build output directory

  • Update artifact upload path to include DLL file

  • Fix line ending formatting in multiple source files


Diagram Walkthrough

flowchart LR
  A["Windows SITL Build"] --> B["Copy cygwin1.dll"]
  B --> C["Build Output Directory"]
  C --> D["Upload Artifacts"]
  D --> E["SITL-WIN Package"]
Loading

File Walkthrough

Relevant files
Configuration changes
ci.yml
Add cygwin1.dll to Windows SITL artifacts                               

.github/workflows/ci.yml

  • Added step to copy cygwin1.dll from system /bin/ to build output
    directory
  • Updated artifact upload path to include both *.exe files and
    cygwin1.dll
  • Ensures Windows SITL binaries have required Cygwin dependency
    available
+5/-1     
Formatting
config.c
Fix line ending formatting                                                             

src/main/target/FLYSPARKF4V4/config.c

  • Line ending conversion from CRLF to LF format
  • No functional code changes
+39/-39 
target.h
Fix line ending formatting                                                             

src/main/target/FLYSPARKF4V4/target.h

  • Line ending conversion from CRLF to LF format
  • No functional code changes
+168/-168
CMakeLists.txt
Fix line ending formatting                                                             

src/main/target/FLYSPARKF4V4/CMakeLists.txt

  • Line ending conversion from CRLF to LF format
  • No functional code changes
+1/-1     

Windows SITL depends on cygwin1.dll.
To always have the correct dll available and to get rid of the SITL binaries floating around in the configurator repro.
@Scavanger Scavanger changed the title Inlcude cygwin1.dll in Windows Ci Build Include cygwin1.dll in Windows Ci Build Nov 26, 2025
@qodo-merge-pro qodo-merge-pro bot changed the title Include cygwin1.dll in Windows Ci Build Inlcude cygwin1.dll in Windows Ci Build Nov 26, 2025
@qodo-merge-pro
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The new workflow step copying cygwin1.dll and modified artifact upload introduce critical
build actions without any auditing or logging metadata beyond default GitHub Actions logs.

Referred Code
- name: Copy cygwin1.dll
  run: cp /bin/cygwin1.dll ./build_SITL/
- name: Upload artifacts
  uses: actions/upload-artifact@v4
  with:
    name: ${{ env.BUILD_NAME }}_SITL-WIN
    path: | 
      ./build_SITL/*.exe
      ./build_SITL/cygwin1.dll

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unchecked copy: The cp /bin/cygwin1.dll ./build_SITL/ step lacks existence checks and error handling for
missing DLLs or permission issues, which may cause silent or opaque failures in CI.

Referred Code
- name: Copy cygwin1.dll
  run: cp /bin/cygwin1.dll ./build_SITL/
- name: Upload artifacts

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Check the DLL into the repository

Instead of copying cygwin1.dll from a hardcoded path on the CI runner, check the
DLL file into the repository. This change will make builds more reproducible and
less dependent on the runner's environment.

Examples:

.github/workflows/ci.yml [259-260]
      - name: Copy cygwin1.dll
        run: cp /bin/cygwin1.dll ./build_SITL/

Solution Walkthrough:

Before:

# .github/workflows/ci.yml

- name: Build SITL
  run: ninja -j4
- name: Strip version number
  run: ...
- name: Copy cygwin1.dll
  run: cp /bin/cygwin1.dll ./build_SITL/
- name: Upload artifacts
  uses: actions/upload-artifact@v4
  with:
    path: | 
      ./build_SITL/*.exe
      ./build_SITL/cygwin1.dll

After:

# .github/workflows/ci.yml
# Assumes cygwin1.dll is checked into the repo, e.g., at tools/cygwin/cygwin1.dll

- name: Build SITL
  run: ninja -j4
- name: Strip version number
  run: ...
- name: Copy cygwin1.dll
  run: cp tools/cygwin/cygwin1.dll ./build_SITL/
- name: Upload artifacts
  uses: actions/upload-artifact@v4
  with:
    path: | 
      ./build_SITL/*.exe
      ./build_SITL/cygwin1.dll
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant flaw in making the CI build dependent on the runner's environment and proposes a best-practice solution to ensure build reproducibility.

Medium
General
Dynamically locate DLL instead of hardcoding

Replace the hardcoded path to cygwin1.dll with a dynamic lookup using the which
command to make the CI workflow more robust.

.github/workflows/ci.yml [259-260]

 - name: Copy cygwin1.dll
-  run: cp /bin/cygwin1.dll ./build_SITL/
+  run: cp "$(which cygwin1.dll)" ./build_SITL/
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that hardcoding the path /bin/cygwin1.dll is fragile and proposes using which to find it dynamically, which significantly improves the CI workflow's robustness.

Low
  • More

@Scavanger Scavanger changed the title Inlcude cygwin1.dll in Windows Ci Build Include cygwin1.dll in Windows Ci Build Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant