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.
| define Package/afetch/description | ||
| Fast and simple system info (for UNIX based operating systems) | ||
| written in POSIX compliant C99. | ||
| endef |
There was a problem hiding this comment.
Why do we need to have this in our repository? What is the usage? As it does not support OpenWrt given the patch, which you want to add here in downstream instead of adding it to upstream, I think majority of users will not use this.
There was a problem hiding this comment.
You're right, there is a real risk that afetch will no longer be maintained, it was precisely the upstream inactivity that led me to not send a patch there first. However, as the last 6 years have been insane, and many have stopped with personal projects, I believe it is worth giving more time for the upstream to return or fork to emerge. Of course there are other actively maintained fetches, but I believe they are too big and bloated for OpenWrt.
Regarding practical utility, afetch and similar tools should have no use other than being a fancy and cool way to share our setup. However, these tools are relatively popular among users of Linux and BSD distros.
Since there are some users who miss something like this in the official OpenWrt repositories, I thought I'd take the opportunity to give back and learn a little more about the project.
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>
51bed4a to
4fe6e2d
Compare
| PKG_VERSION:=2.2.0 | ||
| PKG_RELEASE:=1 | ||
|
|
||
| PKG_SOURCE:=afetch-$(PKG_VERSION).tar.gz |
There was a problem hiding this comment.
| PKG_SOURCE:=afetch-$(PKG_VERSION).tar.gz | |
| PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz |
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.
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