Skip to content

Conversation

@kinke
Copy link
Contributor

@kinke kinke commented 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.

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.
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#21779"

@kinke kinke marked this pull request as ready for review August 25, 2025 13:29
kinke added a commit to the-horo/ldc that referenced this pull request Aug 25, 2025
Based on dlang/dmd#21779. For LDC, always
define IS_MUSL externally, not just to `1` for musl targets.
@the-horo
Copy link
Contributor

Note #21741

I think it's better to carry this musl detection in a common file like druntime/Makefile. This way we don't duplicate it in every test file that needs it. As a plus it also helps ldc by keeping its diff smaller since it uses a different mechanism for detecting musl. Changing it in one place (actually 0 places) is better than having to modify a bunch of different makefiles.

@kinke
Copy link
Contributor Author

kinke commented Aug 25, 2025

I'd have extracted it if a third file needs it. ;) - Unfortunately we cannot put it in existing common.mak, that's too late (as that include requires TESTS to be set up, and we filter those based on musl).

@the-horo
Copy link
Contributor

Yeah, I got it wrong the first time by changing common.mak but moving it up in the dmd-druntime specific makefile is a win-win.

@kinke kinke force-pushed the fix_musl_detection branch from c2d6bda to cc28b26 Compare August 25, 2025 15:09
@kinke
Copy link
Contributor Author

kinke commented Aug 25, 2025

Note #21741

Oh damn, overlooked that at first because I thought you'd link to some post in the LDC repo. 🤦

@kinke
Copy link
Contributor Author

kinke commented Aug 25, 2025

Closing in favor of #21741.

@kinke kinke closed this Aug 25, 2025
@kinke kinke deleted the fix_musl_detection branch August 25, 2025 15:28
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.

3 participants