Skip to content

Conversation

the-horo
Copy link
Contributor

No description provided.

Signed-off-by: Andrei Horodniceanu <[email protected]>
@the-horo the-horo marked this pull request as draft August 18, 2025 12:16
@the-horo
Copy link
Contributor Author

Let's see if there's anything interesting happening with druntime-test-exceptions-release

@kinke
Copy link
Member

kinke commented Aug 18, 2025

Hmm, looks like the non-musl tests are wrongly excluded?

ifneq ($(IS_MUSL),1)
TESTS+=memoryerror_null_read memoryerror_null_write memoryerror_null_call memoryerror_stackoverflow
endif

Maybe some weird Make behavior if the variable isn't defined?

@kinke
Copy link
Member

kinke commented Aug 18, 2025

Or the left-over auto-detection in the Makefile now breaks it because the GHA Linux images might come with an unexpected apk tool now?

ifeq ($(OS),linux)
# FIXME: detect musl libc robustly; just checking Alpine Linux' apk tool for now
ifeq (1,$(shell which apk &>/dev/null && echo 1))
IS_MUSL:=1
endif
endif

@kinke
Copy link
Member

kinke commented Aug 18, 2025

AFAICT from the CI logs, it's got to be the latter - unknown_gc (excluded with IS_MUSL=1) is included on macOS, but not glibc Linux:

ifneq ($(IS_MUSL),1)
TESTS += unknown_gc
endif

Signed-off-by: Andrei Horodniceanu <[email protected]>
Signed-off-by: Andrei Horodniceanu <[email protected]>
@the-horo
Copy link
Contributor Author

Or the left-over auto-detection in the Makefile now breaks it because the GHA Linux images might come with an unexpected apk tool now?

It doesn't seem apk exists by default. Let's see if it somehow pops up by the time the tests are run

@the-horo
Copy link
Contributor Author

Or the left-over auto-detection in the Makefile now breaks it because the GHA Linux images might come with an unexpected apk tool now?

It doesn't seem apk exists by default. Let's see if it somehow pops up by the time the tests are run

Nope, it's still not there. But the tests "detect" it anyways. Is the make shell function doing something we don't expect?

@the-horo
Copy link
Contributor Author

Alright, I genuinely don't have any idea how this is possible:

$(info $(shell which apk ; echo $$?))
$(info $(shell which apk && echo 1 ; echo $$?))
$(info $(shell command -v apk ; echo $$?))
$(info $(shell command -v apk && echo 1 ; echo $$?))
ifeq ($(OS),linux)
# FIXME: detect musl libc robustly; just checking Alpine Linux' apk tool for now
ifeq (1,$(shell command -v apk &>/dev/null && echo 1))
$(warning ~~~ musl detected)
IS_MUSL:=1
else
$(warning ~~~ musl NOT detected)
endif
endif

  • linux-aarch64
1817: 1
1817: 1
1817: 127
1817: 127
1817: Makefile:14: ~~~ musl detected
  • alpine
1822: /sbin/apk 0
1822: /sbin/apk 1 0
1822: /sbin/apk 0
1822: /sbin/apk 1 0
1822: Makefile:14: ~~~ musl detected

@the-horo
Copy link
Contributor Author

  • linux aarch64

$(info $(shell which apk &>/dev/null; echo $$?))
$(info $(shell which apk &>/dev/null && echo 1 ; echo $$?))
$(info $(shell command -v apk &>/dev/null ; echo $$?))
$(info $(shell command -v apk &>/dev/null && echo 1 ; echo $$?))

1817: 0
1817: 1 0
1817: 0
1817: 1 0
1817: Makefile:15: ~~~ musl 1

Why is &> changing the exit status???

Signed-off-by: Andrei Horodniceanu <[email protected]>
@the-horo
Copy link
Contributor Author

/bin/sh is dash in the runner, which doesn't understand &>.


There's already

set(musl "")
if(TARGET_SYSTEM MATCHES "musl")
set(musl "IS_MUSL=1")
endif()
we can just use that, instead of looking for apk. Does that sound good @kinke?

@kinke
Copy link
Member

kinke commented Aug 18, 2025

Oh god; thx for digging!

we can just use that, instead of looking for apk.

Yeah, we here for LDC can, but I think I've added this apk-logic upstream when adding the Alpine CI job (for DMD). So we could skip the logic for IN_LDC, but DMD upstream needs a solution.

kinke added a commit to kinke/dmd that referenced this pull request Aug 25, 2025
As dash apparently doesn't understand the `&>` redirection, see
ldc-developers/ldc#4970 (comment).
kinke added a commit to kinke/dmd that referenced this pull request Aug 25, 2025
As dash apparently doesn't understand the `&>` redirection, see
ldc-developers/ldc#4970 (comment),
causing IS_MUSL to be wrongly set on non-musl distros.
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.

2 participants