Skip to content

Conversation

Ecordonnier
Copy link
Contributor

@Ecordonnier Ecordonnier commented Jun 7, 2025

Fixes #6591

"feat_external_libstdbuf": use an external libstdbuf.so for stdbuf instead of embedding it into
the stdbuf binary.
There are 2 use-cases:

  1. Installation of uutils-coreutils using cargo install (e.g. from crates.io which supports only "cargo install" as installation method). In this case, installing libstdbuf.so is impossible, because "cargo install" installs only binary programs (no cdylib), thus libstdbuf.so must be embedded into stdbuf and written to /tmp at runtime. This is a hack, and may not work on some platforms, e.g. because the SELinux permissions may not allow stdbuf to write to /tmp, /tmp may be read-only, libstdbuf.so may not work at all without SELinux labels, etc.

  2. Installation of uutils-coreutils using an external tool, e.g. dpkg/apt on debian. In this case, libstdbuf.so should be installed separately to its correct location and the environment variable LIBSTDBUF_DIR configures the installation path during the build. E.g. LIBSTDBUF_DIR="/usr/lib"

@@ -3,6 +3,7 @@ linker = "x86_64-unknown-redox-gcc"

[env]
PROJECT_NAME_FOR_VERSION_STRING = "uutils coreutils"
LIBSTDBUF_PATH = "/usr/lib/libstdbuf.so"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I did not want to include this line, and wanted the build to fail per default when enabling feat_external_stdbuf, so that people who are packaging uutils-coreutils notice that they need to set this variable.
The issue is that the pre-commit hook calls clippy with "all-features".

Can I use something like this in the pre-commit file?

sh -c 'LIBSTDBUF_PATH=libstdbuf.so cargo +stable clippy --workspace --all-targets --all-features -- -D warnings'

I guess not, because the pre-commit hook is also supposed to work on windows?

Copy link

github-actions bot commented Jun 7, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@Ecordonnier Ecordonnier force-pushed the eco/stdbuf-external branch from e07d23d to 9f309b9 Compare June 10, 2025 08:34
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@Ecordonnier Ecordonnier force-pushed the eco/stdbuf-external branch from 9f309b9 to 8f1648d Compare June 12, 2025 14:49
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

@@ -3,6 +3,7 @@ linker = "x86_64-unknown-redox-gcc"

[env]
PROJECT_NAME_FOR_VERSION_STRING = "uutils coreutils"
LIBSTDBUF_DIR = "/usr/lib"
Copy link
Contributor

Choose a reason for hiding this comment

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

please add that it is explained in src/uu/stdbuf/Cargo.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sylvestre are there other things which I should change?

Fixes uutils#6591

 "feat_external_stdbuf": use an external libstdbuf.so for stdbuf instead of embedding it into
 the stdbuf binary.
 There are 2 use-cases:
 1. Installation of uutils-coreutils using cargo install (e.g. from crates.io
    which supports only "cargo install" as installation method).  In this case,
    installing libstdbuf.so is impossible, because "cargo install" installs
    only binary programs (no cdylib), thus libstdbuf.so must be embedded into
    stdbuf and written to /tmp at runtime.  This is a hack, and may not work
    on some platforms, e.g. because the SELinux permissions may not allow
    stdbuf to write to /tmp, /tmp may be read-only, libstdbuf.so may not work
    at all without SELinux labels, etc.

 2. Installation of uutils-coreutils using an external tool, e.g. dpkg/apt on
    debian. In this case, libstdbuf.so should be installed separately to its
    correct location and the environment variable LIBSTDBUF_PATH configures the
    installation path during the build. E.g. LIBSTDBUF_PATH="/lib/libstdbuf.so"

Signed-off-by: Etienne Cordonnier <[email protected]>
@Ecordonnier Ecordonnier force-pushed the eco/stdbuf-external branch from 8f1648d to 99aa51a Compare June 13, 2025 09:08
@Ecordonnier Ecordonnier changed the title stdbuf: add feat_external_stdbuf stdbuf: add feat_external_libstdbuf Jun 13, 2025
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@Ecordonnier Ecordonnier requested a review from sylvestre June 15, 2025 20:14
@sylvestre
Copy link
Contributor

ok, let's try with this

@sylvestre sylvestre merged commit ba1833d into uutils:main Jun 18, 2025
74 of 75 checks passed
@Ecordonnier Ecordonnier deleted the eco/stdbuf-external branch June 18, 2025 09:41
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.

Adding back support for an external libstdbuf.so
2 participants