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.

Sign up for GitHub

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

Jump to bottom

#1779 Fixes compatibility with Gradle dependency verification. Previously it was failing with "Failed to create MD5 hash for file". #1782

Closed
wants to merge 3 commits into from
Closed

#1779 Fixes compatibility with Gradle dependency verification. Previously it was failing with "Failed to create MD5 hash for file". #1782

wants to merge 3 commits into from

Conversation

AlexanderBartash
Copy link
Contributor

Pull Request Details

It is pretty much covered in #1779

Description

It is pretty much covered in #1779

Related Issue

#1779

Motivation and Context

It is pretty much covered in #1779

How Has This Been Tested

Manually & integration tests.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • [x ] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [x ] I have read the CONTRIBUTING document.
  • [x ] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [x ] I have included my change in the CHANGELOG.
  • [x ] I have added tests to cover my changes.
  • [ x] All new and existing tests passed.

AlexanderBartash
AlexanderBartash commented Oct 3, 2024
View reviewed changes
...tlin/org/jetbrains/intellij/platform/gradle/extensions/IntelliJPlatformRepositoriesHelper.kt
// It has to be prefixed by "/" because: If this pattern is not a fully-qualified URL, it will be interpreted
// as a file relative to the project directory.
val pattern = "/([type])[artifact]"
artifactPattern(pattern)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the essence of the fix. Now type contains an optional absolute path. I know that I am somewhat misusing the attribute, but there is nothing else to replace it with.

The reason why we've put that absolute path into type is that the name should not have it, because artifact name may come up in files like Gradle's verification-metadata.xml

https://docs.gradle.org/current/userguide/dependency_verification.html

Which will make them not portable between different environments.

