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

(datagrip) Strip .0 suffix from installation directory #2366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

9numbernine9
Copy link

Description

When determining the installation directory for Datagrip, remove any .0 suffix that might be present in the version number. Jetbrains doesn't use this part of the release version number when installing, so this causes Chocolatey to create an empty directory that never gets used or cleaned up.

Motivation and Context

Fixes: #2257

How Has this Been Tested?

choco install -s . datagrip with these changes; observe that spurious .0 directory is no longer created.

Types of changes

  • 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)
  • Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the chocolatey test environment.
  • The changes only affect a single package (not including meta package).

When determining the installation directory for Datagrip, remove
any `.0` suffix that might be present in the version number.
Jetbrains doesn't use this part of the release version number when
installing, so this causes Chocolatey to create an empty
directory that never gets used or cleaned up.

Fixes issue chocolatey-community#2257 .
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

$installDir = "$programFiles\JetBrains\DataGrip $env:ChocolateyPackageVersion"
# If the release version ends in '.0', remove this since JetBrains don't
# use this part of the version number when creating the installation directory.
$installDir = "$programFiles\JetBrains\DataGrip $($env:ChocolateyPackageVersion -replace '.0$', '')"
Copy link
Member

Choose a reason for hiding this comment

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

We're creating the directory further down on line 19. Can we not just remove that line and let the installer install wherever it thinks it should? Why do we need to create the directory?

Copy link
Author

Choose a reason for hiding this comment

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

Well... originally I had assumed (possibly incorrectly) that the New-Item -ItemType Directory step in the original install script was required, but based on my own testing maybe it's not needed anymore? My quick searching seems to imply that it was part of a workaround for this older bug in the Jetbrains installer ( https://youtrack.jetbrains.com/issue/IDEA-202935 ) but it seems that since then the installer bug has been fixed, so this directory creation step might just be vestigial!

I can change the PR to remove the unnecessary cruft if you'd prefer?

Copy link
Member

Choose a reason for hiding this comment

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

The directory creation was indeed just because of a bug in the installer that would cause any custom directories used to fail the installation.

If this is now fixed, and works as expected, then I agree with @pauby that removing the manual creation of the directory instead of replacing it here.

To note though as well. IMO this wouldn't be the correct approach anyway, as if the directory does not respect the 3rd part of a version number if it is zero, we should not use the ChocolateyPackageVersion environment variable, but instead a hard-coded value we automatically insert when creating the package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants