Bug 5237: "make check" fails when ${TRUE} is a multicall binary by renanrodrigo · Pull Request #2442 · squid-cache/squid

renanrodrigo · 2026-06-09T21:22:02Z

Authored-by: Renan Rodrigo rr@ubuntu.com

testHeaders copies the 'true' binary to a file named testHeaders relying
on the fact that GNU's true always exits 0. When 'true' implementation
is a multicall binary (e.g., in rust-coreutils and on Alpine Linux), it
cannot be called by other name. Create an empty executable file instead.

squid-conf-tests also copied the 'true' binary, but that Makefile target
is never executed and, hence, should not create an executable file.

yadij · 2026-06-10T03:59:07Z

Polished the description to fit Anubis requirements. No Substantive change.
Marking for review by myself as I want to look into the side effects of ':' command deeper - we had a lot of portability issues revealed while settling on use of $(TRUE) previously.

yadij

Okay, the ':' command redirection support checks out as good for bash, dash, ksh and csh (for Linux, BSD, and ARM devices). I am not able to test MinGW, Cygwin or MacOS shells specifically for redirection but they should also be okay since we already know they support the command itself.

kinkie

output redirection is done by the calling shell, there's no reason it shouldn't work.
MacOs' shell is either bash or zsh, also not concerned about it.
Good catch!

rousskov · 2026-06-11T07:52:10Z

 		else break; fi; \
 	done; \
-	if test "$$failed" -eq 0; then cp $(TRUE) $@ ; else exit 1; fi
+	if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi


Do we need to make $@ (i.e. squid-conf-tests) executable? Sorry, I cannot easily check this myself right now.

If the answer is "no", then should we switch from rather unusual/obscure redirection code that creates an empty executable file to simple and well-known touch that we already use in, for example, errors/Makefile.am?

We do, because it is executed by other parts of automake. That is why it is true in the first place instead of just a touch (been there tried that years ago).

How about using install -m 775 /dev/null $@ to avoid the unusual redirection then? Assuming true is around, install should also be since it is part of coreutils as well.

i.e., s;cp $(TRUE);install -m 775 /dev/null/; instead of : > $@ && chmod +x

We do, because it is executed by other parts of automake.

Which one? AFAICT, no part requires that squid-conf-tests file is executable, and no part executes that file. Replacing our cp with touch worked in my quick-and-dirty test.

(been there tried that years ago).

Can you give a specific reference? FWIW, there are other use cases where we do execute the copied binary (e.g., testHeaders), but this question is specific to squid-conf-tests.

It seems @rousskov is right. This specific line is not building a target listed in TESTS, so it specifically does not need to be executable.

I have now run a complete re-test and the below change works well and simpler:

Suggested change

if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi

test "$$failed" -eq 0 && touch $@ || exit 1

Please keep if...else structure instead of switching to an a && b || c puzzle.

Off topic: @rousskov if that is such a "puzzle" why have you repeatedly asked me to implement it in other scripts instead of allowing if..else I design?

FWIW I have tested it too, and from my results and at least the testHeaders file needs to be executable.
Just touching the file I get:

FAIL: testHeaders ================= ../cfgaux/test-driver: line 119: ./testHeaders: Permission denied FAIL testHeaders (exit status: 126)

Amos: Off topic: @rousskov if that is such a "puzzle" why have you repeatedly asked me to implement it in other scripts instead of allowing if..else I design?

I have not.

@renanrodrigo: FWIW I have tested it too, and from my results and at least the testHeaders file needs to be executable.

I am sorry you had wasted your time on this. My change request is about squid-conf-tests. It is not (and has never been) about testHeaders.

AFAICT, the minimal fix that would address this change request is the following (untested by me) change:

Suggested change

if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi

if test "$$failed" -eq 0; then touch $@ ; else exit 1; fi

FWIW, we already use the same approach for two similar errors/Makefile.am targets: translate-warn and .po.lang.

yadij · 2026-06-12T10:29:29Z

