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

[ISSUE #4720] Modernize CI license check and Enable Dependabot #4827

Merged
merged 59 commits into from
May 17, 2024

Conversation

Pil0tXia
Copy link
Member

@Pil0tXia Pil0tXia commented Apr 11, 2024

Fixes #4720

Motivation

The list of artifacts recorded in known-dependencies.txt does not help the maintainer manage dependencies effectively. This list lacks a reference hierarchy of artifacts, and it's more practical to print the dependency tree using Gradle.

The purpose of check-dependencies.sh is to inspect the licenses of third-party dependencies, preventing developers from casually introducing untracked new artifacts. However, it can't prevent developers adding Apache 2.0 incompatible licenses.

The presence of known-dependencies.txt blocks Dependabot because it cannot update this file through CI. If our project does not keep up with new versions of dependencies for a long time, it will gradually fall behind and be submerged.

Therefore, I believe it is necessary to cancel the version checking of artifacts of remove known-dependencies.txt in and check-dependencies.sh, and introduce actions/dependency-review-action to check unsupported dependencies.

Why actions/dependency-review-action

About allow-dependencies-licenses attribute

Due to the issue reported in actions/dependency-review-action#670, for dependencies with multiple licenses, this action treats the OR separator as AND, meaning that if any of the licenses are listed in the deny-licenses list, they will be rejected. Ideally, for the OR separator, any dependency should not be rejected as long as at least one license is not listed in the deny-licenses list.

Therefore, I have temporarily added all existing dependencies with multiple licenses in the Repo to the exemptions of this action. Although this action only scans the modified dependencies in the pull request, these exemptions may never be used.

This issue is expected to be fixed in the next version of the action, at which point all exemptions can be removed.

Current implementation of this PR

Use checkDeniedLicense gradle task to check license, instead of dependency-review-action. dependency-review-action will be applied when upstream problems mentioned in #4827 (comment) is resolved.

dependency-review-action workflow files has been backed up to https://github.com/Pil0tXia/eventmesh/tree/pil0txia/action_4720_with-dependency-review-action.

Modifications

Only the artifact name is recorded in known-dependencies.txt, the version number is no longer recorded.

  • Introduce https://github.com/actions/dependency-review-action Use checkDeniedLicense gradle task in CI
  • Remove dependency check shell scripts
  • You can run ./gradlew printAllDependencyTrees > allDeps.log to get dependency trees of all EventMesh submodules.
  • Configure Dependabot and auto-approve its PR to reduce reviewers' pressure.

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (not applicable)
  • If a feature is not applicable for documentation, explain why? See CI logs

@Pil0tXia Pil0tXia added the dependencies Pull requests that update a dependency file label Apr 12, 2024
@Pil0tXia Pil0tXia requested a review from xwm1992 April 15, 2024 03:09
@Pil0tXia Pil0tXia marked this pull request as draft April 15, 2024 04:16
@Pil0tXia Pil0tXia marked this pull request as ready for review April 15, 2024 06:29
@Pil0tXia Pil0tXia changed the title [ISSUE #4720] Simplify known-dependencies and Enable Dependabot [ISSUE #4720] Modernize license check and Enable Dependabot Apr 15, 2024
@Pil0tXia Pil0tXia changed the title [ISSUE #4720] Modernize license check and Enable Dependabot [ISSUE #4720] Modernize CI license check and Enable Dependabot Apr 15, 2024
@Pil0tXia
Copy link
Member Author

I've made some progress in dependency-submission and dependency-review-action. The gradle-build-action@v2 has been upgraded to gradle/actions/setup-gradle@v3 and gradle/actions/[email protected]. And dependency-review-action is now able to detect Gradle dependency changes in GH Actions, however it takes 30min~60min to detect dependency graph's change and it is considered as a bug to be fixed in a future release. So I think this PR does not have technical difficulties for now and can be updated in the next PR when upstream actions' bugs are fixed.

@Pil0tXia Pil0tXia marked this pull request as ready for review April 24, 2024 13:59
@Pil0tXia
Copy link
Member Author

I can write another checkDeniedLicenses Gradle task to check incompatible licenses in another PR before actions/dependency-review-action#632 is resolved and after #4831 is merged. It depends on the output of generateDistLicense task, search for 'incompatible-license-name' in tools/dist-license/LICENSE, and output the line when incompatible license detected.

Although it requires more work and may soon be deprecated after actions/dependency-review-action#632 is resolved, it can save about 30min CI running time before actions/dependency-review-action#632 is resolved.

@Pil0tXia Pil0tXia marked this pull request as draft April 25, 2024 09:43
@Pil0tXia Pil0tXia marked this pull request as ready for review April 25, 2024 11:12
@Pil0tXia
Copy link
Member Author

Pil0tXia commented Apr 25, 2024

@xwm1992 PTAL~

image

The warning of aopalliance is due to this artifact itselves mistake. 1.0 is its only version.

The warning of mysql-connector-j will disappear after this artifact is removed from runtimeClasspath.

The warning of Public Domain can be resolved by upgrading xpp3:xpp3:1.1.4c to org.ogce:xpp3:1.1.6. (bundled by com.aliyun:tea-xml within DingTalk connector)

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.94%. Comparing base (70f9892) to head (b3063e4).
Report is 4 commits behind head on master.

❗ Current head b3063e4 differs from pull request most recent head 7a87a69. Consider uploading reports for the commit 7a87a69 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4827      +/-   ##
============================================
+ Coverage     15.91%   15.94%   +0.03%     
- Complexity     1734     1735       +1     
============================================
  Files           897      897              
  Lines         31982    31943      -39     
  Branches       2737     2734       -3     
============================================
+ Hits           5089     5094       +5     
+ Misses        26413    26370      -43     
+ Partials        480      479       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Pil0tXia
Copy link
Member Author

License check publish guide at apache/eventmesh-site#218.

@Pil0tXia Pil0tXia added the ready for review PR is waiting for reviewer's approval or opinion (used as a strong reminder) label Apr 30, 2024
# Conflicts:
#	tools/dependency-check/known-dependencies.txt
@Pil0tXia
Copy link
Member Author

Pil0tXia commented May 8, 2024

@xwm1992 Conflicts resolved. PTAL~

Copy link
Contributor

@xwm1992 xwm1992 left a comment

Choose a reason for hiding this comment

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

LGTM

@Pil0tXia Pil0tXia requested a review from mxsm May 17, 2024 13:41
@mxsm mxsm merged commit beaa735 into apache:master May 17, 2024
9 checks passed
@Pil0tXia Pil0tXia deleted the pil0txia/action_4720 branch May 18, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file ready for review PR is waiting for reviewer's approval or opinion (used as a strong reminder)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Enable Dependabot on the repository
5 participants