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

feat(subdir): Allow subdir in 'bucket' #5119

Merged
merged 3 commits into from
Sep 28, 2022
Merged

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Aug 22, 2022

Description

Allow subdir in buckets' bucket directory, e.g., main/bucket/#/7zip.json. All scoop command is unchanged (scoop search <xxx>, scoop install <xxx>, checkver, checkurls, checkhashes, etc.)

Motivation and Context

There's some discussion before (to avoid GitHub's 1000 files limit).

This PR won't change too much in scoop ecosystem, but the actual change of buckets structure WILL/MAY affect scoop manifests search engines. (PullRequestHandler GHA failed because of checkver and checkhashes which are fixed in this PR)

How Has This Been Tested?

All tests are passed, and all scoop subcommand and scripts in bin are tested. (With ScoopInstaller/Tests#6)

image
image
image
image
image

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@chawyehsu
Copy link
Member

This PR won't change too much in scoop ecosystem

Though nothing will directly affect end users this is STILL a breaking change, to the ecosystem, so not true.

All scoop command is unchanged (scoop search , scoop install , checkver, checkurls, checkhashes, etc.)

There are many unofficial implementations by users from our community you can find now such as https://github.com/search?q=scoop+search and you need to think of them while doing substantial changes will definitely shape the community.

I think an RFC is needed.

@niheaven
Copy link
Member Author

It of cause is a breaking change, along with config migration (#5116). I've said that this PR won't break the ecosystem, but the actual change of buckets structure WILL/MAY affect scoop manifests search engines. The structure change should be well informed, especially to out official search engines.

For other unofficial implementations of scoop search, we could submit issues to the repos (about 10).

rasa added a commit to rasa/scoop-directory that referenced this pull request Aug 25, 2022
@niheaven
Copy link
Member Author

Anything to improve? Or it should be tested in 'develop'.

Again, this PR doesn't change the current bucket structure and is backward compatible, just provides a feature that user could use nested buckets.

@chawyehsu
Copy link
Member

chawyehsu commented Sep 15, 2022

I'm thinking of introducing the architecture layer besides the manifest name layer, the subdir you described. Combining these two layers, the structure will be eventually looking like this:

bucket/
├── amd64
│   ├── 1
│   │   └── 1password-cli.json
│   ├── 7
│   │   ├── 7zip19.00-helper.json
│   │   └── 7zip.json
│   ├── a
│   │   ├── ...
│   ├── b
│   ├── c
│   ├── d
│   ├── e
│   ├── f
│   ├── g
│   ├── h
│   ├── i
│   ├── j
│   ├── k
│   ├── l
│   ├── m
│   ├── n
│   ├── o
│   ├── p
│   ├── q
│   ├── r
│   ├── s
│   ├── t
│   ├── u
│   ├── v
│   ├── w
│   ├── x
│   ├── y
│   └── z
├── arm64
│   ├── j
│   ├── k
│   ├── l
│   ├── m
│   ├── n
│   ├── o
│   ├── v
│   ├── w
│   ├── x
│   ├── y
│   └── z
└── ia32
    ├── 1
    ├── 7
    ├── a
    ├── b
    ├── c
    ├── d
    ├── e
    ├── f
    ├── g
    ├── h
    ├── i
    ├── j
    ├── k
    ├── l
    ├── m
    ├── n
    ├── o
    ├── p
    ├── q
    ├── r
    ├── s
    ├── t
    ├── u
    ├── v
    ├── w
    ├── x
    ├── y
    └── z

This will solve the long-standing issue of being unable to define different versions of the same package among different architectures. Imagine you need to have version 1.0.0 of package A for 32bit and version 2.0.0 for 64bit and 0.1.0 for arm64. An architecture layer can easily have the capability of storing different versions of the same package, under different architecture dirs.

In addition, I don't quite understand the point of categorizing packages with name starting with digits into # rather than 0..9. According to my experience, most of the other package managers use 0..9, such as dpkg, and the winget you've already known. Is it an optimal way that I need to know well? Giving some references here would be great.

@niheaven
Copy link
Member Author

I'm thinking of introducing the architecture layer besides the manifest name layer, the subdir you described. Combining these two layers, the structure will be eventually looking like this:

Then there will be manifests with the same name, and Scoop should know how to pick correct one for certain architecture. Is it a better solution that use arch-spec version number in manifest? (Which means arch-spec checkver should be supported too)

In addition, I don't quite understand the point of categorizing packages with name starting with digits into # rather than 0..9. According to my experience, most of the other package managers use 0..9, such as dpkg, and the winget you've already known. Is it an optimal way that I need to know well? Giving some references here would be great.

StartMenu in Windows 🤣 : (Damn 360)

CleanShot 2022-09-15 at 23 54 14@2x

I think the reason here is 0-9 or other non-alphabet characters are rarely used as first letter, so merging them to # (the meaning here is 'number') is a considerable option.

@chawyehsu
Copy link
Member

Is it a better solution that use arch-spec version number in manifest?

Polluting a single manifest with more and more arch-spec fields is such a terrible situation you can't ignore. It will end up containing every field for every architecture in one manifest. This will eventually make the manifest hard to inspect and maintain and I'm not a fan of it.

Prefer separated manifests for different architectures over one single messy manifest, that's my point.

I think the reason here is 0-9 or other non-alphabet characters are rarely used as first letter, so merging them to # (the meaning here is 'number') is a considerable option.

not convincing :)

@niheaven
Copy link
Member Author

Just a option, '#' or '0-9' are both okay for me. In ScoopInstaller/Tests#6 I just pick up daily used one which is seen in Start Menu.

A question that may be off topic but about arch-spec manifests, most manifests have architecture section and anything else is the same. Do these manifests need to be splited and go to arch-spec subdir? Or only those need special treatments be moved?

@chawyehsu
Copy link
Member

In an ideal state, all fields will be duplicated to arch-spec manifests whether they are identical or not. Despite redundant content, this is helpful for the feature I want to propose of installing multi-arch packages simultaneously. Ahem, that's another long-standing issue...

Imagine that by adopting the architecture layer, one on an x86_64 machine can only search and install amd64 packages by default, all manifests from other architecture sub-directories are invisible to the user, hence fields redundancy is needed. And by enabling multi-arch configuration, the user can search and install packages (into a different location other than the /apps, maybe /<arch>/apps, I don't have a specific proposal at present) from other architectures. The whole solution would be like some real package managers like dpkg.

It will be a big change compared to this you submit and that's why I think simply adding the manifest name layer is not a good feature because it can only create a new option for you to make nested structures but not solve critical issues.

@niheaven
Copy link
Member Author

Hmm, I got your idea. But the 'subdir' here is for better lookup (manually in Explorer) and GitHub's 1000 files display limit, not for some additional installation or orgnization improvement. 😄 That's really beyond the discussion and this PR.

And back to package managers, dpkg and winget all put their apps under subdir (brew is totally different), why not scoop?

@rashil2000
Copy link
Member

I think the reason here is 0-9 or other non-alphabet characters are rarely used as first letter, so merging them to # (the meaning here is 'number') is a considerable option.

I agree with this option. Having subdirectories for starting character is a good idea but having one each for each digit would be being too pedantic.

@niheaven
Copy link
Member Author

May I merge this or any other improvements? @ScoopInstaller/maintainers

@niheaven niheaven merged commit 7f47f66 into develop Sep 28, 2022
@niheaven niheaven deleted the feat-subdir-in-bucket branch September 28, 2022 03:22
pynappo pushed a commit to pynappo/Scoop that referenced this pull request Sep 29, 2022
@CEbbinghaus
Copy link

This might be a little Bikeshedding but I wanted to respond to a comment made earlier:

Polluting a single manifest with more and more arch-spec fields is such a terrible situation you can't ignore. It will end up containing every field for every architecture in one manifest. This will eventually make the manifest hard to inspect and maintain and I'm not a fan of it.

I personally don't like the idea of having 3 separate files for the same app each for a different architecture in a different directory. I think taking a similar approach to how Windows Terminal settings allow for a default to contain all common parameters with an array to specify all the specific versions and their specific parameters would be ideal. That would also allow the folder names in the bucket to remain arbitrary without having to lock them into a specific naming.

Example manifest

{
    "default": {
        "description": "This is an example to showcase defaults working",
        "extract_dir": "output",
        "homepage": "https://example.com",
        "license": "MIT",
        "depends": "commonDependency"
    },
    "architectures": [
        {
            "arch": "arm",
            "version": "1.0.0",
            "hash": "...",
            "url": "...",
            "checkver": {
                "url": "...",
                "regex": ""
            }
        },
        {
            "arch": "64bit",
            "version": "1.2.0",
            "hash": "...",
            "url": "...",
            "checkver": {
                "url": "...",
                "regex": ""
            }
        },
        ...
    ]
}

HUMORCE added a commit to ScoopInstaller/Main that referenced this pull request May 23, 2023
HUMORCE added a commit to ScoopInstaller/Extras that referenced this pull request May 23, 2023
HUMORCE added a commit to ScoopInstaller/Versions that referenced this pull request May 23, 2023
HUMORCE added a commit to ScoopInstaller/BucketTemplate that referenced this pull request May 23, 2023
rasa pushed a commit to ScoopInstaller/Main that referenced this pull request May 23, 2023
rasa pushed a commit to ScoopInstaller/Extras that referenced this pull request May 23, 2023
rasa pushed a commit to ScoopInstaller/Versions that referenced this pull request May 23, 2023
rasa pushed a commit to ScoopInstaller/BucketTemplate that referenced this pull request May 23, 2023
qwerty12 pushed a commit to qwerty12/scoop-alts that referenced this pull request May 28, 2023
Related: ScoopInstaller/Scoop#5119
Signed-off-by: Faheem Pervez <trippin1@gmail.com>
bodrick added a commit to bodrick/scoop-bucket that referenced this pull request Jun 27, 2023
404NetworkError pushed a commit to 404NetworkError/scoop-bucket that referenced this pull request Jul 10, 2023
kazzarin pushed a commit to kazzarin/bucket that referenced this pull request Jul 12, 2023
aliesbelik added a commit to aliesbelik/poldi that referenced this pull request Dec 20, 2023
LucasOe pushed a commit to LucasOe/scoop-lucasoe that referenced this pull request Apr 23, 2024
LucasOe pushed a commit to LucasOe/scoop-lucasoe that referenced this pull request Oct 10, 2024
bobo2334 pushed a commit to bobo2334/scoop-bucket that referenced this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants