kisslinux / repo

KISS Linux - Official Repositories
https://kisslinux.github.io
MIT License
403 stars 102 forks source link

VERSION variables complicate kiss-find #290

Closed jedahan closed 3 years ago

jedahan commented 3 years ago

RIght now, the logic for extracting versions for packages is very simple (VERSION="$(cat "$PACKAGE/version")")

https://github.com/jedahan/kiss-find/blob/main/lib/generate_fat_db.sh#L53

After VERSION introduction, some packages now now have changed their VERSION number, to part of the url, as an example: https://github.com/kisslinux/repo/commit/6a11373100d59ceacb92e7a49ea90a32dd4561d5

This makes it difficult (or impossible) to match correctly with expected version strings for users.

The fact is, bumping version, sources, and build together seems really low effort, so I am not sure the benefit of magic variables is worth the complexity.

There are other potential problems with VERSION being exported in build files (for example, some go packages read this variable to set their documentation and --version output at build time).

dylanaraps commented 3 years ago

After VERSION introduction, some packages now now have changed their VERSION number, to part of the url, as an example: 6a11373 This makes it difficult (or impossible) to match correctly with expected version strings for users.

Versions being commit hashes is supported, normal and has been used before in this repository. Nothing special about those changes, git packages use either the hash or just git.

The fact is, bumping version, sources, and build together seems really low effort, so I am not sure the benefit of magic variable

This is a simple de-duplication of data that reduces the commit size for each package change by -1 file. It's a quality of life improvement for maintainers as it makes their life easier (bump version + kiss c). (It certainly makes my life easier).

The code required in the package manager for this feature is also very small in size/complexity. As stated elsewhere, this is entirely opt-in. The old behavior is still there and continues to work as it did. If you don't like the feature, don't use it.

There are other potential problems with VERSION being exported in build files (for example, some go packages read this variable to set their documentation and --version output at build time).

VERSION (and all other markers) are not exported as environment variables nor are they environment variables. They're simply a marker that appears in sources files. It has zero affect on build files or other parts of the package manager.

https://github.com/jedahan/kiss-find/blob/main/lib/generate_fat_db.sh#L53

This code will continue to work as it did. No changes have been made to version files at all.

I don't see where the problem is at all. What is broken with this new opt-in behavior? Version files have not changed at all. They are exactly the same. The only difference is that sources files can optionally contain markers (which have no effect anywhere else).

dylanaraps commented 3 years ago

I'll say a few more things.

  1. This is not obfuscation of information as the same information is in the version file which exists right next to the sources file in the same directory. This data is identical or the feature itself would not work.

  2. There's nothing smart about this feature. It is literally just a find/replace of exactly matching (and hard-coded) markers with existing data. It's actually a very dumb feature.

  3. Again, I want to stress that this is opt-in. There is no change in behavior for existing packages and packages without markers in their sources files work the exact same as they always did. The same goes for the environment change. Existing packages work as they did and the new feature is entirely opt-in.

This change has for some reason caused uproar and I don't understand it at all. It's a very minute change in the grand scheme of things (and was meant to benefit users and maintainers alike). To me it seems like an overreaction as there are calls to drop the distribution over what is effectively a find/replace.

To finish off - help me understand your point of view. If there are real concerns about this feature and actual breakage somewhere, open an issue about it. I however, don't see breakage or regression anywhere. Existing behavior is identical, version files are unchanged and this is entirely opt-in.

Edit: Slightly off topic: If people opened bugs here, I wouldn't have to grep the IRC logs, check the subreddit or scour the internet to find bugs to fix. I'm very proactive about issues and go looking for them if they do not come to me. Just wanted to clarify. Also, feel free to send email - I respond to all of it.

dilyn-corner commented 3 years ago

This is not obfuscation of information as the same information is in the version file which exists right next to the sources file in the same directory. This data is identical or the feature itself would not work.

My only qualm with this change is precisely that it DOES do this. That I have to refer to another location for information is exactly obfuscating - things like ${pkgver%%.} are the biggest factor in my distate of PKGBUILDs. But, as you point out, this is entirely optional, the old behavior exists and works just fine, and so I have no real stake in this change.

My only real concern is that this doesn't help with packages using a commit (for instance, using https://github.com/cli/cli/archive/a6710ec5064995aadd18da5808c8c4ff41a8199c.tar.gz) or packages like cbindgen. But I grant that these are niche and alternative cases and shouldn't have much of a bearing on most, if not any, changes to kiss. But this isn't even so much a concern -- as you said, it literally does not matter.

So yes, assuming this causes no breaks/regressions/failures, it's fine -- and it only does this if a user uses a repo with this new option but for some inexplicable reason does NOT update kiss. And that isn't really your fault.

ETA Apparently kiss-outdated was not updated to work with these changes.

dylanaraps commented 3 years ago

My only qualm with this change is precisely that it DOES do this. That I have to refer to another location for information is exactly obfuscating - things like ${pkgver%%.} are the biggest factor in my distate of PKGBUILDs. But, as you point out, this is entirely optional, the old behavior exists and works just fine, and so I have no real stake in this change.

I don't think this is an issue for the following reasons.

  1. The location of the version information is in the same directory as the sources file. This is where it will always be. This never changes.

  2. The markers (if you want to use them) are very simple to understand/memorize as they're just words. (I need to push the docs for them first though(!)).

So yes, assuming this causes no breaks/regressions/failures, it's fine -- and it only does this if a user uses a repo with this new option but for some inexplicable reason does NOT update kiss. And that isn't really your fault.

