Skip to content

alfred: clean up Makefile - #1192

Merged
BKPepe merged 2 commits into
openwrt:masterfrom
BKPepe:alfred-makefile-cleanup
Aug 10, 2026
Merged

alfred: clean up Makefile#1192
BKPepe merged 2 commits into
openwrt:masterfrom
BKPepe:alfred-makefile-cleanup

Conversation

@BKPepe

@BKPepe BKPepe commented Aug 6, 2026

Copy link
Copy Markdown
Member

Cleanup towards moving the package to openwrt/packages (#184):

  • Install binaries with INSTALL_BIN instead of cp -fpR.
  • Replace [ "x$(CONFIG_...)" == "xy" ] && ... ; true shell constructs (the == operator is a bashism) with make-level $(if ...) conditionals.
  • Drop the explicit dependency on libc, which every package depends on implicitly.
  • Indent MAKE_FLAGS consistently with tabs.

No functional change in the resulting package.

Maintainer: @simonwunderlich

Copilot AI lite review requested due to automatic review settings August 6, 2026 07:54
@openwrt openwrt Bot added the not following guidelines Pull request does not follow formatting guidelines label Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@BKPepe
BKPepe force-pushed the alfred-makefile-cleanup branch from 316a7e7 to b2b255a Compare August 6, 2026 08:02
@openwrt openwrt Bot removed the not following guidelines Pull request does not follow formatting guidelines label Aug 6, 2026
@ecsv

ecsv commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@BKPepe I think we had that before. @simonwunderlich is the maintainer. And i can't merge anything here.

@ecsv ecsv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seem to be fine

- Install binaries with INSTALL_BIN instead of `cp -fpR`, which is
  the standard way to install executables with defined permissions.
- Replace the `[ "x$(CONFIG_...)" == "xy" ] && ... ; true` shell
  constructs with make-level $(if ...) conditionals. The `==`
  operator is a bashism and the shell round-trip is not needed to
  decide at install time whether an optional file is packaged.
- Drop the explicit dependency on libc, which every package depends
  on implicitly.
- List the optionally installed /etc/alfred/bat-hosts.lua in
  conffiles, so local modifications survive upgrades.
- Indent MAKE_FLAGS consistently with tabs.

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@BKPepe
BKPepe force-pushed the alfred-makefile-cleanup branch from b2b255a to 7bf37fb Compare August 6, 2026 11:57
get_interface_address() calls io.open() and dereferences the result
without checking it. /sys/class/net does not contain interfaces
only: with the bonding module loaded it also holds the plain file
bonding_masters, which the `ls -1` in get_interfaces_names() returns
and which has no address below it. io.open() then returns nil and
the script dies with "attempt to index a nil value", so no bat-hosts
data is published at all on such a node.

Return nil when the file cannot be opened and skip those entries in
the caller, which would otherwise index the interface table with
nil. get_hostname() gets the same guard.

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@BKPepe
BKPepe merged commit 7e5c48d into openwrt:master Aug 10, 2026
14 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.

3 participants