output redirection is done by the calling shell, there's no reason it shouldn't work. MacOs' shell is either bash or zsh, also not concerned about it. Good catch!

Thanks for that info. I have now also tested zsh - it does work as needed.

rousskov · 2026-06-16T13:12:31Z

 		else break; fi; \
 	done; \
-	if test "$$failed" -eq 0; then cp $(TRUE) $@ ; else exit 1; fi
+	if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi


Amos: Off topic: @rousskov if that is such a "puzzle" why have you repeatedly asked me to implement it in other scripts instead of allowing if..else I design?

I have not.

@renanrodrigo: FWIW I have tested it too, and from my results and at least the testHeaders file needs to be executable.

I am sorry you had wasted your time on this. My change request is about squid-conf-tests. It is not (and has never been) about testHeaders.

AFAICT, the minimal fix that would address this change request is the following (untested by me) change:

Suggested change

if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi

if test "$$failed" -eq 0; then touch $@ ; else exit 1; fi

FWIW, we already use the same approach for two similar errors/Makefile.am targets: translate-warn and .po.lang.

renanrodrigo · 2026-06-16T13:24:39Z

@rousskov not a waste of time at all, just a double check (part of $dayjob anyway)

I will apply your suggested change both to this PR and to the Ubuntu patch.

Thanks for the review!

rousskov

Please add your entry to CONTRIBUTORS. See the diff inside the failed CI test log for the right place to insert that entry.

rousskov · 2026-06-16T13:45:57Z


 testHeaders: $(SOURCES) $(noinst_HEADERS) $(EXTRA_DIST) $(top_srcdir)/test-suite/testHeader.cc.in
-	$(MAKE) $(^:.h=.hdrtest) && cp $(TRUE) $@
+	$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@


@kinkie, will executing an empty file fail on native Windows builds? A "yes" answer is not necessarily a showstopper for this PR, especially if this testHeaders target is currently broken on native Windows builds for other reasons. Is it currently broken?

rousskov · 2026-06-16T14:42:59Z


 testHeaders: $(SOURCES) $(noinst_HEADERS) $(EXTRA_DIST) $(top_srcdir)/test-suite/testHeader.cc.in
-	$(MAKE) $(^:.h=.hdrtest) && cp $(TRUE) $@
+	$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@


Should we use the chmod command found by AC_PATH_PROG in our ./configure?

Suggested change

$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@

$(MAKE) $(^:.h=.hdrtest) && : > $@ && $(CHMOD) +x $@

or, if we also prefer to use touch for file creation:

Suggested change

$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@

$(MAKE) $(^:.h=.hdrtest) && touch $@ && $(CHMOD) +x $@

FWIW, I recommend the latter because we use touch for similar purposes elsewhere and because many readers will find it easier to grok than the : redirection trick.

rousskov · 2026-06-16T15:07:51Z

I will apply your suggested change both to this PR and to the Ubuntu patch.

Thank you. I adjusted PR description to explain squid-conf-tests changes and to mention Alpine Linux as another environment where this fix is needed. I also adjusted PR title to refer to the original Bugzilla report about this bug. PR title and description will form a commit message when this PR is merged by Anubis...

testHeaders copies the 'true' binary to a file named testHeaders relying on the fact that GNU's true always exits 0. When 'true' implementation is a multicall binary (e.g., in rust-coreutils and on Alpine Linux), it cannot be called by other name. Create an empty executable file instead. squid-conf-tests also copied the 'true' binary, but that Makefile target is never executed and, hence, should not create an executable file.

renanrodrigo · 2026-06-18T11:24:08Z

Just updated the changeset - please let me know if this is good now !

rousskov

Thank you for polishing this up!

rousskov

Please note that there is no need to squash fixup commits (and force push). Our merge bot will do that for you.

testHeaders copies the 'true' binary to a file named testHeaders relying on the fact that GNU's true always exits 0. When 'true' implementation is a multicall binary (e.g., in rust-coreutils and on Alpine Linux), it cannot be called by other name. Create an empty executable file instead. squid-conf-tests also copied the 'true' binary, but that Makefile target is never executed and, hence, should not create an executable file.