That's exactly the situation. :P

Apparently kiss-outdated was not updated to work with these changes.

This works fine. The change does not touch it whatsoever. This utility only looks at the version file.

If you're referring to using git hashes as versions, this has always been valid but I stopped doing it as much when we were added to repology (as it basically expects upstream versions by design).

I personally always preferred using hashes as it's way clearer to you as to what version you're using. Right now we use random git hashes as sources and then lie in the version file by saying it's .

kiss-update reporting a version mismatch for commit hashes as versions is a non-issue as you've stepped outside of the versioning scheme and already thrown it away.

It technically is a mismatch anyway so the tool is working as intended - I'd rather it be verbose about this than ignoring these occurrences or reporting that the git hash is up-to-date.

dilyn-corner commented 3 years ago

I don't think this is an issue for the following reasons. snip

I mean you're not wrong, nothing substantial has technically changed. It's just adjusting how I have been navigating packages. But because it's optional... I don't have to care. Especially considering I've been maintaining all of my own packages for a year now anyways. Ergo, I don't really care that it happens, I just have an opinion on the matter. (:

I think for most people it's just a matter of inertia. Honestly I expected more outrage about Wayland.

dylanaraps commented 3 years ago

Documentation has been added: https://kisslinux.xyz/package-system#4.0

Edit: Also, kiss-outdated will continue to work even if KISS is not supported by it. That's the entire point of the tool.

sdsddsd1 commented 3 years ago

Edit: Slightly off topic: If people opened bugs here, I wouldn't have to grep the IRC logs, check the subreddit or scour the internet to find bugs to fix. I'm very proactive about issues and go looking for them if they do not come to me. Just wanted to clarify. Also, feel free to send email - I respond to all of it.

I think people are just lazy. When screaming into the void actually change things, they will just continue to do so.

jedahan commented 3 years ago

I want to apologize for jumping the gun and misunderstanding the scope of this change. My gut reaction was not in line with the actual changes.

Didn't realize commit hashes were supported in VERSION file as it was so rare I was used to just seeing the git string.

Finally, I want to make sure I am using the channels you prefer for communicating - do you mean that making github issues is preferable to discussing stuff on irc? and email preferable to github issues? Or something else?

--

So take these both with a big grain of salt, as my original issue was incorrect, but here are my thoughts on the impact of the change:

As far as support in sources, I can no longer just cat sources to do things like run my own caching tools, search for fallback mirrors. I currently only use this with a non-published mirroring tool, so this is super minor and i can replicate the version string interpolation in said tool.

Most people are users, some are maintainers, few make tools, so I understand optimizing for whats most common - asking tools writers to support VERSION is a small ask.

dylanaraps commented 3 years ago

Finally, I want to make sure I am using the channels you prefer for communicating - do you mean that making github issues is preferable to discussing stuff on irc? and email preferable to github issues? Or something else?

You are fine. The issue tracker and my email are where bugs should be reported. Use whichever is preferred. My comment wasn't directed at anyone in particular. I just mentioned it as most of the issues I fix are never reported anywhere. I come across them by scouring the web and then fix them here.

As far as support in sources, I can no longer just cat sources to do things like run my own caching tools, search for fallback mirrors. I currently only use this with a non-published mirroring tool, so this is super minor and i can replicate the version string interpolation in said tool.

This is a valid concern though you would have already had to parse the special git sources syntax git+<url>[#@]commit/branch, the ?no-extract syntax as well as the optional second field (which is used by the cache to store data). The sources file has been beyond parseable verbatim fields for a long time already basically (all opt-in however).

I can create a KISS utility to resolve the entire file if desirable. ie, it would spit out two fields per line. One is the input URL/file_path with all markers and other syntax resolsed. The second field would contain the path to the file on the disk (in the cache or otherwise). So you'd run kiss source-resolve zlib and it'd spit out something you can work with.

Let me know if this sounds useful.

jedahan commented 3 years ago

Because the second field is a destination / where to extract, it is not used in any of my tools.


Only vim in kisslinux/repo uses git+ syntax, but 30 packages (out of 439) in kiss-community/community do. Point taken that simple tools cannot handle vim, nor many community packages. They must either ignore or handle git+.

Only rust in kisslinux/repo uses ?no-extract, and 0 packages in kiss-community/community do. From what I understand, simple tools don't need any special parsing, and can use the full uri as-is (i.e. downloading that uri should work, etc). I wonder if rust could just use a pre-extract hook so this wouldn't be necessary...

I don't see any packages in kisslinux/repo that use [#@]commit/branch, and only ungoogled-chromium in kisslinux/community, which I guess could be replaced by adding git checkout <commit-hash> in the build file, but that might be excessive.

dylanaraps commented 3 years ago

Because the second field is a destination / where to extract, it is not used in any of my tools.

The second field is also used when saving remote files to the cache! You do need to parse this to check is sources exist in the cache. If second field is test/dir then the source is stored to ~/.cache/kiss/sources/pkg_name/test/dir/source.tar.gz. This is to avoid naming collisions between remote sources (which has happened before).

Yeah, there aren't many packages which make use of this behavior, you're right. My point was that to support everything these pre-existing, low hanging fruit have to be supported by tools anyway.

jedahan commented 3 years ago

Yeah its just frustrating that my tools went from being dead simple and supporting 99% of kisslinux/repo and 90% of kiss-community/community, to 0% of kisslinux/repo and 90% of kiss-community/community.