[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-27 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 added a comment. In D134454#3816460 , @MaskRay wrote: > I am nervous as well as Arch Linux has many derivatives. They likely don't > use `ID=arch` in `/etc/os-release`. The patch won't work for them. > In general, I don't think our current approach

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-27 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 added inline comments. Comment at: clang/lib/Driver/Distro.cpp:213 + // that the Linux OS and distro are properly detected in this cases. + llvm::Triple NormTargetOrHost = llvm::Triple(Twine(TargetOrHost.normalize())); + nickdesaulniers wrote: > 10ne1 wr

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I am nervous as well as Arch Linux has many derivatives. They likely don't use `ID=arch` in `/etc/os-release`. The patch won't work for them. The last few comments of https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606 discuss a

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-26 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. It's not clear to me why Arch is special here. The path may be different, but normalizing the triple seems unnecessary. Blocking this until I'm confident that we're

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-26 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:377-394 + if (Distro.IsArchLinux()) { +std::string Path = (InstallDir + "/../../../../" + TripleStr).str(); +if (getVFS().exists(Path)) + return Path; + } + if (!GCCInstallation.isVa

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-26 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 added inline comments. Comment at: clang/lib/Driver/Distro.cpp:213 + // that the Linux OS and distro are properly detected in this cases. + llvm::Triple NormTargetOrHost = llvm::Triple(Twine(TargetOrHost.normalize())); + nickdesaulniers wrote: > Twine ha

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-26 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 added a comment. In D134454#3812153 , @nickdesaulniers wrote: > Is it worth contacting the package maintainer for LLVM+clang for Arch-Linux > in regards to this patch? Yes, thanks for the suggestion, I will open an issue on the Arch bug tracker

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Is it worth contacting the package maintainer for LLVM+clang for Arch-Linux in regards to this patch? Comment at: clang/lib/Driver/Distro.cpp:210 + // Sometimes the OS can't be detected from the triplet due to ambiguity, for + // eg. ArchLinu

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-23 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 updated this revision to Diff 462423. 10ne1 added a comment. Regenerated diff with git diff HEAD~1 -U99 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134454/new/ https://reviews.llvm.org/D134454 Files: clang/include/clang/Driver/Distro.h clang/lib/Driver/Distro.cpp clan

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-22 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment. Thanks for the patch. Can you please post a full diff (git diff -U). Adding @MaskRay as a reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134454/new/ https://reviews.llvm.org/D134454 ___

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

2022-09-22 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 created this revision. 10ne1 added reviewers: nickdesaulniers, nathanchance, manojgupta. Herald added a subscriber: kristof.beyls. Herald added a project: All. 10ne1 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. While cross-