AlexanderBartash
AlexanderBartash commented Oct 3, 2024
View reviewed changes
...tlin/org/jetbrains/intellij/platform/gradle/extensions/IntelliJPlatformDependenciesHelper.kt
@@ -904,7 +910,7 @@ class IntelliJPlatformDependenciesHelper(
module = name,
revision = version,
),
publications = listOf(artifactPath.toIvyArtifact()),
publications = artifactPath.toLocalPluginIvyArtifacts(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another important part of the fix is that previously localPlugin, bundledPlugin & bundledModule had here a link to a directory which then was processed by CollectorTransformer. This was a problem because Gradle was trying to generate a hash for this dir as if it is a file, because usually artifacts are files. Now this a list of all jars, nor a dir and signature generation works fine. These dependency types now do not require CollectorTransformer, it does nothing for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is an example of hashes generated after the fix (I clipped the file because it is too big).

If the name in the code above had an absolute path it would come in in the name attribute in this file as well. As you can see the names are clean.

<?xml version="1.0" encoding="UTF-8"?>
<verification-metadata xmlns="https://schema.gradle.org/dependency-verification" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="https://schema.gradle.org/dependency-verification https://schema.gradle.org/dependency-verification/dependency-verification-1.3.xsd">
   <configuration>
      <verify-metadata>true</verify-metadata>
      <verify-signatures>true</verify-signatures>
   </configuration>
   <components>
      <component group="bundledModule" name="intellij.platform.coverage" version="242.23339.11">
         <artifact name="intellij.platform.coverage.jar-242.23339.11.jar">
            <md5 value="7153d3560a925ffdaf3e4603bd9c2873" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="226468ff8c4e4ba1a2ec334a2768febec99587a4" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="e4f89b7760e5ae23b7fcdf67a977bc9eb55aedc731d7caa4ac70e769629cfb9f" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="f6e54b8cf67f031876bcd20f0875d4a344ee99049eedd50df49b36fbd34713b963b18db84533a05f2f03b32eaef9238deb1e2e06c13e46f59dd622f7833ba1e9" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
         <artifact name="ivy-242.23339.11.xml">
            <md5 value="99c3e951941f575cd8316b9938bafd35" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="e4933ad772f99bb7086bb5d58ccb4ec36385f120" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="900b85f96b90882a7b6e02344eaa82f6d19ef55f817a640a893a2d9325dbc27a" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="24962aff516afadc5a580dc1618ec6057f44970a1b7c115bd24f49f1b724970de4b5ec86b39267e9e8860cd544c4b9eccceec3f8341481193b3dd35cd851c081" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
      </component>
      <component group="bundledPlugin" name="ByteCodeViewer" version="233.15619.7">
         <artifact name="byteCodeViewer.jar-233.15619.7.jar">
            <md5 value="d4b2b73ee6d0a89c53b692f80c2db1ac" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="cb688dae5c971e0e58029172f003d1a66054743e" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="96888a52c489c4e3e58df364fe19cdf5ae5f013fc92dd1e1f753d22db55e591b" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="f844b78206126128a7a2d9054316846d053bdd9a9f70d037f055eeee068b16734efc8303582c18986cf197cdc57b764ad2d104ae539e20a21fd230de1c44f37f" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
         <artifact name="ivy-233.15619.7.xml">
            <md5 value="3702c3d73de085d561f50fd49c7eb676" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="29a946dabc90d0af3b3f1509aa238113e5934dff" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="fcf19e9ebd38bd7a0947cb83875e243e823b009cd86926a8f4d5c6a8012b8d1d" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="e8dcf993ddc38f4b29d8aa283f6528033008296e9679929c771d877c93b2c58a1aa8eb0112eb9a9c469e9c75f333ccc78cda346a41e6b58296dfd1fd44c6554f" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
      </component>
      <component group="bundledPlugin" name="Coverage" version="233.15619.7">
         <artifact name="ivy-233.15619.7.xml">
            <md5 value="682728aaf751bb6e0669f51441999523" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="23ad37e9d58f57ffb7cf55eb905c91cc2a837491" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="0aed3077c41308ed3df270aa10b1468364158f1dacb43476297c1288a8251c21" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="48cef568c28121e8173acb035b9d94e1912e76890a6b29557c31a58ab7592b8246f1f3ff94e39a2d7f71a7e5866baac690ed96f0b8759045f8ccb6fa1e269d25" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
         <artifact name="jacoco.jar-233.15619.7.jar">
            <md5 value="02944f0d68f98f39579dcfabf1f87c1d" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="9902a47dd143d9828a337406e33c22cee26c50f2" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="2baa4f136a1be79420315285b942ad72f913465b7dd799d4ccf13a80ff5c69e2" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="d3aec04c557fa5107eacce565d2433e6bacc34066c2703ac8be9877e848d1151a8cdacb5fd9de01165caab0cf9dd5fa7953b591295f795f4a90ace4c8b4f1e3b" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
         <artifact name="java-coverage-rt.jar-233.15619.7.jar">
            <md5 value="230912c28c404a3fa17a6323c6fc4f51" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="b5ec497354c9efcd0026c777a6df0f72a4d4110c" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="5f34aaf8ec0c54f2c101521b1cd8ecebbf68c2a2ee8051a66ceacf321c18c9ce" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="5b9ab06ab8cb4a9a47220adef8e4189a2aa218f529b391eea7e4122881070a4bce799da94681f45f43cb1d27364e17cedc6e66bfc80500ed538f879265faf59d" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
         <artifact name="java-coverage.jar-233.15619.7.jar">
            <md5 value="56e660c8da47666caecbac208cb438a8" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha1 value="cbfd1cb59f6e485e90482bdc73ecb23b7d96880e" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha256 value="caae94172afe16c94487ab560f5fbfc2db3f7255745135d3225b6a68cc571b12" origin="Generated by Gradle" reason="Artifact is not signed"/>
            <sha512 value="a2ffca3505223a3b4bc432e509a2e96b66fbae15320ca2d0930776deb34831dab3649f44f7d697c5243268ab95ace4d97ca484c928ee78cf25bba572bfe405b0" origin="Generated by Gradle" reason="Artifact is not signed"/>
         </artifact>
      </component>
   </components>
</verification-metadata>

AlexanderBartash
AlexanderBartash commented Oct 3, 2024
View reviewed changes
src/main/kotlin/org/jetbrains/intellij/platform/gradle/models/IvyModule.kt
return this.explodeIntoIvyJarsArtifacts()
}

/** This method is a bit too universal. It could be split into 3 separate with unnecessary logic removed. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left the comment but did not change it to avoid making this PR too big. I implemented this similarly to how it was wording before.

@AlexanderBartash AlexanderBartash mentioned this pull request Oct 4, 2024
Merged
6 tasks
@AlexanderBartash
JetBrains#1779 Fixes compatibility with Gradle dependency verificatio…
bc89ff1 
…n. Previously it was failing with "Failed to create MD5 hash for file".
@AlexanderBartash AlexanderBartash force-pushed the issue-1779-dependency-verification branch from 3d6e84e to bc89ff1 Compare October 4, 2024 22:56
@hsz
Copy link
Member

hsz commented Oct 5, 2024

@AlexanderBartash Your suggestion doesn't make any difference, as the issue is caused by reading the currently set IntelliJ Platform. If the recommended() helper requests before it is set (so before the dependencies {} block).
As a solution, the recommended() block needs to obtain the IntelliJ Platform lazily.

@hsz hsz added this to the next milestone Oct 5, 2024
@AlexanderBartash
Copy link
Contributor Author

@AlexanderBartash Your suggestion doesn't make any difference, as the issue is caused by reading the currently set IntelliJ Platform. If the recommended() helper requests before it is set (so before the dependencies {} block). As a solution, the recommended() block needs to obtain the IntelliJ Platform lazily.

Ok, I reverted the change in the other PR as well #1782

@AlexanderBartash AlexanderBartash deleted the issue-1779-dependency-verification branch October 5, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsz hsz hsz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.
2 participants
@AlexanderBartash @hsz