-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Do not fail if libstdbuf was not built (not only for Windows) #8770
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
Conversation
| mkdir -p $(DESTDIR)$(LIBSTDBUF_DIR) | ||
| $(INSTALL) -m 755 $(BUILDDIR)/deps/libstdbuf* $(DESTDIR)$(LIBSTDBUF_DIR)/ | ||
| endif | ||
| # Do not fail if libstdbuf was not built (e.g. on Windows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you want to do it the same way I've done it above in the build step?
ifneq ($(findstring stdbuf,$(UTILS)),)
mkdir -p $(DESTDIR)$(LIBSTDBUF_DIR)
$(INSTALL) -m 755 $(BUILDDIR)/deps/libstdbuf* $(DESTDIR)$(LIBSTDBUF_DIR)/
endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I think a call to make install should be added to
coreutils/.github/workflows/CICD.yml
Line 395 in 5c15d79
| make UTILS="rm chmod chown chgrp mv du" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work on SELINUX_ENABLED=1 SKIP_UTILS=stdbuf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the question. what do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand what you mean now. It should be possible to make it work. Can the value of "SKIP_UTILS" not be removed from "UTILS" before we evaluate it at line 226?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the value of "SKIP_UTILS" not be removed from "UTILS" before we evaluate it at line 226?
I agree with it.
Apart from, I met missing libstdbuf when I tried to fix #8767 and considered that stdbuf is not impremented yet for SELinux build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is existense of stdbuf binary also means libstdbuf* at all OS (except for WIndows)? If so, I use existense of stdbuf binary instead.
Also I don't see any reason to avoid installing libstdbuf.dll (not implemented yet?) for simplity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is existense of
stdbufbinary also meanslibstdbuf*at all OS (except for WIndows)? If so, I use existense ofstdbufbinary instead.Also I don't see any reason to avoid installing
libstdbuf.dll(not implemented yet?) for simplity.
-
Well, normally yes, because gnuMakefile enables feat_external_libstdbuf.
-
you can remove the "if windows " for simplicity, even though stdbuf will probably never be implemented on windows
|
GNU testsuite comparison: |
CodSpeed Performance ReportMerging #8770 will not alter performanceComparing Summary
Footnotes
|
|
This looks incorrect solution for SELinux things. |
|
@oech3 I am not sure why you closed this PR. Is it OK if I send one PR as follow-up to fix the libstdbuf installation issue when calling e.g. "make UTILS=ls"? |
|
Maybe, OK. |
OK, I have opened #8782 |
|
@Ecordonnier Is |
LD_PRELOAD does work on Linux. Since the goal of the project is to be a full replacement for GNU coreutils, we can't deprecate stdbuf. It is expected that ldd does not show libstdbuf, since it is loaded at runtime using LD_PRELOAD. |
|
It there any external project using |
|
I was misunderstaiding how is it used. Statically including it is impossible. |
I don't think GNU would drop it without a very good reason. Dropping it would break shell scripts using stdbuf. |
Do not install
libstdbuf*and fail for all OS (not only for Windows).Needed for #8767