squidadm · 2026-06-19T06:19:27Z

queued for backport to v7

squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Jun 9, 2026

This comment was marked as resolved.

Sign in to view

squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Jun 9, 2026

yadij self-requested a review June 10, 2026 03:55

rousskov requested changes Jun 10, 2026

View reviewed changes

Comment thread test-suite/Makefile.am Outdated

renanrodrigo force-pushed the fix-true-test branch from b564217 to 612cb57 Compare June 10, 2026 11:33

yadij previously approved these changes Jun 11, 2026

View reviewed changes

yadij requested a review from rousskov June 11, 2026 04:59

yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box backport-to-v7 maintainer has approved these changes for v7 backporting labels Jun 11, 2026

kinkie previously approved these changes Jun 11, 2026

View reviewed changes

rousskov requested changes Jun 11, 2026

View reviewed changes

rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Jun 11, 2026

yadij requested a review from rousskov June 12, 2026 11:39

rousskov requested changes Jun 16, 2026

View reviewed changes

rousskov changed the title ~~Fix tests when using rust-coreutils (#5542)~~ Bug 5237: "make check" fails when ${TRUE} is a multicall binary Jun 16, 2026

rousskov reviewed Jun 16, 2026

View reviewed changes

renanrodrigo dismissed stale reviews from kinkie and yadij via 85d9a57 June 18, 2026 11:20

renanrodrigo force-pushed the fix-true-test branch from 612cb57 to 85d9a57 Compare June 18, 2026 11:20

renanrodrigo force-pushed the fix-true-test branch from 85d9a57 to ec1e2f8 Compare June 18, 2026 11:21

rousskov previously approved these changes Jun 18, 2026

View reviewed changes

Comment thread CONTRIBUTORS Outdated

Add myself as a contributor

1f1a494

renanrodrigo dismissed rousskov’s stale review via 1f1a494 June 18, 2026 15:04

renanrodrigo force-pushed the fix-true-test branch from ec1e2f8 to 1f1a494 Compare June 18, 2026 15:04

rousskov approved these changes Jun 18, 2026

View reviewed changes

Comment thread CONTRIBUTORS Outdated

rousskov added S-could-use-an-approval An approval may speed this PR merger (but is not required) M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Jun 18, 2026

yadij approved these changes Jun 19, 2026

View reviewed changes

yadij added S-could-use-an-approval An approval may speed this PR merger (but is not required) and removed S-could-use-an-approval An approval may speed this PR merger (but is not required) labels Jun 19, 2026

squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Jun 19, 2026

squid-anubis closed this Jun 19, 2026

squidadm removed the backport-to-v7 maintainer has approved these changes for v7 backporting label Jun 19, 2026

	if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi
	test "$$failed" -eq 0 && touch $@ \|\| exit 1

	if test "$$failed" -eq 0; then : > $@ && chmod +x $@ ; else exit 1; fi
	if test "$$failed" -eq 0; then touch $@ ; else exit 1; fi

	$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@
	$(MAKE) $(^:.h=.hdrtest) && : > $@ && $(CHMOD) +x $@

	$(MAKE) $(^:.h=.hdrtest) && : > $@ && chmod +x $@
	$(MAKE) $(^:.h=.hdrtest) && touch $@ && $(CHMOD) +x $@

Conversation

renanrodrigo commented Jun 9, 2026 • edited by rousskov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

yadij commented Jun 10, 2026

Uh oh!

Uh oh!

yadij left a comment

Choose a reason for hiding this comment

Uh oh!

kinkie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yadij commented Jun 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renanrodrigo commented Jun 16, 2026

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rousskov Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rousskov commented Jun 16, 2026

Uh oh!

renanrodrigo commented Jun 18, 2026

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

squidadm commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

renanrodrigo commented Jun 9, 2026 •

edited by rousskov

Loading

rousskov Jun 16, 2026 •

edited

Loading