-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(chore): Improve git.exe execution and add parallel bucket updates #5122
Conversation
This will almost certainly fix #3877 "Git spawns thousands of times". |
@r15ch13 Couldn't it test to see which git is first in the path, and only implement this logic, if the first git found is scoop's shim? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using standard PowerShell argument [Switch]Log
, see https://docs.microsoft.com/en-us/powershell/scripting/developer/cmdlet/activity-parameters?view=powershell-5.1
717a09e
to
f2e3136
Compare
I tried using Used function Invoke-Git {
[CmdletBinding()]
[OutputType([String])]
param(
[Parameter(Mandatory = $true, Position = 0)]
[Alias('PSPath', 'Path')]
[ValidateNotNullOrEmpty()]
[String]
$WorkingDirectory,
[Parameter(Mandatory = $true, Position = 1)]
[Alias('Args')]
[String[]]
$ArgumentList
)
$proxy = get_config PROXY
$git = Get-HelperPath -Helper Git
$EnvironmentVariables = @{}
if($proxy -and $proxy -ne 'none') {
if(($ArgumentList -join ' ') -Match "\b(clone|checkout|pull|fetch|ls-remote)\b") {
# convert proxy setting for git
if ($proxy.StartsWith('currentuser@')) {
$proxy = $proxy.Replace("currentuser@",":@")
}
$EnvironmentVariables.Add('HTTPS_PROXY', $proxy)
$EnvironmentVariables.Add('HTTP_PROXY', $proxy)
}
}
Invoke-ExternalCommand -FilePath $git -WorkingDirectory $WorkingDirectory -ArgumentList $ArgumentList -EnvironmentVariables $EnvironmentVariables
} |
And for the colorful output, a PS, this is related to |
Yes, the |
And But E.g. $previousCommit = Invoke-Git -Path $currentdir -ArgumentList @('rev-parse', 'HEAD')
$currentRepo = Invoke-Git -Path $currentdir -ArgumentList @('config', 'remote.origin.url')
$currentBranch = Invoke-Git -Path $currentdir -ArgumentList 'branch' |
I think we can skip on using |
Is the And I agreed that |
- By using direct path to ~\scoop\apps\git\current\mingw64\bin\git.exe this prevents spawning subprocesses and improves execution time - Fixes ScoopInstaller#1615 by replacing currentuser@ with :@ - Spawns cmd.exe only if proxy is set and required by the git command
- Use PS7 feature Foreach-Object -Parallel to run bucket updates in parallel to improve execution time - Fallback to sequential updates for PS5
Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
- use arrays for git argumentlist
9d31a9c
to
da4ac18
Compare
Just a rebase onto the current dev branch. |
Abbreviated the hash to 12 hex digits to prevent inconsistent output length. Ready for review 😄 |
It works for me, and two questions here:
if ($PSVersionTable.PSVersion.Major -ge 7) {
# Parallel parameter is available since PowerShell 7
$buckets | Where-Object { $_.valid } | ForEach-Object -ThrottleLimit 5 -Parallel {
. "$using:PSScriptRoot\..\lib\core.ps1"
$bucketLoc = $_.path
$name = $_.name
$previousCommit = Invoke-Git -Path $bucketLoc -ArgumentList @('rev-parse', 'HEAD')
Invoke-Git -Path $bucketLoc -ArgumentList @('pull', '-q')
if ($using:Log) {
Invoke-Git -Path $bucketLoc -ArgumentList @('--no-pager', 'log', '--color', '--no-decorate', "--grep='^(chore)'", '--invert-grep', '--abbrev=12', "--format='tformat: * %C(yellow)%h%Creset %<|(72,trunc)%s %Cgreen$name%Creset %C(cyan)%cr%Creset'", "$previousCommit..HEAD")
}
}
} else {
$buckets | Where-Object { $_.valid } | ForEach-Object {
$bucketLoc = $_.path
$name = $_.name
$previousCommit = Invoke-Git -Path $bucketLoc -ArgumentList @('rev-parse', 'HEAD')
Invoke-Git -Path $bucketLoc -ArgumentList @('pull', '-q')
if ($Log) {
Invoke-Git -Path $bucketLoc -ArgumentList @('--no-pager', 'log', '--color', '--no-decorate', "--grep='^(chore)'", '--invert-grep', '--abbrev=12', "--format='tformat: * %C(yellow)%h%Creset %<|(72,trunc)%s %Cgreen$name%Creset %C(cyan)%cr%Creset'", "$previousCommit..HEAD")
}
}
} |
The idea for Didn't want to truncate the bucket name for the alignment. But it could determine the longest name and align it. |
Yes, but it doesn't update one bucket. So the name is misleading. I know PSScriptAnalyzer doesn't like it 😄 |
It could update one bucket, right? Think about |
LGTM! ## [Unreleased](https://github.com/ScoopInstaller/Scoop/compare/master...develop) And the changelog? (headline above) |
This output is quite wide, IMO. Default terminal emulator widths are like 30-40 columns. May I suggest outputting a PSObject list for this? (similar to We will lose the individual colors, but PowerShell will automatically take care of padding, alignment and terminal adjustment. (Not to mention the output will become parseable by PowerShell. Can't think of a use case for now, but who knows 😅) |
@r15ch13 Occasional the pull process will be outputted, why? |
hm, I don't know 🤷🏽 |
Tend to be appeared after a long time pull. (and maybe a poor network?) Have seen it several times when I don't Change |
How did you reset the repo? I tried a hard reset to an older commit but it doesn't hit the network to change back to the latest. |
|
@r15ch13 If |
My Scoop home is located at a non default directory, with a space in the path. Maybe that's the reason? |
See the For @rashil2000's bug, I have a non-default location of scoop w/o space, and cannot reproduce. Maybe it is the space? |
Looks like all commands related to Git have an issue when Scoop home is a spaced path. |
Description
$appdir\git\current\mingw64\bin\git.exe
this prevents spawning subprocesses and improves execution time$scoopdir\shims\git.exe
calls$appdir\git\current\bin\git.exe
which is also a shim.currentuser@
with:@
PROXY
config value is set and required by the git commandForeach-Object -Parallel
to run bucket updates in parallel to improve execution time[extras]
behind the commit hashUsing a fixed
git.exe
could be a problem if someone uses git not installed via scoop because this will not detect it withGet-Command
anymore.Motivation and Context
How Has This Been Tested?
Checklist:
develop
branch.Before:
![image](https://user-images.githubusercontent.com/432127/186285428-a836f039-3eb3-4562-808f-f8f8c5e352d1.png)
![image](https://user-images.githubusercontent.com/432127/186285709-b2591dc9-cc18-428f-920e-b3b869113b7b.png)
After:
![image](https://user-images.githubusercontent.com/432127/186285407-f8979324-df4e-4500-9c4c-80955c4e2816.png)
![image](https://user-images.githubusercontent.com/432127/186285696-103aa607-6ffe-4bf8-8360-cf98e06ebbcc.png)