Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

py3-awscrt/0.23.4 package update #36021

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

octo-sts[bot]
Copy link
Contributor

@octo-sts octo-sts bot commented Dec 6, 2024

@octo-sts octo-sts bot added request-version-update request for a newer version of a package automated pr labels Dec 6, 2024
Copy link
Contributor Author

octo-sts bot commented Dec 6, 2024

Gen AI suggestions to solve the build error:

Based on the error output and context provided, here's my analysis:

• Detected Error: The error appears to be in the git checkout step, specifically with submodule initialization. While not explicitly shown in the error, this is inferred from the context and the manual s2n update step in the pipeline.

• Error Category: Version/Configuration

• Failure Point: Git checkout step with submodules

• Root Cause Analysis: The current pipeline is trying to update s2n manually after checkout, but this approach may be causing conflicts with the submodule initialization process.

• Suggested Fix:

  1. Remove the manual s2n update step and instead use the sparse-checkout feature in the git-checkout pipeline:
pipeline:
  - uses: git-checkout
    with:
      repository: https://github.com/awslabs/aws-crt-python
      tag: v${{package.version}}
      expected-commit: 4742211db7e3e9c8903b7659389aca8b08ade4ce
      recurse-submodules: true
      submodule-update: true
      submodule-override:
        crt/s2n: 
          repository: https://github.com/aws/s2n-tls
          ref: 1.5.9

• Explanation:

  • The current approach of manually updating s2n after checkout can lead to inconsistencies
  • Using submodule-override provides a cleaner way to specify exact versions of submodules
  • This ensures all dependencies are in sync during the build process
  • Version 1.5.9 of s2n-tls is known to be compatible and includes the necessary fixes

• Additional Notes:

  • The package is using system libcrypto (AWS_CRT_BUILD_USE_SYSTEM_LIBCRYPTO=1) which is correct
  • The current s2n-tls package in the build environment is version 1.5.9-r0
  • This approach maintains better dependency tracking and reproducibility

• References:

@mamccorm mamccorm force-pushed the wolfictl-0c6239d4-db59-4474-a149-72cc1d6a54a1 branch from fe06a8a to 8b70a7b Compare December 7, 2024 19:45
Copy link
Contributor Author

octo-sts bot commented Dec 7, 2024

Gen AI suggestions to solve the build error:

• Detected Error: IndentationError: expected an indented block after 'if' statement on line 351

• Error Category: Build Configuration

• Failure Point: Python setup.py during pip wheel build step

• Root Cause Analysis:
The sed command that modifies setup.py has introduced an indentation error by removing lines without properly maintaining Python's indentation structure. The if statement at line 351 is left without its required indented block.

• Suggested Fix:
Modify the sed command in the pipeline to preserve proper indentation:

- runs: |
    # Allow linking to shared libraries
    sed -i.dist '/libraries = \[/,/libraries\]/{/^[[:space:]]*libraries = \[/!{/^[[:space:]]*\]/!d}}' setup.py
    diff -u setup.py.dist setup.py &&
        { echo "attempted change to setup.py did nothing"; exit 1; }

Or alternatively, use a more targeted approach:

- runs: |
    # Allow linking to shared libraries
    sed -i.dist 's/libraries = \[.*\]/libraries = libraries/' setup.py
    diff -u setup.py.dist setup.py &&
        { echo "attempted change to setup.py did nothing"; exit 1; }

• Explanation:
The current sed command is removing lines related to library linking but breaking the Python code structure. The suggested fix maintains proper indentation while removing the static library configuration, allowing the code to compile correctly.

• Additional Notes:

  • The original code block is attempting to handle static vs. shared library linking
  • The modification needs to preserve Python's strict indentation requirements
  • This change maintains the intended functionality while fixing the syntax error

• References:

Signed-off-by: Dentrax <[email protected]>
@octo-sts octo-sts bot added bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. manual/review-needed labels Dec 9, 2024
@Dentrax Dentrax enabled auto-merge (squash) December 9, 2024 13:05
Signed-off-by: Mark McCormick <[email protected]>
@mamccorm mamccorm requested a review from a team December 9, 2024 18:00
@Dentrax Dentrax merged commit d99b748 into main Dec 9, 2024
14 checks passed
@Dentrax Dentrax deleted the wolfictl-0c6239d4-db59-4474-a149-72cc1d6a54a1 branch December 9, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automated pr bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. manual/review-needed request-version-update request for a newer version of a package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants