-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add explicit dependency to validatePom and generatePom tasks. #12807
Add explicit dependency to validatePom and generatePom tasks. #12807
Conversation
Signed-off-by: Zelin Hao <[email protected]>
Signed-off-by: Zelin Hao <[email protected]>
Compatibility status:Checks if related components are compatible with change 0729327 Incompatible componentsIncompatible components: [https://github.com/opensearch-project/security.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer.git] |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12807 +/- ##
============================================
- Coverage 71.42% 71.36% -0.06%
- Complexity 59978 60209 +231
============================================
Files 4985 5011 +26
Lines 282275 283667 +1392
Branches 40946 41117 +171
============================================
+ Hits 201603 202434 +831
- Misses 63999 64397 +398
- Partials 16673 16836 +163 ☔ View full report in Codecov by Sentry. |
❌ Gradle check result for b6bf6dc: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for b6bf6dc: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for b6bf6dc: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❕ Gradle check result for 0729327: UNSTABLE
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Thanks @zelinh , I think it looks good! |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-12807-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c04dad58bbc941b8d5f18326d5c5cfb7ac312239
# Push it to GitHub
git push --set-upstream origin backport/backport-12807-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
@zelinh could you please backport to |
@reta @zelinh I'm not sure yet, but I think this PR is contributing to failures being seen in security plugin's CI: opensearch-project/security#4156
|
Ah, that is unexpected, thanks a lot for @cwperks ! |
…opensearch-project#12807)" This reverts commit c04dad5. Signed-off-by: Andriy Redko <[email protected]>
@@ -48,17 +48,24 @@ public class PomValidationPrecommitPlugin extends PrecommitPlugin { | |||
public TaskProvider<? extends Task> createTask(Project project) { | |||
TaskProvider<Task> validatePom = project.getTasks().register("validatePom"); | |||
PublishingExtension publishing = project.getExtensions().getByType(PublishingExtension.class); | |||
publishing.getPublications().all(publication -> { | |||
publishing.getPublications().configureEach(publication -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line seemed to cause a breaking change in the security plugin:
* Where:
Build file '/home/runner/work/security/security/build.gradle' line: 417
* What went wrong:
A problem occurred evaluating root project 'opensearch-security'.
> NamedDomainObjectContainer#configureEach(Action) on Publication container cannot be executed in the current context.
Is this change from all
-> configureEach
necessary?
The underlying issue associated with Pom validation was fixed in OpenSearch [1], this causes the 'hack' that was used to unblock pom generation in security to start causing problems in the distribution builds. - [1] opensearch-project/OpenSearch#12807 Signed-off-by: Peter Nied <[email protected]>
The underlying issue associated with Pom validation was fixed in OpenSearch [1], this causes the 'hack' that was used to unblock pom generation in security to start causing problems in the distribution builds. - [1] opensearch-project/OpenSearch#12807 Signed-off-by: Peter Nied <[email protected]>
### Description The underlying issue associated with Pom validation was fixed in OpenSearch [1], this causes the 'hack' that was used to unblock pom generation in security to start causing problems in the distribution builds. - [1] opensearch-project/OpenSearch#12807 ### Issues Resolved - Fixes #4160 ### Testing Added new CI test that executes the same job used by the distribution build and checks the expected artifacts exist ### Check List - [X] New functionality includes testing - [ ] ~New functionality has been documented~ - [X] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). Signed-off-by: Peter Nied <[email protected]>
…arch-project#12807) * Add explicit dependency to PomValidation Signed-off-by: Zelin Hao <[email protected]> * Update CHANGELOG Signed-off-by: Zelin Hao <[email protected]> --------- Signed-off-by: Zelin Hao <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
### Description The underlying issue associated with Pom validation was fixed in OpenSearch [1], this causes the 'hack' that was used to unblock pom generation in security to start causing problems in the distribution builds. - [1] opensearch-project/OpenSearch#12807 ### Issues Resolved - Fixes opensearch-project#4160 ### Testing Added new CI test that executes the same job used by the distribution build and checks the expected artifacts exist ### Check List - [X] New functionality includes testing - [ ] ~New functionality has been documented~ - [X] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). Signed-off-by: Peter Nied <[email protected]>
Description
This PR is to add explicit task dependency on PomGenerate and PomValidate tasks for different publication types.
For plugin with different publication, all
generatePom
tasks such asgeneratePomFileForNebulaPublication
orgeneratePomFileForPluginZipPublication
are creating the Pom file at the same cache location: build/distributions/*******.pom; and Pom validation tasks are reading the output of this Pom.We will get error like this in
job-scheduler
plugin:Reason
Gradle treats any task that reads a file as implicitly dependent on each task that writes to that file.
In our case, Gradle is assuming each validation task is depending on all the Pom generation tasks as implicit dependency.
In this PR, we are adding these implicit dependencies for
generatePomFile
andvalidatePom
into its implementation.We can see same logic is added in few other plugin repos:
In geospatial:
validateNebulaPom.dependsOn(generatePomFileForPluginZipPublication)
In skills:
tasks.validateNebulaPom.dependsOn ':generatePomFileForPluginZipPublication'
Check List
- [x] New functionality includes testing.~~ - [x] All tests pass~~
- [x] New functionality has been documented.~~ - [x] New functionality has javadoc added~~
- [x] Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.