git: Persist system-wide config#7632
Conversation
Fixes ScoopInstaller#7631. This adds 'etc/gitconfig' to the persist field to ensure user configurations are preserved during updates.
|
All changes look good. Wait for review from human collaborators. git
|
z-Fng
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bucket/git.json (1)
37-37: Regex replacement may be overly broad.The pattern
(?<=git/)[\\d.]+(?=/)matches any digit-dot sequence betweengit/and/, which could unintentionally modify version-like strings in other contexts within the config (e.g., commented examples, URLs, or user-defined paths).Consider whether a more specific pattern anchored to known path structures would be safer, or verify this is acceptable for the typical contents of
etc/gitconfig.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/git.json` at line 37, The replacement regex "(?<=git/)[\\d.]+(?=/)" is too generic and may alter unrelated version-like strings; update the PowerShell one-liner (the string containing Get-Content ... -replace ...) to restrict matching to actual Git URL/path lines by anchoring or adding surrounding context—e.g., only apply the replace to lines that contain the Git remote or "url =" prefix or require a semantic version pattern (like x.y.z) with explicit separators—so adjust the regex or pre-filter the file content to target known path structures before calling -replace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bucket/git.json`:
- Around line 29-39: The Get-Content/Set-Content sequence in the pre_install
script risks corrupting non-ASCII text because Get-Content uses the system
default encoding while Set-Content writes with -Encoding utf8 (which on
PowerShell 5.1 produces a BOM). Update the restore block to read and write with
the same explicit encoding: pass -Encoding utf8NoBOM (or a chosen consistent
encoding) to both Get-Content and Set-Content so non-ASCII characters are
preserved and no BOM is introduced; locate the commands referencing Get-Content
and Set-Content in the pre_install script and make the encoding arguments
consistent.
---
Nitpick comments:
In `@bucket/git.json`:
- Line 37: The replacement regex "(?<=git/)[\\d.]+(?=/)" is too generic and may
alter unrelated version-like strings; update the PowerShell one-liner (the
string containing Get-Content ... -replace ...) to restrict matching to actual
Git URL/path lines by anchoring or adding surrounding context—e.g., only apply
the replace to lines that contain the Git remote or "url =" prefix or require a
semantic version pattern (like x.y.z) with explicit separators—so adjust the
regex or pre-filter the file content to target known path structures before
calling -replace.
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bucket/git.json (1)
35-35: Prefer-LiteralPathfor file copies built from variables.Use
-LiteralPathon Line 35 and Line 85 to avoid wildcard interpretation surprises in path strings.Proposed fix
- " Copy-Item -Path $config_path_persisted -Destination $config_path -Force", + " Copy-Item -LiteralPath $config_path_persisted -Destination $config_path -Force", ... - " Copy-Item -Path $config_path -Destination $config_path_persisted -Force", + " Copy-Item -LiteralPath $config_path -Destination $config_path_persisted -Force",Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/git.json` at line 35, Replace the Copy-Item invocations that use the -Path parameter with -LiteralPath to avoid wildcard interpretation for variable-built paths: update the string containing "Copy-Item -Path $config_path_persisted -Destination $config_path -Force" (line with that exact Copy-Item call) and the other Copy-Item occurrence around line 85 to use -LiteralPath instead of -Path, keeping the same -Destination and -Force flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bucket/git.json`:
- Line 37: The current PowerShell replace command is too broad — it replaces any
"git/<digits>/" anywhere; narrow the regex to match only Scoop Git install-paths
by changing the lookbehind to the install path prefix (e.g. use
'(?<=scoop/apps/git/)[\\d.]+' with the same trailing (?=/) ) in the command that
uses Get-Content -Path $config_path -Encoding UTF8 and Set-Content -Path
$config_path -Encoding UTF8 so the replacement of $version only targets
scoop/apps/git/<version>/ entries.
---
Nitpick comments:
In `@bucket/git.json`:
- Line 35: Replace the Copy-Item invocations that use the -Path parameter with
-LiteralPath to avoid wildcard interpretation for variable-built paths: update
the string containing "Copy-Item -Path $config_path_persisted -Destination
$config_path -Force" (line with that exact Copy-Item call) and the other
Copy-Item occurrence around line 85 to use -LiteralPath instead of -Path,
keeping the same -Destination and -Force flags.
|
cc @SorYoshino Could you please take a look and give it a quick test? Just want to be sure nothing slipped through the cracks. |
Apologies for the delayed response. I have been overwhelmed with work recently, both physically and mentally. The related script contains some unnecessary logic and appears somewhat verbose. However, testing results indicate that it functions correctly. The test results are as follows:┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> scoop install "D:\Software\Scoop\Global\apps\Git\_2.53.0.old\git.json"
Installing 'git' (2.51.2) [64bit] from 'D:\Software\Scoop\Global\apps\Git\_2.53.0.old\git.json'
Loading PortableGit-2.51.2-64-bit.7z.exe from cache.
Checking hash of PortableGit-2.51.2-64-bit.7z.exe... OK.
Extracting PortableGit-2.51.2-64-bit.7z.exe... Done.
Running pre_install script... Done.
Linking D:\Software\Scoop\Local\apps\git\current => D:\Software\Scoop\Local\apps\git\2.51.2
Creating shim for 'sh'.
Creating shim for 'git'.
Creating shim for 'git-bash'.
Making D:\Software\Scoop\Local\shims\git-bash.exe a GUI binary.
Creating shim for 'gpg'.
Creating shim for 'gpg-agent'.
Creating shim for 'gpgconf'.
Creating shim for 'gpg-connect-agent'.
Creating shim for 'pinentry'.
Making D:\Software\Scoop\Local\shims\pinentry.exe a GUI binary.
Creating shortcut for Git\Git Bash (git-bash.exe)
Creating shortcut for Git\Git CMD (git-cmd.exe)
Creating shortcut for Git\Git GUI (git-gui.exe)
Adding D:\Software\Scoop\Local\apps\git\current\cmd to your path.
Setting user environment variable: GIT_INSTALL_ROOT = D:\Software\Scoop\Local\apps\git\current
Running post_install script... Done.
'git' (2.51.2) was installed successfully!
Notes
-----
To register file associations, please execute the following command:
reg import "D:\Software\Scoop\Local\apps\git\current\install-associations.reg"
To register the context menu entry, please execute the following command:
reg import "D:\Software\Scoop\Local\apps\git\current\install-context.reg"
To set Git Credential Manager Core for portable Git, please execute the following command:
git config --global credential.helper manager
-----
┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> Get-Content -Path "$(scoop prefix git)\etc\gitconfig" -Encoding utf8 -Tail 2
[credential]
helper = helper-selector
┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> git push
Everything up-to-date
┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> Get-Content -Path "$(scoop prefix git)\etc\gitconfig" -Encoding utf8 -Tail 2
[credential]
helper = !\"D:/Software/Scoop/Local/apps/git/2.51.2/mingw64/bin/git-credential-manager.exe\"
┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> scoop update git
git: 2.51.2 -> 2.53.0
Updating one outdated app:
Updating 'git' (2.51.2 -> 2.53.0)
Downloading new version
Loading PortableGit-2.53.0-64-bit.7z.exe from cache.
Checking hash of PortableGit-2.53.0-64-bit.7z.exe... OK.
Running pre_uninstall script... INFO Saving system-level config to D:\Software\Scoop\Local\persist\git\etc\gitconfig...
Done.
Uninstalling 'git' (2.51.2)
Running uninstaller script... Done.
Removing shim 'sh.shim'.
Removing shim 'sh.exe'.
Removing shim 'git.shim'.
Removing shim 'git.exe'.
Removing shim 'git-bash.shim'.
Removing shim 'git-bash.exe'.
Removing shim 'gpg.shim'.
Removing shim 'gpg.exe'.
Removing shim 'gpg-agent.shim'.
Removing shim 'gpg-agent.exe'.
Removing shim 'gpgconf.shim'.
Removing shim 'gpgconf.exe'.
Removing shim 'gpg-connect-agent.shim'.
Removing shim 'gpg-connect-agent.exe'.
Removing shim 'pinentry.shim'.
Removing shim 'pinentry.exe'.
Unlinking D:\Software\Scoop\Local\apps\git\current
Removing D:\Software\Scoop\Local\apps\git\current\cmd from your path.
Removing user environment variable: GIT_INSTALL_ROOT
Installing 'git' (2.53.0) [64bit] from 'D:\Software\Scoop\Global\apps\Git\_2.53.0.old\git.json'
Loading PortableGit-2.53.0-64-bit.7z.exe from cache.
Extracting PortableGit-2.53.0-64-bit.7z.exe... Done.
Running pre_install script... INFO Restoring system-level config from D:\Software\Scoop\Local\persist\git\etc\gitconfig...
INFO Adjusting paths in D:\Software\Scoop\Local\apps\git\2.53.0\etc\gitconfig...
Done.
Linking D:\Software\Scoop\Local\apps\git\current => D:\Software\Scoop\Local\apps\git\2.53.0
Creating shim for 'sh'.
Creating shim for 'git'.
Creating shim for 'git-bash'.
Making D:\Software\Scoop\Local\shims\git-bash.exe a GUI binary.
Creating shim for 'gpg'.
Creating shim for 'gpg-agent'.
Creating shim for 'gpgconf'.
Creating shim for 'gpg-connect-agent'.
Creating shim for 'pinentry'.
Making D:\Software\Scoop\Local\shims\pinentry.exe a GUI binary.
Creating shortcut for Git\Git Bash (git-bash.exe)
Creating shortcut for Git\Git CMD (git-cmd.exe)
Creating shortcut for Git\Git GUI (git-gui.exe)
Adding D:\Software\Scoop\Local\apps\git\current\cmd to your path.
Setting user environment variable: GIT_INSTALL_ROOT = D:\Software\Scoop\Local\apps\git\current
Running post_install script... Done.
'git' (2.53.0) was installed successfully!
Notes
-----
To register file associations, please execute the following command:
reg import "D:\Software\Scoop\Local\apps\git\current\install-associations.reg"
To register the context menu entry, please execute the following command:
reg import "D:\Software\Scoop\Local\apps\git\current\install-context.reg"
To set Git Credential Manager Core for portable Git, please execute the following command:
git config --global credential.helper manager
-----
┏[ D:\Software\Scoop\Local\buckets\Unofficial][ master ≡ 1]
└─> Get-Content -Path "$(scoop prefix git)\etc\gitconfig" -Encoding utf8 -Tail 2
[credential]
helper = !\"D:/Software/Scoop/Local/apps/git/2.53.0/mingw64/bin/git-credential-manager.exe\"Incidentally, regardless of whether |
z-Fng
changed the title
|
Thanks a lot for the help with testing!
No worries at all! Don't feel pressured to reply right away -- just focus on taking care of yourself first. Hope things get easier soon. Thank you so much for testing.
Switching from |
|
/verify |
|
All changes look good. Wait for review from human collaborators. git
|
3 participants
Fixes #7631. This adds 'etc/gitconfig' to the persist field to ensure user configurations are preserved during updates.
Closes #7631
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit