Skip to content

Honor $(STRIP) in install-strip for cross-compilation - #1024

Merged
steadytao merged 2 commits into
RsyncProject:masterfrom
alessandrodn:fix-install-strip-cross-strip-program
Aug 16, 2026
Merged

Honor $(STRIP) in install-strip for cross-compilation#1024
steadytao merged 2 commits into
RsyncProject:masterfrom
alessandrodn:fix-install-strip-cross-strip-program

Conversation

@alessandrodn

@alessandrodn alessandrodn commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

The install-strip target hard-coded install -s, which strips via the install program using the build host's strip and ignores the STRIP variable, and rsync's configure.ac had no check to detect a cross strip. When cross-compiling, make install-strip therefore ran the host strip against a target-architecture binary and failed.

This makes install-strip portable and cross-aware:

  • configure.ac: add AC_CHECK_TOOL([STRIP], [strip], [strip]), which detects the target-prefixed strip (e.g. aarch64-linux-gnu-strip) when cross-compiling, falls back to plain strip for native builds, and substitutes @STRIP@.
  • Makefile.in: install-strip now performs a normal make install and then runs $(STRIP) on the installed rsync binary.

This replaces an earlier attempt that used install --strip-program=$(or $(STRIP),strip), which assumed GNU Make ($(or ...)) and GNU coreutils install (--strip-program) — neither guaranteed, since AC_PROG_INSTALL may select the bundled install-sh or a BSD install. The new approach depends only on POSIX Make and a strip binary. A plain make install is unaffected.

Verified in a clean Alpine/musl container: a native make install-strip strips the installed binary (1.9 MB → 607 KB), the detected STRIP is honored, and an explicit make install-strip STRIP=... override is respected (as cross toolchains / Conan / Buildroot / Yocto provide).

The install-strip target hard-coded `install -s`, which strips via the
install program using the build host's strip and ignores the STRIP
variable. When cross-compiling this runs the host strip against a
target binary and fails.

Pass --strip-program=$(or $(STRIP),strip) so the target strip is used
when STRIP is set (as cross toolchains and build systems provide),
falling back to plain `strip` for native builds. A plain `make install`
is unaffected.
Comment thread Makefile.in Outdated
- Detect the target strip via AC_CHECK_TOOL([STRIP],[strip],[strip]) in
  configure.ac (picks up the cross-prefixed strip when cross-compiling,
  defaults to plain strip otherwise) and substitute @strip@ in Makefile.in.
- Rewrite install-strip to run a normal install then $(STRIP) on the
  installed rsync binary, dropping the GNU Make $(or ...) and the GNU
  install --strip-program extension that broke with install-sh/BSD install.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alessandrodn added a commit to alessandrodn/conan-center-index that referenced this pull request Jul 13, 2026
Rework the install-strip patch to match the revised upstream fix
(RsyncProject/rsync#1024): instead of the GNU Make $(or ...) and GNU
install --strip-program extension (unsupported by install-sh / BSD
install), install normally then run $(STRIP) on the installed binary.

Add STRIP ?= strip to Makefile.in so the strip comes from the
environment/toolchain when set (e.g. a cross profile's [buildenv] STRIP
or the Android NDK strip) and falls back to plain strip for native
builds. rsync 3.2.7's shipped configure does not substitute @strip@ and
the recipe does not run autoreconf, so ?= (env wins) is used rather than
@strip@.
@alessandrodn
alessandrodn requested a review from steadytao July 13, 2026 12:14
@steadytao

Copy link
Copy Markdown
Member

Cheers, that addresses my portability concern. I'll add the native and cross-strip coverage then give this another passover.

@steadytao steadytao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR.

@steadytao

steadytao commented Jul 16, 2026

Copy link
Copy Markdown
Member

@tridge. See #1027 as well.

@steadytao steadytao added bug Something isn't working NeedsTest labels Jul 16, 2026
@alessandrodn

Copy link
Copy Markdown
Contributor Author

@steadytao @tridge is there anything I can help with to merge this?

@steadytao

Copy link
Copy Markdown
Member

Apologies on the slow reply. We are currently focusing on our security release (3.5.0). We are quite stretched on review time and will get to this once available.

@alessandrodn

Copy link
Copy Markdown
Contributor Author

Thanks for the update @steadytao.
It would be great if this can be part of 3.5.0 as well.
As said, I'm available to support, let me know if I can help with the tests, for example.

@steadytao
steadytao merged commit a49f085 into RsyncProject:master Aug 16, 2026
@alessandrodn
alessandrodn deleted the fix-install-strip-cross-strip-program branch August 16, 2026 12:48
alessandrodn added a commit to alessandrodn/conan-center-index that referenced this pull request Aug 16, 2026
rsync's install-strip target hard-codes `install -s`, which strips using
the build host's strip and ignores the STRIP variable (rsync's configure
has no AC_CHECK_TOOL([STRIP])). When cross-compiling with
tools.build:install_strip=True this runs the host strip against a
target-architecture binary and fails.

Add a patch passing --strip-program=$(or $(STRIP),strip) so the target
strip is used when STRIP is set, falling back to plain strip for native
builds.

Submitted upstream: RsyncProject/rsync#1024
alessandrodn added a commit to alessandrodn/conan-center-index that referenced this pull request Aug 16, 2026
Rework the install-strip patch to match the revised upstream fix
(RsyncProject/rsync#1024): instead of the GNU Make $(or ...) and GNU
install --strip-program extension (unsupported by install-sh / BSD
install), install normally then run $(STRIP) on the installed binary.

Add STRIP ?= strip to Makefile.in so the strip comes from the
environment/toolchain when set (e.g. a cross profile's [buildenv] STRIP
or the Android NDK strip) and falls back to plain strip for native
builds. rsync 3.2.7's shipped configure does not substitute @strip@ and
the recipe does not run autoreconf, so ?= (env wins) is used rather than
@strip@.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working NeedsTest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants