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.mk: include release in version number #7042

Merged
merged 1 commit into from Apr 13, 2024

Conversation

stangri
Copy link
Member

@stangri stangri commented Apr 3, 2024

Fixes issue mentioned in #7009 (comment) where the IPK filename doesn't contain release from the luci-app Makefile.

@stangri
Copy link
Member Author

stangri commented Apr 7, 2024

@jow- @neheb @aparcar @hnyman any feedback? This makes the versioning in IPK filenames consistent with the packages repo.

@jow-
Copy link
Contributor

jow- commented Apr 8, 2024

WIthout having runtime tested it, it looks good to me so far. It will only change filenames for Makefiles that do declare PKG_REVISION, right?

@stangri
Copy link
Member Author

stangri commented Apr 8, 2024

WIthout having runtime tested it, it looks good to me so far. It will only change filenames for Makefiles that do declare PKG_REVISION, right?

I've made the change to the luci.mk within the 23.05 SDK and it ran fine and produced IPK file with the desired filename.

Yes, if PKG_RELEASE is not declared, it should only use PKG_VERSION.

@hnyman
Copy link
Contributor

hnyman commented Apr 8, 2024

It will only change filenames for Makefiles that do declare PKG_REVISION, right?

Yes, if PKG_RELEASE is not declared, it should only use PKG_VERSION.

I guess that @jow- meant that it shouldn't affect the bulk of packages that are not using PKG_VERSION at all and just rely on the automatic versioning. (REVISION was likely misspelling of VERSION, not RELEASE, I think...)

The terminology in the PR is a bit confusing: the commit talks about "revision" but actually changes the handling of PKG_RELEASE. Please adjust.
Otherwise ok.

This whole issue concerns just a few packages...
I looked at the downloadable LuCI packages in master, and (excluding i18n) we have:

Makefiles using include luci.mk:

  • 149 packages in automatic LuCI versioning
  • 9 packages with only PKG_VERSION in Makefile (including yours)
  • 6 packages with PKG_VERSION and PKG_RELEASE, (but PKG_RELEASE currently hidden)
    Those 15 packages are the ones benefiting from this change

Normal Makefile:

  • 5 packages with normal Makefile and versioning
  • 2 packages with normal Makefile, but erroneously with only PKG_RELEASE (ucode-mod-...)
    (the two ucode-mod packages should probably be modified to have PKG_VERSION)

Packages deviating from the default LuCI versioning:

PKG_VERSION, no PKG_RELEASE (but might be part of PKG_VERSION string)
   luci-app-adblock-fast_1.1.1-r7_all.ipk
   luci-app-advanced-reboot_1.0.1-r10_all.ipk
   luci-app-pbr_1.1.4-r7_all.ipk
   luci-proto-nebula_1.8.2-r2_all.ipk
   luci-app-cloudflared_1.2_all.ipk
   luci-proto-yggdrasil_1.1.0_all.ipk
   rpcd-mod-rad2-enc_20190109_arm_cortex-a15_neon-vfpv4.ipk
   rpcd-mod-rrdns_20170710_arm_cortex-a15_neon-vfpv4.ipk
   luci-app-dockerman_v0.5.13-20240317_all.ipk
  
PKG_VERSION,   PKG_RELEASE DEFINED BUT HIDDEN
   luci-app-omcproxy_0.1.0_all.ipk
   luci-app-privoxy_1.0.6_all.ipk
   luci-app-radicale_1.1.0_all.ipk
   luci-app-smartdns_1.2023.42_all.ipk
   luci-app-sshtunnel_1.1.0_all.ipk
   luci-app-tor_1.1.0_all.ipk
  
NORMAL MAKEFILE
   rpcd-mod-luci_20240305-r1_arm_cortex-a15_neon-vfpv4.ipk
   csstidy_2021.06.13~707feaec-r1_arm_cortex-a15_neon-vfpv4.ipk
   liblucihttp-lua_2023.03.15~9b5b683f-r1_arm_cortex-a15_neon-vfpv4.ipk
   liblucihttp-ucode_2023.03.15~9b5b683f-r1_arm_cortex-a15_neon-vfpv4.ipk
   liblucihttp0_2023.03.15~9b5b683f-r1_arm_cortex-a15_neon-vfpv4.ipk
  
NORMAL MAKEFILE, BUT ONLY PKG_RELEASE
   ucode-mod-html_1_arm_cortex-a15_neon-vfpv4.ipk
   ucode-mod-lua_1_arm_cortex-a15_neon-vfpv4.ipk

Fixes issue mentioned in openwrt#7009 (comment)
where the IPK filename doesn't contain release from the luci-app Makefile.

Signed-off-by: Stan Grishin <stangri@melmac.ca>
@stangri
Copy link
Member Author

stangri commented Apr 8, 2024

The terminology in the PR is a bit confusing: the commit talks about "revision" but actually changes the handling of PKG_RELEASE. Please adjust.

Fixed in both commit message and PR.

I guess that @jow- meant that it shouldn't affect the bulk of packages that are not using PKG_VERSION at all and just rely on the automatic versioning. (REVISION was likely misspelling of VERSION, not RELEASE, I think...)

Yes, like the previous code, it should only affect the version if the PKG_VERSION is set, otherwise it only uses PKG_SRC_VERSION.

This whole issue concerns just a few packages...

That was the reason I was using the PKG_VERSION which also contained RELEASE in them in a few of the luci apps, now I can switch back to the same PKG_VERSION/PKG_RELEASE structure in luci apps as used in principal packages.

@stangri stangri changed the title luci.mk: include revision in version number luci.mk: include release in version number Apr 12, 2024
@stangri
Copy link
Member Author

stangri commented Apr 12, 2024

@jow- @hnyman please merge if no further feedback.

@hnyman hnyman merged commit 10be7a3 into openwrt:master Apr 13, 2024
2 of 3 checks passed
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.

None yet

3 participants