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

Fix release pipeline #851

Merged
merged 5 commits into from
Dec 2, 2024
Merged

Fix release pipeline #851

merged 5 commits into from
Dec 2, 2024

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Nov 30, 2024

Issue:
The most recent release failed for 2 different reasons:

  1. Many cross-compilation jobs failed, they all use builder like this
    • failed while doing: mv target/cmake-build/aws-crt-java/* target/cmake-build/
    • error message: "target/cmake-build/deps already present"
  2. The aws-crt-java-manylinux-x64-fips job failed with error "Could not find Go"

Description of changes:

  1. Fix 1st issue by doing mv on the 1 directory we care about, instead of doing mv on a wildcard where lots of stuff we don't care about can collide
  2. Fix 2nd issue by installing golang via package manager
    • Previously, it was manually unpacking golang to a folder, and adding a relative path (.go/go/bin) to $PATH (which is weird usually you add absolute paths). And broke when the new "prebuild aws-lc" logic changed the working directory
  3. Other small fixes, that I'll comment on below in the file diff...

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

- mv wildcard statement failed due to collisions (src and dst dir both had "deps" subfolders). Fix by being more targeted, only moving lib/ folder
- Fix some s3 cp commands. --include has no effect unless you also have --exclude "*"
Apparently you're supposed to set $GOROOT if you install it somewhere arbitrary, so let's put it in the official /usr/local/go
It existed briefly in builder.json in the PR that added this line, ed809e4, but it was removed from builder.json before that PR was merged

JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn -B package -DskipTests -Dshared-lib.skip=true -Dcrt.classifier=$CLASSIFIER

aws s3 cp --recursive --include "*.so" target/cmake-build/lib s3://aws-crt-java-pipeline/${GIT_TAG}/lib
aws s3 cp --recursive --exclude "*" --include "*.so" target/cmake-build/lib s3://aws-crt-java-pipeline/${GIT_TAG}/lib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding --exclude "*" because:

https://docs.aws.amazon.com/cli/latest/reference/s3/#use-of-exclude-and-include-filters

Note that, by default, all files are included. This means that providing only an --include filter will not change what files are transferred. --include will only re-include files that have been excluded from an --exclude filter. If you only want to upload files with a particular extension, you need to first exclude all files, then re-include the files with the particular extension.

This wasn't causing any bugs, but it's code that wasn't doing what it was supposed to be doing, so fixed this here (and in a few other locations)

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@TingDaoK TingDaoK left a comment

Choose a reason for hiding this comment

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

:shipit:

@graebm graebm merged commit 91f9370 into main Dec 2, 2024
50 checks passed
@graebm graebm deleted the fix-release branch December 2, 2024 23:33
graebm added a commit to awslabs/aws-c-common that referenced this pull request Dec 4, 2024
**Issue:**
When aws-crt-java started prebuilding AWS-LC, the release pipeline  broke due to a relative path being passed to CMake (see: awslabs/aws-crt-java#851)

Since the AwsPrebuildDependency changed the working directory while running CMake, the relative path became invalid.

**Description of changes:**
Instead of changing the working directory while running CMake, use the `-B<path-to-build>` option (which exists prior to CMake 3.13 even though it's not documented, note that it's a bit weird pre-3.13 with no space after the B))
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.

2 participants