afetch: add new package (2.2.0)#29292
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an OpenWrt package definition for afetch v2.2.0 and includes a patch intended to tailor afetch output for OpenWrt and reduce upstream code size.
Changes:
- Added
utils/afetch/Makefileto build/installafetchin OpenWrt. - Added a large patch to
src/fetch.cto add OpenWrt branding and remove many other OS/distro branches.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| utils/afetch/Makefile | Introduces the OpenWrt packaging metadata and install recipe for afetch 2.2.0 |
| utils/afetch/patches/100-add_openwrt_suporte_and_reduces_size.patch | Patches upstream fetch.c to add OpenWrt detection/art and significantly trim other OS/distro handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
265f6eb to
51bed4a
Compare
BKPepe
left a comment
There was a problem hiding this comment.
I took a quick look at it and found some inconsistencies that need to be resolved. Overall, however, it leaves me wondering what the actual use case for this package is in OpenWrt, and why we should package afetch to be built for all routers supported by OpenWrt.
We can obviously find it in some GNU/Linux distributions according to https://repology.org/project/afetch/versions, but it doesn't really seem like afetch is actively maintained or developed anymore. It is 5 years old, after all.
And if I understand it correctly, it basically just displays information from /etc/os-release in a fancy layout. Personally, I don't find that interesting or useful enough to justify having a package here along with a massive patch adding OpenWrt support, which we would then have to maintain and modify with every future update.
51bed4a to
4fe6e2d
Compare
BKPepe
left a comment
There was a problem hiding this comment.
I’m still leaning towards a 'no' for including this in its current state. Here are the main technical and practical reasons:
-
Maintenance overhead: The patch you're introducing effectively breaks compatibility with other GNU/Linux distributions. This makes it impossible to push upstream, meaning we would have to manually maintain and rebase this patch every time
fetch.cchanges. We generally avoid adding packages that create this kind of long-term technical debt. -
Actual size reduction: If the justification for such a destructive patch is 'reducing size,' do you have specific numbers? How many kilobytes are we actually saving? It would be much better to use conditional compilation or build flags to exclude non-OpenWrt code paths rather than stripping them out manually.
-
Utility vs. 'Fancy' features: While your point about users missing this is a very diplomatic way of saying you're practicing on a package, we have to be pragmatic. This is a 'fancy' tool for displaying system info that is already available—albeit in a different format—in LuCI or via standard CLI tools.
OpenWrt aims to be lean and maintainable. Adding a package that requires constant babysitting just for aesthetic info seems counterproductive unless the implementation is much cleaner.
Fast and simple system info (for UNIX based operating systems) written in POSIX compliant C99. Link: https://github.com/13-CF/afetch Signed-off-by: Tiago Rocha <tiagorocha@disroot.org>
4fe6e2d to
f6adf59
Compare
|
I redid the patch to just add OpenWrt support without removing strings from other Linux and BSD distros. The binary in my target went from less than 6kb to more than 14kb. Using build flags is probably the best solution, but my skill issue doesn't allow me to do this properly. |
afetch: add new package (2.2.0)
Fast and simple system info (for UNIX based operating systems)
written in POSIX compliant C99.
Link: https://github.com/13-CF/afetch
Signed-off-by: Tiago Rocha tiagorocha@disroot.org