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

luci-mod-status: firmware version should include revision number #7343

Closed
dannil opened this issue Oct 23, 2024 · 18 comments
Closed

luci-mod-status: firmware version should include revision number #7343

dannil opened this issue Oct 23, 2024 · 18 comments

Comments

@dannil
Copy link
Contributor

dannil commented Oct 23, 2024

Steps to reproduce:

  1. Go to: Status -> Overview
  2. Notice that the Firmware version field does not include a revision number.

Actual behavior:

  • Firmware version doesn't include the revision number.

Expected behavior:

  • Firmware version should include the revision number.

Additional Information:

The output of ubus call system board was changed in openwrt/procd@0f88a52 which introduced this problem. First discovered and discussed on the forum at https://forum.openwrt.org/t/strange-firmware-version-string/213745

Here's the SNAPSHOT r27893 at point in time of writing this which doesn't work:

{
        "kernel": "6.6.57",
        "hostname": "labmouse",
        "system": "AMD Ryzen 7 3700X 8-Core Processor",
        "model": "VMware, Inc. VMware Virtual Platform",
        "board_name": "vmware-inc-vmware-virtual-platform",
        "rootfs_type": "ext4",
        "release": {
                "distribution": "OpenWrt",
                "version": "SNAPSHOT",
                "description": "OpenWrt SNAPSHOT",
                "revision": "r27893-b6bbc76c0b",
                "target": "x86/64",
                "builddate": "1729635115"
        }
}

Screenshot_10

and here's a device I run r27687 on which does work

{
        "kernel": "6.6.54",
        "hostname": "JetFuel",
        "system": "ARMv8 Processor rev 4",
        "model": "Linksys MX4200v1",
        "board_name": "linksys,mx4200v1",
        "rootfs_type": "squashfs",
        "release": {
                "distribution": "OpenWrt",
                "version": "SNAPSHOT",
                "revision": "r27687-b62e6f5beb",
                "target": "qualcommax/ipq807x",
                "description": "OpenWrt SNAPSHOT r27687-b62e6f5beb"
        }
}

Screenshot_11

DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='SNAPSHOT'
DISTRIB_REVISION='r27893-b6bbc76c0b'
DISTRIB_TARGET='x86/64'
DISTRIB_ARCH='x86_64'
DISTRIB_DESCRIPTION='OpenWrt SNAPSHOT r27893-b6bbc76c0b'
DISTRIB_TAINTS=''
@dannil dannil changed the title luci-mod-status: missing revision number for SNAPSHOT luci-mod-status: firmware version should include revision number Oct 23, 2024
@dannil
Copy link
Contributor Author

dannil commented Oct 23, 2024

To get to parity as before I assume we would need to change boardinfo.release.description to boardinfo.release.description + ' ' + boardinfo.release.revision at

