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

Refactor function names and fix installing 7zip locally if already globally available #3416

Merged
merged 10 commits into from
May 1, 2019
Merged

Refactor function names and fix installing 7zip locally if already globally available #3416

merged 10 commits into from
May 1, 2019

Conversation

r15ch13
Copy link
Member

@r15ch13 r15ch13 commented Apr 29, 2019 •
edited
Loading

Fixes:

  • Prevents installing 7zip/lessmsi/innounp locally if they are already available globally

Additions:

  • Get-HelperPath
  • Test-HelperInstalled
  • Get-LessmsiPath Get-HelperPath -Helper Lessmsi
  • Test-LessmsiInstalled Test-HelperInstalled -Helper Lessmsi
  • Get-InnounpPath Get-HelperPath -Helper Innounp
  • Test-InnounpInstalled Test-HelperInstalled -Helper Innounp
  • Get-HelperPath -Helper Dark
  • Test-HelperInstalled -Helper Dark

Added tests for:

  • Get-AppFilePath
  • Get-HelperPath
  • Test-HelperInstalled
  • Test-Aria2Enabled
  • Test-CommandAvailable

Refactors:

  • aria2_path > Get-HelperPath -Helper Aria2
  • aria2_installed > Test-HelperInstalled -Helper Aria2
  • aria2_enabled > Test-Aria2Enabled
  • 7zip_path > Get-HelperPath -Helper 7zip
  • 7zip_installed > Test-HelperInstalled -Helper 7zip
  • Expand-7ZipArchive > Expand-7zipArchive
  • Test-7ZipRequirement > Test-7zipRequirement
  • Expand-MSIArchive > Expand-MsiArchive
  • Test-LessMSIRequirement > Test-LessmsiRequirement
  • file_path > Get-AppFilePath

@r15ch13
refactor(decompress): Change function name casing
7ded43d 
7zip_path -> Get-7zipPath
7zip_installed -> Test-7zipInstalled
Expand-7ZipArchive -> Expand-7zipArchive
Test-7ZipRequirement -> Test-7zipRequirement
Expand-MSIArchive -> Expand-MsiArchive
Test-LessMSIRequirement -> Test-LessmsiRequirement
@r15ch13
refactor(core): Rename aria2 helper functions
786f08f 
aria2_path > Get-Aria2Path
aria2_installed > Test-Aria2Installed
aria2_enabled > Test-Aria2Enabled
@r15ch13
feature(core): Add lessmsi and innounp helper functions
b2f3448
@r15ch13
fix(install): Check if 7zip/lessmsi/innounp is installed before addin…
6368823 
…g dependency

Prevents installing 7zip/lessmsi/innounp locally if they are already available globally
@r15ch13 r15ch13 changed the base branch from master to develop April 29, 2019 15:44
@r15ch13
Copy link
Member Author

r15ch13 commented Apr 29, 2019 •
edited
Loading

@Ash258 and @niheaven

se35710 (discord): i have 7zip installed via scoop globally, and now when i installed something via scoop, scoop installed it again "locally". should i report an issue?

@se35710 this should fix this issue

@niheaven
Copy link
Member

Consider Test-CommandAvailable, and will we think about external installation of these tools? That is, test if they are installed (internal or external), call with file_path, if error fallback to cmdname (7z.exe, lessmsi.exe, innounp.exe, like 7zip_using_external do).

PS. I could test it tomorrow~

@r15ch13
Copy link
Member Author

r15ch13 commented Apr 29, 2019

Since these are essential for Scoop to work, I would rather not use external versions because these can cause issues. Older 7zip versions have already caused some issues.

@niheaven
Copy link
Member

Okay, got it.

Since Get-xxxPath return filepath string, it could substitute those string in Expand-xxx function, couldn't it? i.e. & Get-7zipPath x "$Path" -o"$DestinationPath", will it works?

@niheaven
Copy link
Member

niheaven commented Apr 29, 2019 •
edited
Loading 

& (Get-7zipPath) x "$Path" -o"$DestinationPath" (-split $Switches) -y | Out-File $LogLocation

Above cmd works, and I suggest changing them so, for further scoop refactoring.

niheaven
niheaven reviewed Apr 30, 2019
View reviewed changes
Copy link
Member

@niheaven niheaven left a comment

Choose a reason for hiding this comment

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

Some minor fixes

lib/core.ps1 Outdated Show resolved Hide resolved
lib/core.ps1 Outdated Show resolved Hide resolved
lib/core.ps1 Outdated Show resolved Hide resolved
niheaven
niheaven approved these changes May 1, 2019
View reviewed changes
Copy link
Member

@niheaven niheaven left a comment

Choose a reason for hiding this comment

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

LGTM

@r15ch13 r15ch13 merged commit 83e4576 into develop May 1, 2019
@r15ch13 r15ch13 deleted the depends-fix branch May 1, 2019 12:18
@r15ch13 r15ch13 mentioned this pull request May 11, 2019
Merged
r15ch13 added a commit that referenced this pull request May 12, 2019
@r15ch13 @niheaven @Ash258
Bucket extraction and refactoring (#3449)
71f5044 
- The main bucket of Scoop has been separated to https://github.com/ScoopInstaller/Main
- Requires PowerShell 5 and newer #3330
- Improve and refactor decompression functions #3204 #3409 #3411
- Move scoop config from `~\.scoop` to `~\.config\scoop\config.json` #3242
- Add `scoop hold` and `scoop unhold` for excluding programs from updates #3444
- Refactored some Core functions #3314 #3416
- Small fix for starting processes on Windows 7 #3415

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
Co-authored-by: Chawye Hsu <h404bi@outlook.com>
Co-authored-by: Jakub Čábera <cabera.jakub@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niheaven niheaven niheaven approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@r15ch13 @niheaven