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

Fixed issue #1778 by removing a hash of the absolute artifact path appended to the end of the version string. That hash made artifact version different on different PCs and also breaks Gradle dependency locking. #1780

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

AlexanderBartash
Copy link
Contributor

@AlexanderBartash AlexanderBartash commented Sep 30, 2024

Pull Request Details

See comment #1778 (comment) on what this PR improves. Also in the description of #1778 there is a list of reasons why that hash is bad.
Two major points are:

  • My fix allows to lock build classpath in addition to regular configurations.
  • Also with my fix the generated lock file can be committed to VCS and shared among developers, because the hash of the absolute path is not there.

An attempt to fix the issue. I am not really sure why that hash was added initially, but everything seems to work without it. It fixes #1778 but it did not fix #1779 unfortunately.

Please have a look at the new tests which I've added. The ones which are ignored are still broken due to #1779.

The non-ignored tests actually were passing even before the fix & still passing after. I tried a lot of things, but for some reason I could not reproduce it in tests (probably the build classpath is too small there and locks do not make a difference). See #1778 (comment) for context. It is reproducible here https://github.com/AlexanderBartash/intellij-platform-plugin-template/tree/dep-lock-bug and this fix helps to fix dependency locking (this issue), but does not fix dependency verification #1779

Description

In the issue.

Related Issue

#1778

Motivation and Context

#1778

How Has This Been Tested

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.

…t path appended to the end of the version string. That hash made artifact version different on different PCs and also breaks Gradle dependency locking.
@AlexanderBartash
Copy link
Contributor Author

Creating test report Unit Tests Report: Gradle 8.10.2 @ ubuntu-latest
  Processing test results for check run Unit Tests Report: Gradle 8.10.2 @ ubuntu-latest
  Creating check run Unit Tests Report: Gradle 8.10.2 @ ubuntu-latest
Error: HttpError: Resource not accessible by integration

I am not sure that I can fix this.

@hsz
Copy link
Member

hsz commented Oct 2, 2024

The Error: HttpError: Resource not accessible by integration is a common GitHub Actions issue we can see here. Please ignore – I'll re-run the job.

@hsz hsz merged commit dbc44d9 into JetBrains:main Oct 2, 2024
1 check passed
@hsz hsz added this to the next milestone Oct 2, 2024
@AlexanderBartash AlexanderBartash deleted the issue-1778-dependency-locking branch October 4, 2024 22:55
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.

Support dependency verification Support for dependency locking, do not use absolute paths
2 participants