_('Firmware Version'), (L.isObject(boardinfo.release) ? boardinfo.release.description + ' / ' : '') + (luciversion || ''),
I'm a bit unsure where the stable versions end up but if it's baked in to the description as "description": "OpenWrt 24.10" (assuming that'll be the next stable version) that should work.

@systemcrash
Copy link
Contributor

Yeah.👍 if we depend on the new file, this is likely a good update. Feeling up for a PR?

@jow-
Copy link
Contributor

jow- commented Oct 23, 2024

Maybe the OpenWrt side change should be fixed, the commit sort of implies that the intention was to provide the same data and there's probably other downstream users affected by this as well.

systemcrash referenced this issue in openwrt/procd Oct 23, 2024
The information required for output via the ubus is currently read from
the file '/etc/openwrt_release'. The same information can also be found
under '/usr/lib/os-release'. This file contains further information and is
also the most common used file in other Linux distributions.

So let´s use this file.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
@feckert
Copy link
Member

feckert commented Oct 24, 2024

Unfortunately, I didn't know that information was missing here.
The question is whether I should fix this in the procd as suggested by @jow- or in the LuCI as suggested by @dannil.

I would fix this in the procd. And would replace PRETTY_NAME with the information from OPENWRT_RELESE ?
Does the value CONFIG_VERSION_CODE is having the correct value SNAPSHOT?

I don't have access to any system at the moment as I'm travelling. So can someone send me the output of /usr/lib/os-release and /etc/openwrt_release, then we can compare and check what the correct value is.

@feckert
Copy link
Member

feckert commented Oct 24, 2024

Now I was too hasty. I have just seen that these two values DISTRIB_DESCRIPTION in /etc/openwrt_release and OPENWRT_RELEASE in /usr/lib/os-release are the same, so I will create a Patch and use OPENWRT_RELEASE in procd to fix this.

@dannil
Copy link
Contributor Author

dannil commented Oct 24, 2024

I don't have access to any system at the moment as I'm travelling. So can someone send me the output of /usr/lib/os-release and /etc/openwrt_release, then we can compare and check what the correct value is.`

@feckert if you still need it.

root@labmouse:~# cat /usr/lib/os-release
NAME="OpenWrt"
VERSION="SNAPSHOT"
ID="openwrt"
ID_LIKE="lede openwrt"
PRETTY_NAME="OpenWrt SNAPSHOT"
VERSION_ID="snapshot"
HOME_URL="https://openwrt.org/"
BUG_URL="https://bugs.openwrt.org/"
SUPPORT_URL="https://forum.openwrt.org/"
BUILD_ID="r27893-b6bbc76c0b"
OPENWRT_BOARD="x86/64"
OPENWRT_ARCH="x86_64"
OPENWRT_TAINTS=""
OPENWRT_DEVICE_MANUFACTURER="OpenWrt"
OPENWRT_DEVICE_MANUFACTURER_URL="https://openwrt.org/"
OPENWRT_DEVICE_PRODUCT="Generic"
OPENWRT_DEVICE_REVISION="v0"
OPENWRT_RELEASE="OpenWrt SNAPSHOT r27893-b6bbc76c0b"
OPENWRT_BUILD_DATE="1729635115"

root@labmouse:~# cat /etc/openwrt_release
DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='SNAPSHOT'
DISTRIB_REVISION='r27893-b6bbc76c0b'
DISTRIB_TARGET='x86/64'
DISTRIB_ARCH='x86_64'
DISTRIB_DESCRIPTION='OpenWrt SNAPSHOT r27893-b6bbc76c0b'
DISTRIB_TAINTS=''

@feckert
Copy link
Member

feckert commented Oct 25, 2024

@feckert feckert closed this as completed Oct 25, 2024
@systemcrash
Copy link
Contributor

I'm going to leave the issue open for visibility - at least until the patch is merged.

@systemcrash systemcrash reopened this Oct 25, 2024
systemcrash referenced this issue Oct 26, 2024
Signed-off-by: Paul Donald <newtwen+github@gmail.com>
@systemcrash
Copy link
Contributor

@feckert did you also send a PR to repo also?

@dannil
Copy link
Contributor Author

dannil commented Nov 1, 2024

24.10 has now been branched without this fix being merged to procd and at least since 19.07 it hasn't had stable branches so backporting doesn't seem trivial in that repo. How do we go about solving this issue now?

@systemcrash
Copy link
Contributor

Stay the course. There are a few rc to go yet.

@systemcrash
Copy link
Contributor

@feckert any update on the patch?

@feckert
Copy link
Member

feckert commented Nov 6, 2024

@feckert did you also send a PR to repo also?

No. I only send this to patchwork

@feckert any update on the patch?

No. Sorry

@systemcrash Lets highlight the core maintainer to merge the procd fix for this issue.

@jow- @hauke Could anyone merge this into procd? To Fix this issue?

@Ansuel
Copy link
Member

Ansuel commented Nov 6, 2024

@feckert I will take care of that. Just one question the PRETTY_NAME was never used? It got dropped?

@feckert
Copy link
Member

feckert commented Nov 6, 2024

@feckert I will take care of that. Just one question the PRETTY_NAME was never used? It got dropped?

@Ansuel Correct PRETTY_NAME was never used in the old implementation.

Before my change procd reads the variable DISTRIB_DESCRIPTION from /etc/openwrt_release.
After my change procd reads the varialbe PRETTY_NAME from /usr/lib/os-release.

This is not correct. Unfortunately, I overlooked that. The same information can be found in the variable OPENWRT_RELEASE in /usr/lib/os-release. The patch for procd fixes this. So we have the same information available as before.

@Ansuel
Copy link
Member

Ansuel commented Nov 6, 2024

Fixed procd pushed. This will be magically fixed once updated packages gets built.

@Ansuel Ansuel closed this as completed Nov 6, 2024
@feckert
Copy link
Member

feckert commented Nov 6, 2024

@Ansuel Thank you for taking care of this.

@systemcrash
Copy link
Contributor

Thanks @Ansuel

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

No branches or pull requests

5 participants