-
-
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
feature: Prepare extraction of main bucket #3060
Conversation
calling scoop could result into 'scoop is not recognized as command...'
If there is no bucket on update, consider main
Updating works even after this change. If there is no Only inside scoop list will be * Maybe add check in list to ignore if ($install_info.bucket -eq 'main') {
Write-Host ''
} |
How to test?
|
I haven't touched on tests yet. But after recent changes (@r15ch13, 589303f), it should be fine. There is possible to remove default parameter value from https://github.com/lukesampson/scoop/blob/1a845b17ed8f8feb84fdd1052b2e164e8134f50b/test/Scoop-Manifest.Tests.ps1#L1 |
Feel free to suggest any change. |
There is one thing VERY important that you didn't cover above. We need to handle those previously installed packages so that users can still update these packages after this breaking change. packages installed from the {
"bucket": "extras",
"architecture": "64bit"
} but packages installed from {
"architecture": "64bit"
} |
It's already handled :) |
Like brew? Coooool! |
Forgot to mention, that this will make #2121 pretty easy to fix, since there will be option to use So if there will be multiple manifests, just warn user about prefix with desired bucket and exit. |
If a user installs scoop from the first time, the installer script downloads a zip of the core bucket (this is a normal logic, to make scoop work at the first time, git and 7zip are needed). When the user runs But the 'zip core bucket' is not a git repository after the installation. So there should be a handler logic to clone a new core bucket and remove the zip one in sidenote: I'm working on the brand new installer script which is totally independent and does not rely on any function in |
Eveything work as expected. On installing, there is no need to have 'git' repository. If you just unpack zip to buckets\main, shim scoop and run but i see problem on updating, I hoped, that, .git check for scoop update is applied for buckets too. Here come question how to handle it? First thing to appear on my mind: $loc = bucketdir $_ # Line 106
if ($_ -eq 'main' -and !(Test-Path "$loc\.git")) { # main bucket exists, but it's not repo
rm_bucket 'main'
add_bucket 'main'
}
Push-Location ($loc) |
@Ash258 make sense. |
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.
Is there need to change bin\*.ps1
's $Dir
param? main
bucket should has its own bin
dir, and manifest related bin should move there, isn't it?
I'll do a deep review tomorrow, in 16 hrs.
@@ -141,15 +139,6 @@ function find_hash_in_headers([String] $url) { | |||
function get_hash_for_app([String] $app, $config, [String] $version, [String] $url, [Hashtable] $substitutions) { | |||
$hash = $null | |||
|
|||
<# | |||
TODO implement more hashing types |
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.
I suggest leave hash mode comment for illustrations, there is something TODO
.
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.
All are already implemented
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.
I mean TODO
more, and leave these implemented alone as comment. Maybe need to rearrange these lines.
Made |
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.
LGTM
I think this is ready to be merged into develop branch. We can test it today / tomorrow in develop branch and then merge it to master |
@Ash258 can you squash some of the commits? Or should I squash the whole PR? |
Squash whole PR. |
#2939, #2609, #1328
❗❗❗ This PR is not meant to be merged. ❗❗❗
I want to only proof, that extraction of bucket is possible and pretty easy. Bucket folder will not be removed from this PR (to keep diff cleaner, without millions of lines deleted due to bucket folder deletion). If collaborators (This should be done by @r15ch13, since he has access to Excavator) decided to adopt this change, just follow steps bellow (co-authored-by tag would be nice for all commits / PR 😍):
How to?
Slighlty edited and step by step flow from #1328 (comment)
git filter-branch --prune-empty --subdirectory-filter bucket
git remote set-url origin 'https://github.com/ScoopInstaller/scoop-main'
git push -u origin
scoop update
UseCorrectCasing
rulescoop config SCOOP_BRANCH develop
git branch -ne get-config SCOOP_BRANCH { git checkout get-config SCOOP_BRANCH }
Version: X.XX.XX Adopt new change
Core
or something